fix: remove deprecated --hard flag and all references#371
Conversation
There was a problem hiding this comment.
Pull request overview
Removes the deprecated --hard flag from brev reset and consolidates “hard reset” behavior under the brev recreate command, updating related CLI messaging and documentation.
Changes:
- Removed
--hardflag support and deleted the associated hard-reset implementation frompkg/cmd/reset/reset.go. - Renamed “hard reset” functions/wording in the
recreatecommand to berecreate*for clarity. - Updated CLI/docs messaging to eliminate
--hardreferences.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/cmd/reset/reset.go | Drops the deprecated --hard flag and removes hard-reset helper logic from the reset command. |
| pkg/cmd/recreate/recreate.go | Renames hard-reset functions/flow to recreate* naming for consistency. |
| pkg/cmd/recreate/doc.md | Updates example output text to align with updated recreate messaging. |
| pkg/cmd/ls/ls.go | Removes --hard guidance from the “reset breadcrumb” help output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -425,7 +425,7 @@ func displayLsResetBreadCrumb(t *terminal.Terminal, workspaces []entity.Workspac | |||
| } | |||
| } | |||
| if foundAResettableWorkspace { | |||
There was a problem hiding this comment.
The reset breadcrumb now prints only a blank line when a resettable instance is found. Since --hard was removed, this would be a good place to replace the old guidance with an actionable next step (e.g., suggest brev recreate <name> if brev reset doesn’t resolve the issue) rather than emitting an empty line.
| if foundAResettableWorkspace { | |
| if foundAResettableWorkspace { | |
| t.Vprintf("%s", t.Green("If reset doesn't resolve the issue, recreate the instance:\n")) | |
| for _, w := range workspaces { | |
| if w.Status == entity.Failure || getWorkspaceDisplayStatus(w) == entity.Unhealthy { | |
| t.Vprintf("%s", t.Yellow(fmt.Sprintf("\tbrev recreate %s\n", w.Name))) | |
| } | |
| } |
There was a problem hiding this comment.
Intentionally left blank. The prior --hard breadcrumb caused a production incident (nemoclaw-f4a386 / BREV-9066 / the thread in #ep-brev-support) because an AI coding agent auto-executed the destructive suggestion. brev recreate is equally destructive (delete + create), so suggesting it from ls output re-introduces the same vector. Users who need a recreate can discover it via brev --help or docs, which requires a deliberate action rather than reading ls output.
| $ brev recreate payments-frontend | ||
| Starting hard reset 🤙 This can take a couple of minutes. | ||
| recreating 🤙 This can take a couple of minutes. | ||
|
|
||
| Deleting workspace - naive-pubsub. | ||
| Workspace is starting. This can take up to 2 minutes the first time. | ||
| Deleting instance - naive-pubsub. | ||
| Instance is starting. This can take up to 2 minutes the first time. |
There was a problem hiding this comment.
The recreate example is inconsistent: it runs brev recreate payments-frontend but the output references naive-pubsub. Update the command and output to use the same workspace/instance name throughout the example to avoid confusing users.
There was a problem hiding this comment.
Pushed a new commit to address this.
| if foundAResettableWorkspace { | ||
| t.Vprintf("%s", t.Yellow("If this problem persists, run the command again with the --hard flag (warning: the --hard flag will not preserve uncommitted files!) \n\n")) | ||
| t.Vprintf("\n") |
There was a problem hiding this comment.
This change alters the user-facing help text emitted by displayLsResetBreadCrumb, but there doesn’t appear to be any test asserting the breadcrumb output for unhealthy/failed instances in ls_test.go. Adding a focused test that captures the printed guidance would help prevent future regressions in CLI output.
There was a problem hiding this comment.
Same reason as #1 — the line was kept empty on purpose to avoid any CLI output that could confuse the user or be auto-executed by an agent.
There was a problem hiding this comment.
can we just remove the print statement altogether? what's the value in printing a new line?
There was a problem hiding this comment.
Done, bare newline removed. Can we get a review and approval after the new push?
@patelspratik @stephahart
Description
This PR removes the deprecated --hard flag from the
brev resetcommand along with all associated logic, references, and messaging across the CLI and documentation. It ensures the flag is fully eliminated and aligns the codebase with the intended deprecation.Solution
reset.gorecreatecommand for consistency and clarity--hardflag