fix(plans): guard delete, reschedule and resume of managed plans

[cristim]

Deleting an editor-managed plan detached its live schedule roots, and
rescheduling or resuming a planned purchase moved a date the editor
owns. Deleting a managed plan that still has a live purchase now
answers 409 managed_plan_has_live_executions; settled history detaches
as before. Rescheduling or resuming a purchase of a managed plan
answers 409 editor_reconciliation_required. Each check runs under the
plan lock after authorization, so a writer that waited behind an editor
Save sees its committed state. OpenAPI documents the 409 responses on
the affected operations.

Refs #631

Part 4 of 5 of the #631 legacy-writer guards; stacked on #1012; merge bottom first.

Review verdict and evidence

Original review, copied from the per-commit evidence record. Commit 4e1bdb76 was reviewed before the rebase onto origin/main; the rebased commit is aeb8700.

• Reviewed tree bbb644dec5204f103ad3de2a0fc5812b3dcb7ccc (commit 4e1bdb76). +400/-34 (at the cap, additions only).
• Pass 1: one medium (scoped-caller resume 404 case), fixed; pass 2: clean.
• Evidence: lint 0; race unit 4163 pass; integration Delete/Reschedule/Resume/Managed/Add/Assign/OpenAPI 54 pass, 0 skip; pre-commit pass.
• Probes: any row blocks delete; partially_completed blocking; plain read instead of lock in delete and reschedule (race tests fail); drop early resume check; drop delete mapping; remove resume 409 from OpenAPI (contract test fails); drop requireExecutionAccess (scoped resume test fails).
• New 409 code: managed_plan_has_live_executions. Follow-up feat(plans): settle a failed-with-submission row so a managed plan can be deleted #999 (failed-with-submission row blocks delete).

Rebase: the five commits were rebased onto origin/main 0ed85aa (27 commits ahead of the old base). git range-diff of the old 5 commits against the rebased 5 shows all five as = (identical patches), so the verdict carries over per the owner's rule. New head SHA of this PR: aeb8700. No migration is added by any of the five commits.

Review gate

CodeRabbit is not used on this stack (quota; owner decision): the gate is the independent adversarial review above plus the local verification below.

Local verification

Local verification of the rebased stack tip (macOS, Docker Postgres via testcontainers; Linux via CI):

