feat(cli): honest non-interactive init — fail on skew, no false success (rc.2 M4)#687
Open
coderdan wants to merge 1 commit into
Open
feat(cli): honest non-interactive init — fail on skew, no false success (rc.2 M4)#687coderdan wants to merge 1 commit into
coderdan wants to merge 1 commit into
Conversation
…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
🦋 Changeset detectedLatest commit: 27520d6 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) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant initCommand
participant installDepsStep
participant installEqlStep
participant AgentHandoff
User->>initCommand: run stash init
initCommand->>installDepsStep: install dependencies
installDepsStep-->>initCommand: dependency state or CliExit
initCommand->>installEqlStep: install EQL extension
installEqlStep-->>initCommand: eqlInstalled state
initCommand->>AgentHandoff: build prompt from installed skills
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
Comment on lines
+41
to
+48
| 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.` |
There was a problem hiding this comment.
🎯 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 anAGENTS.mdfile this handoff does not write.packages/cli/src/commands/impl/steps/handoff-codex.ts#L60-L67: omit.codex/skills/from generated guidance wheninstalled.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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
Bug Fixes
stash initnow exits with an error when required dependencies have incompatible versions and provides an alignment command.Documentation
stash initguidance for dependency version mismatches and incomplete EQL setup.