feat(cli): honest non-interactive init — fail on skew, no false success (rc.2 M4)#687
Conversation
🦋 Changeset detectedLatest commit: 20e5df9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthrough
ChangesInit outcome handling
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant initCommand
participant installDepsStep
participant installEqlStep
participant installCommand
participant AgentHandoff
User->>initCommand: run stash init
initCommand->>installDepsStep: check dependency versions
installDepsStep-->>initCommand: dependency state or CliExit
initCommand->>installEqlStep: install EQL
installEqlStep->>installCommand: request installation
installCommand-->>installEqlStep: installed or migration-generated
installEqlStep-->>initCommand: EQL state
initCommand->>AgentHandoff: build prompt from installed skills
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@packages/cli/src/commands/impl/steps/handoff-claude.ts`:
- Around line 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f8da61ce-6475-4788-b2d3-6295b53bb730
📒 Files selected for processing (9)
.changeset/honest-noninteractive-init.mdpackages/cli/src/commands/impl/steps/handoff-claude.tspackages/cli/src/commands/impl/steps/handoff-codex.tspackages/cli/src/commands/init/__tests__/init-command.test.tspackages/cli/src/commands/init/index.tspackages/cli/src/commands/init/steps/__tests__/install-deps.test.tspackages/cli/src/commands/init/steps/install-deps.tspackages/cli/src/messages.tsskills/stash-cli/SKILL.md
…ss (rc.2 M4) Non-interactive `stash init` reported success for setups that didn't fully complete. Three honesty gaps closed: 1. Version skew. A non-interactive run can't reconcile a `behind` skew (it won't mutate an install without consent), so instead of warning-and- proceeding — scaffolding against packages older than this CLI expects and then claiming success — it now REFUSES with a non-zero exit (CliExit(1)) and the exact align command. The #661/#666 no-mutation principle is preserved (execSync still not called); only the post-warning action changes from proceed to fail. `ahead` skew stays a warn (install likely fine; update the CLI). 2. False 'Setup complete'. The summary header and the auth/db/scaffold checkmarks were unconditional. Now: when EQL is required but not installed (eqlInstalled=false AND integration !== prisma-next), the summary reads 'Setup incomplete' and init exits non-zero, pointing at `stash eql install`. 'Database connection verified' → 'Database URL resolved' (init resolves a URL, never opens a connection). 'Encryption client scaffolded' is shown only when a client was written (clientFilePath set; skipped for prisma-next). 3. False 'skills loaded'. The Claude/Codex handoff launch prompt referenced the skills dir unconditionally; a stripped build installs no skills, so it told the agent to read files that aren't there. The clause is now conditional on installSkills() actually copying something. Tests: install-deps skew tests updated to assert the refusal (still never mutates); new init-command honest-summary tests (EQL-missing → exit 1 + 'Setup incomplete'; prisma-next eqlInstalled=false → completes). Skill + doc comments updated. Suite 542 unit / 62 e2e green, code:check clean. Changeset: stash minor. Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
27520d6 to
fb77e14
Compare
There was a problem hiding this comment.
Pull request overview
This PR makes stash init reliable for automation by ensuring non-interactive runs never report success when setup is incomplete or potentially inconsistent with the CLI’s expected runtime package versions.
Changes:
- Make non-interactive init refuse (exit non-zero) on behind version skew instead of warning-and-proceeding.
- Make the init summary truthful: “Database URL resolved” (not “connection verified”), “Setup incomplete” + exit non-zero when EQL is missing (except Prisma Next), and only reference skills when they were actually installed.
- Update unit coverage, changeset, and the published
stash-cliskill documentation to match the new behavior.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| skills/stash-cli/SKILL.md | Documents the new “fail on behind skew” and “setup incomplete” non-interactive behavior. |
| packages/cli/src/messages.ts | Adds stable message leaders for non-interactive init failure modes. |
| packages/cli/src/commands/init/steps/install-deps.ts | Refuses non-interactively on behind skew (throws CliExit(1)), while keeping ahead skew as warn-and-proceed. |
| packages/cli/src/commands/init/steps/tests/install-deps.test.ts | Updates skew tests to assert refusal + no mutation in non-interactive mode. |
| packages/cli/src/commands/init/index.ts | Adjusts checkmark copy, adds EQL-missing “Setup incomplete” gate + non-zero exit (except Prisma Next). |
| packages/cli/src/commands/init/tests/init-command.test.ts | Adds unit tests asserting the honest-summary behavior and Prisma Next exception. |
| packages/cli/src/commands/impl/steps/handoff-codex.ts | Makes the “skills” clause in the launch prompt conditional on actual skill installation. |
| packages/cli/src/commands/impl/steps/handoff-claude.ts | Makes the “skills” clause in the launch prompt conditional on actual skill installation. |
| .changeset/honest-noninteractive-init.md | Adds release notes describing the non-interactive honesty changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Two "only claim what actually happened" gaps found in review: - Copilot: the "✓ Encryption client scaffolded" checkmark was gated on `state.clientFilePath`, which build-schema sets even when it KEEPS an existing file (writes nothing). Gate on `state.schemaGenerated` (true only when a placeholder was written); show "✓ Encryption client kept (existing file)" for the keep path. Prisma Next (no clientFilePath) still shows neither. - CodeRabbit: the setup-prompt "Skills loaded" section pointed at skill directories and `AGENTS.md` regardless of what the handoff wrote — so a stripped build (no skills) sent the agent to read files that don't exist, and the Claude handoff (which never writes AGENTS.md) named it as the doctrine source. New `skillsLoadedLines(handoff, installedSkills)` renders the section honestly: no skills → point only at what was written (AGENTS.md for codex/agents-md, the docs for claude-code); claude-code → doctrine is in the installed skills, not AGENTS.md. Also hedged "list the skills loaded (if any)". Added setup-prompt honesty tests (empty-skills × handoff) and de-duped the two identical inline blocks. 557 tests green; build + biome clean. Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
Differential Review: PR #687Agent: Codex/gpt-5.6-sol Executive Summary
Overall risk: High Recommendation: Changes requested. The new completion gate still reports a Key metrics:
What ChangedCommit range: Timeline: 2026-07-17 to 2026-07-18
FindingsHigh: Drizzle still exits successfully before EQL is installedFile: Commit: Blast radius: Every Test coverage: Missing end-to-end state coverage. The new init test injects The new gate trusts
This preserves the exact false-success contract the PR is intended to remove. Concrete scenario:
Recommendation: Make Test Coverage AnalysisAll 557 CLI unit tests and all 65 CLI E2E tests passed. The CLI package build The important uncovered transition is:
Current tests cover each endpoint with mocks, which is why the contradictory Blast Radius Analysis
Historical Context
RecommendationsBlocking
Follow-up
Analysis MethodologyStrategy: Focused review of a large TypeScript repository (692 source files), Techniques used:
Limitations: No live database or credentials were used. The Drizzle finding Confidence: High. |
coderdan
left a comment
coderdan
left a comment
There was a problem hiding this comment.
Test-coverage review — stash init honesty changes (PR #687)
The behaviour that carries the exit-code contract is well covered: the non-interactive behind-skew refusal (exit 1, never mutates), the ahead-skew non-interactive non-refusal, the unreadable-manifest → behind path (install-deps.test.ts), and the EQL-missing → "Setup incomplete"/exit-1 gate plus the Prisma-Next exemption (init-command.test.ts). Nice work locking the leaders the e2e suite asserts on.
Three new branches introduced by this diff have no test exercising them:
- The encryption-client summary line now forks three ways (
scaffolded/kept (existing file)/ nothing) and none of the outcomes is asserted. - & 3. The
skillsClauseempty-vs-populated branch in each agent handoff prompt (handoff-claude.ts,handoff-codex.ts) — the launch-prompt analog of thesetup-promptchange that did get a dedicated test.impl.test.tsmockshowToProceedStep.runout, so no test drives either handoff prompt.
Details inline.
Additional coverage gaps not posted inline (minor)
install-deps.ts:213— the refusal test asserts thep.notealign commands and exit, but not thatp.log.error(messages.init.skewNonInteractive)is emitted (the why message). Low value; the exit-1 is the contract.setup-prompt.tsskillsLoadedLines— theagents-md+ no-skills combination isn't tested directly; it shares thewroteAgentsMdbranch withcodex(which is tested), so it's covered by construction rather than by assertion.
No security concerns noted in this diff.
Differential review (Codex) found the completion gate still reported a successful, complete setup for `stash init --drizzle` before EQL was in the database: the Drizzle path only *generates* a v2 migration (applied later with `drizzle-kit migrate`), yet `installEqlStep` returned `eqlInstalled: true` for every non-throwing outcome — the exact false-success this PR removes elsewhere. - `installCommand` now returns a structured `InstallOutcome` (`installed` | `already-installed` | `migration-generated` | `dry-run`). - `installEqlStep` maps `migration-generated` to a new `eqlMigrationPending` state (EQL not in the DB yet) instead of `eqlInstalled`. - The init summary reports it honestly — "○ EQL migration generated — apply it with `drizzle-kit migrate`" — and excuses it from the "Setup incomplete" hard-fail (exit 0), the same treatment Prisma Next already gets. - Changeset + stash-cli skill updated to match. Test-gap review: added the previously-uncovered branches — - init summary's three-way encryption-client checkmark (scaffolded / kept); - the Drizzle migration-pending gate (cross-layer: outcome → step → summary); - handoff-claude / handoff-codex launch-prompt skills clause (stripped build drops the skills dir; codex keeps AGENTS.md). Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
|
Addressed the High finding (Drizzle exits successfully before EQL is installed) in The gate no longer trusts a boolean that meant two different things. The init summary now handles that outcome honestly and separately, exactly as recommended:
On the test gap: rather than injecting the final boolean, coverage now meets in the middle at Deferred (your follow-up bullet): persisting init outcomes in |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/cli/src/commands/init/steps/__tests__/install-eql.test.ts (1)
85-104: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo test exercises the real Drizzle chain end-to-end (
db/install.ts→install-eql.ts→init/index.ts). Each layer's test mocks its immediate dependency, so a future contributor could break theInstallOutcome→eqlMigrationPending→ completion-gate contract at any hop without a failing test — the same class of bug this PR fixes.
packages/cli/src/commands/init/steps/__tests__/install-eql.test.ts#L85-L104: keep this unit test, but add a companion test that callsinstallEqlStep.runwith the realinstallCommand(mock only its internals — e.g.generateDrizzleMigration,pg/EQLInstaller) to prove the real function actually returns'migration-generated'for the Drizzle path and thatinstallEqlStepcorrectly maps it.packages/cli/src/commands/init/__tests__/init-command.test.ts#L118-L141: add a companion test that runsinitCommandwith the realinstallEqlStep.run(noteqlRun), mocking onlyinstallCommand, to prove the field set by the real step actually satisfiesinitCommand's completion gate for the Drizzle flow.Based on the PR's own review comments recommending "adding a cross-layer regression test covering the Drizzle flow through the final init status."
🤖 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/init/steps/__tests__/install-eql.test.ts` around lines 85 - 104, The Drizzle regression coverage stops at mocked layer boundaries and does not verify the real status propagation chain. In packages/cli/src/commands/init/steps/__tests__/install-eql.test.ts:85-104, retain the existing unit test and add a companion using the real installCommand while mocking only its internals, asserting the real Drizzle path returns migration-generated and installEqlStep maps it to eqlMigrationPending. In packages/cli/src/commands/init/__tests__/init-command.test.ts:118-141, add a companion using the real installEqlStep.run while mocking only installCommand, and assert initCommand's final Drizzle completion status honors the field produced by the real step.
🤖 Prompt for all review comments with 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.
Nitpick comments:
In `@packages/cli/src/commands/init/steps/__tests__/install-eql.test.ts`:
- Around line 85-104: The Drizzle regression coverage stops at mocked layer
boundaries and does not verify the real status propagation chain. In
packages/cli/src/commands/init/steps/__tests__/install-eql.test.ts:85-104,
retain the existing unit test and add a companion using the real installCommand
while mocking only its internals, asserting the real Drizzle path returns
migration-generated and installEqlStep maps it to eqlMigrationPending. In
packages/cli/src/commands/init/__tests__/init-command.test.ts:118-141, add a
companion using the real installEqlStep.run while mocking only installCommand,
and assert initCommand's final Drizzle completion status honors the field
produced by the real step.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0544bc31-27b7-4347-acd8-e699865a706d
📒 Files selected for processing (17)
.changeset/honest-noninteractive-init.mdpackages/cli/src/commands/db/install.tspackages/cli/src/commands/impl/steps/__tests__/handoff-claude.test.tspackages/cli/src/commands/impl/steps/__tests__/handoff-codex.test.tspackages/cli/src/commands/impl/steps/handoff-claude.tspackages/cli/src/commands/impl/steps/handoff-codex.tspackages/cli/src/commands/init/__tests__/init-command.test.tspackages/cli/src/commands/init/index.tspackages/cli/src/commands/init/lib/__tests__/setup-prompt.test.tspackages/cli/src/commands/init/lib/setup-prompt.tspackages/cli/src/commands/init/steps/__tests__/install-deps.test.tspackages/cli/src/commands/init/steps/__tests__/install-eql.test.tspackages/cli/src/commands/init/steps/install-deps.tspackages/cli/src/commands/init/steps/install-eql.tspackages/cli/src/commands/init/types.tspackages/cli/src/messages.tsskills/stash-cli/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/cli/src/messages.ts
- packages/cli/src/commands/impl/steps/handoff-claude.ts
- packages/cli/src/commands/impl/steps/handoff-codex.ts
- .changeset/honest-noninteractive-init.md
- packages/cli/src/commands/init/index.ts
|
Human and AI review (copilot, coderabbit, Claude/Opus and Codex/gpt-5.6-sol) |
|
Tracked by #714. |
Problem (rc.2 M4)
Non-interactive
stash initreported success for setups that hadn't fully completed — so CI/agents couldn't trust its exit code or its summary. Three honesty gaps:behindskew (an installed@cipherstash/*older than this CLI expects) — it won't mutate an install without consent — so it warned and kept going, scaffolding config/client against mismatched packages, then printed "Setup complete".eqlInstalled=false), the header still said complete — and "✓ Database connection verified" was printed although init only resolves a URL, never connects.installSkills()copied nothing (stripped build) — telling it to read files that aren't there.Fix
behindskew → refuse with a non-zero exit (CliExit(1)) and the exact align command, instead of proceeding. The npm dist-tags deliver broken/stale packages: adapters' latest=0.0.0, stack latest=0.19.0; stash init installs unpinned #661/fix(cli): pin init's installs to the release's own package versions (#661) #666 no-mutation principle is preserved (execSyncstill never called — only the post-warning action changes).aheadskew stays a warn (install likely fine; update the CLI). Interactive runs still offer to align.!eqlInstalled && integration !== 'prisma-next'), the summary reads "Setup incomplete" and init exits non-zero, pointing atstash eql install. "Database connection verified" → "Database URL resolved"; "Encryption client scaffolded" shows only when a client was actually written (skipped for Prisma Next).installSkills()returning something.Scope notes
prisma-next migration apply, soeqlInstalled=falseis expected there and is NOT treated as incomplete (tested).Verification
install-depsskew tests (refuses + exit 1, still never mutates); newinit-commandhonest-summary tests (EQL-missing → exit 1 + "Setup incomplete"; prisma-nexteqlInstalled=false→ completes).code:checkclean. Changeset:stashminor. Skill (stash-cli) + doc comments updated.initneeds live auth + a DB (which is why there's no init e2e); the honest-summary and skew-fail logic is unit-covered.https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
Summary by CodeRabbit
stash initin non-interactive mode now fails (“Setup incomplete” / non-zero exit) when EQL setup is missing or when dependency version skew can’t be safely reconciled, including printing the exact alignment command..*/skills/when skills weren’t installed.stash initdocumentation to clarify exact version pinning, honest non-interactive mismatch behavior, and EQL/migration status expectations.