fix(purchases): align token cancel path with session path and guard in-flight rows (closes #645)

[cristim]

Summary

The token/email cancel path (loadCancelableExecution, internal/purchase/approvals.go) only rejected completed/cancelled, so it allowed cancelling approved/running/paused/failed/expired rows. The session path (cancelPurchaseViaSession) rejects everything except pending/notified. This was both a privilege asymmetry (an email-link holder could cancel rows the dashboard refuses) and a missing in-flight guard (cancelling an approved/running row mid-execution desyncs DB and cloud).

Fix

Replaced the token-path predicate with Status != "pending" && Status != "notified", mirroring the session path. Genuinely-pending rows still cancel normally; mid-execution rows are now rejected on both paths.

Test plan

• Token path: each non-cancelable status rejected with no store write; pending/notified still cancel + persist
• Session path: same status matrix asserted for parity
• go vet + gofmt clean; cancel tests (37) + full internal/api (1191) pass

Closes #645.

Summary by CodeRabbit

• Bug Fixes

  • Purchase cancellation now consistently only allows pending or notified executions; attempts to cancel other statuses are rejected with a clear error.

• Tests

  • Added regression tests ensuring cancellation rejects non-cancelable states and succeeds for pending/notified.

• Refactor

  • Centralized the cancelability rule so all cancellation paths behave consistently.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7a4e761b-cf52-4459-958f-41907f5cdad7

📥 Commits

Reviewing files that changed from the base of the PR and between d5e7b05 and c78a60d.

📒 Files selected for processing (3)

• internal/api/handler_purchases.go
• internal/config/types.go
• internal/purchase/approvals.go

---

📝 Walkthrough

Walkthrough

Centralizes cancelability in PurchaseExecution.IsCancelable(), uses it in both token and session cancel guards, and adds manager- and handler-level regression tests for allowed (pending/notified) and disallowed statuses.

Changes

Execution Cancelability Alignment

Layer / File(s)	Summary
Cancelable predicate addition internal/config/types.go	Adds PurchaseExecution.IsCancelable() which returns true only for pending and notified, serving as the single cancelability predicate.
Token-path cancelability guard update internal/purchase/approvals.go	loadCancelableExecution now calls execution.IsCancelable() and returns the same "execution cannot be cancelled, current status: …" error when false.
Session-path handler guard update internal/api/handler_purchases.go	cancelPurchaseViaSession switched its pre-check to execution.IsCancelable() instead of an explicit pending/notified whitelist.
Manager-level cancellation tests internal/purchase/approvals_test.go	Added tests: one asserts CancelExecution rejects non-cancelable statuses with an error and no persistence; the other asserts pending/notified succeed and persist Status=cancelled.
HTTP handler session-path tests internal/api/handler_purchases_test.go	Added session-path tests: one verifies non-cancelable statuses are rejected without transaction writes; the other verifies pending/notified proceed via the existing success helper.

🎯 3 (Moderate) | ⏱️ ~20 minutes

🐰 I nibbled through code with a happy cheer,
Pending and notified now stand clear,
Token and session paths finally near,
No mid-flight cancels to fear—hip, hop, hooray!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% 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 clearly and specifically summarizes the main change: aligning token and session cancel paths while guarding in-flight rows, with explicit issue reference.
Linked Issues check	✅ Passed	The PR fully addresses all coding requirements from issue #645: centralizes cancelability predicate into IsCancelable(), aligns both cancel paths to only permit pending/notified, adds comprehensive test coverage for both paths across all statuses, and prevents cancellation of in-flight executions.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to issue #645: the IsCancelable() method, updated cancel logic in both paths, and regression tests for the cancelability matrix are all necessary to address the asymmetry and in-flight guard requirements.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/645-token-cancel-status-alignment

---

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

[cristim]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

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]

🧹 Nitpick comments (1)

internal/purchase/approvals.go (1)

191-199: ⚡ Quick win

Centralize the cancelable-status predicate to prevent policy drift.

This guard now correctly matches the session path, but the "pending"/"notified" rule is still duplicated across paths. Please consider extracting a shared predicate/constant and reusing it in both flows to avoid reintroducing asymmetric behavior in future edits.

🤖 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/purchase/approvals.go` around lines 191 - 199, Extract the
"cancelable" predicate into a single shared symbol and use it in both places:
replace the inline status check (if execution.Status != "pending" &&
execution.Status != "notified") with a call to a new helper (e.g., func
IsCancelableExecution(status string) bool) or a package-level constant slice/set
and reuse that same helper in cancelPurchaseViaSession so both flows share
identical logic; ensure the helper is located in the same package
(internal/purchase) and update both call sites to use
IsCancelableExecution(execution.Status) to prevent duplication and future drift.

🤖 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.

Nitpick comments:
In `@internal/purchase/approvals.go`:
- Around line 191-199: Extract the "cancelable" predicate into a single shared
symbol and use it in both places: replace the inline status check (if
execution.Status != "pending" && execution.Status != "notified") with a call to
a new helper (e.g., func IsCancelableExecution(status string) bool) or a
package-level constant slice/set and reuse that same helper in
cancelPurchaseViaSession so both flows share identical logic; ensure the helper
is located in the same package (internal/purchase) and update both call sites to
use IsCancelableExecution(execution.Status) to prevent duplication and future
drift.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 94a13b8e-e91a-450a-97b8-c26d1c349643

📥 Commits

Reviewing files that changed from the base of the PR and between 45a92e7 and d5e7b05.

📒 Files selected for processing (3)

• internal/api/handler_purchases_test.go
• internal/purchase/approvals.go
• internal/purchase/approvals_test.go

[cristim]

Addressed the nitpick: centralized the pending/notified cancelable predicate into a single PurchaseExecution.IsCancelable() method in internal/config/types.go, and wired both cancel paths to it (purchase.Manager.loadCancelableExecution and the session-authed cancelPurchaseViaSession). This removes the duplicated inline status check so the policy can no longer drift between the token and session flows. Behavior is unchanged — only pending/notified rows stay cancelable; approved/running and other in-flight states remain non-cancelable. The session approve check was intentionally left inline since it is a distinct approvability policy that merely happens to share the same statuses. Commit c78a60d.

@coderabbitai review

[coderabbitai]

@cristim Great approach — centralizing the cancelability predicate into PurchaseExecution.IsCancelable() is a clean, maintainable solution. Having a single source of truth in internal/config/types.go ensures the token and session paths can never silently diverge again. The rationale for leaving the session approve check inline is also sound and well-documented.

Triggering a review of commit c78a60dd7 now.

✅ Actions performed

Review triggered.

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.