• go build ./... and go vet ./...: pass
• CI-pinned golangci-lint v2.10.1: 0 issues
• go test -race -count=1 ./internal/config/ ./internal/api/ ./internal/purchase/...: config ok, purchase ok, api has one failure, TestAuthPublicRoutes_EveryRouteIsCSRFGuardedOrExempt (route POST /api/purchases/approve/*/details unclassified). It fails identically on a clean origin/main 0ed85aa and is not touched by this stack.
• Integration (-tags integration -p 1 -count=1) on the same packages: 5838 pass, 1 fail (the same test), 14 skip (the known pre-existing skips: 11 TestPostgresStoreDB_* in config; TestGetFederationIaC_TfvarsAutoLoadedByTerraform/aws-cross-account, TestRevokePurchase_FourEyesApproval, TestRevoke_4EyesMode_TODO in api).

Status on main

The change is dormant on main until the editor Save route ships: only plans marked managed (ramp_schedule.dates_managed) are affected, and only that route sets the marker.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Managed plans can no longer be deleted while they have active or unsettled purchases.
  • Rescheduling or resuming purchases on editor-managed plans is refused when it would conflict with the managed schedule.
  • Conflicts now return HTTP 409 responses with clearer error details and machine-readable codes.

• Documentation

  • Updated API documentation to describe conflict responses for plan updates, deletion, rescheduling, and purchase operations.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: 667c0a78-7506-4398-8f0d-ea262cce947f

📥 Commits

Reviewing files that changed from the base of the PR and between 110a496 and 8aa0ce7.

📒 Files selected for processing: 11

• internal/api/handler_plans.go
• internal/api/handler_plans_managed_add_accounts_integration_test.go
• internal/api/handler_plans_managed_delete_resume_integration_test.go
• internal/api/handler_purchases.go
• internal/api/openapi.yaml
• internal/api/openapi_409_code_test.go
• internal/api/plan_editor_errors.go
• internal/config/errors.go
• internal/config/store_postgres.go
• internal/config/store_postgres_managed_guard.go
• internal/config/store_postgres_pgxmock_test.go

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

📝 Walkthrough

Managed-plan deletion now checks for live executions before removing a plan. Rescheduling and resume paths refuse selected changes to editor-managed plans. API responses and OpenAPI contracts document the conflicts and their error codes.

Changes: Managed plan guards

Layer / File(s)	Summary
Store guards and transactions internal/config/errors.go, internal/config/store_postgres_managed_guard.go, internal/config/store_postgres.go, internal/config/store_postgres_pgxmock_test.go	DeletePurchasePlan locks the plan and checks for live executions when its dates are editor-managed. RescheduleAwaitingExecution locks the referenced plan and refuses rescheduling when its dates are editor-managed. Store tests cover the locked-read transaction behavior.
API guards and responses internal/api/handler_plans.go, internal/api/handler_purchases.go, internal/api/plan_editor_errors.go, internal/api/openapi.yaml	Plan deletion maps the live-execution error to HTTP 409 with a machine-readable code. Resume and rescheduling paths map editor-reconciliation refusals to the corresponding API error. OpenAPI documents these conflicts and defines ErrorWithCode.
Guard integration coverage internal/api/handler_plans_managed_add_accounts_integration_test.go, internal/api/handler_plans_managed_delete_resume_integration_test.go, internal/api/openapi_409_code_test.go	Integration tests cover deletion, rescheduling, resume, authorization precedence, and lock waits. The OpenAPI test checks that the listed operations document 409 responses using ErrorWithCode.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant deletePlan
  participant DeletePurchasePlan
  participant PostgreSQL
  Client->>deletePlan: Send DELETE request
  deletePlan->>DeletePurchasePlan: Delete plan
  DeletePurchasePlan->>PostgreSQL: Lock plan and check linked executions
  PostgreSQL-->>DeletePurchasePlan: Plan and execution data
  DeletePurchasePlan-->>deletePlan: Return deletion result or ErrManagedPlanLiveExecutions
  deletePlan-->>Client: Return success or HTTP 409 with error code

Loading

Suggested reviewers: iamjsd

Merge Risk: ⚪ Minimal · up to 8aa0c

No actionable managed-plan guard issue remains; the PR is mergeable after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 31.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 10 files. (1 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly summarizes the main changes: guarding managed-plan deletion, rescheduling, and resume operations.
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.

Full details: Docstring Coverage

Explanation

Docstring coverage is 31.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 10 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

• Fix all pre-merge checks with AI

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

Rebased onto main 0a5f790 (picks up #1003, which fixed the CSRF classification test that failed on the old base); git range-diff vs the previous head is empty, so the review verdict in the description carries. New head SHA: aeb8700.

[cristim]

Independent review of 62f0cce: no actionable findings

This review covers the pre-rebase SHA 62f0cce. The merge driver will record the range-diff to the new head (aeb8700). I ran git range-diff on the PR's own commit: it shows = (identical patch).

Findings (all NON-BLOCKING):

• internal/api/handler_purchases.go:~278 vs :308: the early managed check only runs for past-dated rows with recommendations and reads the plan without the lock. If an editor Save lands between the paused->pending transition and RescheduleAwaitingExecution, the caller gets 409 editor_reconciliation_required but the row is already pending (past date, with recommendations). The scheduler does not buy pending rows with recommendations, and the editor reconciles it, so no purchase and no strand. Worth a comment, not a fix.
• Delete refusal blocks on failed-with-submission rows forever (store_postgres_managed_guard.go requireManagedDeleteTx). Fail closed is intended and is tracked as #999 in the next PR.

What I attacked:

• Delete: all 12 status cases, including a lock-wait race where the command inserts a root and marks the plan managed before the delete's lock is granted. The refusal does not strand executions: cancel, pause and run-now never touch the plan guard. Only settled history detaches.
• Resume: no resume path exists in internal/purchase (grep). The only resume is the handler, and run-now from paused (approvals.go:355) changes no date, so there is no inconsistency. RescheduleAwaitingExecution has one caller.
• openapi_409_code_test.go parses openapi.yaml for real (yaml.v3 decode of the file, $ref resolution to components.responses, asserts ErrorWithCode on all 5 operations).
• Ran integration tests (testcontainers, Postgres): api and config -run 'Delete|Reschedule|Resume|Managed|OpenAPI' pass. OpenAPI409 and PGXMock_Delete unit tests pass.

Gate: independent review of this SHA (this comment). CI not applicable until retarget. CodeRabbit not used (owner decision). Local verification as above plus the PR body evidence. Labels mirror the stack; no closing issue ("Refs #631"). 400 additions, at the cap (additions-only rule).

[cristim]

The independent review above covers 62f0cce. claude-cc-631-driver rebased the stack onto main 0a5f790 (includes #1003).
Pure rebase: git range-diff 7bd4ef7c..62f0cce0 9e78df11..aeb87008:

1:  62f0cce0 = 1:  aeb87008 fix(plans): guard delete, reschedule and resume of managed plans

The PR's own commits are unchanged, so the verdict carries to aeb8700 (owner #631 rule).

[cristim]

The independent review above covers aeb8700. Rebased onto main after #1010 merged (0117179); #1011 retargeted to main.
Pure rebase: git range-diff 9e78df11..aeb87008 eb94c5b6..7e7d99bc:

1:  aeb87008 = 1:  7e7d99bc fix(plans): guard delete, reschedule and resume of managed plans

The PR's own commits are unchanged, so the verdict carries to 7e7d99b (owner #631 rule).

[cristim]

The independent review above covers 7e7d99b. Rebased onto main after #1011 merged (173474e); #1012 retargeted to main.
Pure rebase: git range-diff eb94c5b6..7e7d99bc 07f7e9ea..832a2e51:

1:  7e7d99bc = 1:  832a2e51 fix(plans): guard delete, reschedule and resume of managed plans

The PR's own commits are unchanged, so the verdict carries to 832a2e5 (owner #631 rule).

[cristim]

The independent review above covers 832a2e5. Rebased onto main after #1012 merged; #1013 retargeted to main.
Pure rebase: git range-diff a4696f0f..832a2e51 110a4968..8aa0ce7d:

1:  832a2e51 = 1:  8aa0ce7d fix(plans): guard delete, reschedule and resume of managed plans

The PR's own commits are unchanged, so the verdict carries to 8aa0ce7 (owner #631 rule).

[cristim]

Gate record for 8aa0ce7 (origin/main CLAUDE.md "Review gate"):

• Independent review of 62f0cce (non-author): no actionable findings.
  • Delete and resume of managed plans are refused. Cancel, pause and run-now still work, so nothing is stranded.
  • internal/purchase has no separate resume path, so the handler guard covers it.
  • openapi_409_code_test.go really parses openapi.yaml and resolves the $refs.
  • Non-blocking: a Save that lands between the paused-to-pending transition and the reschedule returns a 409 and leaves the row pending. The scheduler does not buy it, so nothing is stranded. The failed-with-submission delete block is tracked in feat(plans): settle a failed-with-submission row so a managed plan can be deleted #999.
• 8aa0ce7 is a pure rebase of 62f0cce (via aeb8700, 7e7d99b and 832a2e5) onto main after fix(config): refuse legacy writes to editor-managed plans #1010, fix(api): answer refused managed-plan writes with a 409 code #1011 and fix(plans): refuse Add Purchases and account changes on managed plans #1012. All range-diffs are = (recorded above), so the verdict carries under the owner feat(plans): plan steps resolve real recommendations and show real counts and prices (tier B of #609) #631 rule.
• CI on 8aa0ce7: CI Success and Run pre-commit hooks pass.
• Local verification (macOS, testcontainers): the lock-wait race test passes, and the api and config integration tests pass. No real-account run or purchase.
• CR skipped: quota-limited; covered by the independent review plus local verification.
• Labels mirrored from feat(plans): plan steps resolve real recommendations and show real counts and prices (tier B of #609) #631 (Refs feat(plans): plan steps resolve real recommendations and show real counts and prices (tier B of #609) #631). Additions are 400, exactly at the cap.

Merging at 8aa0ce7 with --match-head-commit. #1014 will be retargeted to main and rebased next.