Repository navigation
fix(projects): take --volume-size back out of upgrade-instance - #316
Conversation
Disk resizes are admin-only: provisioned disk is not billed, so customers cannot grow it themselves. update --storage-size now points at support. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Instance-type-only upgrade src/types.ts, src/commands/projects/manage.ts, src/lib/guard/risk-registry.ts, src/commands/projects/manage.test.ts |
The upgrade request now requires an instance type, and the command validates and confirms only that change. Disk-size validation, API and analytics data, and resizing output were removed. The risk assessment and tests no longer describe disk resizing. The projects update --storage-size error now refers users to InsForge support. |
Priority:
Estimated code review effort: 2 (Simple) | ~12 minutes
Change: Bug fix
Suggested reviewers: carmendou, jwfing
Merge Risk: 
08ff8
-
Disk-size changes are directed to support, while instance upgrades remain available by instance type. No identified issue blocks merging after normal checks.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: removing --volume-size from upgrade-instance. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
A rabbit checks the instance type,
And finds the disk-size path is gone.
“Ask InsForge,” the prompt now says,
The type change gets a restart note.
The bunny hops through tests at dawn.
Comment @coderabbitai help to get the list of available commands.
![agent-zhang-beihai[bot]](https://avatars.githubusercontent.com/in/3908133?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed by Wang Miao
I've finished reading the diff and its callers, and I'm writing up the review.
This reverts the --volume-size option that the base commit added to projects upgrade-instance. upgrade-instance goes back to taking a required instance type. The guard risk entry, UpgradeInstanceBody/UpgradeInstanceResult and the confirmation prompt shrink to match, and the hidden projects update --storage-size flag now refuses, before login, with a message pointing at InsForge support. The revert is complete and nothing ships broken, so I'd approve it.
I found no problems.
- Leftovers: nothing in
src/,README.mdordocsstill mentionsvolumeSize,volume-sizeorpreviousVolumeSizeGiB. - Instance type: it is now a required positional argument, so the
ctx.args[0] ?? '?'fallback in the risk registry is never hit. - No released users affected:
--volume-sizenever went out in a release.v0.2.8is the latest tag, and none of the tags contain16a7a83, the commit that introduced it.
I couldn't run the tests or tsc because dependencies aren't installed in this checkout.
 [Medium impact] The changes affect the bounded project upgrade workflow. The PR appears safe to merge, with a non-blocking gap in tests for the changed upgrade command. Findings
SummaryThis PR removes customer disk resizing from
Reviews (1) · Last reviewed commit: "fix(projects): take --volume-size back o..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| describe('projects disk resizing', () => { | ||
| let exitSpy: ReturnType<typeof vi.spyOn>; | ||
| let logSpy: ReturnType<typeof vi.spyOn>; | ||
| describe('projects update --storage-size', () => { |
There was a problem hiding this comment.
Removing every upgrade-instance test leaves the main change in this PR unchecked. Add replacement tests that verify upgrade-instance large sends only { instanceType: 'large' }, and that the removed --volume-size flag and a missing instance type fail without calling upgradeInstance. These checks would catch disk resizing being accidentally restored or instance upgrades being broken.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Declining. After this PR, upgrade-instance is byte-identical to main before PR 315, which had no tests for it, so these would be new tests for behaviour this PR does not add. Commander already rejects an unknown --volume-size or a missing positional. The control that stops customers resizing the disk is server-side, in InsForge/insforge-cloud-backend PR 1000, and it has its own test.
There was a problem hiding this comment.
That’s fair. Since this restores the pre-315 CLI contract, Commander already rejects --volume-size and a missing instance type, and the actual disk-resize restriction is enforced and tested in the backend, my request for replacement upgrade-instance tests was a coverage preference—not a defect in this PR. I’m withdrawing the comment.
Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.
There was a problem hiding this comment.
Reviewed by Yang Dong
This removes customer-facing disk resizing from projects upgrade-instance, restores the required instance-type argument, and keeps the legacy storage flag as a support redirect. The rollback is internally consistent, so I would approve it.
No findings.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/commands/projects/manage.test.ts">
<violation number="1" location="src/commands/projects/manage.test.ts:36">
P2: Add replacement `upgrade-instance` tests for the instance-type request, rejection of `--volume-size`, and missing-type failure; this change removes the command’s existing coverage without testing its new behavior.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| describe('projects disk resizing', () => { | ||
| let exitSpy: ReturnType<typeof vi.spyOn>; | ||
| let logSpy: ReturnType<typeof vi.spyOn>; | ||
| describe('projects update --storage-size', () => { |
There was a problem hiding this comment.
P2: Add replacement upgrade-instance tests for the instance-type request, rejection of --volume-size, and missing-type failure; this change removes the command’s existing coverage without testing its new behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/commands/projects/manage.test.ts, line 36:
<comment>Add replacement `upgrade-instance` tests for the instance-type request, rejection of `--volume-size`, and missing-type failure; this change removes the command’s existing coverage without testing its new behavior.</comment>
<file context>
@@ -30,89 +26,39 @@ vi.mock('../../lib/analytics.js', () => ({
-describe('projects disk resizing', () => {
- let exitSpy: ReturnType<typeof vi.spyOn>;
- let logSpy: ReturnType<typeof vi.spyOn>;
+describe('projects update --storage-size', () => {
let errors: string[];
- let errorSpy: ReturnType<typeof vi.spyOn>;
</file context>
There was a problem hiding this comment.
Same as the thread above: declined. The command is restored to its tested-by-nothing pre-315 form, and the server-side control is tested in insforge-cloud-backend PR 1000.
Summary
Disk resizes are admin-only. Provisioned disk is not billed, so customers can't grow it themselves. This reverts the
--volume-sizepart of PR 315, which is merged but has not been released (npm latest is 0.2.8).projects upgrade-instance <instanceType>is back to taking only an instance type.risk-registry.tsandUpgradeInstanceBodymatch the code from before 315.projects update --storage-sizestays hidden and fails before login or any API call with "Disk size changes go through InsForge support." Released versions shipped this flag, and it only saved a number.UpgradeInstanceResultdropsprevious/newVolumeSizeGiB. The customer endpoint no longer returns them.Companion backend PR: the customer
upgrade-instanceroute rejectsvolumeSizeGiBandsilent.Testing
manage.test.tschecks thatupdate --storage-size 20exits beforerequireAuthandupdateProject, and that the error names support. Removing the early throw turns it red. Fullvitest runpasses (839), and eslint and build are clean.🤖 Generated with Claude Code
Summary by cubic
Reverts the unreleased
--volume-sizedisk resize support, since provisioned disk is not billed and disk resizes go through InsForge support.projects upgrade-instance <instanceType>takes only an instance type again;--volume-sizeis removed and the result no longer includes volume sizes.projects update --storage-sizestill errors, but now fails before authentication with "Disk size changes go through InsForge support."Written for commit 08ff842. Summary will update on new commits.
Summary by CodeRabbit