From cb8fa1d894392f36689cfa0dcff86fa2d484a5b1 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 8 Jul 2026 17:04:28 +1000 Subject: [PATCH 01/10] fix(cli): actionable config-scaffold errors instead of dead-ends (#578, #579) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related config-scaffold failures reported from testing the 0.17 CLI: - `eql status` (any read command) in a project with no stash.config.ts printed a "hand-write this file" message and exited — no scaffold, no pointer to the commands that set it up (#578). - `npx stash eql install` scaffolded a stash.config.ts, then immediately crashed with a raw `Cannot find module 'stash'`: the config imports `stash` (and the client imports `@cipherstash/stack`), which resolve only once those are project dependencies — something only `stash init` did. Run standalone via npx, they live in the npx cache, not the project (#579). Fixes: - Missing-config message now recommends `stash init` / `stash eql install` (runner-aware) before the hand-written fallback. - `eql install` checks for the config's dependencies after scaffolding and offers to install the missing ones (or prints the exact commands in non-interactive contexts) before jiti loads the config. - `loadStashConfig` translates a missing-module load failure into the same actionable guidance for every command, instead of a jiti/Node stack trace. Tests: config-scaffold.ts had zero tests and the install-command tests run inside the monorepo where `stash` always resolves via the workspace — which is why the crash escaped CI. Adds unit coverage for the dependency guard and the load-error translation (mocked jiti), plus e2e coverage in a throwaway temp project for the missing-config and missing-deps paths. --- .changeset/stash-cli-config-scaffold-dx.md | 18 ++++ packages/cli/src/__tests__/config.test.ts | 67 +++++++++++++ .../db/__tests__/config-scaffold.test.ts | 77 +++++++++++++++ .../cli/src/commands/db/config-scaffold.ts | 98 +++++++++++++++++++ packages/cli/src/commands/db/install.ts | 15 ++- packages/cli/src/commands/init/utils.ts | 7 +- packages/cli/src/config/index.ts | 69 ++++++++++++- .../cli/tests/e2e/config-scaffold.e2e.test.ts | 59 +++++++++++ 8 files changed, 406 insertions(+), 4 deletions(-) create mode 100644 .changeset/stash-cli-config-scaffold-dx.md create mode 100644 packages/cli/src/commands/db/__tests__/config-scaffold.test.ts create mode 100644 packages/cli/tests/e2e/config-scaffold.e2e.test.ts diff --git a/.changeset/stash-cli-config-scaffold-dx.md b/.changeset/stash-cli-config-scaffold-dx.md new file mode 100644 index 00000000..7d04b2e8 --- /dev/null +++ b/.changeset/stash-cli-config-scaffold-dx.md @@ -0,0 +1,18 @@ +--- +"stash": patch +--- + +Fix two config-scaffold dead-ends in the CLI (#578, #579). + +- **Missing config is now actionable.** When a command can't find + `stash.config.ts`, the error recommends `stash init` / `stash eql install` + (runner-aware) instead of only telling you to hand-write the file. +- **Standalone `stash eql install` no longer crashes with a raw + `Cannot find module 'stash'`.** The scaffolded `stash.config.ts` imports + `stash` (and the client it points at imports `@cipherstash/stack`), which + only resolve once those packages are project dependencies — something only + `stash init` did. `eql install` now checks for them after scaffolding and + offers to install the missing ones (or prints the exact install commands in + non-interactive contexts). As a safety net, `loadStashConfig` translates a + missing-module load failure into the same actionable guidance for every + command, rather than dumping a jiti/Node stack trace. diff --git a/packages/cli/src/__tests__/config.test.ts b/packages/cli/src/__tests__/config.test.ts index 5048519b..a820141a 100644 --- a/packages/cli/src/__tests__/config.test.ts +++ b/packages/cli/src/__tests__/config.test.ts @@ -36,6 +36,73 @@ describe('loadStashConfig', () => { await expect(loadStashConfig()).rejects.toThrow('process.exit') }) + it('points the user at `init` / `eql install` when config is missing (#578)', async () => { + process.cwd = () => tmpDir + vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit') + }) + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + + const { loadStashConfig } = await import('@/config/index.ts') + await expect(loadStashConfig()).rejects.toThrow('process.exit') + + const output = errorSpy.mock.calls.map((c) => c.join(' ')).join('\n') + expect(output).toContain('stash init') + expect(output).toContain('stash eql install') + }) + + it.each([ + ['stash', `Cannot find module 'stash'`], + ['@cipherstash/stack', `Cannot find package '@cipherstash/stack'`], + ])('translates a missing `%s` module into actionable guidance (#579)', async (pkg, message) => { + fs.writeFileSync( + path.join(tmpDir, 'stash.config.ts'), + `import 'stash'\nexport default {}`, + ) + process.cwd = () => tmpDir + vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit') + }) + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + + const moduleErr = Object.assign(new Error(message), { + code: 'MODULE_NOT_FOUND', + }) + const { createJiti } = await import('jiti') + vi.mocked(createJiti).mockReturnValue({ + import: vi.fn().mockRejectedValue(moduleErr), + } as never) + + const { loadStashConfig } = await import('@/config/index.ts') + await expect(loadStashConfig()).rejects.toThrow('process.exit') + + const output = errorSpy.mock.calls.map((c) => c.join(' ')).join('\n') + expect(output).toContain(`\`${pkg}\` is not installed`) + expect(output).toContain('stash init') + // The raw jiti stack trace must NOT be forwarded to the user. + expect(output).not.toContain('Failed to load') + }) + + it('still surfaces the raw error for unrelated config load failures', async () => { + fs.writeFileSync(path.join(tmpDir, 'stash.config.ts'), 'export default {}') + process.cwd = () => tmpDir + vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit') + }) + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + + const { createJiti } = await import('jiti') + vi.mocked(createJiti).mockReturnValue({ + import: vi.fn().mockRejectedValue(new SyntaxError('Unexpected token')), + } as never) + + const { loadStashConfig } = await import('@/config/index.ts') + await expect(loadStashConfig()).rejects.toThrow('process.exit') + + const output = errorSpy.mock.calls.map((c) => c.join(' ')).join('\n') + expect(output).toContain('Failed to load') + }) + it('validates required fields', async () => { // Write a config file that exists but exports an empty object fs.writeFileSync(path.join(tmpDir, 'stash.config.ts'), 'export default {}') diff --git a/packages/cli/src/commands/db/__tests__/config-scaffold.test.ts b/packages/cli/src/commands/db/__tests__/config-scaffold.test.ts new file mode 100644 index 00000000..439e74a3 --- /dev/null +++ b/packages/cli/src/commands/db/__tests__/config-scaffold.test.ts @@ -0,0 +1,77 @@ +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + ensureConfigDependencies, + missingConfigDependencies, +} from '../config-scaffold.js' + +/** Create a fake installed package under `/node_modules/`. */ +function fakeInstall(cwd: string, name: string): void { + const dir = path.join(cwd, 'node_modules', name) + fs.mkdirSync(dir, { recursive: true }) + fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify({ name })) +} + +describe('config-scaffold dependency guard', () => { + let tmpDir: string + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'stash-config-scaffold-')) + }) + + afterEach(() => { + vi.restoreAllMocks() + if (tmpDir && fs.existsSync(tmpDir)) { + fs.rmSync(tmpDir, { recursive: true, force: true }) + } + }) + + describe('missingConfigDependencies', () => { + it('reports both packages missing in a bare project', () => { + expect(missingConfigDependencies(tmpDir)).toEqual({ + prod: ['@cipherstash/stack'], + dev: ['stash'], + }) + }) + + it('reports nothing missing when both are installed', () => { + fakeInstall(tmpDir, 'stash') + fakeInstall(tmpDir, '@cipherstash/stack') + expect(missingConfigDependencies(tmpDir)).toEqual({ prod: [], dev: [] }) + }) + + it('reports only the package that is actually missing', () => { + fakeInstall(tmpDir, '@cipherstash/stack') + expect(missingConfigDependencies(tmpDir)).toEqual({ + prod: [], + dev: ['stash'], + }) + }) + }) + + describe('ensureConfigDependencies', () => { + it('returns true (no prompt) when both packages are present', async () => { + fakeInstall(tmpDir, 'stash') + fakeInstall(tmpDir, '@cipherstash/stack') + await expect(ensureConfigDependencies(tmpDir)).resolves.toBe(true) + }) + + it('warns and returns false in non-interactive contexts when deps are missing (#579)', async () => { + // Under vitest, process.stdin.isTTY is undefined → the guard takes the + // non-interactive branch: it must print guidance and stop cleanly rather + // than spawn a package manager or hang on a prompt. Capture stdout (where + // clack writes) to assert the guidance surfaced. + let out = '' + vi.spyOn(process.stdout, 'write').mockImplementation((chunk) => { + out += String(chunk) + return true + }) + + await expect(ensureConfigDependencies(tmpDir)).resolves.toBe(false) + expect(out).toContain('not installed') + expect(out).toContain('stash init') + }) + }) +}) diff --git a/packages/cli/src/commands/db/config-scaffold.ts b/packages/cli/src/commands/db/config-scaffold.ts index 35947f32..f1842e87 100644 --- a/packages/cli/src/commands/db/config-scaffold.ts +++ b/packages/cli/src/commands/db/config-scaffold.ts @@ -1,9 +1,26 @@ +import { execSync } from 'node:child_process' import { existsSync, writeFileSync } from 'node:fs' import { resolve } from 'node:path' import * as p from '@clack/prompts' +import { + combinedInstallCommands, + detectPackageManager, + isPackageInstalled, + runnerCommand, +} from '../init/utils.js' export const CONFIG_FILENAME = 'stash.config.ts' +/** + * The packages a scaffolded `stash.config.ts` depends on. `stash` (dev) exports + * the `defineConfig`/`resolveDatabaseUrl` the config imports; `@cipherstash/stack` + * (prod) is imported by the encryption client the config points at. Both must + * resolve from the project's node_modules or `loadStashConfig` fails to load the + * config with `Cannot find module 'stash'`. + */ +const CLI_PACKAGE = 'stash' +const STACK_PACKAGE = '@cipherstash/stack' + /** * Common locations where an encryption client file might live. Checked in * order of priority during auto-detection. @@ -108,3 +125,84 @@ export async function ensureStashConfig( p.log.success(`Created ${CONFIG_FILENAME}`) return true } + +/** + * Which config dependencies the project is missing, split by install kind. + * Pure (only the filesystem probe in `isPackageInstalled`), so the decision is + * unit-testable without spawning a package manager. + */ +export function missingConfigDependencies(cwd: string = process.cwd()): { + prod: string[] + dev: string[] +} { + return { + prod: isPackageInstalled(STACK_PACKAGE, cwd) ? [] : [STACK_PACKAGE], + dev: isPackageInstalled(CLI_PACKAGE, cwd) ? [] : [CLI_PACKAGE], + } +} + +/** + * Ensure the packages a `stash.config.ts` imports are installed before the CLI + * tries to load it. Offers to install any missing ones interactively; in + * non-interactive contexts (or on cancel / install failure) it prints the exact + * install commands and returns `false` so the caller can stop cleanly. + * + * Without this, a standalone `npx stash eql install` scaffolds a config and then + * crashes with a raw `Cannot find module 'stash'` because the CLI packages were + * never added as project dependencies (only `stash init` does that) — #579. + * Returns `true` when nothing is missing or the install succeeded. + */ +export async function ensureConfigDependencies( + cwd: string = process.cwd(), +): Promise { + const { prod, dev } = missingConfigDependencies(cwd) + if (prod.length === 0 && dev.length === 0) return true + + const pm = detectPackageManager() + const commands = combinedInstallCommands(pm, prod, dev) + const missing = [...prod, ...dev] + const missingList = missing.join(', ') + const verb = missing.length === 1 ? 'is' : 'are' + + const isTTY = Boolean(process.stdin.isTTY) && process.env.CI !== 'true' + if (!isTTY) { + p.log.warn( + `${CONFIG_FILENAME} imports \`${CLI_PACKAGE}\`, but ${missingList} ${verb} not installed in this project.`, + ) + p.note( + `Install, then re-run:\n ${commands.join('\n ')}\n\nOr run \`${runnerCommand(pm, 'stash init')}\` to set everything up.`, + 'Missing dependencies', + ) + return false + } + + const proceed = await p.confirm({ + message: `Install ${missingList}? (${commands.join(' && ')})`, + }) + if (p.isCancel(proceed) || !proceed) { + p.note( + `Install manually, then re-run:\n ${commands.join('\n ')}`, + 'Missing dependencies', + ) + return false + } + + for (const cmd of commands) { + p.log.step(`Running: ${cmd}`) + try { + execSync(cmd, { cwd, stdio: 'inherit' }) + } catch { + p.log.error(`Install failed: ${cmd}`) + return false + } + } + + // Re-check from disk — a package manager can exit 0 without the package + // actually resolving (registry hiccup, workspace mismatch). + const still = missingConfigDependencies(cwd) + if (still.prod.length > 0 || still.dev.length > 0) { + p.log.warn(`Still missing: ${[...still.prod, ...still.dev].join(', ')}.`) + return false + } + return true +} diff --git a/packages/cli/src/commands/db/install.ts b/packages/cli/src/commands/db/install.ts index b4c97249..1f303250 100644 --- a/packages/cli/src/commands/db/install.ts +++ b/packages/cli/src/commands/db/install.ts @@ -16,7 +16,10 @@ import { loadBundledEqlSql, } from '@/installer/index.js' import { ensureEncryptionClient } from './client-scaffold.js' -import { ensureStashConfig } from './config-scaffold.js' +import { + ensureConfigDependencies, + ensureStashConfig, +} from './config-scaffold.js' import { detectDrizzle, detectSupabase, @@ -98,6 +101,16 @@ export async function installCommand(options: InstallOptions) { process.exit(0) } + // The config (and the client it points at) `import` `stash` / + // `@cipherstash/stack`; make sure they're installed before jiti loads the + // config, so a standalone `npx stash eql install` gives actionable guidance + // instead of a raw `Cannot find module 'stash'` (#579). + const depsReady = await ensureConfigDependencies() + if (!depsReady) { + p.outro('Installation aborted.') + process.exit(1) + } + const s = p.spinner() s.start('Loading stash.config.ts...') diff --git a/packages/cli/src/commands/init/utils.ts b/packages/cli/src/commands/init/utils.ts index 7d8e3c4e..405e4bbd 100644 --- a/packages/cli/src/commands/init/utils.ts +++ b/packages/cli/src/commands/init/utils.ts @@ -13,8 +13,11 @@ import type { Integration, SchemaDef } from './types.js' * `Cannot find module 'stash'` at the jiti import. Requiring the manifest * matches what Node's resolver actually needs to load the module. */ -export function isPackageInstalled(packageName: string): boolean { - const modulePath = resolve(process.cwd(), 'node_modules', packageName) +export function isPackageInstalled( + packageName: string, + cwd: string = process.cwd(), +): boolean { + const modulePath = resolve(cwd, 'node_modules', packageName) const manifestPath = resolve(modulePath, 'package.json') return existsSync(modulePath) && existsSync(manifestPath) } diff --git a/packages/cli/src/config/index.ts b/packages/cli/src/config/index.ts index a4d732b5..5f73b6f4 100644 --- a/packages/cli/src/config/index.ts +++ b/packages/cli/src/config/index.ts @@ -3,11 +3,50 @@ import path from 'node:path' import type { EncryptionClient } from '@cipherstash/stack/encryption' import type { EncryptConfig } from '@cipherstash/stack/schema' import { z } from 'zod' +import { + combinedInstallCommands, + detectPackageManager, + runnerCommand, +} from '../commands/init/utils.js' import { type ResolveDatabaseUrlOptions, withResolverContext, } from './database-url.js' +/** The npm packages a `stash.config.ts` (and the client it points at) import. */ +const CLI_PACKAGE = 'stash' +const STACK_PACKAGE = '@cipherstash/stack' + +/** + * When jiti fails to evaluate the config because a required CipherStash package + * isn't installed in the project, Node throws a `MODULE_NOT_FOUND` / + * `ERR_MODULE_NOT_FOUND`. Return the offending package name so the caller can + * print actionable guidance instead of a raw stack trace; return `undefined` + * for any other failure (which should surface as-is). + * + * This is the standalone `npx stash eql install` failure mode: the scaffolded + * config `import`s `stash`, which resolves only when the CLI packages were added + * as project dependencies (via `stash init`) — see #579. + */ +function missingConfigModule(error: unknown): string | undefined { + if (!(error instanceof Error)) return undefined + const code = (error as { code?: string }).code + if (code !== 'MODULE_NOT_FOUND' && code !== 'ERR_MODULE_NOT_FOUND') { + return undefined + } + // Check the scoped package first — `stash` is a substring of paths that + // mention it, but the quoted `'@cipherstash/stack'` is the specific specifier. + for (const pkg of [STACK_PACKAGE, CLI_PACKAGE]) { + if ( + error.message.includes(`'${pkg}'`) || + error.message.includes(`"${pkg}"`) + ) { + return pkg + } + } + return undefined +} + export interface StashConfig { /** PostgreSQL connection string */ databaseUrl: string @@ -96,9 +135,13 @@ export async function loadStashConfig( const configPath = findConfigFile(process.cwd()) if (!configPath) { + const stash = runnerCommand(detectPackageManager(), 'stash') console.error(`Error: Could not find ${CONFIG_FILENAME} -Create a ${CONFIG_FILENAME} file in your project root: +Run \`${stash} init\` to set up CipherStash (recommended), or +\`${stash} eql install\` to scaffold a ${CONFIG_FILENAME} and install EQL. + +To create it by hand, add ${CONFIG_FILENAME} to your project root: import { defineConfig, resolveDatabaseUrl } from 'stash' @@ -126,6 +169,30 @@ Create a ${CONFIG_FILENAME} file in your project root: jiti.import(configPath, { default: true }), ) } catch (error) { + // A missing CipherStash package (the config `import`s `stash`; the client it + // points at `import`s `@cipherstash/stack`) is the common standalone-npx + // failure — translate jiti's raw `Cannot find module 'stash'` into + // actionable guidance instead of a stack trace (#579). + const missingPkg = missingConfigModule(error) + if (missingPkg) { + const pm = detectPackageManager() + const stash = runnerCommand(pm, 'stash') + const install = combinedInstallCommands( + pm, + [STACK_PACKAGE], + [CLI_PACKAGE], + ) + console.error( + `Error: ${CONFIG_FILENAME} could not load — \`${missingPkg}\` is not installed in this project. + +Install the CipherStash packages, then re-run: + ${install.join('\n ')} + +Or run \`${stash} init\` to set everything up. +`, + ) + process.exit(1) + } console.error(`Error: Failed to load ${CONFIG_FILENAME} at ${configPath}\n`) console.error(error) process.exit(1) diff --git a/packages/cli/tests/e2e/config-scaffold.e2e.test.ts b/packages/cli/tests/e2e/config-scaffold.e2e.test.ts new file mode 100644 index 00000000..c8f74759 --- /dev/null +++ b/packages/cli/tests/e2e/config-scaffold.e2e.test.ts @@ -0,0 +1,59 @@ +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { run } from '../helpers/run.js' + +/** + * E2E coverage for the config-scaffold DX fixes (#578, #579). These run the + * built CLI in a throwaway temp project OUTSIDE the monorepo, so `stash` / + * `@cipherstash/stack` genuinely don't resolve from `node_modules` — the exact + * condition that produced the raw `Cannot find module 'stash'` crash. + */ +describe('config-scaffold DX (missing config / missing deps)', () => { + let tmpDir: string + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'stash-config-e2e-')) + fs.writeFileSync( + path.join(tmpDir, 'package.json'), + JSON.stringify({ name: 'bare', version: '1.0.0' }), + ) + }) + + afterEach(() => { + if (tmpDir && fs.existsSync(tmpDir)) { + fs.rmSync(tmpDir, { recursive: true, force: true }) + } + }) + + it('a read command with no config points at `init` / `eql install` (#578)', async () => { + const r = await run(['eql', 'status'], { cwd: tmpDir }) + expect(r.output).toContain('Could not find stash.config.ts') + expect(r.output).toContain('stash init') + expect(r.output).toContain('stash eql install') + }) + + it('`eql install` with a config but missing deps guides instead of crashing (#579)', async () => { + fs.writeFileSync( + path.join(tmpDir, 'stash.config.ts'), + "import { defineConfig, resolveDatabaseUrl } from 'stash'\n" + + 'export default defineConfig({ databaseUrl: await resolveDatabaseUrl() })\n', + ) + + const r = await run(['eql', 'install'], { cwd: tmpDir }) + // Actionable guidance, not a raw module-resolution stack trace. + expect(r.output).toContain('not installed') + expect(r.output).toContain('stash init') + expect(r.output).not.toContain('MODULE_NOT_FOUND') + expect(r.output).not.toContain('Cannot find module') + expect(r.exitCode).toBe(1) + }) + + // The `loadStashConfig` catch that translates a missing-module error into + // guidance (the read-command path for #579) is covered by unit tests in + // src/__tests__/config.test.ts with a mocked jiti rejection. It can't be + // reproduced end-to-end here: inside the monorepo jiti resolves `stash` via + // the workspace self-reference even from a temp dir, so the import never + // fails — which is exactly why the original bug escaped the test suite. +}) From 4d11cfc4eaed49bf3b9603297105773b4232f392 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 8 Jul 2026 18:07:16 +1000 Subject: [PATCH 02/10] refactor(cli): decouple `eql install` from stash.config.ts (approach B) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the previous commit: rather than guarding the scaffolded config's dependencies, remove the requirement entirely. `eql install` only needs a database URL, so it now resolves one directly (`--database-url` → env → supabase → prompt) via resolveDatabaseUrl instead of scaffolding a config and loading it. - An existing stash.config.ts is still authoritative (later workflow commands load the encryption client through it) — load it when present. - Without one, resolve the URL directly and *offer* to scaffold a config for the rest of the workflow (auto-creates it in non-interactive contexts) — it's a convenience, never a prerequisite. A standalone `npx stash eql install --database-url ...` now works in a bare project with zero deps: there's no `import 'stash'` to resolve, so the `Cannot find module 'stash'` crash can't happen. - Drop the now-redundant dependency guard (ensureConfigDependencies / missingConfigDependencies) and revert the isPackageInstalled cwd param. - Keep the improved messages: the actionable missing-config error (#578) and loadStashConfig's missing-module translation (#579) still cover the commands that genuinely load a config. Replaces the guard unit tests with offerStashConfig coverage; the e2e now asserts `eql install` resolves the URL directly and fails cleanly (no config written, no module crash) when no URL is available. --- .changeset/stash-cli-config-scaffold-dx.md | 27 ++-- .../db/__tests__/config-scaffold.test.ts | 80 ++++------ .../cli/src/commands/db/config-scaffold.ts | 143 +++++------------- packages/cli/src/commands/db/install.ts | 85 ++++++----- packages/cli/src/commands/init/utils.ts | 7 +- packages/cli/src/config/index.ts | 6 +- .../cli/tests/e2e/config-scaffold.e2e.test.ts | 32 ++-- 7 files changed, 154 insertions(+), 226 deletions(-) diff --git a/.changeset/stash-cli-config-scaffold-dx.md b/.changeset/stash-cli-config-scaffold-dx.md index 7d04b2e8..17082a68 100644 --- a/.changeset/stash-cli-config-scaffold-dx.md +++ b/.changeset/stash-cli-config-scaffold-dx.md @@ -4,15 +4,18 @@ Fix two config-scaffold dead-ends in the CLI (#578, #579). -- **Missing config is now actionable.** When a command can't find - `stash.config.ts`, the error recommends `stash init` / `stash eql install` - (runner-aware) instead of only telling you to hand-write the file. -- **Standalone `stash eql install` no longer crashes with a raw - `Cannot find module 'stash'`.** The scaffolded `stash.config.ts` imports - `stash` (and the client it points at imports `@cipherstash/stack`), which - only resolve once those packages are project dependencies — something only - `stash init` did. `eql install` now checks for them after scaffolding and - offers to install the missing ones (or prints the exact install commands in - non-interactive contexts). As a safety net, `loadStashConfig` translates a - missing-module load failure into the same actionable guidance for every - command, rather than dumping a jiti/Node stack trace. +- **Missing config is now actionable.** When a command that needs a + `stash.config.ts` can't find one, the error recommends `stash init` / + `stash eql install` (runner-aware) instead of only telling you to hand-write + the file. +- **`stash eql install` no longer requires a `stash.config.ts`.** It only needs + a database URL, so it now resolves one directly (`--database-url` → env → + `supabase status` → prompt) instead of scaffolding a config and loading it. + That means a standalone `npx stash eql install --database-url ...` works in a + bare project with **zero dependencies** — no more crash with a raw + `Cannot find module 'stash'` from the config's `import`. An existing config is + still honoured (later workflow commands rely on it), and a fresh one is offered + as a convenience for the rest of the workflow rather than being a prerequisite. +- As a safety net, `loadStashConfig` translates a missing-module load failure + (a project that *has* a config but lacks the CLI packages) into the same + actionable guidance for every command, instead of a jiti/Node stack trace. diff --git a/packages/cli/src/commands/db/__tests__/config-scaffold.test.ts b/packages/cli/src/commands/db/__tests__/config-scaffold.test.ts index 439e74a3..0bfe470b 100644 --- a/packages/cli/src/commands/db/__tests__/config-scaffold.test.ts +++ b/packages/cli/src/commands/db/__tests__/config-scaffold.test.ts @@ -3,18 +3,12 @@ import os from 'node:os' import path from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { - ensureConfigDependencies, - missingConfigDependencies, + CONFIG_FILENAME, + DEFAULT_CLIENT_PATH, + offerStashConfig, } from '../config-scaffold.js' -/** Create a fake installed package under `/node_modules/`. */ -function fakeInstall(cwd: string, name: string): void { - const dir = path.join(cwd, 'node_modules', name) - fs.mkdirSync(dir, { recursive: true }) - fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify({ name })) -} - -describe('config-scaffold dependency guard', () => { +describe('offerStashConfig (optional config scaffold)', () => { let tmpDir: string beforeEach(() => { @@ -28,50 +22,38 @@ describe('config-scaffold dependency guard', () => { } }) - describe('missingConfigDependencies', () => { - it('reports both packages missing in a bare project', () => { - expect(missingConfigDependencies(tmpDir)).toEqual({ - prod: ['@cipherstash/stack'], - dev: ['stash'], - }) - }) + it('non-interactively creates a config with the default client path', async () => { + // Under vitest process.stdin.isTTY is undefined → the non-interactive branch + // writes a config instead of prompting (which would hang in CI / agents). + const clientPath = await offerStashConfig(tmpDir) + + expect(clientPath).toBe(DEFAULT_CLIENT_PATH) + const written = fs.readFileSync(path.join(tmpDir, CONFIG_FILENAME), 'utf-8') + expect(written).toContain("from 'stash'") + expect(written).toContain(`client: '${DEFAULT_CLIENT_PATH}'`) + }) + + it('points the config at a detected client file when one exists', async () => { + fs.mkdirSync(path.join(tmpDir, 'src')) + fs.writeFileSync(path.join(tmpDir, 'src', 'encryption.ts'), '// client') - it('reports nothing missing when both are installed', () => { - fakeInstall(tmpDir, 'stash') - fakeInstall(tmpDir, '@cipherstash/stack') - expect(missingConfigDependencies(tmpDir)).toEqual({ prod: [], dev: [] }) - }) + const clientPath = await offerStashConfig(tmpDir) - it('reports only the package that is actually missing', () => { - fakeInstall(tmpDir, '@cipherstash/stack') - expect(missingConfigDependencies(tmpDir)).toEqual({ - prod: [], - dev: ['stash'], - }) - }) + expect(clientPath).toBe('./src/encryption.ts') + expect( + fs.readFileSync(path.join(tmpDir, CONFIG_FILENAME), 'utf-8'), + ).toContain(`client: './src/encryption.ts'`) }) - describe('ensureConfigDependencies', () => { - it('returns true (no prompt) when both packages are present', async () => { - fakeInstall(tmpDir, 'stash') - fakeInstall(tmpDir, '@cipherstash/stack') - await expect(ensureConfigDependencies(tmpDir)).resolves.toBe(true) - }) + it('never overwrites an existing config', async () => { + const configPath = path.join(tmpDir, CONFIG_FILENAME) + fs.writeFileSync(configPath, '// hand-written, do not touch') - it('warns and returns false in non-interactive contexts when deps are missing (#579)', async () => { - // Under vitest, process.stdin.isTTY is undefined → the guard takes the - // non-interactive branch: it must print guidance and stop cleanly rather - // than spawn a package manager or hang on a prompt. Capture stdout (where - // clack writes) to assert the guidance surfaced. - let out = '' - vi.spyOn(process.stdout, 'write').mockImplementation((chunk) => { - out += String(chunk) - return true - }) + const clientPath = await offerStashConfig(tmpDir) - await expect(ensureConfigDependencies(tmpDir)).resolves.toBe(false) - expect(out).toContain('not installed') - expect(out).toContain('stash init') - }) + expect(clientPath).toBe(DEFAULT_CLIENT_PATH) + expect(fs.readFileSync(configPath, 'utf-8')).toBe( + '// hand-written, do not touch', + ) }) }) diff --git a/packages/cli/src/commands/db/config-scaffold.ts b/packages/cli/src/commands/db/config-scaffold.ts index f1842e87..72f23da7 100644 --- a/packages/cli/src/commands/db/config-scaffold.ts +++ b/packages/cli/src/commands/db/config-scaffold.ts @@ -1,25 +1,12 @@ -import { execSync } from 'node:child_process' import { existsSync, writeFileSync } from 'node:fs' import { resolve } from 'node:path' import * as p from '@clack/prompts' -import { - combinedInstallCommands, - detectPackageManager, - isPackageInstalled, - runnerCommand, -} from '../init/utils.js' +import { detectPackageManager, runnerCommand } from '../init/utils.js' export const CONFIG_FILENAME = 'stash.config.ts' -/** - * The packages a scaffolded `stash.config.ts` depends on. `stash` (dev) exports - * the `defineConfig`/`resolveDatabaseUrl` the config imports; `@cipherstash/stack` - * (prod) is imported by the encryption client the config points at. Both must - * resolve from the project's node_modules or `loadStashConfig` fails to load the - * config with `Cannot find module 'stash'`. - */ -const CLI_PACKAGE = 'stash' -const STACK_PACKAGE = '@cipherstash/stack' +/** Default encryption-client path used when the project has none yet. */ +export const DEFAULT_CLIENT_PATH = './src/encryption/index.ts' /** * Common locations where an encryption client file might live. Checked in @@ -100,109 +87,51 @@ export default defineConfig({ } /** - * Create a `stash.config.ts` at the project root if one doesn't already exist. - * Returns `true` if a config is present (either pre-existing or freshly - * written), `false` if the user cancelled the prompt. + * Offer to create a `stash.config.ts` for the rest of the workflow. * - * Invoked by `eql install` when no `stash.config.ts` exists, so users don't - * need to run a separate `setup` step before installing EQL. - */ -export async function ensureStashConfig( - cwd: string = process.cwd(), -): Promise { - const configPath = resolve(cwd, CONFIG_FILENAME) - if (existsSync(configPath)) return true - - p.log.info(`No ${CONFIG_FILENAME} found — let's create one.`) - - const clientPath = await resolveClientPath(cwd) - if (!clientPath) { - p.cancel('Setup cancelled.') - return false - } - - writeFileSync(configPath, generateConfig(clientPath), 'utf-8') - p.log.success(`Created ${CONFIG_FILENAME}`) - return true -} - -/** - * Which config dependencies the project is missing, split by install kind. - * Pure (only the filesystem probe in `isPackageInstalled`), so the decision is - * unit-testable without spawning a package manager. - */ -export function missingConfigDependencies(cwd: string = process.cwd()): { - prod: string[] - dev: string[] -} { - return { - prod: isPackageInstalled(STACK_PACKAGE, cwd) ? [] : [STACK_PACKAGE], - dev: isPackageInstalled(CLI_PACKAGE, cwd) ? [] : [CLI_PACKAGE], - } -} - -/** - * Ensure the packages a `stash.config.ts` imports are installed before the CLI - * tries to load it. Offers to install any missing ones interactively; in - * non-interactive contexts (or on cancel / install failure) it prints the exact - * install commands and returns `false` so the caller can stop cleanly. + * `eql install` itself doesn't need a config — it resolves the database URL + * directly — but `db push` / `schema build` / `encrypt *` load the encryption + * client through it, so we create it now as a convenience. Declining (or a + * non-interactive context) never blocks the EQL install. * - * Without this, a standalone `npx stash eql install` scaffolds a config and then - * crashes with a raw `Cannot find module 'stash'` because the CLI packages were - * never added as project dependencies (only `stash init` does that) — #579. - * Returns `true` when nothing is missing or the install succeeded. + * Returns the encryption-client path the config points at, falling back to + * {@link DEFAULT_CLIENT_PATH} when no config is written — so the caller can + * still scaffold the client file at a sensible location. + * + * Should be called only when no config exists yet (the caller loads an existing + * one instead); it never overwrites a present `stash.config.ts`. */ -export async function ensureConfigDependencies( +export async function offerStashConfig( cwd: string = process.cwd(), -): Promise { - const { prod, dev } = missingConfigDependencies(cwd) - if (prod.length === 0 && dev.length === 0) return true - - const pm = detectPackageManager() - const commands = combinedInstallCommands(pm, prod, dev) - const missing = [...prod, ...dev] - const missingList = missing.join(', ') - const verb = missing.length === 1 ? 'is' : 'are' +): Promise { + const configPath = resolve(cwd, CONFIG_FILENAME) + if (existsSync(configPath)) return DEFAULT_CLIENT_PATH const isTTY = Boolean(process.stdin.isTTY) && process.env.CI !== 'true' if (!isTTY) { - p.log.warn( - `${CONFIG_FILENAME} imports \`${CLI_PACKAGE}\`, but ${missingList} ${verb} not installed in this project.`, - ) - p.note( - `Install, then re-run:\n ${commands.join('\n ')}\n\nOr run \`${runnerCommand(pm, 'stash init')}\` to set everything up.`, - 'Missing dependencies', - ) - return false + // Non-interactive (CI / agents / pipes): create with a detected or default + // client path rather than prompting, which would hang. + const clientPath = detectClientPath(cwd) ?? DEFAULT_CLIENT_PATH + writeFileSync(configPath, generateConfig(clientPath), 'utf-8') + p.log.success(`Created ${CONFIG_FILENAME}`) + return clientPath } - const proceed = await p.confirm({ - message: `Install ${missingList}? (${commands.join(' && ')})`, + const create = await p.confirm({ + message: `Create a ${CONFIG_FILENAME}? (needed later for db push / schema build / encrypt)`, + initialValue: true, }) - if (p.isCancel(proceed) || !proceed) { - p.note( - `Install manually, then re-run:\n ${commands.join('\n ')}`, - 'Missing dependencies', + if (p.isCancel(create) || !create) { + p.log.info( + `Skipped ${CONFIG_FILENAME}. Create it later with \`${runnerCommand(detectPackageManager(), 'stash init')}\`.`, ) - return false + return DEFAULT_CLIENT_PATH } - for (const cmd of commands) { - p.log.step(`Running: ${cmd}`) - try { - execSync(cmd, { cwd, stdio: 'inherit' }) - } catch { - p.log.error(`Install failed: ${cmd}`) - return false - } - } + const clientPath = await resolveClientPath(cwd) + if (!clientPath) return DEFAULT_CLIENT_PATH - // Re-check from disk — a package manager can exit 0 without the package - // actually resolving (registry hiccup, workspace mismatch). - const still = missingConfigDependencies(cwd) - if (still.prod.length > 0 || still.dev.length > 0) { - p.log.warn(`Still missing: ${[...still.prod, ...still.dev].join(', ')}.`) - return false - } - return true + writeFileSync(configPath, generateConfig(clientPath), 'utf-8') + p.log.success(`Created ${CONFIG_FILENAME}`) + return clientPath } diff --git a/packages/cli/src/commands/db/install.ts b/packages/cli/src/commands/db/install.ts index 1f303250..2684acc8 100644 --- a/packages/cli/src/commands/db/install.ts +++ b/packages/cli/src/commands/db/install.ts @@ -9,17 +9,15 @@ import { import * as p from '@clack/prompts' import pg from 'pg' import { detectPackageManager, runnerCommand } from '@/commands/init/utils.js' -import { loadStashConfig } from '@/config/index.js' +import { resolveDatabaseUrl } from '@/config/database-url.js' +import { findConfigFile, loadStashConfig } from '@/config/index.js' import { downloadEqlSql, EQLInstaller, loadBundledEqlSql, } from '@/installer/index.js' import { ensureEncryptionClient } from './client-scaffold.js' -import { - ensureConfigDependencies, - ensureStashConfig, -} from './config-scaffold.js' +import { offerStashConfig } from './config-scaffold.js' import { detectDrizzle, detectSupabase, @@ -79,6 +77,40 @@ export interface InstallOptions { /** Resolved install mode for the Supabase non-Drizzle branch. */ export type SupabaseInstallMode = 'migration' | 'direct' +/** + * Resolve the database URL + encryption-client path for the install. + * + * A pre-existing `stash.config.ts` is authoritative — later workflow commands + * (db push / schema build / encrypt) load the client through it — so we load + * it. Without one, `eql install` doesn't need a config: resolve the URL + * directly (flag → env → supabase → prompt), then offer to scaffold a config + * for the rest of the workflow. Decoupling install from the config is what lets + * `npx stash eql install --database-url ...` run in a bare project without the + * `stash` / `@cipherstash/stack` dependencies the config would otherwise import + * (#579). + */ +async function resolveInstallContext( + options: InstallOptions, + s: ReturnType, +): Promise<{ databaseUrl: string; clientPath: string }> { + if (findConfigFile(process.cwd())) { + s.start('Loading stash.config.ts...') + const config = await loadStashConfig({ + databaseUrlFlag: options.databaseUrl, + supabase: options.supabase, + }) + s.stop('Configuration loaded.') + return { databaseUrl: config.databaseUrl, clientPath: config.client } + } + + const databaseUrl = await resolveDatabaseUrl({ + databaseUrlFlag: options.databaseUrl, + supabase: options.supabase, + }) + const clientPath = await offerStashConfig() + return { databaseUrl, clientPath } +} + export async function installCommand(options: InstallOptions) { p.intro(runnerCommand(detectPackageManager(), 'stash eql install')) @@ -93,41 +125,24 @@ export async function installCommand(options: InstallOptions) { process.exit(1) } - // Scaffold stash.config.ts if missing. `eql install` is the single command - // that gets a project from zero to installed EQL — no separate setup step - // (CIP-2986). - const configReady = await ensureStashConfig() - if (!configReady) { - process.exit(0) - } - - // The config (and the client it points at) `import` `stash` / - // `@cipherstash/stack`; make sure they're installed before jiti loads the - // config, so a standalone `npx stash eql install` gives actionable guidance - // instead of a raw `Cannot find module 'stash'` (#579). - const depsReady = await ensureConfigDependencies() - if (!depsReady) { - p.outro('Installation aborted.') - process.exit(1) - } - const s = p.spinner() - s.start('Loading stash.config.ts...') - const config = await loadStashConfig({ - databaseUrlFlag: options.databaseUrl, - supabase: options.supabase, - }) - s.stop('Configuration loaded.') + // `eql install` only needs a database URL to install EQL. It does NOT require + // a stash.config.ts: an existing config is authoritative (later workflow + // commands rely on it), but without one we resolve the URL directly — so a + // standalone `npx stash eql install --database-url ...` works with zero + // project dependencies — and offer to scaffold a config for the rest of the + // workflow (CIP-2986 / #579). + const { databaseUrl, clientPath } = await resolveInstallContext(options, s) // Safety net: if the user ran `eql install` without first running `init`, - // scaffold the encryption client file so config.client points somewhere - // real. No-op when the file already exists. - ensureEncryptionClient(config.client, process.cwd(), config.databaseUrl) + // scaffold the encryption client file so clientPath points somewhere real. + // No-op when the file already exists. + ensureEncryptionClient(clientPath, process.cwd(), databaseUrl) // Auto-detect provider hints when the user didn't explicitly pass flags. // CIP-2985. - const resolved = resolveProviderOptions(options, config.databaseUrl) + const resolved = resolveProviderOptions(options, databaseUrl) const eqlVersion: 2 | 3 = options.eqlVersion === '3' ? 3 : 2 @@ -196,7 +211,7 @@ export async function installCommand(options: InstallOptions) { } const installer = new EQLInstaller({ - databaseUrl: config.databaseUrl, + databaseUrl, }) s.start('Checking database permissions...') @@ -262,7 +277,7 @@ export async function installCommand(options: InstallOptions) { } s.start('Installing cs_migrations tracking schema...') - const migrationsDb = new pg.Client({ connectionString: config.databaseUrl }) + const migrationsDb = new pg.Client({ connectionString: databaseUrl }) try { await migrationsDb.connect() await installMigrationsSchema(migrationsDb) diff --git a/packages/cli/src/commands/init/utils.ts b/packages/cli/src/commands/init/utils.ts index 405e4bbd..7d8e3c4e 100644 --- a/packages/cli/src/commands/init/utils.ts +++ b/packages/cli/src/commands/init/utils.ts @@ -13,11 +13,8 @@ import type { Integration, SchemaDef } from './types.js' * `Cannot find module 'stash'` at the jiti import. Requiring the manifest * matches what Node's resolver actually needs to load the module. */ -export function isPackageInstalled( - packageName: string, - cwd: string = process.cwd(), -): boolean { - const modulePath = resolve(cwd, 'node_modules', packageName) +export function isPackageInstalled(packageName: string): boolean { + const modulePath = resolve(process.cwd(), 'node_modules', packageName) const manifestPath = resolve(modulePath, 'package.json') return existsSync(modulePath) && existsSync(manifestPath) } diff --git a/packages/cli/src/config/index.ts b/packages/cli/src/config/index.ts index 5f73b6f4..4e4352bd 100644 --- a/packages/cli/src/config/index.ts +++ b/packages/cli/src/config/index.ts @@ -91,9 +91,11 @@ const stashConfigSchema = z.object({ * Search for `stash.config.ts` starting from `startDir` and walking up * parent directories until the filesystem root is reached. * - * Returns the absolute path if found, or `undefined` if not. + * Returns the absolute path if found, or `undefined` if not. Exported so + * commands can branch on whether a config is present without loading it (e.g. + * `eql install` resolves the database URL directly when there's no config). */ -function findConfigFile(startDir: string): string | undefined { +export function findConfigFile(startDir: string): string | undefined { let dir = path.resolve(startDir) while (true) { diff --git a/packages/cli/tests/e2e/config-scaffold.e2e.test.ts b/packages/cli/tests/e2e/config-scaffold.e2e.test.ts index c8f74759..7a96989a 100644 --- a/packages/cli/tests/e2e/config-scaffold.e2e.test.ts +++ b/packages/cli/tests/e2e/config-scaffold.e2e.test.ts @@ -28,32 +28,32 @@ describe('config-scaffold DX (missing config / missing deps)', () => { }) it('a read command with no config points at `init` / `eql install` (#578)', async () => { + // `eql status` loads the encryption client, so it genuinely needs a config. const r = await run(['eql', 'status'], { cwd: tmpDir }) expect(r.output).toContain('Could not find stash.config.ts') expect(r.output).toContain('stash init') expect(r.output).toContain('stash eql install') }) - it('`eql install` with a config but missing deps guides instead of crashing (#579)', async () => { - fs.writeFileSync( - path.join(tmpDir, 'stash.config.ts'), - "import { defineConfig, resolveDatabaseUrl } from 'stash'\n" + - 'export default defineConfig({ databaseUrl: await resolveDatabaseUrl() })\n', - ) - + it('`eql install` does not require a config — it resolves the URL directly (#579)', async () => { + // With no config and no database URL, install must NOT scaffold a config and + // then crash on `Cannot find module 'stash'`. It resolves the URL first and + // fails cleanly asking for one; no stash.config.ts is written. const r = await run(['eql', 'install'], { cwd: tmpDir }) - // Actionable guidance, not a raw module-resolution stack trace. - expect(r.output).toContain('not installed') - expect(r.output).toContain('stash init') - expect(r.output).not.toContain('MODULE_NOT_FOUND') + + expect(r.output).toContain('Cannot resolve DATABASE_URL') + expect(r.output).toContain('--database-url') expect(r.output).not.toContain('Cannot find module') + expect(r.output).not.toContain('MODULE_NOT_FOUND') + expect(fs.existsSync(path.join(tmpDir, 'stash.config.ts'))).toBe(false) expect(r.exitCode).toBe(1) }) // The `loadStashConfig` catch that translates a missing-module error into - // guidance (the read-command path for #579) is covered by unit tests in - // src/__tests__/config.test.ts with a mocked jiti rejection. It can't be - // reproduced end-to-end here: inside the monorepo jiti resolves `stash` via - // the workspace self-reference even from a temp dir, so the import never - // fails — which is exactly why the original bug escaped the test suite. + // guidance (the path for a project that HAS a config but lacks the CLI + // packages) is covered by unit tests in src/__tests__/config.test.ts with a + // mocked jiti rejection. It can't be reproduced end-to-end here: inside the + // monorepo jiti resolves `stash` via the workspace self-reference even from a + // temp dir, so the import never fails — which is exactly why the original bug + // escaped the test suite. }) From 7321b53acfac437788797accae97907d6a536c83 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 8 Jul 2026 18:13:11 +1000 Subject: [PATCH 03/10] feat(cli): treat `eql install --database-url` as a one-shot (no scaffolding) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An explicit --database-url signals "install EQL against this DB now", so resolveInstallContext no longer scaffolds project files in that case — neither stash.config.ts nor the encryption client. Without the flag (URL from env / supabase / prompt) the config is still offered/created for the rest of the workflow. An existing config is honoured either way. --- .changeset/stash-cli-config-scaffold-dx.md | 6 ++-- packages/cli/src/commands/db/install.ts | 32 +++++++++++++------ .../cli/tests/e2e/config-scaffold.e2e.test.ts | 23 +++++++++++++ 3 files changed, 50 insertions(+), 11 deletions(-) diff --git a/.changeset/stash-cli-config-scaffold-dx.md b/.changeset/stash-cli-config-scaffold-dx.md index 17082a68..1360cac5 100644 --- a/.changeset/stash-cli-config-scaffold-dx.md +++ b/.changeset/stash-cli-config-scaffold-dx.md @@ -14,8 +14,10 @@ Fix two config-scaffold dead-ends in the CLI (#578, #579). That means a standalone `npx stash eql install --database-url ...` works in a bare project with **zero dependencies** — no more crash with a raw `Cannot find module 'stash'` from the config's `import`. An existing config is - still honoured (later workflow commands rely on it), and a fresh one is offered - as a convenience for the rest of the workflow rather than being a prerequisite. + still honoured (later workflow commands rely on it). An explicit + `--database-url` is treated as a one-shot install and leaves the project + untouched (no config or client scaffolded); without it, a config is offered as + a convenience for the rest of the workflow rather than being a prerequisite. - As a safety net, `loadStashConfig` translates a missing-module load failure (a project that *has* a config but lacks the CLI packages) into the same actionable guidance for every command, instead of a jiti/Node stack trace. diff --git a/packages/cli/src/commands/db/install.ts b/packages/cli/src/commands/db/install.ts index 2684acc8..8919e3cf 100644 --- a/packages/cli/src/commands/db/install.ts +++ b/packages/cli/src/commands/db/install.ts @@ -83,16 +83,20 @@ export type SupabaseInstallMode = 'migration' | 'direct' * A pre-existing `stash.config.ts` is authoritative — later workflow commands * (db push / schema build / encrypt) load the client through it — so we load * it. Without one, `eql install` doesn't need a config: resolve the URL - * directly (flag → env → supabase → prompt), then offer to scaffold a config - * for the rest of the workflow. Decoupling install from the config is what lets - * `npx stash eql install --database-url ...` run in a bare project without the - * `stash` / `@cipherstash/stack` dependencies the config would otherwise import - * (#579). + * directly (flag → env → supabase → prompt). Decoupling install from the config + * is what lets `npx stash eql install --database-url ...` run in a bare project + * without the `stash` / `@cipherstash/stack` dependencies the config would + * otherwise import (#579). + * + * An explicit `--database-url` signals a one-shot install against that database, + * so we do NOT scaffold project files (config or client) — `clientPath` comes + * back `null`. Without the flag we offer to scaffold a config (and return its + * client path) to set the project up for the rest of the workflow. */ async function resolveInstallContext( options: InstallOptions, s: ReturnType, -): Promise<{ databaseUrl: string; clientPath: string }> { +): Promise<{ databaseUrl: string; clientPath: string | null }> { if (findConfigFile(process.cwd())) { s.start('Loading stash.config.ts...') const config = await loadStashConfig({ @@ -107,6 +111,12 @@ async function resolveInstallContext( databaseUrlFlag: options.databaseUrl, supabase: options.supabase, }) + + // One-shot install (explicit --database-url): don't touch the project. + if (options.databaseUrl) { + return { databaseUrl, clientPath: null } + } + const clientPath = await offerStashConfig() return { databaseUrl, clientPath } } @@ -131,14 +141,18 @@ export async function installCommand(options: InstallOptions) { // a stash.config.ts: an existing config is authoritative (later workflow // commands rely on it), but without one we resolve the URL directly — so a // standalone `npx stash eql install --database-url ...` works with zero - // project dependencies — and offer to scaffold a config for the rest of the + // project dependencies. A one-shot `--database-url` run leaves the project + // untouched; otherwise we offer to scaffold a config for the rest of the // workflow (CIP-2986 / #579). const { databaseUrl, clientPath } = await resolveInstallContext(options, s) // Safety net: if the user ran `eql install` without first running `init`, // scaffold the encryption client file so clientPath points somewhere real. - // No-op when the file already exists. - ensureEncryptionClient(clientPath, process.cwd(), databaseUrl) + // No-op when the file already exists. Skipped for a one-shot `--database-url` + // install (clientPath === null), which leaves the project untouched. + if (clientPath) { + ensureEncryptionClient(clientPath, process.cwd(), databaseUrl) + } // Auto-detect provider hints when the user didn't explicitly pass flags. // CIP-2985. diff --git a/packages/cli/tests/e2e/config-scaffold.e2e.test.ts b/packages/cli/tests/e2e/config-scaffold.e2e.test.ts index 7a96989a..fe14d53b 100644 --- a/packages/cli/tests/e2e/config-scaffold.e2e.test.ts +++ b/packages/cli/tests/e2e/config-scaffold.e2e.test.ts @@ -49,6 +49,29 @@ describe('config-scaffold DX (missing config / missing deps)', () => { expect(r.exitCode).toBe(1) }) + it('`eql install --database-url` is one-shot — it never scaffolds project files', async () => { + // An explicit --database-url means "install EQL against this DB now"; it + // must not drop a stash.config.ts or an encryption client into the project. + // The bogus URL fails fast at connect (past the config stage), which is all + // we need to observe the no-scaffold behaviour. + const r = await run( + [ + 'eql', + 'install', + '--database-url', + 'postgres://u:p@127.0.0.1:1/db?connect_timeout=2', + ], + { cwd: tmpDir }, + ) + + expect(fs.existsSync(path.join(tmpDir, 'stash.config.ts'))).toBe(false) + expect( + fs.existsSync(path.join(tmpDir, 'src', 'encryption', 'index.ts')), + ).toBe(false) + expect(r.output).not.toContain('Created stash.config.ts') + expect(r.output).not.toContain('Cannot find module') + }) + // The `loadStashConfig` catch that translates a missing-module error into // guidance (the path for a project that HAS a config but lacks the CLI // packages) is covered by unit tests in src/__tests__/config.test.ts with a From d56e008f051d412061fd000dfbb143936134d94e Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 8 Jul 2026 18:22:06 +1000 Subject: [PATCH 04/10] docs(cli): document DATABASE_URL resolution priority order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the precedence discoverable in three places: - `resolveDatabaseUrl` gets a function-level JSDoc with the authoritative order (flag → DATABASE_URL env → supabase status → prompt → fail), plus the nuance that stash.config.ts is a container, not a separate tier — its default `databaseUrl: await resolveDatabaseUrl()` re-runs the same chain, so the flag wins even with a config present (except a hand-edited literal, which bypasses the resolver). The module header's duplicated list now points here. - The registry `--database-url` descriptor (feeds `stash manifest`). - The `stash --help` banner's DB / EQL Flags entry. --- packages/cli/src/cli/registry.ts | 2 +- packages/cli/src/config/database-url.ts | 59 +++++++++++++++---------- 2 files changed, 36 insertions(+), 25 deletions(-) diff --git a/packages/cli/src/cli/registry.ts b/packages/cli/src/cli/registry.ts index 9f4176b2..eee1ba7e 100644 --- a/packages/cli/src/cli/registry.ts +++ b/packages/cli/src/cli/registry.ts @@ -54,7 +54,7 @@ const DATABASE_URL_FLAG: Flag = { name: '--database-url', value: '', description: - 'Override DATABASE_URL for this run only — never written to disk.', + 'Database URL for this run only — never written to disk. Highest precedence in the resolution order: --database-url flag → DATABASE_URL env → supabase status → interactive prompt. A stash.config.ts is not a separate tier (its default databaseUrl re-runs this same chain).', env: 'DATABASE_URL', } diff --git a/packages/cli/src/config/database-url.ts b/packages/cli/src/config/database-url.ts index 94d66da6..7ffde5ab 100644 --- a/packages/cli/src/config/database-url.ts +++ b/packages/cli/src/config/database-url.ts @@ -1,6 +1,7 @@ /** - * Layered DATABASE_URL resolution. Called from inside the user's - * `stash.config.ts` via: + * Layered DATABASE_URL resolution. Called both directly by CLI commands (e.g. + * `eql install` when there's no config) and from inside the user's + * `stash.config.ts`: * * import { defineConfig, resolveDatabaseUrl } from 'stash' * export default defineConfig({ @@ -8,29 +9,14 @@ * }) * * The CLI's `loadStashConfig` wraps the jiti-import in - * `withResolverContext({ databaseUrlFlag, supabase })` (an - * `AsyncLocalStorage` scope) before evaluating the config file. Any - * `resolveDatabaseUrl()` call inside the file then sees those options - * via `als.getStore()` and walks: + * `withResolverContext({ databaseUrlFlag, supabase })` (an `AsyncLocalStorage` + * scope) before evaluating the config file, so a `resolveDatabaseUrl()` call + * inside the config picks up the CLI's flags via `als.getStore()`. See the + * {@link resolveDatabaseUrl} doc comment for the authoritative priority order. * - * 1. `--database-url ` flag (explicit override). - * 2. `process.env.DATABASE_URL` (shell, mise, direnv, dotenv files - * loaded by `bin/stash.ts`). - * 3. `supabase status --output env` → `DB_URL`, when `--supabase` is - * set OR a `supabase/config.toml` is detected. - * 4. Interactive `p.text` prompt (skipped under `CI=true` or non-TTY - * stdin). - * 5. Hard-fail with a source-naming error. - * - * Returns the resolved URL string. The CLI never mutates - * `process.env.DATABASE_URL` — the URL is only carried in the value - * `defineConfig` returns. The connection string is never persisted to - * disk; `stash.config.ts` references this function, not a literal. - * - * Concurrency: the ALS context is per-async-flow, so multiple - * concurrent `loadStashConfig` calls (e.g. parallel test cases or a - * programmatic batch invocation) each get isolated options without - * stepping on each other. + * Concurrency: the ALS context is per-async-flow, so multiple concurrent + * `loadStashConfig` calls (e.g. parallel test cases or a programmatic batch + * invocation) each get isolated options without stepping on each other. */ import { AsyncLocalStorage } from 'node:async_hooks' @@ -172,6 +158,31 @@ async function promptForUrl(cwd: string): Promise { * * Exits 1 when no source resolves a URL. */ +/** + * Resolve the database connection URL from the first available source, in + * strict priority order (first hit wins): + * + * 1. `--database-url ` flag (or `databaseUrlFlag` in the resolver + * context) — explicit, highest precedence. + * 2. `process.env.DATABASE_URL` — shell, mise/direnv, or a `.env*` file + * loaded at startup by `bin/stash.ts`. + * 3. `supabase status --output env` → `DB_URL` — only when `--supabase` is + * set OR a `supabase/config.toml` is detected. + * 4. Interactive prompt — skipped under CI / non-TTY stdin. + * 5. Hard-fail with a source-naming error. + * + * `stash.config.ts` is NOT a separate tier. The scaffolded config's field is + * `databaseUrl: await resolveDatabaseUrl()`, so loading a config just re-runs + * this same chain — and because `loadStashConfig` threads `--database-url` in + * via `withResolverContext`, the flag still wins even when a config is present. + * The one exception is a hand-edited config that assigns a literal + * `databaseUrl` string: that bypasses this resolver entirely, so the literal + * takes precedence over the flag and env. + * + * The resolved URL is returned only — never written to `process.env` or + * persisted to disk. `opts` override the ALS context + * (`{ ...als.getStore(), ...opts }`). + */ export async function resolveDatabaseUrl( opts: ResolveDatabaseUrlOptions = {}, ): Promise { From f7a5a32ee23cd13ec9875264b0c839cb771f6563 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 8 Jul 2026 19:04:27 +1000 Subject: [PATCH 05/10] fix(cli): `stash init` must still create a config (review #1 / #581) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The one-shot rule was keyed on `options.databaseUrl` being set, but that field is populated by two callers meaning different things: the CLI sets it only when `--database-url` is passed (a genuine one-shot), while `stash init` always sets it (a resolved URL, just to avoid re-prompting). So init tripped the one-shot branch and never scaffolded stash.config.ts — leaving every downstream command to dead-end on "Could not find stash.config.ts". Make the scaffold intent explicit and caller-owned instead of inferred: - InstallOptions gains `scaffoldConfig: 'ensure' | 'offer' | 'skip'`. - runInstall (CLI) passes 'skip' when --database-url is present, else 'offer'. - init's install-eql passes 'ensure' (create without a yes/no prompt). - resolveInstallContext switches on scaffoldConfig, not options.databaseUrl. - offerStashConfig gains an `ensure` mode; also folds the duplicated writeFileSync+log into a `writeStashConfig` helper. Tests: new install-eql.test.ts asserts init requests scaffoldConfig: 'ensure' (the exact wiring that regressed), plus an ensure-mode scaffold test. Fixed the stale install-eql comment (installCommand now scaffolds but no longer loads the config during init). --- packages/cli/src/bin/main.ts | 3 ++ .../db/__tests__/config-scaffold.test.ts | 15 +++++-- .../cli/src/commands/db/config-scaffold.ts | 42 +++++++++++------- packages/cli/src/commands/db/install.ts | 25 ++++++++--- .../init/steps/__tests__/install-eql.test.ts | 43 +++++++++++++++++++ .../src/commands/init/steps/install-eql.ts | 12 +++--- 6 files changed, 109 insertions(+), 31 deletions(-) create mode 100644 packages/cli/src/commands/init/steps/__tests__/install-eql.test.ts diff --git a/packages/cli/src/bin/main.ts b/packages/cli/src/bin/main.ts index 3bebf438..3a0ed517 100644 --- a/packages/cli/src/bin/main.ts +++ b/packages/cli/src/bin/main.ts @@ -190,6 +190,9 @@ async function runInstall( migrationsDir: values['migrations-dir'], eqlVersion: values['eql-version'], databaseUrl: values['database-url'], + // An explicit `--database-url` is a one-shot install against that DB — leave + // the project untouched. Otherwise offer to scaffold a config for later. + scaffoldConfig: values['database-url'] !== undefined ? 'skip' : 'offer', }) } diff --git a/packages/cli/src/commands/db/__tests__/config-scaffold.test.ts b/packages/cli/src/commands/db/__tests__/config-scaffold.test.ts index 0bfe470b..9b45e5b2 100644 --- a/packages/cli/src/commands/db/__tests__/config-scaffold.test.ts +++ b/packages/cli/src/commands/db/__tests__/config-scaffold.test.ts @@ -25,7 +25,7 @@ describe('offerStashConfig (optional config scaffold)', () => { it('non-interactively creates a config with the default client path', async () => { // Under vitest process.stdin.isTTY is undefined → the non-interactive branch // writes a config instead of prompting (which would hang in CI / agents). - const clientPath = await offerStashConfig(tmpDir) + const clientPath = await offerStashConfig({ cwd: tmpDir }) expect(clientPath).toBe(DEFAULT_CLIENT_PATH) const written = fs.readFileSync(path.join(tmpDir, CONFIG_FILENAME), 'utf-8') @@ -33,11 +33,20 @@ describe('offerStashConfig (optional config scaffold)', () => { expect(written).toContain(`client: '${DEFAULT_CLIENT_PATH}'`) }) + it('ensure mode creates the config (init path)', async () => { + // `ensure` is how `stash init` requests a config unconditionally; it must + // write one regardless of the prompt path. + const clientPath = await offerStashConfig({ ensure: true, cwd: tmpDir }) + + expect(clientPath).toBe(DEFAULT_CLIENT_PATH) + expect(fs.existsSync(path.join(tmpDir, CONFIG_FILENAME))).toBe(true) + }) + it('points the config at a detected client file when one exists', async () => { fs.mkdirSync(path.join(tmpDir, 'src')) fs.writeFileSync(path.join(tmpDir, 'src', 'encryption.ts'), '// client') - const clientPath = await offerStashConfig(tmpDir) + const clientPath = await offerStashConfig({ cwd: tmpDir }) expect(clientPath).toBe('./src/encryption.ts') expect( @@ -49,7 +58,7 @@ describe('offerStashConfig (optional config scaffold)', () => { const configPath = path.join(tmpDir, CONFIG_FILENAME) fs.writeFileSync(configPath, '// hand-written, do not touch') - const clientPath = await offerStashConfig(tmpDir) + const clientPath = await offerStashConfig({ cwd: tmpDir }) expect(clientPath).toBe(DEFAULT_CLIENT_PATH) expect(fs.readFileSync(configPath, 'utf-8')).toBe( diff --git a/packages/cli/src/commands/db/config-scaffold.ts b/packages/cli/src/commands/db/config-scaffold.ts index 72f23da7..3ebb990a 100644 --- a/packages/cli/src/commands/db/config-scaffold.ts +++ b/packages/cli/src/commands/db/config-scaffold.ts @@ -86,13 +86,23 @@ export default defineConfig({ ` } +/** Write the config with the given client path and report it. */ +function writeStashConfig(configPath: string, clientPath: string): string { + writeFileSync(configPath, generateConfig(clientPath), 'utf-8') + p.log.success(`Created ${CONFIG_FILENAME}`) + return clientPath +} + /** - * Offer to create a `stash.config.ts` for the rest of the workflow. + * Create a `stash.config.ts` for the rest of the workflow (`db push` / + * `schema build` / `encrypt *` load the encryption client through it). + * `eql install` itself doesn't need one — it resolves the database URL + * directly — so this is a setup convenience, never a blocker. * - * `eql install` itself doesn't need a config — it resolves the database URL - * directly — but `db push` / `schema build` / `encrypt *` load the encryption - * client through it, so we create it now as a convenience. Declining (or a - * non-interactive context) never blocks the EQL install. + * - `opts.ensure` (used by `stash init`, where the user has already committed + * to setup) creates the config without a yes/no prompt. + * - Otherwise, an interactive run offers to create it; a non-interactive run + * creates it with a detected/default client path rather than hanging. * * Returns the encryption-client path the config points at, falling back to * {@link DEFAULT_CLIENT_PATH} when no config is written — so the caller can @@ -102,19 +112,21 @@ export default defineConfig({ * one instead); it never overwrites a present `stash.config.ts`. */ export async function offerStashConfig( - cwd: string = process.cwd(), + opts: { ensure?: boolean; cwd?: string } = {}, ): Promise { + const cwd = opts.cwd ?? process.cwd() const configPath = resolve(cwd, CONFIG_FILENAME) if (existsSync(configPath)) return DEFAULT_CLIENT_PATH const isTTY = Boolean(process.stdin.isTTY) && process.env.CI !== 'true' - if (!isTTY) { - // Non-interactive (CI / agents / pipes): create with a detected or default - // client path rather than prompting, which would hang. - const clientPath = detectClientPath(cwd) ?? DEFAULT_CLIENT_PATH - writeFileSync(configPath, generateConfig(clientPath), 'utf-8') - p.log.success(`Created ${CONFIG_FILENAME}`) - return clientPath + + // `ensure` (init) and non-interactive contexts create without a yes/no + // prompt: init already committed to setup, and a non-TTY run can't prompt. + if (opts.ensure || !isTTY) { + return writeStashConfig( + configPath, + detectClientPath(cwd) ?? DEFAULT_CLIENT_PATH, + ) } const create = await p.confirm({ @@ -131,7 +143,5 @@ export async function offerStashConfig( const clientPath = await resolveClientPath(cwd) if (!clientPath) return DEFAULT_CLIENT_PATH - writeFileSync(configPath, generateConfig(clientPath), 'utf-8') - p.log.success(`Created ${CONFIG_FILENAME}`) - return clientPath + return writeStashConfig(configPath, clientPath) } diff --git a/packages/cli/src/commands/db/install.ts b/packages/cli/src/commands/db/install.ts index 8919e3cf..bfb70524 100644 --- a/packages/cli/src/commands/db/install.ts +++ b/packages/cli/src/commands/db/install.ts @@ -66,6 +66,15 @@ export interface InstallOptions { * never persisted. See `src/config/database-url.ts`. */ databaseUrl?: string + /** + * How to handle a missing `stash.config.ts` — the caller owns this intent + * rather than it being inferred from whether a URL was supplied: + * - `'ensure'` — create it without asking (`stash init`, where the user has + * already committed to setting the project up). + * - `'offer'` — offer to create it (plain `stash eql install`). Default. + * - `'skip'` — never scaffold (a one-shot `eql install --database-url ...`). + */ + scaffoldConfig?: 'ensure' | 'offer' | 'skip' /** * EQL generation to install: `'2'` (default, composite `eql_v2_encrypted`) * or `'3'` (native `eql_v3.*` domain schema). v3 currently supports the @@ -88,10 +97,12 @@ export type SupabaseInstallMode = 'migration' | 'direct' * without the `stash` / `@cipherstash/stack` dependencies the config would * otherwise import (#579). * - * An explicit `--database-url` signals a one-shot install against that database, - * so we do NOT scaffold project files (config or client) — `clientPath` comes - * back `null`. Without the flag we offer to scaffold a config (and return its - * client path) to set the project up for the rest of the workflow. + * Whether a missing config gets scaffolded is the caller's explicit intent + * ({@link InstallOptions.scaffoldConfig}), not inferred from whether a URL was + * supplied — `stash init` passes a resolved URL but still wants a config, while + * a one-shot `--database-url` run wants the project left untouched. When no + * config is created, `clientPath` comes back `null` so the caller skips the + * client scaffold too. */ async function resolveInstallContext( options: InstallOptions, @@ -112,12 +123,12 @@ async function resolveInstallContext( supabase: options.supabase, }) - // One-shot install (explicit --database-url): don't touch the project. - if (options.databaseUrl) { + const mode = options.scaffoldConfig ?? 'offer' + if (mode === 'skip') { return { databaseUrl, clientPath: null } } - const clientPath = await offerStashConfig() + const clientPath = await offerStashConfig({ ensure: mode === 'ensure' }) return { databaseUrl, clientPath } } diff --git a/packages/cli/src/commands/init/steps/__tests__/install-eql.test.ts b/packages/cli/src/commands/init/steps/__tests__/install-eql.test.ts new file mode 100644 index 00000000..46398e76 --- /dev/null +++ b/packages/cli/src/commands/init/steps/__tests__/install-eql.test.ts @@ -0,0 +1,43 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { InitProvider, InitState } from '../../types.js' + +// installCommand is the unit under test's collaborator — mock it so we assert +// what init asks for without touching a database. +vi.mock('../../../db/install.js', () => ({ installCommand: vi.fn() })) +// `stash` must appear installed so the precondition guard doesn't short-circuit. +vi.mock('../../utils.js', () => ({ isPackageInstalled: vi.fn(() => true) })) +// Auto-approve the "install EQL now?" prompt; no-op the rest of clack. +vi.mock('@clack/prompts', () => ({ + confirm: vi.fn(async () => true), + isCancel: vi.fn(() => false), + log: { info: vi.fn(), error: vi.fn(), success: vi.fn(), warn: vi.fn() }, + note: vi.fn(), +})) + +import { installCommand } from '../../../db/install.js' +import { installEqlStep } from '../install-eql.js' + +const baseState = { + integration: 'postgresql', + databaseUrl: 'postgresql://localhost:5432/app', +} as unknown as InitState +const provider = { name: 'postgresql' } as unknown as InitProvider + +describe('installEqlStep', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it("requests scaffoldConfig: 'ensure' so init still creates a stash.config.ts (#581 regression)", async () => { + // Regression guard: init passes a resolved databaseUrl only to avoid + // re-prompting. If installCommand treated a present databaseUrl as a + // one-shot `--database-url` run, init would finish with no config and every + // downstream command would dead-end on 'Could not find stash.config.ts'. + await installEqlStep.run(baseState, provider) + + expect(installCommand).toHaveBeenCalledTimes(1) + const opts = vi.mocked(installCommand).mock.calls[0][0] + expect(opts.scaffoldConfig).toBe('ensure') + expect(opts.databaseUrl).toBe('postgresql://localhost:5432/app') + }) +}) diff --git a/packages/cli/src/commands/init/steps/install-eql.ts b/packages/cli/src/commands/init/steps/install-eql.ts index 7dd48fb5..f054a423 100644 --- a/packages/cli/src/commands/init/steps/install-eql.ts +++ b/packages/cli/src/commands/init/steps/install-eql.ts @@ -62,11 +62,10 @@ export const installEqlStep: InitStep = { } // installCommand scaffolds stash.config.ts (which `import`s from `stash`) - // and immediately loads it via jiti. If `stash` isn't actually loadable - // from the project, that load throws `Cannot find module 'stash'` from - // deep inside jiti — confusing and fatal mid-flow. Detect the precondition - // and bail with a clear message instead. install-deps is what installs - // the package, so a "no" there leaves us here. + // for the rest of the workflow. `stash` must be installed or the config the + // user relies on next (db push / schema build / encrypt) can't load. Detect + // the precondition and bail with a clear message instead. install-deps is + // what installs the package, so a "no" there leaves us here. if (!isPackageInstalled('stash')) { p.log.error( '`stash` is not installed in this project. The previous step (install-deps) was skipped or failed. Re-run `stash init` and accept the dependency install when prompted, or install it manually:', @@ -83,6 +82,9 @@ export const installEqlStep: InitStep = { supabase: supabase || undefined, drizzle: drizzle || undefined, databaseUrl: state.databaseUrl, + // init passes a resolved URL to avoid re-prompting, but still wants a + // config scaffolded — this is NOT a one-shot `--database-url` run. + scaffoldConfig: 'ensure', }) } catch { // Don't echo the underlying error — Postgres client errors routinely From 9cbb7ff673367ceef5de628d7b10c724ffbb78de Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 8 Jul 2026 19:09:50 +1000 Subject: [PATCH 06/10] fix(cli): don't scaffold files the user didn't ask for (review #2, #4, #5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit offerStashConfig returned a truthy DEFAULT_CLIENT_PATH even when it created nothing, so installCommand's `if (clientPath)` guard scaffolded the encryption client anyway. Three related fixes, all on this one surface: - #2: return null when no config is created (declined / cancelled / existing), so the caller skips the client scaffold too. Declining now touches nothing. - #4: in 'offer' mode a non-interactive run no longer silently writes a config (+ client) into a bare project — those files import stash / @cipherstash/stack and would just re-defer the MODULE_NOT_FOUND crash to the next command. It returns null; the missing-config guidance points the user at `stash init`. ('ensure'/init still creates unconditionally; init installs the deps.) - #5: use the shared isCiEnv() (handles CI=1 / CI=TRUE) instead of the raw `process.env.CI !== 'true'`, matching the sibling resolveDatabaseUrl so the two steps classify the same environment consistently. Tests: offerStashConfig now asserts null + no write for the non-interactive offer and existing-config cases; a new e2e drives `eql install` with the URL from env (non-TTY) and asserts neither a config nor a client is written. --- .../db/__tests__/config-scaffold.test.ts | 35 ++++++++++--------- .../cli/src/commands/db/config-scaffold.ts | 34 ++++++++++-------- .../cli/tests/e2e/config-scaffold.e2e.test.ts | 19 ++++++++++ 3 files changed, 57 insertions(+), 31 deletions(-) diff --git a/packages/cli/src/commands/db/__tests__/config-scaffold.test.ts b/packages/cli/src/commands/db/__tests__/config-scaffold.test.ts index 9b45e5b2..c81b72d9 100644 --- a/packages/cli/src/commands/db/__tests__/config-scaffold.test.ts +++ b/packages/cli/src/commands/db/__tests__/config-scaffold.test.ts @@ -22,10 +22,10 @@ describe('offerStashConfig (optional config scaffold)', () => { } }) - it('non-interactively creates a config with the default client path', async () => { - // Under vitest process.stdin.isTTY is undefined → the non-interactive branch - // writes a config instead of prompting (which would hang in CI / agents). - const clientPath = await offerStashConfig({ cwd: tmpDir }) + it('ensure mode creates the config with the default client path (init path)', async () => { + // `ensure` is how `stash init` requests a config unconditionally; it must + // write one without prompting. + const clientPath = await offerStashConfig({ ensure: true, cwd: tmpDir }) expect(clientPath).toBe(DEFAULT_CLIENT_PATH) const written = fs.readFileSync(path.join(tmpDir, CONFIG_FILENAME), 'utf-8') @@ -33,20 +33,11 @@ describe('offerStashConfig (optional config scaffold)', () => { expect(written).toContain(`client: '${DEFAULT_CLIENT_PATH}'`) }) - it('ensure mode creates the config (init path)', async () => { - // `ensure` is how `stash init` requests a config unconditionally; it must - // write one regardless of the prompt path. - const clientPath = await offerStashConfig({ ensure: true, cwd: tmpDir }) - - expect(clientPath).toBe(DEFAULT_CLIENT_PATH) - expect(fs.existsSync(path.join(tmpDir, CONFIG_FILENAME))).toBe(true) - }) - - it('points the config at a detected client file when one exists', async () => { + it('ensure mode points the config at a detected client file when one exists', async () => { fs.mkdirSync(path.join(tmpDir, 'src')) fs.writeFileSync(path.join(tmpDir, 'src', 'encryption.ts'), '// client') - const clientPath = await offerStashConfig({ cwd: tmpDir }) + const clientPath = await offerStashConfig({ ensure: true, cwd: tmpDir }) expect(clientPath).toBe('./src/encryption.ts') expect( @@ -54,13 +45,23 @@ describe('offerStashConfig (optional config scaffold)', () => { ).toContain(`client: './src/encryption.ts'`) }) - it('never overwrites an existing config', async () => { + it('offer mode writes nothing and returns null in a non-interactive context (#2, #4)', async () => { + // Under vitest process.stdin.isTTY is undefined → non-interactive. Without + // `ensure`, offer must NOT silently drop a config into the project, and the + // null return is what makes the caller skip the client scaffold too. + const clientPath = await offerStashConfig({ cwd: tmpDir }) + + expect(clientPath).toBeNull() + expect(fs.existsSync(path.join(tmpDir, CONFIG_FILENAME))).toBe(false) + }) + + it('never overwrites an existing config (returns null)', async () => { const configPath = path.join(tmpDir, CONFIG_FILENAME) fs.writeFileSync(configPath, '// hand-written, do not touch') const clientPath = await offerStashConfig({ cwd: tmpDir }) - expect(clientPath).toBe(DEFAULT_CLIENT_PATH) + expect(clientPath).toBeNull() expect(fs.readFileSync(configPath, 'utf-8')).toBe( '// hand-written, do not touch', ) diff --git a/packages/cli/src/commands/db/config-scaffold.ts b/packages/cli/src/commands/db/config-scaffold.ts index 3ebb990a..24282481 100644 --- a/packages/cli/src/commands/db/config-scaffold.ts +++ b/packages/cli/src/commands/db/config-scaffold.ts @@ -1,6 +1,7 @@ import { existsSync, writeFileSync } from 'node:fs' import { resolve } from 'node:path' import * as p from '@clack/prompts' +import { isCiEnv } from '../../config/tty.js' import { detectPackageManager, runnerCommand } from '../init/utils.js' export const CONFIG_FILENAME = 'stash.config.ts' @@ -101,34 +102,39 @@ function writeStashConfig(configPath: string, clientPath: string): string { * * - `opts.ensure` (used by `stash init`, where the user has already committed * to setup) creates the config without a yes/no prompt. - * - Otherwise, an interactive run offers to create it; a non-interactive run - * creates it with a detected/default client path rather than hanging. + * - Otherwise it *offers* to create it interactively. A non-interactive run + * (CI / agents / pipes) can't prompt, so it does nothing rather than silently + * writing files that reference packages a bare project may not have installed. * - * Returns the encryption-client path the config points at, falling back to - * {@link DEFAULT_CLIENT_PATH} when no config is written — so the caller can - * still scaffold the client file at a sensible location. + * Returns the encryption-client path the config points at when a config was + * written, or `null` when nothing was created (declined, non-interactive, or an + * existing config) — so the caller skips the client scaffold too. * * Should be called only when no config exists yet (the caller loads an existing * one instead); it never overwrites a present `stash.config.ts`. */ export async function offerStashConfig( opts: { ensure?: boolean; cwd?: string } = {}, -): Promise { +): Promise { const cwd = opts.cwd ?? process.cwd() const configPath = resolve(cwd, CONFIG_FILENAME) - if (existsSync(configPath)) return DEFAULT_CLIENT_PATH + if (existsSync(configPath)) return null - const isTTY = Boolean(process.stdin.isTTY) && process.env.CI !== 'true' - - // `ensure` (init) and non-interactive contexts create without a yes/no - // prompt: init already committed to setup, and a non-TTY run can't prompt. - if (opts.ensure || !isTTY) { + // `ensure` (init) creates the config without asking — the user already + // committed to setup by running `stash init`. + if (opts.ensure) { return writeStashConfig( configPath, detectClientPath(cwd) ?? DEFAULT_CLIENT_PATH, ) } + // 'offer' mode. A non-interactive run can't prompt; don't write into the + // project unasked (that could drop files importing uninstalled packages) — + // the missing-config guidance points the user at `stash init` later. + const isTTY = Boolean(process.stdin.isTTY) && !isCiEnv() + if (!isTTY) return null + const create = await p.confirm({ message: `Create a ${CONFIG_FILENAME}? (needed later for db push / schema build / encrypt)`, initialValue: true, @@ -137,11 +143,11 @@ export async function offerStashConfig( p.log.info( `Skipped ${CONFIG_FILENAME}. Create it later with \`${runnerCommand(detectPackageManager(), 'stash init')}\`.`, ) - return DEFAULT_CLIENT_PATH + return null } const clientPath = await resolveClientPath(cwd) - if (!clientPath) return DEFAULT_CLIENT_PATH + if (!clientPath) return null return writeStashConfig(configPath, clientPath) } diff --git a/packages/cli/tests/e2e/config-scaffold.e2e.test.ts b/packages/cli/tests/e2e/config-scaffold.e2e.test.ts index fe14d53b..f343020a 100644 --- a/packages/cli/tests/e2e/config-scaffold.e2e.test.ts +++ b/packages/cli/tests/e2e/config-scaffold.e2e.test.ts @@ -72,6 +72,25 @@ describe('config-scaffold DX (missing config / missing deps)', () => { expect(r.output).not.toContain('Cannot find module') }) + it('non-interactive `eql install` (URL from env) writes no config or client (#2, #4)', async () => { + // No --database-url flag but DATABASE_URL in the env: offer mode, but a + // non-TTY run can't prompt. It must NOT silently scaffold a config (which + // imports `stash`) or a client (which imports `@cipherstash/stack`) into a + // bare project. offerStashConfig returns null and the client scaffold is + // skipped; the bogus URL then fails fast at connect. + const r = await run(['eql', 'install'], { + cwd: tmpDir, + env: { DATABASE_URL: 'postgres://u:p@127.0.0.1:1/db?connect_timeout=2' }, + }) + + expect(fs.existsSync(path.join(tmpDir, 'stash.config.ts'))).toBe(false) + expect( + fs.existsSync(path.join(tmpDir, 'src', 'encryption', 'index.ts')), + ).toBe(false) + expect(r.output).not.toContain('Created stash.config.ts') + expect(r.output).not.toContain('Cannot find module') + }) + // The `loadStashConfig` catch that translates a missing-module error into // guidance (the path for a project that HAS a config but lacks the CLI // packages) is covered by unit tests in src/__tests__/config.test.ts with a From 33c2cc6b863def0ab1aa8e9fcdab27cefc3a7a08 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 8 Jul 2026 19:17:40 +1000 Subject: [PATCH 07/10] fix(cli): actionable guidance when the client's @cipherstash/stack is missing (review #3, #6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The missing-package translation only wrapped the CONFIG load, which imports bare `stash` — so its `@cipherstash/stack` branch never fired. The package actually fails to resolve in the CLIENT file (imported by loadEncryptConfig, used by db push / schema build / encrypt), whose catch dumped a raw jiti stack trace. - #3: apply the translation to loadEncryptConfig too, so a missing @cipherstash/stack (incl. subpaths like @cipherstash/stack/schema) yields the same "install it / run stash init" guidance for every command, not a trace. - #6: replace the brittle substring matching (includes("'stash'")) with a shared, subpath-aware helper. New src/module-error.ts holds the primitives (isModuleNotFound + moduleNotFoundSpecifier's `Cannot find (module|package)` regex); native.ts now reuses them instead of its own copy, and a new config/missing-package.ts reduces a specifier to its package name before matching (so @cipherstash/stack/schema resolves to @cipherstash/stack). Tests: unit coverage for missingCipherStashPackage (subpath, non-matches, code gate) and a loadEncryptConfig test asserting the client-load path translates a missing @cipherstash/stack into guidance rather than a raw trace. --- packages/cli/src/__tests__/config.test.ts | 106 +++++++++++++----- .../config/__tests__/missing-package.test.ts | 55 +++++++++ packages/cli/src/config/index.ts | 78 +++---------- packages/cli/src/config/missing-package.ts | 57 ++++++++++ packages/cli/src/module-error.ts | 28 +++++ packages/cli/src/native.ts | 23 ++-- 6 files changed, 241 insertions(+), 106 deletions(-) create mode 100644 packages/cli/src/config/__tests__/missing-package.test.ts create mode 100644 packages/cli/src/config/missing-package.ts create mode 100644 packages/cli/src/module-error.ts diff --git a/packages/cli/src/__tests__/config.test.ts b/packages/cli/src/__tests__/config.test.ts index a820141a..d3094584 100644 --- a/packages/cli/src/__tests__/config.test.ts +++ b/packages/cli/src/__tests__/config.test.ts @@ -54,34 +54,37 @@ describe('loadStashConfig', () => { it.each([ ['stash', `Cannot find module 'stash'`], ['@cipherstash/stack', `Cannot find package '@cipherstash/stack'`], - ])('translates a missing `%s` module into actionable guidance (#579)', async (pkg, message) => { - fs.writeFileSync( - path.join(tmpDir, 'stash.config.ts'), - `import 'stash'\nexport default {}`, - ) - process.cwd = () => tmpDir - vi.spyOn(process, 'exit').mockImplementation(() => { - throw new Error('process.exit') - }) - const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) - - const moduleErr = Object.assign(new Error(message), { - code: 'MODULE_NOT_FOUND', - }) - const { createJiti } = await import('jiti') - vi.mocked(createJiti).mockReturnValue({ - import: vi.fn().mockRejectedValue(moduleErr), - } as never) - - const { loadStashConfig } = await import('@/config/index.ts') - await expect(loadStashConfig()).rejects.toThrow('process.exit') - - const output = errorSpy.mock.calls.map((c) => c.join(' ')).join('\n') - expect(output).toContain(`\`${pkg}\` is not installed`) - expect(output).toContain('stash init') - // The raw jiti stack trace must NOT be forwarded to the user. - expect(output).not.toContain('Failed to load') - }) + ])( + 'translates a missing `%s` module into actionable guidance (#579)', + async (pkg, message) => { + fs.writeFileSync( + path.join(tmpDir, 'stash.config.ts'), + `import 'stash'\nexport default {}`, + ) + process.cwd = () => tmpDir + vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit') + }) + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + + const moduleErr = Object.assign(new Error(message), { + code: 'MODULE_NOT_FOUND', + }) + const { createJiti } = await import('jiti') + vi.mocked(createJiti).mockReturnValue({ + import: vi.fn().mockRejectedValue(moduleErr), + } as never) + + const { loadStashConfig } = await import('@/config/index.ts') + await expect(loadStashConfig()).rejects.toThrow('process.exit') + + const output = errorSpy.mock.calls.map((c) => c.join(' ')).join('\n') + expect(output).toContain(`\`${pkg}\` is not installed`) + expect(output).toContain('stash init') + // The raw jiti stack trace must NOT be forwarded to the user. + expect(output).not.toContain('Failed to load') + }, + ) it('still surfaces the raw error for unrelated config load failures', async () => { fs.writeFileSync(path.join(tmpDir, 'stash.config.ts'), 'export default {}') @@ -148,3 +151,50 @@ describe('loadStashConfig', () => { }) }) }) + +describe('loadEncryptConfig', () => { + let tmpDir: string + let originalCwd: () => string + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'stash-encrypt-config-test-')) + originalCwd = process.cwd + }) + + afterEach(() => { + process.cwd = originalCwd + vi.restoreAllMocks() + if (tmpDir && fs.existsSync(tmpDir)) { + fs.rmSync(tmpDir, { recursive: true, force: true }) + } + }) + + it('translates a missing @cipherstash/stack in the client file into guidance (#3)', async () => { + // The client (not the config) is what imports @cipherstash/stack, incl. + // subpaths. This path used to dump a raw jiti stack trace. + fs.writeFileSync(path.join(tmpDir, 'client.ts'), '// encryption client') + process.cwd = () => tmpDir + vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit') + }) + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + + const moduleErr = Object.assign( + new Error("Cannot find module '@cipherstash/stack/schema'"), + { code: 'MODULE_NOT_FOUND' }, + ) + const { createJiti } = await import('jiti') + vi.mocked(createJiti).mockReturnValue({ + import: vi.fn().mockRejectedValue(moduleErr), + } as never) + + const { loadEncryptConfig } = await import('@/config/index.ts') + await expect(loadEncryptConfig('client.ts')).rejects.toThrow('process.exit') + + const output = errorSpy.mock.calls.map((c) => c.join(' ')).join('\n') + expect(output).toContain('`@cipherstash/stack` is not installed') + expect(output).toContain('stash init') + // Not the raw stack-trace path. + expect(output).not.toContain('Failed to load encrypt client') + }) +}) diff --git a/packages/cli/src/config/__tests__/missing-package.test.ts b/packages/cli/src/config/__tests__/missing-package.test.ts new file mode 100644 index 00000000..10b5eeb9 --- /dev/null +++ b/packages/cli/src/config/__tests__/missing-package.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it } from 'vitest' +import { missingCipherStashPackage } from '../missing-package.js' + +function moduleErr(message: string, code = 'MODULE_NOT_FOUND'): Error { + return Object.assign(new Error(message), { code }) +} + +describe('missingCipherStashPackage', () => { + it('detects a missing bare `stash` (the config import)', () => { + expect( + missingCipherStashPackage(moduleErr("Cannot find module 'stash'")), + ).toBe('stash') + }) + + it('detects a missing `@cipherstash/stack` (ESM "package" wording)', () => { + expect( + missingCipherStashPackage( + moduleErr( + "Cannot find package '@cipherstash/stack'", + 'ERR_MODULE_NOT_FOUND', + ), + ), + ).toBe('@cipherstash/stack') + }) + + it('reduces a subpath specifier to its package name (#3)', () => { + // The client imports subpaths like `@cipherstash/stack/schema`; the brittle + // substring match this replaced missed those. + expect( + missingCipherStashPackage( + moduleErr("Cannot find module '@cipherstash/stack/schema'"), + ), + ).toBe('@cipherstash/stack') + }) + + it('ignores a missing third-party package', () => { + expect( + missingCipherStashPackage(moduleErr("Cannot find module 'left-pad'")), + ).toBeUndefined() + }) + + it('ignores errors that are not module-not-found', () => { + expect( + missingCipherStashPackage( + moduleErr("Cannot find module 'stash'", 'EACCES'), + ), + ).toBeUndefined() + expect(missingCipherStashPackage(new SyntaxError('boom'))).toBeUndefined() + }) + + it('ignores non-Error values', () => { + expect(missingCipherStashPackage(undefined)).toBeUndefined() + expect(missingCipherStashPackage('boom')).toBeUndefined() + }) +}) diff --git a/packages/cli/src/config/index.ts b/packages/cli/src/config/index.ts index 4e4352bd..42df89e2 100644 --- a/packages/cli/src/config/index.ts +++ b/packages/cli/src/config/index.ts @@ -3,49 +3,15 @@ import path from 'node:path' import type { EncryptionClient } from '@cipherstash/stack/encryption' import type { EncryptConfig } from '@cipherstash/stack/schema' import { z } from 'zod' -import { - combinedInstallCommands, - detectPackageManager, - runnerCommand, -} from '../commands/init/utils.js' +import { detectPackageManager, runnerCommand } from '../commands/init/utils.js' import { type ResolveDatabaseUrlOptions, withResolverContext, } from './database-url.js' - -/** The npm packages a `stash.config.ts` (and the client it points at) import. */ -const CLI_PACKAGE = 'stash' -const STACK_PACKAGE = '@cipherstash/stack' - -/** - * When jiti fails to evaluate the config because a required CipherStash package - * isn't installed in the project, Node throws a `MODULE_NOT_FOUND` / - * `ERR_MODULE_NOT_FOUND`. Return the offending package name so the caller can - * print actionable guidance instead of a raw stack trace; return `undefined` - * for any other failure (which should surface as-is). - * - * This is the standalone `npx stash eql install` failure mode: the scaffolded - * config `import`s `stash`, which resolves only when the CLI packages were added - * as project dependencies (via `stash init`) — see #579. - */ -function missingConfigModule(error: unknown): string | undefined { - if (!(error instanceof Error)) return undefined - const code = (error as { code?: string }).code - if (code !== 'MODULE_NOT_FOUND' && code !== 'ERR_MODULE_NOT_FOUND') { - return undefined - } - // Check the scoped package first — `stash` is a substring of paths that - // mention it, but the quoted `'@cipherstash/stack'` is the specific specifier. - for (const pkg of [STACK_PACKAGE, CLI_PACKAGE]) { - if ( - error.message.includes(`'${pkg}'`) || - error.message.includes(`"${pkg}"`) - ) { - return pkg - } - } - return undefined -} +import { + missingCipherStashPackage, + reportMissingCipherStashPackage, +} from './missing-package.js' export interface StashConfig { /** PostgreSQL connection string */ @@ -171,30 +137,11 @@ To create it by hand, add ${CONFIG_FILENAME} to your project root: jiti.import(configPath, { default: true }), ) } catch (error) { - // A missing CipherStash package (the config `import`s `stash`; the client it - // points at `import`s `@cipherstash/stack`) is the common standalone-npx - // failure — translate jiti's raw `Cannot find module 'stash'` into - // actionable guidance instead of a stack trace (#579). - const missingPkg = missingConfigModule(error) - if (missingPkg) { - const pm = detectPackageManager() - const stash = runnerCommand(pm, 'stash') - const install = combinedInstallCommands( - pm, - [STACK_PACKAGE], - [CLI_PACKAGE], - ) - console.error( - `Error: ${CONFIG_FILENAME} could not load — \`${missingPkg}\` is not installed in this project. - -Install the CipherStash packages, then re-run: - ${install.join('\n ')} - -Or run \`${stash} init\` to set everything up. -`, - ) - process.exit(1) - } + // A missing CipherStash package (the config `import`s `stash`) is the common + // standalone-npx failure — translate jiti's raw `Cannot find module 'stash'` + // into actionable guidance instead of a stack trace (#579). + const missingPkg = missingCipherStashPackage(error) + if (missingPkg) reportMissingCipherStashPackage(missingPkg) console.error(`Error: Failed to load ${CONFIG_FILENAME} at ${configPath}\n`) console.error(error) process.exit(1) @@ -247,6 +194,11 @@ export async function loadEncryptConfig( // the user re-exports it as `default` or as a named binding. moduleExports = (await jiti.import(resolvedPath)) as Record } catch (error) { + // The client `import`s `@cipherstash/stack` (incl. subpaths). If that isn't + // installed, translate the raw jiti stack trace into the same actionable + // guidance the config load gives, rather than leaking it (#579 / review #3). + const missingPkg = missingCipherStashPackage(error) + if (missingPkg) reportMissingCipherStashPackage(missingPkg) console.error( `Error: Failed to load encrypt client file at ${resolvedPath}\n`, ) diff --git a/packages/cli/src/config/missing-package.ts b/packages/cli/src/config/missing-package.ts new file mode 100644 index 00000000..e2bb37c8 --- /dev/null +++ b/packages/cli/src/config/missing-package.ts @@ -0,0 +1,57 @@ +// Turn a raw jiti/Node `Cannot find module 'stash'` into actionable guidance. +// A `stash.config.ts` `import`s `stash`; the encryption client it points at +// `import`s `@cipherstash/stack` (incl. subpaths like `@cipherstash/stack/schema`). +// Both resolve only once those are project dependencies — via `stash init`, or a +// manual install — so a bare project (e.g. `npx stash eql install` before init) +// otherwise crashes with a stack trace. See #579. + +import { + combinedInstallCommands, + detectPackageManager, + runnerCommand, +} from '../commands/init/utils.js' +import { isModuleNotFound, moduleNotFoundSpecifier } from '../module-error.js' + +const CLI_PACKAGE = 'stash' +const STACK_PACKAGE = '@cipherstash/stack' + +/** Reduce a specifier (`@cipherstash/stack/schema`, `stash/foo`) to its package name. */ +function packageNameOf(specifier: string): string { + const parts = specifier.split('/') + return specifier.startsWith('@') ? parts.slice(0, 2).join('/') : parts[0] +} + +/** + * If `error` is a module-resolution failure for one of the CipherStash packages + * a config (`stash`) or its encryption client (`@cipherstash/stack`, incl. + * subpaths) imports, return that package name. Returns `undefined` for any other + * error, so the caller can surface it raw. + */ +export function missingCipherStashPackage(error: unknown): string | undefined { + if (!isModuleNotFound(error)) return undefined + const specifier = moduleNotFoundSpecifier(error) + if (!specifier) return undefined + const pkg = packageNameOf(specifier) + return pkg === CLI_PACKAGE || pkg === STACK_PACKAGE ? pkg : undefined +} + +/** + * Print actionable guidance for a missing CipherStash package and exit, instead + * of leaking jiti's raw stack trace. Shared by every command that jiti-loads the + * config or the encryption client. + */ +export function reportMissingCipherStashPackage(pkg: string): never { + const pm = detectPackageManager() + const stash = runnerCommand(pm, 'stash') + const install = combinedInstallCommands(pm, [STACK_PACKAGE], [CLI_PACKAGE]) + console.error( + `Error: \`${pkg}\` is not installed in this project. + +Install the CipherStash packages, then re-run: + ${install.join('\n ')} + +Or run \`${stash} init\` to set everything up. +`, + ) + process.exit(1) +} diff --git a/packages/cli/src/module-error.ts b/packages/cli/src/module-error.ts new file mode 100644 index 00000000..4f5f06c9 --- /dev/null +++ b/packages/cli/src/module-error.ts @@ -0,0 +1,28 @@ +// Shared helpers for Node module-resolution failures. A missing dependency +// surfaces as a `MODULE_NOT_FOUND` (CJS require) or `ERR_MODULE_NOT_FOUND` (ESM +// import) whose message names the unresolved specifier. These turn that raw +// error into structured data callers translate into guidance — missing native +// binaries in `native.ts`, missing CipherStash packages in +// `config/missing-package.ts`. + +/** A Node module-resolution error. */ +export interface ModuleError extends Error { + code?: string + requireStack?: string[] +} + +/** True when `err` is a CJS `MODULE_NOT_FOUND` or ESM `ERR_MODULE_NOT_FOUND`. */ +export function isModuleNotFound(err: unknown): err is ModuleError { + if (!(err instanceof Error)) return false + const code = (err as ModuleError).code + return code === 'MODULE_NOT_FOUND' || code === 'ERR_MODULE_NOT_FOUND' +} + +/** + * The quoted specifier from a module-not-found message, or `undefined`. CJS says + * "Cannot find module 'X'"; ESM says "Cannot find package 'X'". Handles subpath + * specifiers too (e.g. `@cipherstash/stack/schema`). + */ +export function moduleNotFoundSpecifier(err: ModuleError): string | undefined { + return /Cannot find (?:module|package) '([^']+)'/.exec(err.message)?.[1] +} diff --git a/packages/cli/src/native.ts b/packages/cli/src/native.ts index 732957fa..969b3547 100644 --- a/packages/cli/src/native.ts +++ b/packages/cli/src/native.ts @@ -15,11 +15,11 @@ import { type PackageManager, runnerCommand, } from './commands/init/utils.js' - -interface ModuleError extends Error { - code?: string - requireStack?: string[] -} +import { + isModuleNotFound, + type ModuleError, + moduleNotFoundSpecifier, +} from './module-error.js' // Matches the platform-suffixed optional package, e.g. // `@cipherstash/protect-ffi-darwin-arm64` or `@cipherstash/auth-linux-x64-gnu`. @@ -37,13 +37,8 @@ export function currentTarget(): string { * to a missing top-level package or any other module error. */ export function isNativeBinaryMissing(err: unknown): err is ModuleError { - if (!(err instanceof Error)) return false - const e = err as ModuleError - // CJS require throws `MODULE_NOT_FOUND`; ESM throws `ERR_MODULE_NOT_FOUND`. - if (e.code !== 'MODULE_NOT_FOUND' && e.code !== 'ERR_MODULE_NOT_FOUND') { - return false - } - const haystack = `${e.message}\n${(e.requireStack ?? []).join('\n')}` + if (!isModuleNotFound(err)) return false + const haystack = `${err.message}\n${(err.requireStack ?? []).join('\n')}` // A platform-suffixed @cipherstash package, or a failure surfaced from the // neon loader, both mean the optional native binary wasn't installed. return ( @@ -58,9 +53,7 @@ function missingModuleName(err: ModuleError): string | undefined { // `-` guess would miss. const pkg = PLATFORM_PKG.exec(haystack)?.[0] if (pkg) return pkg - // Fall back to the quoted name. CJS says "Cannot find module 'X'"; ESM - // (ERR_MODULE_NOT_FOUND) says "Cannot find package 'X'". - return /Cannot find (?:module|package) '([^']+)'/.exec(err.message)?.[1] + return moduleNotFoundSpecifier(err) } const LOCKFILE: Record = { From fecf6987cdde3c55e8467cb101fe4529b29b651c Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 8 Jul 2026 19:23:18 +1000 Subject: [PATCH 08/10] =?UTF-8?q?refactor(cli):=20review=20cleanups=20?= =?UTF-8?q?=E2=80=94=20dedupe=20default=20path,=20message,=20config=20walk?= =?UTF-8?q?=20(#7,=20#8,=20#10)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - #7: collapse three copies of the default encryption-client path (DEFAULT_ENCRYPT_CLIENT_PATH in config/index.ts, DEFAULT_CLIENT_PATH in config-scaffold.ts, and a private copy in build-schema.ts) into one exported DEFAULT_CLIENT_PATH in config/index.ts, imported everywhere — so the scaffold default and the Zod default can't drift. - #8: move the "`` is not installed" guidance into messages.ts (db.missingCipherStashPackage) per the assertion-stable-strings convention. - #10: loadStashConfig accepts an optional knownConfigPath; resolveInstallContext passes the path it already located so the cwd→root filesystem walk runs once instead of twice on the existing-config path. No behaviour change; message output is byte-identical. Full unit + e2e green. --- packages/cli/src/commands/db/config-scaffold.ts | 15 ++++++++------- packages/cli/src/commands/db/install.ts | 16 +++++++++++----- .../cli/src/commands/init/steps/build-schema.ts | 3 +-- packages/cli/src/config/index.ts | 15 ++++++++++++--- packages/cli/src/config/missing-package.ts | 9 ++------- packages/cli/src/messages.ts | 12 ++++++++++++ 6 files changed, 46 insertions(+), 24 deletions(-) diff --git a/packages/cli/src/commands/db/config-scaffold.ts b/packages/cli/src/commands/db/config-scaffold.ts index 24282481..c5a6439e 100644 --- a/packages/cli/src/commands/db/config-scaffold.ts +++ b/packages/cli/src/commands/db/config-scaffold.ts @@ -1,20 +1,21 @@ import { existsSync, writeFileSync } from 'node:fs' import { resolve } from 'node:path' import * as p from '@clack/prompts' +import { DEFAULT_CLIENT_PATH } from '../../config/index.js' import { isCiEnv } from '../../config/tty.js' import { detectPackageManager, runnerCommand } from '../init/utils.js' export const CONFIG_FILENAME = 'stash.config.ts' -/** Default encryption-client path used when the project has none yet. */ -export const DEFAULT_CLIENT_PATH = './src/encryption/index.ts' +// Re-exported so scaffold consumers (and their tests) have a single import site. +export { DEFAULT_CLIENT_PATH } /** * Common locations where an encryption client file might live. Checked in - * order of priority during auto-detection. + * order of priority during auto-detection — the default path leads. */ const COMMON_CLIENT_PATHS = [ - './src/encryption/index.ts', + DEFAULT_CLIENT_PATH, './src/encryption.ts', './encryption/index.ts', './encryption.ts', @@ -56,9 +57,9 @@ export async function resolveClientPath( const clientPath = await p.text({ message: 'Where is your encryption client file?', - placeholder: './src/encryption/index.ts', - defaultValue: './src/encryption/index.ts', - initialValue: detected ?? './src/encryption/index.ts', + placeholder: DEFAULT_CLIENT_PATH, + defaultValue: DEFAULT_CLIENT_PATH, + initialValue: detected ?? DEFAULT_CLIENT_PATH, validate(value) { if (!value || value.trim().length === 0) { return 'Client file path is required.' diff --git a/packages/cli/src/commands/db/install.ts b/packages/cli/src/commands/db/install.ts index bfb70524..523edd4e 100644 --- a/packages/cli/src/commands/db/install.ts +++ b/packages/cli/src/commands/db/install.ts @@ -108,12 +108,18 @@ async function resolveInstallContext( options: InstallOptions, s: ReturnType, ): Promise<{ databaseUrl: string; clientPath: string | null }> { - if (findConfigFile(process.cwd())) { + const configPath = findConfigFile(process.cwd()) + if (configPath) { s.start('Loading stash.config.ts...') - const config = await loadStashConfig({ - databaseUrlFlag: options.databaseUrl, - supabase: options.supabase, - }) + // Pass the path we already located so loadStashConfig doesn't re-walk the + // filesystem to find it. + const config = await loadStashConfig( + { + databaseUrlFlag: options.databaseUrl, + supabase: options.supabase, + }, + configPath, + ) s.stop('Configuration loaded.') return { databaseUrl: config.databaseUrl, clientPath: config.client } } diff --git a/packages/cli/src/commands/init/steps/build-schema.ts b/packages/cli/src/commands/init/steps/build-schema.ts index c6212a30..210dce16 100644 --- a/packages/cli/src/commands/init/steps/build-schema.ts +++ b/packages/cli/src/commands/init/steps/build-schema.ts @@ -1,6 +1,7 @@ import { existsSync, mkdirSync, writeFileSync } from 'node:fs' import { dirname, resolve } from 'node:path' import * as p from '@clack/prompts' +import { DEFAULT_CLIENT_PATH } from '../../../config/index.js' import { detectDrizzle, detectPrismaNext, @@ -17,8 +18,6 @@ import type { import { CancelledError } from '../types.js' import { generatePlaceholderClient } from '../utils.js' -const DEFAULT_CLIENT_PATH = './src/encryption/index.ts' - /** * Pick the integration template by reading the same signals `eql install` * uses — Drizzle config / dependency for `drizzle`, Supabase host in diff --git a/packages/cli/src/config/index.ts b/packages/cli/src/config/index.ts index 42df89e2..7103d7ba 100644 --- a/packages/cli/src/config/index.ts +++ b/packages/cli/src/config/index.ts @@ -44,13 +44,18 @@ export function defineConfig(config: StashConfig): StashConfig { const CONFIG_FILENAME = 'stash.config.ts' -const DEFAULT_ENCRYPT_CLIENT_PATH = './src/encryption/index.ts' +/** + * Default encryption-client path — the single source of truth for the location + * a scaffolded config points at and the Zod default when `client` is omitted. + * Imported by the scaffolder and the init schema builder so they can't drift. + */ +export const DEFAULT_CLIENT_PATH = './src/encryption/index.ts' const stashConfigSchema = z.object({ databaseUrl: z .string({ required_error: 'databaseUrl is required' }) .min(1, 'databaseUrl must not be empty'), - client: z.string().default(DEFAULT_ENCRYPT_CLIENT_PATH), + client: z.string().default(DEFAULT_CLIENT_PATH), }) /** @@ -95,12 +100,16 @@ export function findConfigFile(startDir: string): string | undefined { * command. This is how the CLI passes flag context into config * evaluation without mutating `process.env` or relying on globals. * + * `knownConfigPath` lets a caller that already located the config (via + * {@link findConfigFile}) skip a second cwd→root filesystem walk. + * * Exits with code 1 if the config file is not found or fails validation. */ export async function loadStashConfig( resolverOptions: ResolveDatabaseUrlOptions = {}, + knownConfigPath?: string, ): Promise { - const configPath = findConfigFile(process.cwd()) + const configPath = knownConfigPath ?? findConfigFile(process.cwd()) if (!configPath) { const stash = runnerCommand(detectPackageManager(), 'stash') diff --git a/packages/cli/src/config/missing-package.ts b/packages/cli/src/config/missing-package.ts index e2bb37c8..fc5665ec 100644 --- a/packages/cli/src/config/missing-package.ts +++ b/packages/cli/src/config/missing-package.ts @@ -10,6 +10,7 @@ import { detectPackageManager, runnerCommand, } from '../commands/init/utils.js' +import { messages } from '../messages.js' import { isModuleNotFound, moduleNotFoundSpecifier } from '../module-error.js' const CLI_PACKAGE = 'stash' @@ -45,13 +46,7 @@ export function reportMissingCipherStashPackage(pkg: string): never { const stash = runnerCommand(pm, 'stash') const install = combinedInstallCommands(pm, [STACK_PACKAGE], [CLI_PACKAGE]) console.error( - `Error: \`${pkg}\` is not installed in this project. - -Install the CipherStash packages, then re-run: - ${install.join('\n ')} - -Or run \`${stash} init\` to set everything up. -`, + `Error: ${messages.db.missingCipherStashPackage(pkg, install.join('\n '), stash)}\n`, ) process.exit(1) } diff --git a/packages/cli/src/messages.ts b/packages/cli/src/messages.ts index 9195e6b7..0e7e418a 100644 --- a/packages/cli/src/messages.ts +++ b/packages/cli/src/messages.ts @@ -96,5 +96,17 @@ export const messages = { /** Nudge shown after a prompt-sourced run completes. */ urlHint: (file: string) => `Set DATABASE_URL in ${file} to skip this prompt next time.`, + /** + * Shown when a `stash.config.ts` (or the encryption client it points at) + * can't load because a CipherStash package isn't installed. `installCommands` + * is the newline-joined install invocation and `stash` the runner-aware + * `stash` prefix (e.g. `npx stash`). + */ + missingCipherStashPackage: ( + pkg: string, + installCommands: string, + stash: string, + ) => + `\`${pkg}\` is not installed in this project.\n\nInstall the CipherStash packages, then re-run:\n ${installCommands}\n\nOr run \`${stash} init\` to set everything up.`, }, } as const From 7877bb4755c0ef254ed0a9ca8ea74aa803b7c1e0 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 8 Jul 2026 19:44:53 +1000 Subject: [PATCH 09/10] =?UTF-8?q?fix(cli):=20address=20PR=20review=20?= =?UTF-8?q?=E2=80=94=20dry-run=20scaffolding,=20test=20env=20isolation,=20?= =?UTF-8?q?doc=20caveat?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Dry run no longer mutates the project (CodeRabbit): `eql install --dry-run` reached offerStashConfig()/ensureEncryptionClient() before the dry-run guard, so a config or client file could be written despite "no changes will be made". resolveInstallContext now skips scaffolding under dryRun, and the client scaffold is guarded on `!options.dryRun` (an existing config still yields a clientPath). Adds an e2e that asserts no client file is written on a dry run. - Isolate DATABASE_URL in the "resolves URL directly" e2e (CodeRabbit): pass `env: { DATABASE_URL: '' }` so a value in the parent env can't leak in and resolve past the guidance the test asserts on. - Document the literal-databaseUrl exception in the --database-url help (Copilot): a hand-set literal `databaseUrl` in stash.config.ts bypasses the resolver and wins over the flag/env/etc. --- packages/cli/src/cli/registry.ts | 2 +- packages/cli/src/commands/db/install.ts | 10 ++++-- .../cli/tests/e2e/config-scaffold.e2e.test.ts | 35 ++++++++++++++++++- 3 files changed, 42 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/cli/registry.ts b/packages/cli/src/cli/registry.ts index eee1ba7e..eb7d3fa9 100644 --- a/packages/cli/src/cli/registry.ts +++ b/packages/cli/src/cli/registry.ts @@ -54,7 +54,7 @@ const DATABASE_URL_FLAG: Flag = { name: '--database-url', value: '', description: - 'Database URL for this run only — never written to disk. Highest precedence in the resolution order: --database-url flag → DATABASE_URL env → supabase status → interactive prompt. A stash.config.ts is not a separate tier (its default databaseUrl re-runs this same chain).', + 'Database URL for this run only — never written to disk. Highest precedence in the resolution order: --database-url flag → DATABASE_URL env → supabase status → interactive prompt. A stash.config.ts is not a separate tier (its default databaseUrl re-runs this same chain); a hand-set literal databaseUrl in the config bypasses the resolver and wins over all of these.', env: 'DATABASE_URL', } diff --git a/packages/cli/src/commands/db/install.ts b/packages/cli/src/commands/db/install.ts index 523edd4e..a5808692 100644 --- a/packages/cli/src/commands/db/install.ts +++ b/packages/cli/src/commands/db/install.ts @@ -129,8 +129,10 @@ async function resolveInstallContext( supabase: options.supabase, }) + // A dry run must not write scaffold files, so it never enters the scaffold + // path (nor does an explicit `skip`). const mode = options.scaffoldConfig ?? 'offer' - if (mode === 'skip') { + if (mode === 'skip' || options.dryRun) { return { databaseUrl, clientPath: null } } @@ -166,8 +168,10 @@ export async function installCommand(options: InstallOptions) { // Safety net: if the user ran `eql install` without first running `init`, // scaffold the encryption client file so clientPath points somewhere real. // No-op when the file already exists. Skipped for a one-shot `--database-url` - // install (clientPath === null), which leaves the project untouched. - if (clientPath) { + // install (clientPath === null) or a dry run, which leave the project + // untouched — an existing config still yields a clientPath, so guard on dryRun + // too. + if (clientPath && !options.dryRun) { ensureEncryptionClient(clientPath, process.cwd(), databaseUrl) } diff --git a/packages/cli/tests/e2e/config-scaffold.e2e.test.ts b/packages/cli/tests/e2e/config-scaffold.e2e.test.ts index f343020a..f874a317 100644 --- a/packages/cli/tests/e2e/config-scaffold.e2e.test.ts +++ b/packages/cli/tests/e2e/config-scaffold.e2e.test.ts @@ -39,7 +39,12 @@ describe('config-scaffold DX (missing config / missing deps)', () => { // With no config and no database URL, install must NOT scaffold a config and // then crash on `Cannot find module 'stash'`. It resolves the URL first and // fails cleanly asking for one; no stash.config.ts is written. - const r = await run(['eql', 'install'], { cwd: tmpDir }) + // Clear DATABASE_URL explicitly so a value in the parent env can't leak in + // and resolve past the guidance we're asserting on. + const r = await run(['eql', 'install'], { + cwd: tmpDir, + env: { DATABASE_URL: '' }, + }) expect(r.output).toContain('Cannot resolve DATABASE_URL') expect(r.output).toContain('--database-url') @@ -91,6 +96,34 @@ describe('config-scaffold DX (missing config / missing deps)', () => { expect(r.output).not.toContain('Cannot find module') }) + it('`eql install --dry-run` writes no files, even with an existing config', async () => { + // Dry run must not mutate the project. With an existing config pointing at a + // not-yet-created client, the client scaffold used to run before the dry-run + // guard — so assert the client file is NOT written. + fs.writeFileSync( + path.join(tmpDir, 'stash.config.ts'), + "import { defineConfig, resolveDatabaseUrl } from 'stash'\n" + + "export default defineConfig({ databaseUrl: await resolveDatabaseUrl(), client: './src/encryption/index.ts' })\n", + ) + + const r = await run( + [ + 'eql', + 'install', + '--dry-run', + '--database-url', + 'postgres://u:p@127.0.0.1:1/db?connect_timeout=2', + ], + { cwd: tmpDir }, + ) + + expect(r.output).toContain('Dry run') + expect( + fs.existsSync(path.join(tmpDir, 'src', 'encryption', 'index.ts')), + ).toBe(false) + expect(r.exitCode).toBe(0) + }) + // The `loadStashConfig` catch that translates a missing-module error into // guidance (the path for a project that HAS a config but lacks the CLI // packages) is covered by unit tests in src/__tests__/config.test.ts with a From edf4db25370e14f8efea28e3b6fe14ef8b88a1cc Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Wed, 8 Jul 2026 21:38:26 +1000 Subject: [PATCH 10/10] =?UTF-8?q?fix(cli):=20address=20James's=20PR=20#580?= =?UTF-8?q?=20review=20=E2=80=94=20one-shot=20install=20bypasses=20config?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four findings from @freshtonic's review: 1. `--database-url` was silently overridden by a parent-directory config with a literal `databaseUrl` (findConfigFile walks up). A one-shot install (scaffoldConfig 'skip') now bypasses config loading entirely, so the flag always wins and no parent config can redirect the install. For the other modes, warn when a config's literal databaseUrl overrides the flag. 2. One-shot `--database-url` no longer scaffolds the client file when a config already exists — the config branch is skipped, so the project is truly left untouched. Added a non-dry-run e2e covering existing-config + --database-url. 3. Removed the orphaned duplicate JSDoc block on resolveDatabaseUrl. 4. Extracted shared isInteractive() into config/tty.ts and routed the DATABASE_URL resolver, config scaffolder, and Supabase install-mode gate through it instead of re-deriving `isTTY && !CI` inline. --- .changeset/stash-cli-config-scaffold-dx.md | 12 ++++--- .../cli/src/commands/db/config-scaffold.ts | 5 ++- packages/cli/src/commands/db/install.ts | 35 ++++++++++++++++--- packages/cli/src/config/database-url.ts | 20 ++++------- packages/cli/src/config/tty.ts | 11 ++++++ .../cli/tests/e2e/config-scaffold.e2e.test.ts | 29 +++++++++++++++ 6 files changed, 85 insertions(+), 27 deletions(-) diff --git a/.changeset/stash-cli-config-scaffold-dx.md b/.changeset/stash-cli-config-scaffold-dx.md index 1360cac5..ade7f56d 100644 --- a/.changeset/stash-cli-config-scaffold-dx.md +++ b/.changeset/stash-cli-config-scaffold-dx.md @@ -13,11 +13,13 @@ Fix two config-scaffold dead-ends in the CLI (#578, #579). `supabase status` → prompt) instead of scaffolding a config and loading it. That means a standalone `npx stash eql install --database-url ...` works in a bare project with **zero dependencies** — no more crash with a raw - `Cannot find module 'stash'` from the config's `import`. An existing config is - still honoured (later workflow commands rely on it). An explicit - `--database-url` is treated as a one-shot install and leaves the project - untouched (no config or client scaffolded); without it, a config is offered as - a convenience for the rest of the workflow rather than being a prerequisite. + `Cannot find module 'stash'` from the config's `import`. A plain + `stash eql install` still honours an existing config (later workflow commands + rely on it) and offers to scaffold one otherwise. An explicit `--database-url` + is a one-shot install: it resolves that URL directly and leaves the project + untouched — no config or client is scaffolded, and an existing config is + bypassed so the flag can't be silently overridden by a hand-edited literal + `databaseUrl` (including one in a parent directory). - As a safety net, `loadStashConfig` translates a missing-module load failure (a project that *has* a config but lacks the CLI packages) into the same actionable guidance for every command, instead of a jiti/Node stack trace. diff --git a/packages/cli/src/commands/db/config-scaffold.ts b/packages/cli/src/commands/db/config-scaffold.ts index c5a6439e..8bec16da 100644 --- a/packages/cli/src/commands/db/config-scaffold.ts +++ b/packages/cli/src/commands/db/config-scaffold.ts @@ -2,7 +2,7 @@ import { existsSync, writeFileSync } from 'node:fs' import { resolve } from 'node:path' import * as p from '@clack/prompts' import { DEFAULT_CLIENT_PATH } from '../../config/index.js' -import { isCiEnv } from '../../config/tty.js' +import { isInteractive } from '../../config/tty.js' import { detectPackageManager, runnerCommand } from '../init/utils.js' export const CONFIG_FILENAME = 'stash.config.ts' @@ -133,8 +133,7 @@ export async function offerStashConfig( // 'offer' mode. A non-interactive run can't prompt; don't write into the // project unasked (that could drop files importing uninstalled packages) — // the missing-config guidance points the user at `stash init` later. - const isTTY = Boolean(process.stdin.isTTY) && !isCiEnv() - if (!isTTY) return null + if (!isInteractive()) return null const create = await p.confirm({ message: `Create a ${CONFIG_FILENAME}? (needed later for db push / schema build / encrypt)`, diff --git a/packages/cli/src/commands/db/install.ts b/packages/cli/src/commands/db/install.ts index a5808692..9cb013b9 100644 --- a/packages/cli/src/commands/db/install.ts +++ b/packages/cli/src/commands/db/install.ts @@ -11,6 +11,7 @@ import pg from 'pg' import { detectPackageManager, runnerCommand } from '@/commands/init/utils.js' import { resolveDatabaseUrl } from '@/config/database-url.js' import { findConfigFile, loadStashConfig } from '@/config/index.js' +import { isInteractive } from '@/config/tty.js' import { downloadEqlSql, EQLInstaller, @@ -103,12 +104,24 @@ export type SupabaseInstallMode = 'migration' | 'direct' * a one-shot `--database-url` run wants the project left untouched. When no * config is created, `clientPath` comes back `null` so the caller skips the * client scaffold too. + * + * A one-shot run (`mode === 'skip'`, set when `--database-url` is passed alone) + * bypasses config loading entirely: it must leave the project untouched, so it + * neither honours nor scaffolds a config or client. This also means the flag + * always wins — loading a config here could pick up a parent-directory config + * with a hand-edited literal `databaseUrl` that silently overrides the flag and + * installs EQL against the wrong database. */ async function resolveInstallContext( options: InstallOptions, s: ReturnType, ): Promise<{ databaseUrl: string; clientPath: string | null }> { - const configPath = findConfigFile(process.cwd()) + const mode = options.scaffoldConfig ?? 'offer' + + // A one-shot `--database-url` install leaves the project untouched: don't load + // an existing config (see doc comment re: parent-dir literal override) and + // don't scaffold. Resolve the URL directly and return a null clientPath. + const configPath = mode === 'skip' ? null : findConfigFile(process.cwd()) if (configPath) { s.start('Loading stash.config.ts...') // Pass the path we already located so loadStashConfig doesn't re-walk the @@ -121,6 +134,19 @@ async function resolveInstallContext( configPath, ) s.stop('Configuration loaded.') + + // A config with a hand-edited literal `databaseUrl` bypasses the resolver, + // so a `--database-url` flag would be silently ignored. Surface it rather + // than installing against a different database than the user asked for — + // especially since `findConfigFile` walks up into parent directories. + if ( + options.databaseUrl !== undefined && + config.databaseUrl !== options.databaseUrl.trim() + ) { + p.log.warn( + `Ignoring --database-url: ${configPath} sets an explicit databaseUrl that takes precedence. Installing against the config's database.`, + ) + } return { databaseUrl: config.databaseUrl, clientPath: config.client } } @@ -131,7 +157,6 @@ async function resolveInstallContext( // A dry run must not write scaffold files, so it never enters the scaffold // path (nor does an explicit `skip`). - const mode = options.scaffoldConfig ?? 'offer' if (mode === 'skip' || options.dryRun) { return { databaseUrl, clientPath: null } } @@ -680,12 +705,12 @@ async function resolveSupabaseInstallMode( options: InstallOptions, projectInfo: SupabaseProjectInfo, ): Promise { - const isTTY = Boolean(process.stdin.isTTY) && process.env.CI !== 'true' - const decided = chooseSupabaseInstallMode(options, projectInfo, isTTY) + const interactive = isInteractive() + const decided = chooseSupabaseInstallMode(options, projectInfo, interactive) if (decided !== null) { if ( - !isTTY && + !interactive && options.migration === undefined && options.direct === undefined ) { diff --git a/packages/cli/src/config/database-url.ts b/packages/cli/src/config/database-url.ts index 7ffde5ab..eade7999 100644 --- a/packages/cli/src/config/database-url.ts +++ b/packages/cli/src/config/database-url.ts @@ -27,7 +27,7 @@ import * as p from '@clack/prompts' import { detectSupabaseProject } from '../commands/db/detect.js' import { detectPackageManager, runnerCommand } from '../commands/init/utils.js' import { messages } from '../messages.js' -import { isCiEnv } from './tty.js' +import { isCiEnv, isInteractive } from './tty.js' export interface ResolveDatabaseUrlOptions { /** Value of `--database-url` if the user passed one. */ @@ -150,14 +150,6 @@ async function promptForUrl(cwd: string): Promise { return value.trim() } -/** - * Walk the resolution chain and return a usable DATABASE_URL. Reads - * options from the surrounding `withResolverContext` scope (set by the - * CLI before evaluating the config file); any explicit `opts` passed - * here override the scoped values. - * - * Exits 1 when no source resolves a URL. - */ /** * Resolve the database connection URL from the first available source, in * strict priority order (first hit wins): @@ -216,12 +208,12 @@ export async function resolveDatabaseUrl( } } - // 4. Interactive prompt — skipped in CI / non-TTY. `isCiEnv` accepts the - // common CI-truthy spellings (`true`, `1`, case-insensitive) since not every - // CI provider sets `CI=true` exactly; shared with the region resolver. + // 4. Interactive prompt — skipped in CI / non-TTY. `isInteractive` (shared + // with the config scaffolder and region resolver) accepts the common + // CI-truthy spellings (`true`, `1`, case-insensitive) since not every CI + // provider sets `CI=true` exactly. const isCi = isCiEnv() - const isInteractive = Boolean(process.stdin.isTTY) && !isCi - if (isInteractive) { + if (isInteractive()) { const fromPrompt = await promptForUrl(cwd) if (fromPrompt) { p.log.info(messages.db.urlResolvedFromPrompt) diff --git a/packages/cli/src/config/tty.ts b/packages/cli/src/config/tty.ts index e8c6cf44..09fd476a 100644 --- a/packages/cli/src/config/tty.ts +++ b/packages/cli/src/config/tty.ts @@ -13,3 +13,14 @@ export function isCiEnv(): boolean { const ciVar = process.env.CI?.trim() return ciVar !== undefined && /^(1|true)$/i.test(ciVar) } + +/** + * True when it's safe to show an interactive clack prompt: stdin is a TTY and + * we're not in CI. Every prompt gate (the DATABASE_URL resolver, the config + * scaffolder, the Supabase install-mode selector) must decide this the same + * way, so they all call here rather than re-deriving `isTTY && !CI` inline and + * drifting apart. + */ +export function isInteractive(): boolean { + return Boolean(process.stdin.isTTY) && !isCiEnv() +} diff --git a/packages/cli/tests/e2e/config-scaffold.e2e.test.ts b/packages/cli/tests/e2e/config-scaffold.e2e.test.ts index f874a317..6eba9e5b 100644 --- a/packages/cli/tests/e2e/config-scaffold.e2e.test.ts +++ b/packages/cli/tests/e2e/config-scaffold.e2e.test.ts @@ -77,6 +77,35 @@ describe('config-scaffold DX (missing config / missing deps)', () => { expect(r.output).not.toContain('Cannot find module') }) + it('`eql install --database-url` leaves an existing config untouched — no client scaffolded', async () => { + // One-shot install with an EXISTING stash.config.ts pointing at a + // not-yet-created client. The config branch must be bypassed entirely: no + // client file is written, and the `stash` import in the config never runs + // (so no `Cannot find module` in this bare project). The bogus URL fails + // fast at connect, past the config/scaffold stage. + fs.writeFileSync( + path.join(tmpDir, 'stash.config.ts'), + "import { defineConfig, resolveDatabaseUrl } from 'stash'\n" + + "export default defineConfig({ databaseUrl: await resolveDatabaseUrl(), client: './src/encryption/index.ts' })\n", + ) + + const r = await run( + [ + 'eql', + 'install', + '--database-url', + 'postgres://u:p@127.0.0.1:1/db?connect_timeout=2', + ], + { cwd: tmpDir }, + ) + + expect( + fs.existsSync(path.join(tmpDir, 'src', 'encryption', 'index.ts')), + ).toBe(false) + expect(r.output).not.toContain('Created stash.config.ts') + expect(r.output).not.toContain('Cannot find module') + }) + it('non-interactive `eql install` (URL from env) writes no config or client (#2, #4)', async () => { // No --database-url flag but DATABASE_URL in the env: offer mode, but a // non-TTY run can't prompt. It must NOT silently scaffold a config (which