feat(cli): add branch remove for preview branches#110
Conversation
WalkthroughThis pull request adds a new 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
AmanVarshney01
force-pushed
the
feat/branch-remove
branch
from
b7c4576 to
a820e74
Compare
Preview branch cleanup previously required the Console. branch remove takes an explicit branch id or git name in the resolved project, the exact-id --confirm convention, and maps the platform's guarantees to structured codes: BRANCH_PROTECTED for production/default branches (422) and BRANCH_NOT_EMPTY when live apps or databases remain (409), so removal never cascades into member resources. Removal is a platform soft-delete and never touches local Git branches. Branch creation deliberately stays implicit (git-push automation and app deploy); there is no branch create, per team alignment.
AmanVarshney01
force-pushed
the
feat/branch-remove
branch
from
a820e74 to
0884425
Compare
|
@AmanVarshney01 here's some feedback on the PR ( in collab with GPT ;)) Maintainability and readability feedbackWe should make the branch removal flow easier to read, extend, and safely change over time. The current implementation works, but some parts rely on inline types, repeated string values, and controller-level branching that will become harder to maintain as more branch lifecycle rules are added. 1. Name the branch removal result typeWe should avoid defining complex union return types directly inside method signatures. Instead of: removeBranch(
branchId: string,
):
| { outcome: "removed"; branch: BranchRecord }
| { outcome: "not-found" }
| { outcome: "protected" }
| { outcome: "not-empty" } {Do this: type BranchRemoveResult =
| { outcome: "removed"; branch: BranchRecord }
| { outcome: "not-found" }
| { outcome: "protected" }
| { outcome: "not-empty" };
removeBranch(branchId: BranchId): BranchRemoveResult {This helps because the method signature becomes easier to scan, and the result now has a clear domain name. It also gives us one place to update the result shape if branch removal gains more outcomes later. 2. Avoid raw strings for important domain statesWe should avoid repeating important values like Instead of: if (branch.role === "production") {
return { outcome: "protected" };
}Do this: const BranchRole = {
Production: "production",
Preview: "preview",
} as const;
const BranchRemoveOutcome = {
Removed: "removed",
NotFound: "not-found",
Protected: "protected",
NotEmpty: "not-empty",
} as const;
if (branch.role === BranchRole.Production) {
return { outcome: BranchRemoveOutcome.Protected };
}This helps because these values are part of the product behavior, not incidental strings. Centralizing them makes typos less likely and makes it easier to see the supported branch states and removal outcomes. 3. Extract branch removal rules into small helper functionsWe should keep the removal method focused on the flow, not the details of every rule. Instead of: const hasDatabases = (this.data.databases ?? []).some(
(database) => database.branchId === branchId,
);
const hasDeployments = this.data.deployments.some(
(deployment) =>
deployment.projectId === branch.projectId &&
deployment.branch === branch.name,
);
if (hasDatabases || hasDeployments) {
return { outcome: "not-empty" };
}Do this: function hasLiveBranchResources(data: MockApiData, branch: BranchRecord): boolean {
return hasBranchDatabases(data, branch.id) || hasBranchDeployments(data, branch);
}
function hasBranchDatabases(data: MockApiData, branchId: BranchId): boolean {
return (data.databases ?? []).some((database) => database.branchId === branchId);
}
function hasBranchDeployments(data: MockApiData, branch: BranchRecord): boolean {
return data.deployments.some(
(deployment) =>
deployment.projectId === branch.projectId &&
deployment.branch === branch.name,
);
}This helps because the main method can read like the product rule: “find branch, reject protected branch, reject branch with live resources, remove branch.” The details are still there, but they are behind names that explain intent. 4. Keep
|
luanvdw
left a comment
luanvdw
left a comment
There was a problem hiding this comment.
Thanks @AmanVarshney01
If I tried this correctly, the BRANCH_NOT_EMPTY recovery experience currently sends the user into a dead end. The flow:
- User runs
prisma-cli branch remove preview --confirm br_123 - CLI correctly refuses because the branch still has live resources
- CLI suggests removing the child app with
prisma-cli app remove --app <name> --branch preview - That command fails, because
app removedoes not currently support--branch
So the user is blocked, follows the CLI’s next step, and gets unknown option '--branch'. Can we add --branch support to app remove rather than only changing the message, because branch cleanup naturally needs “remove this app from this branch, then retry branch removal.”
Let me know if I miss-understood the flow.
Then lastly, (as a follow up, maybe as a separate PR) I think we should support an explicit cascade path for non-production branches. Blocking by default is reasonable, but users should not have to manually tear down every child resource forever. Something like:
prisma-cli branch remove feat-login --confirm br_123 --cascade
The important guardrail is that cascade should only be allowed when the resolved branch is not role=production, and it should stay refused for production/default branches. Ideally the cascade response also makes the blast radius clear by listing the apps/databases that will be removed, so this is explicit destructive intent rather than a hidden side effect of confirming the branch id.
wdyt?
Review feedback from #110: - app remove now accepts --branch, honored as-is like the other read-branch commands, so branch cleanup can target apps on branches that are not checked out locally. The BRANCH_NOT_EMPTY recovery command previously pointed at this flag before it existed, which was a dead end. - branch remove --cascade removes the branch's apps, then its databases, then the branch, for preview branches only: production and default branches stay refused before any member resource is touched. The result lists every removed resource (human output and result.removed in JSON) so the blast radius is explicit. Cascade is client-orchestrated because the platform's branch delete refuses non-empty branches; a mid-cascade failure stops immediately with BRANCH_CASCADE_INCOMPLETE, whose meta lists what was already removed. - BRANCH_NOT_EMPTY recovery steps now offer the cascade rerun first, then individual app/database cleanup.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/controllers/app.ts (1)
2041-2066: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider an options object for
runAppRemove's growing parameter list.
runAppRemovenow takes 5 positional args (context, appName, projectRef, configTarget, branchName), most optional strings — easy to transpose at call sites and hard to extend further. BundlingappName/projectRef/configTarget/branchNameinto a single options object would improve call-site clarity and align with the sibling branch-remove cohort's push toward clearer, named inputs.♻️ Suggested direction
-export async function runAppRemove( - context: CommandContext, - appName: string | undefined, - projectRef?: string, - configTarget?: string, - branchName?: string, -): Promise<CommandSuccess<AppRemoveResult>> { +export async function runAppRemove( + context: CommandContext, + appName: string | undefined, + options: { + projectRef?: string; + configTarget?: string; + branchName?: string; + } = {}, +): Promise<CommandSuccess<AppRemoveResult>> { + const { projectRef, configTarget, branchName } = options;🤖 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/controllers/app.ts` around lines 2041 - 2066, `runAppRemove` has a growing positional parameter list that is easy to misuse at call sites. Refactor it to accept a single options object for the app-specific inputs (`appName`, `projectRef`, `configTarget`, `branchName`) while keeping `context` separate, and update the `runAppRemove` implementation to destructure those named fields before calling `resolveComputeManagementContext` and `requireProviderAndProjectContext`. Then update every caller to pass named properties so the signature is clearer and easier to extend.
🤖 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/controllers/branch.ts`:
- Line 47: The fixture-backed branch flow is dropping the isDefault flag, so
default-branch protection can be bypassed in tests. Update the branch handling
path in branch.ts to preserve and pass through isDefault from fixture/mock data
just like the real API path does, and make sure the mock-api.ts BranchRecord
includes the field. Add a regression test around branch remove to verify default
branches still hit the protection check in fixture mode.
In `@packages/cli/src/presenters/branch.ts`:
- Around line 88-100: The branch presenter in the details array is inlining
duplicated ternary logic and hardcoded removal messages; extract this into a
small helper used by branch summary rendering. Centralize the removed-resource
copy for apps and databases so the presenter just calls a helper from branch.ts
rather than nesting conditionals inline, and reuse the helper to format
live-resource summary text consistently.
In `@packages/cli/tests/branch-remove.test.ts`:
- Around line 55-75: The branch removal test only checks a partial object, so it
does not verify that non-cascade removals omit the removed payload. Update the
assertion in branch-remove.test.ts for the branch.remove JSON result to
explicitly check the result shape from executeCli/payload, and add a negative
assertion that result.removed is absent when removing the preview branch by git
name. Keep the existing checks for command, projectId, and branch identity, but
make the test fail if removed is unexpectedly included.
- Around line 156-173: The branch remove --cascade test has a duplicate setup
helper that repeats setupLinkedProject() logic, so refactor setup() to reuse
setupLinkedProject() instead of reimplementing the auth/login and local project
fixture steps. Keep the cascade-specific test setup focused only on its extra
needs, and reference the existing setupLinkedProject() helper so both tests stay
aligned if the fixture changes.
---
Outside diff comments:
In `@packages/cli/src/controllers/app.ts`:
- Around line 2041-2066: `runAppRemove` has a growing positional parameter list
that is easy to misuse at call sites. Refactor it to accept a single options
object for the app-specific inputs (`appName`, `projectRef`, `configTarget`,
`branchName`) while keeping `context` separate, and update the `runAppRemove`
implementation to destructure those named fields before calling
`resolveComputeManagementContext` and `requireProviderAndProjectContext`. Then
update every caller to pass named properties so the signature is clearer and
easier to extend.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 26d87594-0344-4a67-bc9e-2f8a59ef1e25
📒 Files selected for processing (15)
docs/product/command-spec.mddocs/product/error-conventions.mddocs/product/resource-model.mdpackages/cli/src/adapters/mock-api.tspackages/cli/src/commands/app/index.tspackages/cli/src/commands/branch/index.tspackages/cli/src/controllers/app.tspackages/cli/src/controllers/branch.tspackages/cli/src/presenters/branch.tspackages/cli/src/shell/command-meta.tspackages/cli/src/types/branch.tspackages/cli/tests/app-controller.test.tspackages/cli/tests/branch-remove.test.tspackages/cli/tests/branch.test.tspackages/cli/tests/shell.test.ts
| id: string; | ||
| gitName: string; | ||
| role: BranchRole; | ||
| isDefault?: boolean; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map relevant files/symbols first
ast-grep outline packages/cli/src/controllers/branch.ts --view expanded || true
printf '\n--- branch.ts lines ---\n'
sed -n '1,260p' packages/cli/src/controllers/branch.ts
printf '\n--- search isDefault and BranchRecord ---\n'
rg -n "isDefault|BranchRecord|listBranchesForProject|listBranches\(" packages/cli/src -SRepository: prisma/prisma-cli
Length of output: 13917
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline packages/cli/src/controllers/branch.ts --view expanded || true
printf '\n--- branch.ts lines ---\n'
sed -n '1,260p' packages/cli/src/controllers/branch.ts
printf '\n--- search isDefault and BranchRecord ---\n'
rg -n "isDefault|BranchRecord|listBranchesForProject|listBranches\(" packages/cli/src -SRepository: prisma/prisma-cli
Length of output: 13917
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- mock-api BranchRecord and listBranchesForProject ---\n'
sed -n '1,140p' packages/cli/src/adapters/mock-api.ts
sed -n '250,310p' packages/cli/src/adapters/mock-api.ts
sed -n '390,430p' packages/cli/src/adapters/mock-api.ts
printf '\n--- branch.ts listBranches and toBranchSummary ---\n'
sed -n '470,525p' packages/cli/src/controllers/branch.ts
printf '\n--- app-provider branch mapping ---\n'
sed -n '760,900p' packages/cli/src/lib/app/app-provider.ts
printf '\n--- tests mentioning branch protection/default ---\n'
rg -n "branchProtectedError|isDefault|default branches|branch remove|branch.list|production" packages/cli/src -g '*test*' -g '*spec*' -SRepository: prisma/prisma-cli
Length of output: 10653
Map isDefault through fixture branches
Fixture-backed branch remove drops isDefault, so a default branch in test runs bypasses the protection check. Real mode already forwards isDefault from the platform API, so this is limited to the mock/fixture path. Add isDefault to mock-api.ts's BranchRecord, pass it through here, and add a regression test.
🤖 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/controllers/branch.ts` at line 47, The fixture-backed branch
flow is dropping the isDefault flag, so default-branch protection can be
bypassed in tests. Update the branch handling path in branch.ts to preserve and
pass through isDefault from fixture/mock data just like the real API path does,
and make sure the mock-api.ts BranchRecord includes the field. Add a regression
test around branch remove to verify default branches still hit the protection
check in fixture mode.
| details: [ | ||
| "The branch was removed from the platform. Local Git branches are untouched.", | ||
| ...(result.removed | ||
| ? [ | ||
| result.removed.apps.length > 0 | ||
| ? `Removed apps: ${result.removed.apps.map((app) => app.name).join(", ")}` | ||
| : "No apps were on the branch.", | ||
| result.removed.databases.length > 0 | ||
| ? `Removed databases: ${result.removed.databases.map((database) => database.name).join(", ")}` | ||
| : "No databases were on the branch.", | ||
| ] | ||
| : []), | ||
| ], |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Extract removed-resource summary into a helper.
The apps/databases ternary pair duplicates shape and hardcodes outcome strings inline. Per PR feedback on this stack, centralize removal-outcome copy and extract a small helper for live-resource summarization instead of inlining nested ternaries in the presenter.
♻️ Suggested extraction
+function formatRemovedResourceLine(
+ label: string,
+ resources: BranchRemovedResource[],
+): string {
+ return resources.length > 0
+ ? `Removed ${label}: ${resources.map((r) => r.name).join(", ")}`
+ : `No ${label} were on the branch.`;
+}
+
details: [
"The branch was removed from the platform. Local Git branches are untouched.",
...(result.removed
- ? [
- result.removed.apps.length > 0
- ? `Removed apps: ${result.removed.apps.map((app) => app.name).join(", ")}`
- : "No apps were on the branch.",
- result.removed.databases.length > 0
- ? `Removed databases: ${result.removed.databases.map((database) => database.name).join(", ")}`
- : "No databases were on the branch.",
- ]
+ ? [
+ formatRemovedResourceLine("apps", result.removed.apps),
+ formatRemovedResourceLine("databases", result.removed.databases),
+ ]
: []),
],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| details: [ | |
| "The branch was removed from the platform. Local Git branches are untouched.", | |
| ...(result.removed | |
| ? [ | |
| result.removed.apps.length > 0 | |
| ? `Removed apps: ${result.removed.apps.map((app) => app.name).join(", ")}` | |
| : "No apps were on the branch.", | |
| result.removed.databases.length > 0 | |
| ? `Removed databases: ${result.removed.databases.map((database) => database.name).join(", ")}` | |
| : "No databases were on the branch.", | |
| ] | |
| : []), | |
| ], | |
| function formatRemovedResourceLine( | |
| label: string, | |
| resources: BranchRemovedResource[], | |
| ): string { | |
| return resources.length > 0 | |
| ? `Removed ${label}: ${resources.map((r) => r.name).join(", ")}` | |
| : `No ${label} were on the branch.`; | |
| } | |
| details: [ | |
| "The branch was removed from the platform. Local Git branches are untouched.", | |
| ...(result.removed | |
| ? [ | |
| formatRemovedResourceLine("apps", result.removed.apps), | |
| formatRemovedResourceLine("databases", result.removed.databases), | |
| ] | |
| : []), | |
| ], |
🤖 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/presenters/branch.ts` around lines 88 - 100, The branch
presenter in the details array is inlining duplicated ternary logic and
hardcoded removal messages; extract this into a small helper used by branch
summary rendering. Centralize the removed-resource copy for apps and databases
so the presenter just calls a helper from branch.ts rather than nesting
conditionals inline, and reuse the helper to format live-resource summary text
consistently.
| it("removes an empty preview branch by git name", async () => { | ||
| const { cwd, stateDir } = await setupLinkedProject(); | ||
|
|
||
| const result = await executeCli({ | ||
| argv: ["branch", "remove", "staging", "--confirm", "br_345", "--json"], | ||
| cwd, | ||
| stateDir, | ||
| fixturePath, | ||
| }); | ||
| const payload = JSON.parse(result.stdout); | ||
|
|
||
| expect(result.exitCode).toBe(0); | ||
| expect(payload).toMatchObject({ | ||
| ok: true, | ||
| command: "branch.remove", | ||
| result: { | ||
| projectId: "proj_123", | ||
| branch: { id: "br_345", name: "staging", role: "preview" }, | ||
| }, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Assertion doesn't actually verify removed is absent.
toMatchObject permits extra keys, so this test would still pass if result.removed were unexpectedly present for a non-cascade removal.
expect(payload).toMatchObject({
ok: true,
command: "branch.remove",
result: {
projectId: "proj_123",
branch: { id: "br_345", name: "staging", role: "preview" },
},
});
+ expect(payload.result).not.toHaveProperty("removed");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it("removes an empty preview branch by git name", async () => { | |
| const { cwd, stateDir } = await setupLinkedProject(); | |
| const result = await executeCli({ | |
| argv: ["branch", "remove", "staging", "--confirm", "br_345", "--json"], | |
| cwd, | |
| stateDir, | |
| fixturePath, | |
| }); | |
| const payload = JSON.parse(result.stdout); | |
| expect(result.exitCode).toBe(0); | |
| expect(payload).toMatchObject({ | |
| ok: true, | |
| command: "branch.remove", | |
| result: { | |
| projectId: "proj_123", | |
| branch: { id: "br_345", name: "staging", role: "preview" }, | |
| }, | |
| }); | |
| }); | |
| it("removes an empty preview branch by git name", async () => { | |
| const { cwd, stateDir } = await setupLinkedProject(); | |
| const result = await executeCli({ | |
| argv: ["branch", "remove", "staging", "--confirm", "br_345", "--json"], | |
| cwd, | |
| stateDir, | |
| fixturePath, | |
| }); | |
| const payload = JSON.parse(result.stdout); | |
| expect(result.exitCode).toBe(0); | |
| expect(payload).toMatchObject({ | |
| ok: true, | |
| command: "branch.remove", | |
| result: { | |
| projectId: "proj_123", | |
| branch: { id: "br_345", name: "staging", role: "preview" }, | |
| }, | |
| }); | |
| expect(payload.result).not.toHaveProperty("removed"); | |
| }); |
🤖 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/tests/branch-remove.test.ts` around lines 55 - 75, The branch
removal test only checks a partial object, so it does not verify that
non-cascade removals omit the removed payload. Update the assertion in
branch-remove.test.ts for the branch.remove JSON result to explicitly check the
result shape from executeCli/payload, and add a negative assertion that
result.removed is absent when removing the preview branch by git name. Keep the
existing checks for command, projectId, and branch identity, but make the test
fail if removed is unexpectedly included.
| describe("branch remove --cascade", () => { | ||
| async function setup() { | ||
| const cwd = await createTempCwd(); | ||
| const stateDir = path.join(cwd, ".state"); | ||
| await executeCli({ | ||
| argv: ["auth", "login", "--provider", "github", "--user", "usr_456"], | ||
| cwd, | ||
| stateDir, | ||
| fixturePath, | ||
| }); | ||
| await mkdir(path.join(cwd, ".prisma"), { recursive: true }); | ||
| await writeFile( | ||
| path.join(cwd, ".prisma/local.json"), | ||
| `${JSON.stringify({ workspaceId: "ws_123", projectId: "proj_123" }, null, 2)}\n`, | ||
| "utf8", | ||
| ); | ||
| return { cwd, stateDir }; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Duplicate setup fixture.
This setup() reimplements setupLinkedProject() (Lines 9-25) verbatim. Reuse it to avoid the two drifting apart.
♻️ Proposed fix
describe("branch remove --cascade", () => {
- async function setup() {
- const cwd = await createTempCwd();
- const stateDir = path.join(cwd, ".state");
- await executeCli({
- argv: ["auth", "login", "--provider", "github", "--user", "usr_456"],
- cwd,
- stateDir,
- fixturePath,
- });
- await mkdir(path.join(cwd, ".prisma"), { recursive: true });
- await writeFile(
- path.join(cwd, ".prisma/local.json"),
- `${JSON.stringify({ workspaceId: "ws_123", projectId: "proj_123" }, null, 2)}\n`,
- "utf8",
- );
- return { cwd, stateDir };
- }
+ const setup = setupLinkedProject;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| describe("branch remove --cascade", () => { | |
| async function setup() { | |
| const cwd = await createTempCwd(); | |
| const stateDir = path.join(cwd, ".state"); | |
| await executeCli({ | |
| argv: ["auth", "login", "--provider", "github", "--user", "usr_456"], | |
| cwd, | |
| stateDir, | |
| fixturePath, | |
| }); | |
| await mkdir(path.join(cwd, ".prisma"), { recursive: true }); | |
| await writeFile( | |
| path.join(cwd, ".prisma/local.json"), | |
| `${JSON.stringify({ workspaceId: "ws_123", projectId: "proj_123" }, null, 2)}\n`, | |
| "utf8", | |
| ); | |
| return { cwd, stateDir }; | |
| } | |
| describe("branch remove --cascade", () => { | |
| const setup = setupLinkedProject; |
🤖 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/tests/branch-remove.test.ts` around lines 156 - 173, The branch
remove --cascade test has a duplicate setup helper that repeats
setupLinkedProject() logic, so refactor setup() to reuse setupLinkedProject()
instead of reimplementing the auth/login and local project fixture steps. Keep
the cascade-specific test setup focused only on its extra needs, and reference
the existing setupLinkedProject() helper so both tests stay aligned if the
fixture changes.
3 participants
Preview branch cleanup previously required the Console. Per the team breakout: removal yes, creation no. Branch creation stays implicit (git-push automation and
app deploy); there is deliberately nobranch create.Design notes
--confirmconvention;--yesnever satisfies it.BRANCH_PROTECTED(422: production/default branch) andBRANCH_NOT_EMPTY(409: live apps or databases remain). Plain removal never touches member resources. Production/default is also refused client-side before the confirm prompt.DELETE /v1/branches/{branchId}; local Git branches are untouched.BRANCH_NOT_FOUND,BRANCH_PROTECTED,BRANCH_NOT_EMPTY,BRANCH_CASCADE_INCOMPLETE) registered in error-conventions.md.Review follow-ups (this PR, per @luanvdw)
app remove --branch <name>: theBRANCH_NOT_EMPTYrecovery hint used to point at a flag that did not exist. The flag is real now, so an app can be removed from a branch that is not checked out locally (or from a directory with no git repo at all).branch remove --cascade: explicit opt-in teardown for non-production branches. Guardrails: the protection check (production or default role) runs before any member resource is touched, so--cascadenever widens what can be removed; the exact-id--confirmis still required. The CLI removes the branch's apps, then its databases, then the branch, and lists the blast radius in the output and in JSONresult.removed. A partial failure stops immediately and reports exactly what was already removed (BRANCH_CASCADE_INCOMPLETE).BRANCH_NOT_EMPTYnext steps now lead with the--cascadererun, then the per-resource path.Testing: 9 branch-remove integration tests (confirmation gate, git-name removal, protected, not-empty with cascade-first next steps, not-found, explicit
--project, cascade success with blast radius, cascade refused on production) plus anapp remove --branchcontroller test; full suite 601 green. Also live-verified end to end against a real workspace: real 409 toBRANCH_NOT_EMPTYmapping,app remove --branchfrom a git-less directory, cascade removing app + database + branch with the blast radius listed, production refusal with--cascade, wrong--confirmrejected.