Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions .changeset/honest-noninteractive-init.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
'stash': minor
---

`stash init` is honest non-interactively — it no longer reports success for a
setup that didn't fully complete.

- **Fails on version skew.** A non-interactive run can't reconcile an
already-installed `@cipherstash/*` package that's *older* than this CLI
expects (it won't mutate an install without consent), so instead of warning
and proceeding — scaffolding against mismatched packages and then claiming
success — it now refuses with a non-zero exit and the exact align command.
Interactive runs still offer to align. A *newer* install stays a warn (the
install is likely fine; update the CLI instead).
- **No false "Setup complete".** If the EQL extension isn't installed at the
end (and the integration isn't Prisma Next, which installs it via
`migration apply`), the summary reads "Setup incomplete" and init exits
non-zero, pointing at `stash eql install`.
- **Honest checkmarks.** The summary no longer claims "Database connection
verified" (init resolves a URL but doesn't open a connection) — it now says
"Database URL resolved" — and only shows "Encryption client scaffolded" when
a client was actually written (skipped for Prisma Next).
- **No false "skills loaded".** The agent handoff prompt only points at the
skills directory when skills were actually copied (a stripped build installs
none), instead of telling the agent to read files that aren't there.
11 changes: 9 additions & 2 deletions packages/cli/src/commands/impl/steps/handoff-claude.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,17 @@ export const handoffClaudeStep: HandoffStep = {
writeArtifacts(cwd, state, 'claude-code', installed)

const mode = state.mode ?? 'implement'
// Only point the agent at the skills dir when skills were actually copied.
// A stripped CLI build (no bundled skills) returns [] — claiming they're
// there would send the agent to read files that don't exist.
const skillsClause =
installed.length > 0
? `The installed skills under ${CLAUDE_SKILLS_DIR}/ have the rules; `
: ''
const launchPrompt =
mode === 'plan'
? `Read ${SETUP_PROMPT_REL_PATH} and produce the planning deliverable it describes. The installed skills under ${CLAUDE_SKILLS_DIR}/ have the rules; ${CONTEXT_REL_PATH} has the project facts. Do not edit code or run mutating commands during this phase.`
: `Read ${SETUP_PROMPT_REL_PATH} and complete the setup steps. The installed skills under ${CLAUDE_SKILLS_DIR}/ have the rules; ${CONTEXT_REL_PATH} has the project facts.`
? `Read ${SETUP_PROMPT_REL_PATH} and produce the planning deliverable it describes. ${skillsClause}${CONTEXT_REL_PATH} has the project facts. Do not edit code or run mutating commands during this phase.`
: `Read ${SETUP_PROMPT_REL_PATH} and complete the setup steps. ${skillsClause}${CONTEXT_REL_PATH} has the project facts.`
Comment on lines +41 to +48

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Make all generated handoff guidance honor the installed-skills state.

The launch prompts now conditionally omit skill-directory references, but the setup prompt they always load still references those directories when no skills were installed.

  • packages/cli/src/commands/impl/steps/handoff-claude.ts#L41-L48: make the empty-skills setup prompt valid for Claude; it currently also references an AGENTS.md file this handoff does not write.
  • packages/cli/src/commands/impl/steps/handoff-codex.ts#L60-L67: omit .codex/skills/ from generated guidance when installed.length === 0.
📍 Affects 2 files
  • packages/cli/src/commands/impl/steps/handoff-claude.ts#L41-L48 (this comment)
  • packages/cli/src/commands/impl/steps/handoff-codex.ts#L60-L67
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/cli/src/commands/impl/steps/handoff-claude.ts` around lines 41 - 48,
Make all generated handoff guidance respect the installed-skills state: in
handoff-claude.ts, update the setup prompt loaded by the launchPrompt flow so
the empty-skills variant does not reference skill directories or the unwritten
AGENTS.md file, while retaining valid skill guidance when skills exist; in
handoff-codex.ts, update its generated guidance so .codex/skills/ is omitted
when installed.length is zero.


if (!state.agents?.cli.claudeCode) {
p.note(
Expand Down
11 changes: 9 additions & 2 deletions packages/cli/src/commands/impl/steps/handoff-codex.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,17 @@ export const handoffCodexStep: HandoffStep = {
writeArtifacts(cwd, state, 'codex', installed)

const mode = state.mode ?? 'implement'
// Only reference the skills dir when skills were actually copied (a
// stripped build returns []); the durable rules live in AGENTS.md either
// way, so the prompt stays useful without them.
const skillsClause =
installed.length > 0
? `the skills under ${CODEX_SKILLS_DIR}/ have the API details; `
: ''
const launchPrompt =
mode === 'plan'
? `Read ${SETUP_PROMPT_REL_PATH} and produce the planning deliverable it describes. AGENTS.md has the durable rules; the skills under ${CODEX_SKILLS_DIR}/ have the API details; ${CONTEXT_REL_PATH} has the project facts. Do not edit code or run mutating commands during this phase.`
: `Read ${SETUP_PROMPT_REL_PATH} and complete the setup steps. AGENTS.md has the durable rules; the skills under ${CODEX_SKILLS_DIR}/ have the API details; ${CONTEXT_REL_PATH} has the project facts.`
? `Read ${SETUP_PROMPT_REL_PATH} and produce the planning deliverable it describes. AGENTS.md has the durable rules; ${skillsClause}${CONTEXT_REL_PATH} has the project facts. Do not edit code or run mutating commands during this phase.`
: `Read ${SETUP_PROMPT_REL_PATH} and complete the setup steps. AGENTS.md has the durable rules; ${skillsClause}${CONTEXT_REL_PATH} has the project facts.`

if (!state.agents?.cli.codex) {
p.note(
Expand Down
47 changes: 46 additions & 1 deletion packages/cli/src/commands/init/__tests__/init-command.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import * as p from '@clack/prompts'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { CliExit } from '../../../cli/exit.js'
import { messages } from '../../../messages.js'
import type { InitState } from '../types.js'

// `--region` is the non-interactive escape hatch for `stash init`; it must land
Expand All @@ -9,6 +12,10 @@ import type { InitState } from '../types.js'
// (`authenticate`, `install-deps`) out of the fast suite.
const authRun = vi.hoisted(() => vi.fn(async (state: InitState) => state))
const passthrough = { run: async (s: InitState) => s }
// Controllable so the honest-summary tests can vary whether EQL installed.
const eqlRun = vi.hoisted(() =>
vi.fn(async (s: InitState) => ({ ...s, eqlInstalled: true })),
)

vi.mock('../steps/authenticate.js', () => ({
authenticateStep: { id: 'authenticate', name: 'Authenticate', run: authRun },
Expand All @@ -26,7 +33,10 @@ vi.mock('../steps/install-deps.js', () => ({
installDepsStep: { id: 'install-deps', ...passthrough },
}))
vi.mock('../steps/install-eql.js', () => ({
installEqlStep: { id: 'install-eql', ...passthrough },
// A successful init installs EQL — the default mark keeps the honest-summary
// gate (`eqlPending` → exit 1) happy for the region-threading runs. The
// honest-summary tests override `eqlRun` per case.
installEqlStep: { id: 'install-eql', run: eqlRun },
}))
vi.mock('../steps/gather-context.js', () => ({
gatherContextStep: { id: 'gather-context', ...passthrough },
Expand Down Expand Up @@ -70,3 +80,38 @@ describe('initCommand — region threading', () => {
expect(stateArg.regionFlag).toBeUndefined()
})
})

describe('initCommand — honest summary', () => {
it('exits non-zero and reports "Setup incomplete" when EQL was not installed', async () => {
eqlRun.mockImplementationOnce(async (s: InitState) => ({
...s,
eqlInstalled: false,
}))

await expect(initCommand({}, {})).rejects.toBeInstanceOf(CliExit)
// The summary titles the run as incomplete, and the EQL fix is surfaced.
expect(vi.mocked(p.note)).toHaveBeenCalledWith(
expect.any(String),
messages.init.setupIncomplete,
)
expect(vi.mocked(p.log.error)).toHaveBeenCalledWith(
expect.stringContaining(messages.init.eqlNotInstalled),
)
})

it('completes (no throw) when EQL was not installed but the integration is prisma-next', async () => {
// Prisma Next installs EQL via `migration apply`, so eqlInstalled=false is
// expected there and must NOT be treated as an incomplete setup.
eqlRun.mockImplementationOnce(async (s: InitState) => ({
...s,
integration: 'prisma-next',
eqlInstalled: false,
}))

await expect(initCommand({}, {})).resolves.toBeUndefined()
expect(vi.mocked(p.note)).toHaveBeenCalledWith(
expect.any(String),
'Setup complete',
)
})
})
27 changes: 25 additions & 2 deletions packages/cli/src/commands/init/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as p from '@clack/prompts'
import { CliExit } from '../../cli/exit.js'
import { messages } from '../../messages.js'
import { HANDOFF_CHOICES } from '../impl/steps/how-to-proceed.js'
import { planCommand } from '../plan/index.js'
import { createBaseProvider } from './providers/base.js'
Expand Down Expand Up @@ -99,17 +100,39 @@ export async function initCommand(

const pm = detectPackageManager()
const cli = runnerCommand(pm, 'stash')
// Only claim what actually happened. Auth throws on failure (reaching here
// means it succeeded); the database step *resolves* a URL but never opens a
// connection, so don't claim "verified"; the client scaffold is skipped for
// Prisma Next (no `clientFilePath` on state).
const checkmarks: string[] = [
'✓ Authenticated to CipherStash',
'✓ Database connection verified',
'✓ Encryption client scaffolded',
'✓ Database URL resolved',
]
if (state.clientFilePath) {
checkmarks.push('✓ Encryption client scaffolded')
}
if (state.stackInstalled) {
checkmarks.push('✓ `@cipherstash/stack` installed')
}
if (state.cliInstalled) checkmarks.push('✓ `stash` CLI installed')
if (state.eqlInstalled) checkmarks.push('✓ EQL extension installed')

// EQL is required for encryption. Prisma Next installs it via `migration
// apply` (so `eqlInstalled` is false by design there); every other
// integration needs it installed here. If it's missing, setup is NOT
// complete — say so and exit non-zero so automation can't read a false
// success from a run where encryption would fail at query time.
const eqlPending =
!state.eqlInstalled && state.integration !== 'prisma-next'
if (eqlPending) {
checkmarks.push('✗ EQL extension NOT installed')
p.note(checkmarks.join('\n'), messages.init.setupIncomplete)
p.log.error(
`${messages.init.eqlNotInstalled} Run \`${cli} eql install\` before running any encryption.`,
)
throw new CliExit(1)
}

p.note(checkmarks.join('\n'), 'Setup complete')

// Offer to chain straight into `stash plan` so first-time users don't
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { CliExit } from '../../../../cli/exit.js'
import type { InitProvider, InitState } from '../../types.js'

const execSyncMock = vi.hoisted(() => vi.fn())
Expand Down Expand Up @@ -199,15 +200,20 @@ describe('installDepsStep', () => {
)
})

it('non-interactive: warns on skew, prints align commands, never mutates', async () => {
it('non-interactive: REFUSES on skew (exit 1), prints align commands, never mutates', async () => {
vi.mocked(isInteractive).mockReturnValue(false)
present('@cipherstash/stack', 'stash')
resolvedVersions({
'@cipherstash/stack': '0.19.0',
stash: FIXTURE_VERSIONS.stash,
})

const result = await installDepsStep.run(baseState, provider)
// M4: a non-interactive run can't reconcile a `behind` skew (won't mutate
// without consent), so it refuses with a non-zero exit rather than
// proceeding and reporting a false success.
await expect(
installDepsStep.run(baseState, provider),
).rejects.toBeInstanceOf(CliExit)

expect(p.log.warn).toHaveBeenCalledWith(
expect.stringContaining('@cipherstash/stack: installed 0.19.0'),
Expand All @@ -217,9 +223,8 @@ describe('installDepsStep', () => {
expect.stringContaining('npm install @cipherstash/stack@9.9.9-test.1'),
'Version skew',
)
// Never mutates — the #661/#666 principle survives the refusal.
expect(execSyncMock).not.toHaveBeenCalled()
expect(result.stackInstalled).toBe(true)
expect(result.cliInstalled).toBe(true)
})

it('a NEWER install gets an update-stash warning, never a downgrade command', async () => {
Expand Down Expand Up @@ -284,7 +289,11 @@ describe('installDepsStep', () => {
stash: FIXTURE_VERSIONS.stash,
})

await installDepsStep.run(baseState, provider)
// An unreadable manifest classifies as `behind` skew, so a non-interactive
// run refuses (exit 1) after warning — same as any other skew.
await expect(
installDepsStep.run(baseState, provider),
).rejects.toBeInstanceOf(CliExit)

expect(p.log.warn).toHaveBeenCalledWith(
expect.stringContaining(
Expand Down
27 changes: 24 additions & 3 deletions packages/cli/src/commands/init/steps/install-deps.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { execSync } from 'node:child_process'
import * as p from '@clack/prompts'
import { CliExit } from '../../../cli/exit.js'
import { isInteractive } from '../../../config/tty.js'
import { messages } from '../../../messages.js'
import {
compareVersions,
expectedVersion,
Expand Down Expand Up @@ -146,9 +148,12 @@ function splitProdDev(packages: readonly string[]): {
* before any prompt or early exit, so no path (decline, partial failure,
* everything-already-present) proceeds silently on a stale or placeholder
* install. Interactively, init offers to align the skewed packages to this
* release in the same confirm as the missing installs; non-interactively it
* NEVER mutates an existing install — it warns, prints the exact align
* commands, and proceeds.
* release in the same confirm as the missing installs. Non-interactively it
* still NEVER mutates an existing install without consent — but rather than
* proceeding on a `behind` skew (which would scaffold against packages older
* than this CLI expects and then report a false success), it REFUSES with a
* non-zero exit and the exact align commands (M4). An `ahead` skew is not
* fatal — the install is likely fine and the fix is updating the CLI.
*
* When everything is already present at matching versions this logs a
* success line and moves on with no prompts.
Expand Down Expand Up @@ -193,6 +198,22 @@ export const installDepsStep: InitStep = {
p.log.warn(`Version skew detected:\n ${skewLines(skewed)}`)
}

// A non-interactive run can't reconcile a `behind` skew: it won't mutate an
// existing install without consent (the #661/#666 rule), so proceeding
// would scaffold config/client against packages older than this CLI
// expects and then report a false success. Refuse with a non-zero exit and
// the exact align commands, instead of warning-and-continuing. (Interactive
// runs still offer to align — see below. `ahead` skew is handled
// separately: the install is likely fine, so it warns and proceeds.)
if (skewed.length > 0 && !isInteractive()) {
p.note(
`Align these packages, then re-run init:\n ${alignCommands.join('\n ')}`,
'Version skew',
)
p.log.error(messages.init.skewNonInteractive)
throw new CliExit(1)
}

// What's missing outright (pinned, prod/dev split).
const missing: string[] = []
if (!stackPresent) missing.push(STACK_PACKAGE)
Expand Down
11 changes: 11 additions & 0 deletions packages/cli/src/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,17 @@ export const messages = {
nameRequiresValue: '--name requires a value',
unexpectedArgument: 'Unexpected argument',
},
init: {
/**
* Honest non-interactive init. These exit non-zero so automation never
* reads a false success — the e2e suite asserts on the leaders.
*/
setupIncomplete: 'Setup incomplete',
eqlNotInstalled: 'EQL is not installed — encryption queries will fail.',
/** Shown when a non-interactive run hits version skew it won't reconcile. */
skewNonInteractive:
'Version skew on already-installed packages — refusing to proceed non-interactively. Align the packages (below) and re-run, or run init interactively.',
},
telemetry: {
/**
* The one-time first-run notice. Printed to stderr so it never pollutes
Expand Down
2 changes: 1 addition & 1 deletion skills/stash-cli/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ npx stash init # PostgreSQL / Drizzle / Prisma
npx stash init --supabase # Supabase
```

`stash init` installs the CLI as a project dev dependency, so subsequent commands can drop the `npx`. The CLI is package-manager aware — before init, use whichever one-shot runner your project uses (`npx`, `pnpm dlx`, `bunx`, `yarn dlx`). Installs are **pinned to the exact `@cipherstash/*` versions this CLI release shipped with** (never bare dist-tags, which can lag behind a release), and init warns if an already-installed package's resolved version differs from the release's. Treat that warning as a real problem — but the fix depends on direction, and init says which applies: an **older** install should be aligned to the release (init offers the exact command); a **newer** install must NOT be downgraded — update the `stash` CLI to the matching release instead (init prints that command too).
`stash init` installs the CLI as a project dev dependency, so subsequent commands can drop the `npx`. The CLI is package-manager aware — before init, use whichever one-shot runner your project uses (`npx`, `pnpm dlx`, `bunx`, `yarn dlx`). Installs are **pinned to the exact `@cipherstash/*` versions this CLI release shipped with** (never bare dist-tags, which can lag behind a release), and init flags any already-installed `@cipherstash/*` package whose resolved version differs from the release's. The fix depends on direction, and init says which applies: an **older** install should be aligned to the release (init offers the exact command); a **newer** install must NOT be downgraded — update the `stash` CLI to the matching release instead (init prints that command too). **Non-interactively, an older ("behind") skew is fatal** — init refuses with a non-zero exit and the align command rather than scaffolding against mismatched packages and reporting a false success. Interactively it offers to align. Likewise, if the EQL extension isn't installed at the end (and the integration isn't Prisma Next, which installs it via `migration apply`), init reports **"Setup incomplete"** and exits non-zero — it never claims a setup is complete when encryption would fail at query time.

**If you are an agent, do this first:**

Expand Down
Loading