fix(cli): detect plan before impl commences#441
Conversation
🦋 Changeset detectedLatest commit: 5d8e699 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
📝 WalkthroughWalkthroughThis PR adds a ChangesStatus Command Plan Existence Tracking
Sequence DiagramsequenceDiagram
participant Status as statusCommand
participant BuildLog as buildLog
participant BuildQuest as buildQuestLog
participant Render as renderQuestLog*
participant ProjectStatus as ProjectStatus
Status->>ProjectStatus: read planExists
Status->>BuildLog: call with project state
BuildLog->>ProjectStatus: extract planExists
BuildLog->>BuildQuest: call with planExists
BuildQuest->>BuildQuest: populate QuestLog.planExists
BuildQuest-->>BuildLog: return QuestLog with planExists
BuildLog->>Render: call with QuestLog
Render->>Render: branch on planExists<br/>true: show impl + plan.md<br/>false: show plan guidance
Render-->>Status: rendered output
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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.
🧹 Nitpick comments (1)
packages/cli/src/commands/status/__tests__/status.test.ts (1)
408-410: ⚡ Quick winConsider using a simpler assertion to avoid the ReDoS warning.
The RegExp is constructed from the
CLIvariable, which triggers a static analysis warning about potential ReDoS. While this is a false positive in the test context (CLI is a constant), you can eliminate the warning by using a simpler assertion approach.🔧 Proposed refactor to avoid ReDoS warning
- expect(out).not.toMatch( - new RegExp(`${CLI.replace(/ /g, '\\s')}\\s+plan\\b`), - ) + // Ensure we don't suggest running plan when a plan already exists + expect(out).not.toContain(`${CLI} plan`)This simpler check achieves the same goal (ensuring we don't suggest
planwhen a plan exists) while avoiding the regex complexity warning.🤖 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/status/__tests__/status.test.ts` around lines 408 - 410, Replace the regex-based assertion that uses CLI with a simple substring check to avoid the ReDoS warning: locate the assertion using expect(out).not.toMatch(new RegExp(`${CLI.replace(/ /g, '\\s')}\\s+plan\\b`)) and change it to a plain string containment assertion such as expect(out).not.toContain(`${CLI} plan`) (or use !out.includes(`${CLI} plan`)) to achieve the same negative check without constructing a RegExp from the CLI variable.
🤖 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/status/__tests__/status.test.ts`:
- Around line 408-410: Replace the regex-based assertion that uses CLI with a
simple substring check to avoid the ReDoS warning: locate the assertion using
expect(out).not.toMatch(new RegExp(`${CLI.replace(/ /g, '\\s')}\\s+plan\\b`))
and change it to a plain string containment assertion such as
expect(out).not.toContain(`${CLI} plan`) (or use !out.includes(`${CLI} plan`))
to achieve the same negative check without constructing a RegExp from the CLI
variable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1044405f-14de-4e45-9fcb-a8ceb8a888aa
📒 Files selected for processing (5)
.changeset/ten-deserts-hammer.mdpackages/cli/src/commands/status/__tests__/status.test.tspackages/cli/src/commands/status/index.tspackages/cli/src/commands/status/quest.tspackages/cli/src/commands/status/render.ts
2 participants
Summary by CodeRabbit
stash statuscommand now properly distinguishes between "no encryption rollout plan drafted yet" and "plan already drafted but not executed." When a plan exists, users are now directed to runstash implto execute the drafted plan, rather than being directed to create a new plan first.