diff --git a/packages/cli/AGENTS.md b/packages/cli/AGENTS.md index 2adbd5c6..0a358728 100644 --- a/packages/cli/AGENTS.md +++ b/packages/cli/AGENTS.md @@ -19,10 +19,10 @@ Update `tests/e2e/**` whenever you: - Add or rename a top-level command, subcommand, or flag (smoke tests assert on help text, command names, and unknown-command behavior). - Change the user-facing string for an exit message that an existing E2E - asserts on (e.g. "Setup cancelled.", "Unknown auth command:", - "not yet implemented"). Either update the assertion or, preferably, route - the string through the future `messages.ts` module (see - `docs/plans/cli-pty-integration-tests.md`, phase 2). + asserts on (e.g. cancellation text, "Unknown auth command", the + `db migrate` stub warning). Strings that tests assert on live in + `src/messages.ts` — update the constant there and both prod and tests + pick it up. *Don't* hard-code the new wording in a test. - Touch `src/bin/stash.ts` argv parsing, exit codes, or top-level error handling. - Add a new clack prompt that changes the *first* prompt rendered for a @@ -65,11 +65,13 @@ exercise the same code paths. `selectRegion()` runs before any network I/O. Don't move the cancel assertion to a command that hits the auth server or DB before the first prompt — flaky. -- **Don't assert on full prompt strings if avoidable.** Prefer stable - substrings. Phase 2 (planned) introduces a `messages.ts` module so test - assertions can import handles and survive copy changes; until then, - assert on the most stable fragment ("Select a region", not the full - rendered prompt frame). +- **Use `src/messages.ts` for assertion-stable strings.** The module is a + single typed `as const` object grouping copy by area (`cli`, `auth`, + `db`). Prod call sites import the same constants the tests do, so a copy + tweak only needs to land in one place. Add to `messages.ts` only when a + test actually asserts on the string — premature extraction is worse + than copy-paste here. For literals tests don't touch (e.g. command + names like `init`, `db install`), keep them inline. - **Telemetry.** The CLI source no longer imports `posthog-node` (analytics moved to `packages/wizard`). The dep is still listed in `package.json` and should be removed in a follow-up. If you re-introduce telemetry to diff --git a/packages/cli/src/bin/stash.ts b/packages/cli/src/bin/stash.ts index fa31d2bc..39e2513d 100644 --- a/packages/cli/src/bin/stash.ts +++ b/packages/cli/src/bin/stash.ts @@ -23,6 +23,7 @@ import { testConnectionCommand, upgradeCommand, } from '../commands/index.js' +import { messages } from '../messages.js' function isModuleNotFound(err: unknown): boolean { return ( @@ -54,9 +55,9 @@ const pkg = JSON.parse( ) const HELP = ` -CipherStash CLI v${pkg.version} +${messages.cli.versionBannerPrefix}${pkg.version} -Usage: npx @cipherstash/cli [options] +${messages.cli.usagePrefix} [options] Commands: init Initialize CipherStash for your project @@ -192,10 +193,10 @@ async function runDbCommand( await testConnectionCommand() break case 'migrate': - p.log.warn('"npx @cipherstash/cli db migrate" is not yet implemented.') + p.log.warn(messages.db.migrateNotImplemented) break default: - p.log.error(`Unknown db subcommand: ${sub ?? '(none)'}`) + p.log.error(`${messages.db.unknownSubcommand}: ${sub ?? '(none)'}`) console.log() console.log(HELP) process.exit(1) @@ -256,7 +257,7 @@ async function main() { await envCommand({ write: flags.write }) break default: - console.error(`Unknown command: ${command}\n`) + console.error(`${messages.cli.unknownCommand}: ${command}\n`) console.log(HELP) process.exit(1) } diff --git a/packages/cli/src/commands/auth/index.ts b/packages/cli/src/commands/auth/index.ts index 0d7e511e..e52358b3 100644 --- a/packages/cli/src/commands/auth/index.ts +++ b/packages/cli/src/commands/auth/index.ts @@ -1,7 +1,8 @@ +import { messages } from '../../messages.js' import { bindDevice, login, selectRegion } from './login.js' const HELP = ` -Usage: npx @cipherstash/cli auth [options] +${messages.auth.usagePrefix} [options] Commands: login Authenticate with CipherStash @@ -44,7 +45,7 @@ export async function authCommand( } break default: - console.error(`Unknown auth command: ${subcommand}\n`) + console.error(`${messages.auth.unknownSubcommand}: ${subcommand}\n`) console.log(HELP) process.exit(1) } diff --git a/packages/cli/src/commands/auth/login.ts b/packages/cli/src/commands/auth/login.ts index 0833ac4a..95e1848c 100644 --- a/packages/cli/src/commands/auth/login.ts +++ b/packages/cli/src/commands/auth/login.ts @@ -1,5 +1,6 @@ import auth from '@cipherstash/auth' import * as p from '@clack/prompts' +import { messages } from '../../messages.js' const { beginDeviceCodeFlow, bindClientDevice } = auth // TODO: pull from the CTS API @@ -15,12 +16,12 @@ export const regions = [ export async function selectRegion(): Promise { const region = await p.select({ - message: 'Select a region', + message: messages.auth.selectRegion, options: regions, }) if (p.isCancel(region)) { - p.cancel('Cancelled.') + p.cancel(messages.auth.cancelled) process.exit(0) } diff --git a/packages/cli/src/messages.ts b/packages/cli/src/messages.ts new file mode 100644 index 00000000..75238656 --- /dev/null +++ b/packages/cli/src/messages.ts @@ -0,0 +1,29 @@ +/** + * User-facing message handles for strings that E2E tests assert on. + * + * Production code imports these instead of inlining literals so that copy + * tweaks (rename, rephrase, capitalisation) only need to land in one place + * and tests stay green automatically. + * + * Scope: only strings the E2E suite asserts on. Inline strings that no test + * depends on stay inline — premature extraction is worse than copy-paste + * here. See `packages/cli/AGENTS.md` for guidance on what to add. + */ +export const messages = { + cli: { + versionBannerPrefix: 'CipherStash CLI v', + usagePrefix: 'Usage: npx @cipherstash/cli', + unknownCommand: 'Unknown command', + }, + auth: { + usagePrefix: 'Usage: npx @cipherstash/cli auth', + unknownSubcommand: 'Unknown auth command', + selectRegion: 'Select a region', + cancelled: 'Cancelled.', + }, + db: { + unknownSubcommand: 'Unknown db subcommand', + migrateNotImplemented: + '"npx @cipherstash/cli db migrate" is not yet implemented.', + }, +} as const diff --git a/packages/cli/tests/e2e/auth-login-cancel.e2e.test.ts b/packages/cli/tests/e2e/auth-login-cancel.e2e.test.ts index e456dfd7..2a25ae2b 100644 --- a/packages/cli/tests/e2e/auth-login-cancel.e2e.test.ts +++ b/packages/cli/tests/e2e/auth-login-cancel.e2e.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from 'vitest' +import { messages } from '../../src/messages.js' import { render } from '../helpers/pty.js' describe('stash auth login — interactive cancel', () => { @@ -7,12 +8,12 @@ describe('stash auth login — interactive cancel', () => { // First clack prompt — `selectRegion()` runs synchronously before any // network activity, so this is a deterministic assertion target. - await r.waitFor('Select a region') + await r.waitFor(messages.auth.selectRegion) r.key('CtrlC') const { exitCode } = await r.exit expect(exitCode).toBe(0) - expect(r.output).toContain('Cancelled.') + expect(r.output).toContain(messages.auth.cancelled) }) }) diff --git a/packages/cli/tests/e2e/smoke.e2e.test.ts b/packages/cli/tests/e2e/smoke.e2e.test.ts index fd7d51f7..c80aee6b 100644 --- a/packages/cli/tests/e2e/smoke.e2e.test.ts +++ b/packages/cli/tests/e2e/smoke.e2e.test.ts @@ -2,6 +2,7 @@ import { readFileSync } from 'node:fs' import { dirname, resolve } from 'node:path' import { fileURLToPath } from 'node:url' import { describe, expect, it } from 'vitest' +import { messages } from '../../src/messages.js' import { render } from '../helpers/pty.js' const __dirname = dirname(fileURLToPath(import.meta.url)) @@ -14,8 +15,10 @@ describe('stash CLI — non-interactive smoke', () => { const r = render(['--help']) const { exitCode } = await r.exit expect(exitCode).toBe(0) - expect(r.output).toContain('CipherStash CLI v') - expect(r.output).toContain('Usage: npx @cipherstash/cli') + expect(r.output).toContain(messages.cli.versionBannerPrefix) + expect(r.output).toContain(messages.cli.usagePrefix) + // Command-list items — these are the literal command names users type, not + // copy strings, so they stay inline. expect(r.output).toContain('init') expect(r.output).toContain('db install') }) @@ -31,18 +34,18 @@ describe('stash CLI — non-interactive smoke', () => { const r = render(['definitely-not-a-command']) const { exitCode } = await r.exit expect(exitCode).toBe(1) - // Assert the stable phrase and the user-supplied token separately so a - // copy tweak to the surrounding wording doesn't break the test. - expect(r.output).toContain('Unknown command') + // Stable phrase + user-supplied token asserted separately so a copy tweak + // around the wording doesn't break the test. + expect(r.output).toContain(messages.cli.unknownCommand) expect(r.output).toContain('definitely-not-a-command') - expect(r.output).toContain('Usage: npx @cipherstash/cli') + expect(r.output).toContain(messages.cli.usagePrefix) }) it('auth with no subcommand prints auth help and exits 0', async () => { const r = render(['auth']) const { exitCode } = await r.exit expect(exitCode).toBe(0) - expect(r.output).toContain('Usage: npx @cipherstash/cli auth') + expect(r.output).toContain(messages.auth.usagePrefix) expect(r.output).toContain('login') }) @@ -50,7 +53,7 @@ describe('stash CLI — non-interactive smoke', () => { const r = render(['auth', 'bogus-sub']) const { exitCode } = await r.exit expect(exitCode).toBe(1) - expect(r.output).toContain('Unknown auth command') + expect(r.output).toContain(messages.auth.unknownSubcommand) expect(r.output).toContain('bogus-sub') }) @@ -58,7 +61,7 @@ describe('stash CLI — non-interactive smoke', () => { const r = render(['db', 'bogus-sub']) const { exitCode } = await r.exit expect(exitCode).toBe(1) - expect(r.output).toContain('Unknown db subcommand') + expect(r.output).toContain(messages.db.unknownSubcommand) expect(r.output).toContain('bogus-sub') }) @@ -66,6 +69,6 @@ describe('stash CLI — non-interactive smoke', () => { const r = render(['db', 'migrate']) const { exitCode } = await r.exit expect(exitCode).toBe(0) - expect(r.output).toContain('not yet implemented') + expect(r.output).toContain(messages.db.migrateNotImplemented) }) })