feat(mcp): require confirm_name for delete_workspace and send X-Confirm-Name header #7
Reference in New Issue
Block a user
Delete Branch "feat/delete-workspace-confirm-name-mcp58"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes molecule-mcp-server#58.
The tenant's destructive-action gate refuses workspace deletion unless the caller echoes the workspace's exact name in the
X-Confirm-Nameheader. The previousdelete_workspacetool only sent?confirm=true, so agents could not complete deletion even after human approval.Changes
apiCallnow accepts an optionalheadersmap.handleDeleteWorkspacerequiresconfirm_nameand sends it asX-Confirm-Name, while preserving?confirm=true.deprovision_workspaceas an alias fordelete_workspace(both names are used in the issue).CLAUDE.md, and tests updated; refusal and alias are tested.Test plan
npm testpasses (153 passed, 1 skipped).npm run buildpasses.SOP Checklist
confirmboolean was already insufficient and is replaced by the explicit name check.🤖 Generated with Claude Code
SOP checklist
SOP checklist is now complete in the PR body and CI is green. Awaiting a peer review and
/sop-ackso this can land. Thanks!0b9834062bto5f749bd5845f749bd584to7426a8ec8e5-axis review: current delta requires confirm_name before destructive workspace deletion, sends X-Confirm-Name through apiCall, adds deprovision alias coverage, and keeps response-format tests updated. CI/all-required is green; note a non-required channel test was still pending when checked.
/sop-ack 1
/sop-ack 2
/sop-ack 3
/sop-ack 4
/sop-ack 5
/sop-ack 6
/sop-ack 7
/sop-ack comprehensive-testing
/sop-ack local-postgres-e2e
/sop-ack staging-smoke
/sop-ack root-cause
/sop-ack five-axis-review
/sop-ack no-backwards-compat
/sop-ack memory-consulted
/sop-ack comprehensive-testing
/sop-ack local-postgres-e2e
/sop-ack staging-smoke
/sop-ack five-axis-review
/sop-ack memory-consulted
5-axis review: REQUEST_CHANGES.
Correctness/security: The delete confirmation change itself is directionally sound: requiring
confirm_nameand sendingX-Confirm-Namecomposes with the tenant destructive-action gate, and the alias is covered by tests.Blocking scope/backcompat issue: the current diff is stacked with the unrelated provider-migration tool additions from molecule-mcp#6 (
handleMigrateWorkspaceProvider,handleGetWorkspaceMigrationStatus, CP helpers, tool-count changes, and migration tests). If #7 lands as-is before #6, it silently ships/closes the migration-provider surface under a delete-confirmation PR; if #6 lands first, this PR should be rebased so the diff contains only the delete/deprovision confirmation work. Please rebase/split so #7's current diff is scoped to delete confirmation.Tests/ops: current Gitea status at
7426a8eshows server/all-required green and channels/claude skipped, not failed; the blocker is the stacked unrelated code, not CI.New commits pushed, approval review dismissed automatically according to repository settings
5-axis review: REQUEST_CHANGES. This stacked head still contains the migration-status endpoint bug from #6: get_workspace_migration_status calls GET /migrate-provider instead of /migration-status, so the status tool is wired to the wrong CP route. Also the delete-confirmation delta adds a deprovision_workspace alias and raises registered tools to 91, but server/src/index.ts still logs '90 tools available', so operational startup logs drift from actual registration. CI/all-required is green; delete confirm_name/X-Confirm-Name behavior itself looks well covered.
3817bd2c60to46fa096de0APPROVED after re-review at head
46fa096d. The prior migration-status route issue is gone because this branch is rebased to contain only the delete-confirmation delta; no migrate-provider/migration-status code remains in the diff. The stable registered tool count is now 89, matching one added deprovision_workspace alias over the 88-tool base. CI/all-required is green; delete_workspace now requires confirm_name, sends X-Confirm-Name, and tests cover the success/refusal/alias paths.5-axis re-review: APPROVED.
The prior blocker is resolved at head
46fa096d: the PR is no longer stacked with the provider-migration tool additions from #6, and the tool-count assertion is now the expected focused +1 (89) from adding thedeprovision_workspacealias. Correctness: delete/deprovision now requireconfirm_name, refuse empty values before network calls, and sendX-Confirm-Namewith the existing?confirm=truedestructive-action path. Tests: server build/test, channels/claude, and all-required are green; tests cover header send, refusal, alias, and response invariants. Security: strengthens destructive delete confirmation. Scope/backcompat: intentional tool schema tightening for a previously insufficient destructive call. Ops: CI context adjustment keeps required channels/claude posted successfully.46fa096de0to8f4bdeab71APPROVED at head
8f4bdeab. Diff is now scoped to delete/deprovision confirmation only: no migration-tool route code remains, the stable tool-count assertion is 89, delete_workspace requires confirm_name before making the destructive call, and apiCall sends X-Confirm-Name. Tests cover success, missing confirm_name refusal, alias behavior, and response formatting. CI/all-required and server build/test are green./sop-ack comprehensive-testing
/sop-ack local-postgres-e2e
/sop-ack staging-smoke
/sop-ack root-cause
/sop-ack five-axis-review
/sop-ack no-backwards-compat
/sop-ack memory-consulted
/sop-ack root-cause
/sop-ack no-backwards-compat
/sop-ack comprehensive-testing
/sop-ack local-postgres-e2e
/sop-ack staging-smoke
/sop-ack five-axis-review
/sop-ack memory-consulted
Re-triggering sop-checklist-gate after molecule-mcp#9 (trusted-acker fallback) merged to main.
Re-triggering sop-checklist-gate after molecule-mcp#9 merged to main. Existing /sop-ack comments from reviewers should now satisfy all items via the trusted-acker fallback.
APPROVED. 5-axis review on current head
f29eb82e: delete_workspace now requires confirm_name and sends X-Confirm-Name while preserving confirm=true, and deprovision_workspace is only an alias to the same handler. apiCall header extension is additive and tests cover header emission, missing confirmation refusal, alias behavior, and tool count. This strengthens destructive-action safety without broadening access.APPROVE on
f29eb82e. 5-axis review: destructive workspace deletion now requires non-empty confirm_name and sends it as X-Confirm-Name for platform exact-name enforcement; blank confirmations fail before fetch, deprovision alias shares the same handler, tests cover header/refusal/alias paths, and docs describe the requirement. CI and SOP gate are green.New commits pushed, approval review dismissed automatically according to repository settings
New commits pushed, approval review dismissed automatically according to repository settings
5efe7e4b05to8b7f631fddAPPROVED on rebased head
8b7f631f. Re-reviewed the conflict-resolved diff: delete_workspace now requires non-empty confirm_name, sends X-Confirm-Name with the existing confirm=true delete request, and deprovision_workspace is a same-handler alias. apiCall header support is additive, tests cover header emission/refusal/alias behavior, and tool counts were correctly rebased after #6. No correctness, robustness, security, performance, or readability blockers found.APPROVE on
8b7f631. Rebased current head is sound: delete_workspace now fails before fetch without non-empty confirm_name, sends the trimmed exact name as X-Confirm-Name for platform-side confirmation, and deprovision_workspace aliases the same handler/schema. Tests cover header emission, blank-confirm refusal, alias behavior, and response-format call sites; tool count/logging is updated to 91 after main advanced. CI and SOP gate are green.