Skip to content

fix(api): resume of a planned purchase: capped-user, double-read and empty-step cap edges (#1847 follow-ups) #1866

Closed

Description

@cristim

Follow-ups from the cold reviews of #1847 (the resume amount cap, closes #1130). None blocks that change; each is a small edge in resumePlannedPurchase (internal/api/handler_purchases.go).

Work

  1. A legacy row with an empty CommitmentCurrency cannot be resumed by a capped user. The row counts as non-USD, so the refusal tells them to "refresh the recommendations so they carry a USD price", and a paused row cannot be refreshed. This follows approve's existing semantics, so it is not new in kind. An admin or an uncapped user has to resume such a row. Say so in the refusal text, or find a path that lets the caller refresh a paused row.
  2. The row is read twice. requireExecutionAccess and loadPlannedExecution each read it, so the tenant check and the cap use separate snapshots. Nothing changes a paused row's accounts or recommendations in between today, so there is no gap, but passing one read through would remove the question.
  3. The caller's cap does not follow a step that has no recommendations yet. A row with no recommendations skips the cap check on resume (there is no amount to compare), and the recommendations attach later. At attach time the plan's own max_step_commitment binds the purchase (attachWithinCap, plan_step_attach.go, and CheckStepCommitmentCap, step_cap.go), and a plan with no cap refuses auto-purchase, so the amount is bounded by the plan owner's cap, not unbounded. Decide whether the resuming caller's own cap should also be carried to the attach, or whether a capped caller should be refused an empty-step resume. CodeRabbit raised this on fix(api): cap resume of a planned purchase by the caller's amount limit #1847 and it was answered as by design there.

Done when

Each item is either fixed with a test or closed with a reason on this issue.

Related: #1130, #1847.

Activity

  1. cristim commented on Nov 10, 2026

    @cristim
    MemberAuthor

    Claimed by claude-ui-fix-1 (coordinator: take the next unclaimed platform item after #1904, which needs owner decisions; see the comment there). Investigating the three resume edges against current main first, then a failing test per edge that is real.

  2. cristim commented on Nov 10, 2026

    @cristim
    MemberAuthor

    Landed on main as c1ffc30 (claude-ui-fix-1), closing this.

    1. Fixed: the shared refusal now says a stored or paused purchase cannot be refreshed and that an administrator or a user without a spend cap must act on it instead (the refresh remedy stays for direct execute). A test covers a legacy empty-currency paused row.
    2. Closed, no change: the first read (requireExecutionAccess) only feeds the account-scope check for scoped sessions; recommendations attach only to pending or notified rows, so nothing rewrites a paused row between the two reads, and the pinned resume transition (TransitionExecutionStatusPinned with the recommendation-set emptiness) already loses the CAS if the set changes. One read would leave the same window.
    3. Closed, by design as decided on fix(api): cap resume of a planned purchase by the caller's amount limit #1847: a resumed empty row only restores the plan's pre-pause state, and attach is bounded by the plan's own cap (attachWithinCap, then recheckStepCap after the claim); a plan with no usable cap refuses. Raising an armed plan's cap needs the editor's own arm grant.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions