feat(cli): add project rename, remove, and transfer#107
Conversation
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis pull request adds three new project lifecycle commands—rename, remove, and transfer—to the Prisma CLI, spanning documentation, type definitions, token storage enhancements for pinned workspace resolution, recipient session validation for transfers, mock API fixture/adapter updates, controller logic, presenters, CLI command registration, and integration tests. Separately, it adds rendering of command success warnings in human-mode CLI output. Changes
Sequence Diagram(s)sequenceDiagram
participant Controller
participant resolveRecipientWorkspaceSession
participant FileTokenStorage
participant ManagementApiSdk
Controller->>resolveRecipientWorkspaceSession: workspaceRef
resolveRecipientWorkspaceSession->>FileTokenStorage: resolveWorkspace(workspaceRef)
resolveRecipientWorkspaceSession->>ManagementApiSdk: GET /v1/workspaces (probe)
ManagementApiSdk-->>resolveRecipientWorkspaceSession: refreshed tokens
resolveRecipientWorkspaceSession-->>Controller: RecipientWorkspaceSession
Estimated code review effort: 4 (High) - The change spans many files with new destructive/mutating operations, recipient token validation, local pin lifecycle management, and outcome-union error handling that require careful cross-file verification. Related issues: None specified in the provided context. Related PRs: None specified in the provided context. Suggested labels: cli, feature, project-management Suggested reviewers: None specified in the provided context. Poem A rabbit hops through workspaces new, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@docs/product/command-spec.md`:
- Line 699: Update the `prisma-cli project rename` heading to mark `--project
<id-or-name>` as optional, matching the bracket convention used by other
commands like `project link`; adjust the command title in the command-spec docs
so it no longer implies the flag is required and stays consistent with the body
text and examples.
In `@packages/cli/src/adapters/mock-api.ts`:
- Around line 211-214: removeProject() in mock-api.ts currently deletes matching
databases but leaves their databaseConnections behind, creating orphaned state.
Update removeProject() to mirror removeDatabase() by collecting the removed
database IDs from this.data.databases and filtering
this.data.databaseConnections to drop any entries whose databaseId matches those
removed databases. Keep the change localized to the removeProject() flow and the
database/databaseConnections state management symbols.
In `@packages/cli/src/adapters/token-storage.ts`:
- Around line 51-57: The pinned/read-only token path in token-storage must not
mutate the active workspace. Update the workspace resolution flow in
token-storage so `resolveWorkspace()` uses a non-migrating read path instead of
`listWorkspaces()`, and ensure any refresh logic bypasses
`maybeActivateWorkspaceId` whenever `pinnedWorkspaceId` is set. This should keep
`activeWorkspaceId` untouched during pinned SDK auth/refresh while still
resolving the pinned workspace’s credentials correctly.
- Line 359: The return in the token storage match handling uses an unnecessary
non-null assertion, since the existing length checks already guarantee a match
in the relevant branch. Update the logic in token-storage.ts around the
match-selection flow so the return from the matches array uses the indexed value
directly without the non-null assertion, keeping the surrounding validation in
place.
In `@packages/cli/src/controllers/project.ts`:
- Around line 802-866: `requireProjectCommandContext` and
`requireProjectMutationContext` both duplicate real-mode auth/client setup, and
`requireProjectCommandContext` also calls `requireProjectClient(context)` twice.
Refactor these paths to create the `ManagementApiClient` once per command
invocation, reuse it for both `listProjects` and
`createManagementProjectProvider`, and have `requireProjectMutationContext`
delegate to `requireProjectClient` instead of redoing the
`requireComputeAuth`/`authRequiredError` flow. Keep the changes centered around
`requireProjectCommandContext`, `requireProjectMutationContext`, and
`requireProjectClient`.
In `@packages/cli/src/lib/project/provider.ts`:
- Around line 51-60: The project rename flow in provider.ts is casting the API
payload directly to RawProjectRecord without validating its shape. Update the
logic around the result.data.data handling to add a runtime check for the
required project fields (at least id and name, and url when present) before
building the return object. If the payload is invalid, fail fast with the same
error path used by projectRenameFailedError so malformed Management API
responses do not propagate into ProjectSummary.
- Around line 41-61: renameProject currently collapses every failure into
PROJECT_RENAME_FAILED, which hides non-name-related errors; update the error
handling in renameProject to match removeProject/transferProject by
special-casing the expected validation failure and otherwise passing through the
underlying API error via projectApiError so the original error code/status is
preserved. Keep the existing success path and use the same client.PATCH result
handling around result.error, result.data, and projectRenameFailedError to
locate the change.
🪄 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: 0fb346c3-7d41-482e-8f6b-c60f907b46d5
📒 Files selected for processing (15)
docs/product/command-spec.mddocs/product/resource-model.mdpackages/cli/fixtures/mock-api.jsonpackages/cli/src/adapters/mock-api.tspackages/cli/src/adapters/token-storage.tspackages/cli/src/commands/project/index.tspackages/cli/src/controllers/project.tspackages/cli/src/lib/auth/recipient.tspackages/cli/src/lib/project/provider.tspackages/cli/src/presenters/project.tspackages/cli/src/shell/command-meta.tspackages/cli/src/types/project.tspackages/cli/tests/project-mutations.test.tspackages/cli/tests/project-usecases.test.tspackages/cli/tests/project.test.ts
luanvdw
left a comment
There was a problem hiding this comment.
Looks good!
One small thing for us humans, the new remove/transfer flows collect best-effort local pin cleanup warnings, but those warnings only make it into the command success object/JSON. The human presenters render only result, so if the remote mutation succeeds but deleting/rewriting/clearing .prisma/local.json fails, human users see the success message without any hint that their local binding still points at stale or inaccessible state. That will make the next command failure feel surprising.
Could we surface those cleanup warnings in human output too, either as renderMutate warning alerts for project remove / project transfer, or via a generic success-warning renderer in the command runner? The rest of the behavior looks good, this is just making the partial local-state failure visible.
43070e8
|
@luanvdw Fixed in the latest commit, and I took the generic option: the command runner now renders |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Completes the project group's remote lifecycle: - project rename <name>: renames the resolved Project (durable binding or --project); pins bind by id, so directory bindings stay valid - project remove <project> --confirm <project-id>: permanent removal with the exact-id confirmation convention; surfaces the platform's active-deployments block as PROJECT_REMOVE_BLOCKED and clears this directory's local pin when it pointed at the removed project - project transfer <project> (--to-workspace <id-or-name> | --recipient-token <token>) --confirm <project-id>: moves a Project to another workspace. --to-workspace resolves a locally stored OAuth session (the auth workspace use targets) and authorizes the transfer with it, refreshing through the SDK via a workspace-pinned token storage view that never touches the active-workspace pointer. --recipient-token is the cross-account and headless path. A matching local pin is rewritten to the recipient workspace when known, otherwise cleared. Structured recovery codes for agents: PROJECT_RENAME_FAILED, PROJECT_REMOVE_BLOCKED, PROJECT_TRANSFER_REJECTED, TRANSFER_RECIPIENT_REQUIRED, TRANSFER_RECIPIENT_UNAVAILABLE, plus the existing workspace selection errors and CONFIRMATION_REQUIRED with expectedConfirm/receivedConfirm meta. Spec-first: command-spec.md and resource-model.md updated alongside the implementation.
- resolveWorkspace no longer runs the legacy auth-context migration, so it is genuinely read-only, and pinned token-storage views skip workspace activation entirely, even when no active workspace is set; the transfer contract no longer depends on call order - renameProject maps only 400/422 to PROJECT_RENAME_FAILED and surfaces other API failures (401/403/5xx) through the generic error passthrough instead of misreporting them as a rejected name - requireProjectCommandContext builds the ManagementApiClient once per invocation; requireProjectMutationContext reuses requireProjectClient - mock removeProject also deletes the removed databases' connections - drop a redundant non-null assertion in resolveWorkspace
…runner remove/transfer confirmation nextSteps, transfer recipient errors, the post-transfer workspace-use hint, and provider recovery commands now render through the project's package runner (pnpm dlx / bunx / npx -y with @prisma/cli@latest) instead of the hardcoded prisma-cli bin name, matching the agent group's convention.
Best-effort steps (local pin cleanup on remove/transfer) collected warnings that only reached the JSON envelope; human users saw a clean success even when their local binding was left stale. The command runner now renders success warnings as ⚠ lines in human mode for every command, so partial local-state failures are visible. Also registers the new project/transfer error codes in error-conventions.md.
0aeb5c9 to
31eb1ec
Compare
|
@coderabbitai approve |
✅ Action performedComments resolved and changes approved. |
Completes the
projectgroup's remote lifecycle: rename, remove, and transfer. Spec-first (command-spec.md+resource-model.mdupdated in the same change).Design notes
--to-workspaceresolves a locally stored OAuth session (same targets asauth workspace use) through a new workspace-pinnedFileTokenStorageview that refreshes via the SDK and can never move the active-workspace selection;--recipient-tokenis the cross-account/headless path. UnderPRISMA_SERVICE_TOKEN,--to-workspacefails withTRANSFER_RECIPIENT_UNAVAILABLE.--confirm..prisma/local.json; transfer rewrites it to the recipient workspace when known, else clears it; both best-effort with warnings.PROJECT_REMOVE_BLOCKED(active deployments),PROJECT_TRANSFER_REJECTED,PROJECT_RENAME_FAILED,TRANSFER_RECIPIENT_REQUIRED/_UNAVAILABLE,CONFIRMATION_REQUIREDwith meta.Testing: 14 new integration tests, full suite green, and all three commands verified against the live Management API with scratch projects, including a real cross-workspace transfer (project visible in the recipient workspace, pin rewritten, cleaned up afterwards).
Note: merge after #106 or rebase; both touch
mock-api.ts/fixtures.