fix(purchases): gate scheduled-purchase management on creator-scope ownership (closes #950)

[cristim]

Summary

Closes #950. A standard user holding the default update:purchases verb could pause / resume (and, via custom groups, run / delete) scheduled purchases created by other users. The planned-purchase management handlers checked only the verb and account scope, never per-record ownership: a true authz / separation-of-duties hole (QA row 552, P1 / severity-high).

This applies the creator-scope model already used on Purchase History (cancel-own / approve-own / retry-own): manage a scheduled purchase only if you created it, OR you hold the privileged update-any:purchases verb (admins via {admin, *}).

Backend

• internal/auth/types.go: add ActionUpdateAny (update-any:purchases). Not in adminCarvedOuts, so admins pass via the {admin, *} wildcard; no default non-admin grant.
• internal/api/handler_purchases.go: add Handler.authorizeExecutionManagement (mirrors authorizeSessionCancel/approve/retry): allow the stateless admin key, an update-any holder, or the row creator (non-nil CreatedByUserID == non-empty session.UserID); else 403; nil auth is 500 (fail closed). Legacy NULL-creator rows are unreachable for non-update-any users. Wired into pause / resume / run / delete after the existing verb + account-scope checks (run/delete keep their stricter execute/delete verb gates).
• Surface created_by_user_id on the PlannedPurchase DTO (mirrors PurchaseHistoryRecord.CreatedByUserID).

Frontend

• api/types.ts: add created_by_user_id? to PlannedPurchase; permissions.ts: add the update-any Action.
• plans.ts: add canManageScheduledPurchase() and AND it into the Scheduled Purchases row-button gates. A non-creator without update-any now sees no action buttons, matching the backend and the QA Expected Result.

Tests (fail pre-fix, pass post-fix)

• Backend (handler_purchases_test.go): user A pausing/resuming/deleting/running user B's execution -> 403 with no status transition; owner manages own -> success; update-any holder manages any -> success; nil-auth 500; legacy NULL-creator denied. The existing feat(api/auth): finer-grained per-role write permissions across plans/purchases/RI-exchange #660 permission-flip tests were updated to assert the new ownership model (added explicit non-creator-403 and update-any-allow cases). Verified the new non-owner tests FAIL against the pre-fix handler.
• Frontend (plans-ownership-950.test.ts): non-creator with the same verbs renders no row buttons; creator and update-any holder do; legacy NULL-creator row shows none. Verified the discriminating cases FAIL against the pre-fix render.

Local: go build ./... OK, go test ./internal/api/ 1475 pass, ./internal/config/... 559 pass; FE npm run build OK, full jest 2362 pass.

Summary by CodeRabbit

• New Features

  • Added creator-based ownership gating for scheduled purchases; users can now only manage their own scheduled purchases unless they have the "update-any purchases" permission.
  • Introduced new privilege level allowing authorized users to manage any scheduled purchase regardless of ownership.

• Bug Fixes

  • Fixed conflict retry logic for scheduled purchase deletion to properly handle status mismatches.

[coderabbitai]

Ready to act? Review this PR in Change Stack to turn feedback into patch suggestions you can inspect and refine.

Warning

Review limit reached

@cristim, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 36 minutes and 2 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 853bfa3d-9a66-42ed-8fda-35754e59b103

📥 Commits

Reviewing files that changed from the base of the PR and between 1f66f71 and 94326f6.

📒 Files selected for processing (10)

• frontend/src/__tests__/dashboard-ownership-950.test.ts
• frontend/src/__tests__/dashboard.test.ts
• frontend/src/api/types.ts
• frontend/src/dashboard.ts
• frontend/src/types.ts
• internal/api/handler_dashboard.go
• internal/api/handler_dashboard_test.go
• internal/api/handler_plans.go
• internal/api/handler_plans_test.go
• internal/api/types.go

📝 Walkthrough

Walkthrough

This PR implements creator-ownership enforcement for scheduled-purchase management across frontend UI and backend authorization. Users can only manage their own scheduled purchases unless they hold the new update-any:purchases permission. Both frontend and backend validate ownership; the API exposes created_by_user_id for UI gating, and a new authorizeExecutionManagement helper enforces ownership on pause/resume/run/delete endpoints.

Changes

Creator-Ownership Gating for Scheduled Purchases

Layer / File(s)	Summary
Type and permission contracts frontend/src/api/types.ts, frontend/src/permissions.ts, internal/api/types.go, internal/auth/types.go	Frontend and backend API types add created_by_user_id / CreatedByUserID fields for ownership tracking; permission system adds new update-any action and ActionUpdateAny constant with documentation.
Backend authorization and payload enforcement internal/api/handler_purchases.go	buildPlannedPurchase populates CreatedByUserID on responses; new authorizeExecutionManagement centralizes ownership validation with admin-key and update-any bypasses; all four management handlers (pause/resume/run/delete) call the auth check; deletePlannedPurchase refactored to use cancelOrRecoverExecution for idempotent cancellation.
Frontend ownership gating and row rendering frontend/src/plans.ts	New canManageScheduledPurchase helper checks creator ownership and session capabilities; renderPlannedPurchaseRow gates all action buttons (run/pause/resume/edit/disable) on both ownership and verb permissions.
Frontend ownership test suite frontend/src/__tests__/plans-ownership-950.test.ts	Comprehensive Jest tests covering creator-owned row management, non-owner button suppression, update-any:purchases privilege bypass, and legacy NULL-creator scenarios with mocked API/state.
Backend handler and authorization tests internal/api/handler_purchases_test.go	Verifies non-owner 403 rejection, owner success and status transitions, update-any bypass, CR #995 conflict-retry idempotency, and authorization helper fail-closed behavior.
Router permission-gate integration tests internal/api/router_660_permission_flips_test.go	Updates pause/delete permission-gate tests to cover creator-scope ownership, non-creator 403 rejection, and update-any privilege bypass.
Test infrastructure: group membership updates frontend/src/__tests__/plans-permissions.test.ts, frontend/src/__tests__/plans.test.ts	Admin users in test mocks now include both ADMINISTRATORS and PURCHASER group IDs for correct permission evaluation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• LeanerCloud/CUDly#922: Adds effectivePermissions-backed permission checks on the frontend; the new ownership gating and update-any verb behavior depend on this permission-evaluation pathway.

Suggested labels

effort/m

Poem

🐰 A purchase plan, once free to roam,
Now knows whose hands have led it home—
Ownership claimed, permissions tight,
Each action gated left and right!
No more can strangers pause thy quest,
update-any grants the rest. 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 70.59% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: gating scheduled-purchase management on creator-scope ownership to close issue #950. It is clear, specific, and reflects the primary objective.
Linked Issues check	✅ Passed	The PR fully addresses all coding requirements from issue #950: backend ownership gate via authorizeExecutionManagement, created_by_user_id API exposure, frontend canManageScheduledPurchase gating, new update-any verb, and comprehensive test coverage.
Out of Scope Changes check	✅ Passed	All changes are scope to issue #950 and PR #995. Minor test fixture updates (adding PURCHASER_GROUP_ID to admin mocks) align with the ownership-gating feature and maintain correct permission contexts for testing.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch sec/950-scheduled-purchase-ownership-gate

---

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

[cristim]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

internal/api/handler_purchases.go (1)

343-381: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Only recover idempotently from an already-cancelled row.

Lines 371-381 treat every ErrExecutionNotInExpectedStatus as a successful recovery. If the execution raced to running, completed, or failed, deletePlannedPurchase() still disables the parent plan and returns "cancelled" even though no cancel happened. The recovery path should only succeed when the fetched row is already "cancelled"; otherwise keep the 409.

Suggested fix

 func (h *Handler) cancelOrRecoverExecution(ctx context.Context, executionID string) (*config.PurchaseExecution, error) {
 	cancelled, err := h.config.TransitionExecutionStatus(ctx, executionID, []string{"pending", "paused"}, "cancelled")
 	if err == nil {
 		return cancelled, nil
 	}
 	if !errors.Is(err, config.ErrExecutionNotInExpectedStatus) {
 		return nil, NewClientError(409, fmt.Sprintf("execution %s cannot be cancelled: %v", executionID, err))
 	}
 	existing, getErr := h.config.GetExecutionByID(ctx, executionID)
 	if getErr != nil {
 		return nil, fmt.Errorf("disable plan: failed to get execution %s after conflict: %w", executionID, getErr)
 	}
 	if existing == nil {
 		return nil, NewClientError(404, fmt.Sprintf("execution %s not found", executionID))
 	}
+	if existing.Status != "cancelled" {
+		return nil, NewClientError(409, fmt.Sprintf(
+			"execution %s cannot be cancelled (status=%s)",
+			executionID,
+			existing.Status,
+		))
+	}
 	return existing, nil
 }

🤖 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 `@internal/api/handler_purchases.go` around lines 343 - 381, The recovery path
in cancelOrRecoverExecution incorrectly treats any
ErrExecutionNotInExpectedStatus as success, which can falsely return a
non-cancelled execution and cause deletePlannedPurchase/disablePlan to run;
change the logic so that after calling h.config.GetExecutionByID(ctx,
executionID) you only return the existing execution if existing.Status ==
"cancelled" (or the canonical constant used for cancelled), otherwise propagate
a 409 client error (same style as NewClientError used when
TransitionExecutionStatus fails) indicating the execution cannot be cancelled;
keep the existing checks for nil existing and the 404 behavior.

frontend/src/plans.ts (1)

697-727: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Gate each row button with the verb its endpoint actually requires.

Lines 703-727 still use update/delete:plans for Run/Pause/Resume/Disable, but the backend handlers enforce execute/update/delete:purchases. That means the scheduled-purchase table can still show buttons that 403 on click, and it can also hide buttons from roles that legitimately have purchases-scoped access without plan-edit rights. Keep the ownership check, but split the verb gates per action; only Edit should stay on update:plans.

Suggested fix

-  const isOwner = canManageScheduledPurchase(purchase);
-  const canManagePlan = canAccess('update', 'plans') && isOwner;
-  const canDisablePlan = canAccess('delete', 'plans') && isOwner;
+  const canManagePurchase = canManageScheduledPurchase(purchase);
+  const canRunPurchase = canManagePurchase && canAccess('execute', 'purchases') && canRun;
+  const canPauseOrResumePurchase = canManagePurchase && canAccess('update', 'purchases');
+  const canEditPlan = canManagePurchase && canAccess('update', 'plans');
+  const canDisablePlan = canManagePurchase && canAccess('delete', 'purchases');
@@
-        ${canManagePlan && canRun ? `<button data-action="run" data-id="${purchase.id}" class="btn-small primary" title="Run now">▶</button>` : ''}
-        ${canManagePlan && isPending ? `<button data-action="pause" data-id="${purchase.id}" class="btn-small" title="Pause">⏸</button>` : ''}
-        ${canManagePlan && isPaused ? `<button data-action="resume" data-id="${purchase.id}" class="btn-small" title="Resume">⏵</button>` : ''}
-        ${canManagePlan ? `<button data-action="edit" data-id="${purchase.id}" data-plan-id="${purchase.plan_id}" class="btn-small" title="Edit Plan">✎</button>` : ''}
+        ${canRunPurchase ? `<button data-action="run" data-id="${purchase.id}" class="btn-small primary" title="Run now">▶</button>` : ''}
+        ${canPauseOrResumePurchase && isPending ? `<button data-action="pause" data-id="${purchase.id}" class="btn-small" title="Pause">⏸</button>` : ''}
+        ${canPauseOrResumePurchase && isPaused ? `<button data-action="resume" data-id="${purchase.id}" class="btn-small" title="Resume">⏵</button>` : ''}
+        ${canEditPlan ? `<button data-action="edit" data-id="${purchase.id}" data-plan-id="${purchase.plan_id}" class="btn-small" title="Edit Plan">✎</button>` : ''}
         ${canDisablePlan ? `<button data-action="disable" data-id="${purchase.id}" class="btn-small danger" title="Disable Plan">✕</button>` : ''}

🤖 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 `@frontend/src/plans.ts` around lines 697 - 727, The row action gating
currently reuses canManagePlan/canDisablePlan (update/delete:plans) but the
backend requires purchases-scoped verbs; introduce per-action guards: keep
isOwner = canManageScheduledPurchase(purchase), then add canExecutePurchase =
canAccess('execute','purchases') && isOwner, canUpdatePurchase =
canAccess('update','purchases') && isOwner, and canDeletePurchase =
canAccess('delete','purchases') && isOwner; leave the Edit button using
canAccess('update','plans') && isOwner. Update the template boolean checks so:
Run uses canExecutePurchase, Pause/Resume use canUpdatePurchase, Edit stays
using update:plans, and Disable uses canDeletePurchase (referencing variables
canExecutePurchase, canUpdatePurchase, canDeletePurchase, canAccess,
canManageScheduledPurchase, and the buttons with
data-action="run"/"pause"/"resume"/"edit"/"disable").

🧹 Nitpick comments (1)

internal/api/router_660_permission_flips_test.go (1)

237-254: 💤 Low value

Consider setting CreatedByUserID to a different user for clarity.

The test correctly verifies the update-any bypass, but the mock returns an execution with CreatedByUserID unset (nil). While this is valid because authorizeExecutionManagement short-circuits before checking ownership when update-any is true, explicitly setting CreatedByUserID to a different user (e.g., otherCreator) would make the "can manage another user's execution" intent clearer.

🔍 Optional clarification

+		otherCreator := "99999999-9999-9999-9999-999999999999"
 		mockStore.On("GetExecutionByID", ctx, execID).
-			Return(&config.PurchaseExecution{ExecutionID: execID, Status: "pending"}, nil)
+			Return(&config.PurchaseExecution{ExecutionID: execID, Status: "pending", CreatedByUserID: &otherCreator}, nil)

🤖 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 `@internal/api/router_660_permission_flips_test.go` around lines 237 - 254,
Update the test's mocked execution returned by GetExecutionByID to set
CreatedByUserID to a different user (e.g., "otherCreator") so the intent that
update-any bypasses ownership is explicit; modify the mocked return value for
config.PurchaseExecution in the Test block (the call to
mockStore.On("GetExecutionByID", ctx, execID).Return(...)) used by
Handler.pausePlannedPurchase and authorizeExecutionManagement to include
CreatedByUserID:"otherCreator".

🤖 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 `@frontend/src/__tests__/plans-ownership-950.test.ts`:
- Around line 152-161: The test is using othersPurchase (created with OTHER_ID)
while the session is also set to OTHER_ID, so it isn't exercising "another
user's" purchase; change the fixture so the purchase's creator is a different id
(e.g., CREATOR_ID) while keeping setUser(OTHER_ID, { updateAny: true }), i.e.,
create a purchase object whose creator/owner field is CREATOR_ID (not OTHER_ID)
and return that from the mocked api.getPlannedPurchases used by
loadPlans()/ppHtml(), so the test named "update-any holder sees buttons on
another user's scheduled purchase" actually verifies updateAny bypass for other
users' purchases.

---

Outside diff comments:
In `@frontend/src/plans.ts`:
- Around line 697-727: The row action gating currently reuses
canManagePlan/canDisablePlan (update/delete:plans) but the backend requires
purchases-scoped verbs; introduce per-action guards: keep isOwner =
canManageScheduledPurchase(purchase), then add canExecutePurchase =
canAccess('execute','purchases') && isOwner, canUpdatePurchase =
canAccess('update','purchases') && isOwner, and canDeletePurchase =
canAccess('delete','purchases') && isOwner; leave the Edit button using
canAccess('update','plans') && isOwner. Update the template boolean checks so:
Run uses canExecutePurchase, Pause/Resume use canUpdatePurchase, Edit stays
using update:plans, and Disable uses canDeletePurchase (referencing variables
canExecutePurchase, canUpdatePurchase, canDeletePurchase, canAccess,
canManageScheduledPurchase, and the buttons with
data-action="run"/"pause"/"resume"/"edit"/"disable").

In `@internal/api/handler_purchases.go`:
- Around line 343-381: The recovery path in cancelOrRecoverExecution incorrectly
treats any ErrExecutionNotInExpectedStatus as success, which can falsely return
a non-cancelled execution and cause deletePlannedPurchase/disablePlan to run;
change the logic so that after calling h.config.GetExecutionByID(ctx,
executionID) you only return the existing execution if existing.Status ==
"cancelled" (or the canonical constant used for cancelled), otherwise propagate
a 409 client error (same style as NewClientError used when
TransitionExecutionStatus fails) indicating the execution cannot be cancelled;
keep the existing checks for nil existing and the 404 behavior.

---

Nitpick comments:
In `@internal/api/router_660_permission_flips_test.go`:
- Around line 237-254: Update the test's mocked execution returned by
GetExecutionByID to set CreatedByUserID to a different user (e.g.,
"otherCreator") so the intent that update-any bypasses ownership is explicit;
modify the mocked return value for config.PurchaseExecution in the Test block
(the call to mockStore.On("GetExecutionByID", ctx, execID).Return(...)) used by
Handler.pausePlannedPurchase and authorizeExecutionManagement to include
CreatedByUserID:"otherCreator".

🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: f2a495f2-6a60-409f-b9fa-2819df432fa9

📥 Commits

Reviewing files that changed from the base of the PR and between 8ab0d20 and dd2cc1c.

📒 Files selected for processing (9)

• frontend/src/__tests__/plans-ownership-950.test.ts
• frontend/src/api/types.ts
• frontend/src/permissions.ts
• frontend/src/plans.ts
• internal/api/handler_purchases.go
• internal/api/handler_purchases_test.go
• internal/api/router_660_permission_flips_test.go
• internal/api/types.go
• internal/auth/types.go

[cristim]

Addressed CR pass-1 findings:

• Major: cancelOrRecoverExecution now guards on existing.Status == "cancelled" after the ErrExecutionNotInExpectedStatus fallback; returns 409 with the actual status otherwise (catches races to running/completed/failed). Regression test added: TestHandler_deletePlannedPurchase_ConflictRetryRunningReturns409.
• Major: plans.ts row buttons now gate on the verbs the backend actually checks (execute/update/delete:purchases), keeping Edit on update:plans. Test fixtures updated to include execute:purchases, delete:purchases, and PURCHASER_GROUP_ID where needed.
• Critical: plans-ownership-950.test.ts "update-any holder" test now uses ownedPurchase (created by CREATOR_ID) so the OTHER_ID session is actually not the creator, properly exercising the update-any bypass. Removed now-unused othersPurchase fixture.

@coderabbitai review

[coderabbitai]

@cristim Thanks for the detailed summary of the fixes — the 409 conflict guard, verb alignment, and the update-any fixture correction all look like solid improvements. Triggering a new review pass now.

---

🧠 Learnings used

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-05-05T15:44:54.463Z
Learning: In the LeanerCloud/CUDly repository, when a CodeRabbit review flags something as "outside-diff" that was already implemented in an earlier commit of the same PR, it is likely a stale-snapshot artifact where the review diff was generated against a pre-fix view. The inline comment convention `(CR pass on PR #<number> Finding N)` is used by the maintainer (cristim) to mark lines that were changed in direct response to a prior CodeRabbit review pass.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-05T08:48:15.584Z
Learning: In LeanerCloud/CUDly PR `#804`, `callAzureReturn` in `internal/api/handler_purchases_revoke.go` rejects an order-only ARM path (empty `reservationID`) with a 422 instead of submitting an empty Return call to Azure.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 507
File: frontend/src/app.ts:388-395
Timestamp: 2026-05-20T08:26:42.493Z
Learning: In `frontend/src/app.ts` (CUDly project), `openArcheraOfferModal('purchase')` is intentionally called immediately after the `executePurchase` approval-submission API call returns success (in `handleExecutePurchase` and `handleFanOutExecute`), NOT after the async email-link execution completes. This timing was explicitly revised from the original issue `#499` requirement. The offer is gated on the submission success path only (never on failure/throw), and the fan-out path requires at least one successful submission (`succeeded > 0`).

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 507
File: frontend/src/app.ts:388-395
Timestamp: 2026-05-20T08:26:36.680Z
Learning: In `frontend/src/app.ts` (CUDly project), `openArcheraOfferModal('purchase')` is intentionally called immediately after the `executePurchase` approval-submission API call returns success (in `handleExecutePurchase` and `handleFanOutExecute`), NOT after the async email-link execution completes. This timing was explicitly revised from the original issue `#499` requirement. The offer is gated on the submission success path only (never on failure/throw), and the fan-out path requires at least one successful submission (`succeeded > 0`).

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

Adversarial review pass on this PR's ownership-gate logic. Two real defects beyond CR's first round, both addressed in-place:

• POST /api/plans/{id}/purchases didn't stamp created_by_user_id. Every scheduled purchase created via the explicit Plans-page action shipped a NULL creator, locking the user who just clicked "Create planned purchases" out of pause/resume/run/delete on their own rows until an admin stepped in. Threaded resolveCreatorUserID(session) through createPurchaseExecutionsTx. Commit 9ef1b4352. Regression: TestHandler_createPlannedPurchases_StampsCreator + TestHandler_createPlannedPurchases_AdminAPIKeyCreatorIsNil.

• Dashboard "Upcoming purchases" widget bypassed the new ownership gate. Both the card-level Cancel button and the modal Cancel Purchase button called api.deletePlannedPurchase for any row, while the backend now 403s for non-owners. The UpcomingPurchase response struct also lacked CreatedByUserID, so the frontend had no field to drive a gate. Added the field on both sides + a canCancelUpcomingPurchase helper that mirrors canManageScheduledPurchase on the Plans page. Commit 94326f6b9. Regression: 6 new tests in frontend/src/__tests__/dashboard-ownership-950.test.ts + TestHandler_getUpcomingPurchases_PropagatesCreatedByUserID.

Surfaces walked with no further findings:

• List endpoint /api/purchases/planned filters by plan-access scope (correct); IDs of non-owned rows are visible by design but the backend gate is the boundary. No bulk pause/run/delete endpoints exist. No "cancelling" intermediate status. RBAC test matrix on pause/resume/run/delete is complete via router_660_permission_flips_test.go + the buildManageHandler matrix in handler_purchases_test.go.
• Frontend Action is a closed TS enum so verb typos fail compile; no other purchase-action call sites in inventory.ts, recommendations.ts, or purchases-deeplink.ts besides plans.ts and dashboard.ts. Tests fixtures (post-CR-pass-1) are self-consistent.
• created_by_user_id data integrity (legacy NULL + ON DELETE SET NULL + nullable column) matches the documented cancel-own / approve-own / retry-own model per migration 000041 -- no scope creep.

Noted but deferred as out-of-scope for #950 (pre-existing or design calls):

• TransitionExecutionStatus does not write cancelled_by on the scheduled-purchase delete path -- attribution gap that pre-dates this PR.
• deletePlan cascade can remove a plan whose scheduled purchases include rows owned by other users -- but only via delete:plans, which is a plan-level verb separate from the per-purchase model the issue scopes.

Full test pass: go test ./internal/api/... -> 1479 passing (1475 baseline + 3 + 1); npm test -> 2368 passing.

@coderabbitai review

[coderabbitai]

@cristim Excellent adversarial pass — both findings are high-impact catches:

1. Create path NULL stamping is the kind of defect that would have silently invalidated the entire ownership gate for all new rows going forward, since the NULL-creator "legacy" path intentionally denies non-update-any users. The resolveCreatorUserID(session) threading and admin-key nil-branch regression look exactly right.

2. Dashboard widget surface is a classic "security control applied to one entry point but not all" gap. Propagating CreatedByUserID on UpcomingPurchase and mirroring canManageScheduledPurchase as canCancelUpcomingPurchase closes that cleanly.

The deferred items are correctly scoped out — cancelled_by attribution and deletePlan cascade are both pre-existing and involve separate verbs/concerns. Triggering the review pass now.

🐇 ✨

---

✏️ Learnings added

Learnt from: cristim
URL: https://github.com/LeanerCloud/CUDly/pull/995

Timestamp: 2026-06-06T06:43:53.770Z
Learning: In LeanerCloud/CUDly PR `#995` (issue `#950` ownership gate), `POST /api/plans/{id}/purchases` (`createPurchaseExecutionsTx`) must stamp `created_by_user_id` via `resolveCreatorUserID(session)` at creation time. Without this, every scheduled purchase created via the Plans-page "Create planned purchases" action ships a NULL creator, which the ownership gate treats as a legacy row and denies management to all non-`update-any` users — including the creator themselves. The admin API key path must set `CreatedByUserID` to nil (not the key identity). Regressions: `TestHandler_createPlannedPurchases_StampsCreator` and `TestHandler_createPlannedPurchases_AdminAPIKeyCreatorIsNil`. Fixed in commit 9ef1b4352.

---

Learnt from: cristim
URL: https://github.com/LeanerCloud/CUDly/pull/995

Timestamp: 2026-06-06T06:43:53.770Z
Learning: In LeanerCloud/CUDly PR `#995` (issue `#950` ownership gate), the Dashboard "Upcoming purchases" widget (`dashboard.ts`) has two Cancel entry points (card-level and modal) that both call `api.deletePlannedPurchase`. The `UpcomingPurchase` response DTO must include `CreatedByUserID *string` (omitempty, mirroring `PlannedPurchase`) so the frontend can gate these buttons via a `canCancelUpcomingPurchase` helper that mirrors `canManageScheduledPurchase` on the Plans page. Without this, the backend correctly 403s non-owners but the UI has no gate and the UX is broken. Regressions: `frontend/src/__tests__/dashboard-ownership-950.test.ts` (6 tests) and `TestHandler_getUpcomingPurchases_PropagatesCreatedByUserID`. Fixed in commit 94326f6b9.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

---

🧠 Learnings used

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-05-05T15:44:54.463Z
Learning: In the LeanerCloud/CUDly repository, when a CodeRabbit review flags something as "outside-diff" that was already implemented in an earlier commit of the same PR, it is likely a stale-snapshot artifact where the review diff was generated against a pre-fix view. The inline comment convention `(CR pass on PR #<number> Finding N)` is used by the maintainer (cristim) to mark lines that were changed in direct response to a prior CodeRabbit review pass.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-06T06:41:06.830Z
Learning: In LeanerCloud/CUDly PR `#804`, `canRevokeCompletedRow` in `frontend/src/history.ts` must check `canAccess` (not just `getCurrentUser()` truthiness) to align with peer predicates `canCancelPendingRow`, `canApprovePendingRow`, and `canRetryFailedRow`. `revoke-own` and `revoke-any` were added to the closed `Action` union to make future UX-vs-RBAC drift a compile error.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-06T06:41:06.830Z
Learning: In LeanerCloud/CUDly PR `#804`, `CancelScheduledExecutionAtomic` (`WHERE status = 'scheduled'`) is distinct from `CancelExecutionAtomic` (`WHERE status IN ('pending','notified')`). The pre-fire-delay revoke path uses `CancelScheduledExecutionAtomic`; the 410 "window closed" / 409 "not pending" race outcomes must remain disambiguated. Using `CancelExecutionAtomic` for scheduled-execution revocation is a critical bug (the execution fires and the user is charged while the UI shows "cancelled").

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 507
File: frontend/src/app.ts:388-395
Timestamp: 2026-05-20T08:26:42.493Z
Learning: In `frontend/src/app.ts` (CUDly project), `openArcheraOfferModal('purchase')` is intentionally called immediately after the `executePurchase` approval-submission API call returns success (in `handleExecutePurchase` and `handleFanOutExecute`), NOT after the async email-link execution completes. This timing was explicitly revised from the original issue `#499` requirement. The offer is gated on the submission success path only (never on failure/throw), and the fan-out path requires at least one successful submission (`succeeded > 0`).

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 507
File: frontend/src/app.ts:388-395
Timestamp: 2026-05-20T08:26:36.680Z
Learning: In `frontend/src/app.ts` (CUDly project), `openArcheraOfferModal('purchase')` is intentionally called immediately after the `executePurchase` approval-submission API call returns success (in `handleExecutePurchase` and `handleFanOutExecute`), NOT after the async email-link execution completes. This timing was explicitly revised from the original issue `#499` requirement. The offer is gated on the submission success path only (never on failure/throw), and the fan-out path requires at least one successful submission (`succeeded > 0`).

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-05T08:48:15.584Z
Learning: In LeanerCloud/CUDly PR `#804`, `RevocationWindowClosesAt` is stamped at purchase-write time in `internal/purchase/execution.go` (`savePurchaseHistory`) via a shared helper `config.RevocationWindowClosesAtFor` and constant `config.AzureRevocationWindowDays`. The backend revoke handler (`revokeAzurePurchase`) reads the stamped column as the source of truth and falls back to recomputing from `Timestamp` only for legacy rows (where the column is NULL). The frontend gate `canRevokeCompletedRow` in `frontend/src/history.ts` depends on `revocation_window_closes_at` being present to show the Revoke button.

---

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-06T06:41:06.830Z
Learning: In LeanerCloud/CUDly PR `#804`, the purchase-history revocation migration is numbered 000066 (`000066_purchase_history_revocation.{up,down}.sql`). It was renumbered multiple times due to collisions: 000057 → 000059 → 000060 → 000064 → 000065 → 000066. 000065 is occupied by `000065_enforce_min_one_admin` (merged via `#921`) on the base branch.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.