fix(plans): a ramp step counts as complete when one of N accounts buys, and both silent-under-buy paths are log-only

[cristim]

Split out of #1669 (fix in flight on fix/1669-ramp-step-idempotent), which deliberately kept its scope to "one ramp step must not be counted twice". Two problems remain, and they are the same problem seen from two sides: a plan can quietly buy less commitment than the customer intended, and nothing durable says so.

1. A ramp step counts as complete when ONE account's execution runs clean

CompletePlanStep advances the ramp when any execution for that step finishes successfully. A multi-account plan fans one ramp step out into one execution per cloud account, each of which succeeds or fails independently, so an operator who repairs and retries a single failed account moves the plan to "step N done" while the other accounts have bought nothing for step N.

This predates #1669 and is unchanged by it: the pre-#1669 blind CurrentStep++ had the same granularity, plus the double-count that #1669 removes. It is visible directly in that fix's own regression test (internal/purchase/ramp_step_progress_integration_test.go): a 3-account plan whose step-3 fan-out committed only account A reaches CurrentStep = 3 as soon as account B's retry succeeds, while account C is still failed.

The original issue asked for this decision to be settled explicitly:

a ramp step is only complete when every account's row for that step has succeeded, not when any one of them has. Worth settling that explicitly in this issue before implementing.

It was not settled, and #1669 does not settle it. Settling it is this issue.

Options, roughly in order of preference:

1. Derive progress instead of accumulating it (fix(plans): retrying two failed accounts of one ramp step advances CurrentStep twice, so a later step silently never purchases #1669's option 2): compute CurrentStep from the executions table as the highest step whose per-account rows are all terminal-and-successful, rather than storing an incrementally-mutated counter. This also removes the migration/backfill class of problem entirely, because there is no stored convention left to get wrong.
2. Gate the advance on sibling completeness: keep the counter, but only advance when no per-account row for that step is outstanding. Cheaper, but needs care that a permanently-failed account cannot freeze the ramp forever, which is the failure mode option 1 avoids by construction.

Whichever is chosen, "the plan's target accounts at the time the step ran" has to be pinned down, since a plan's account set can change between steps.

2. Two reachable money-path decisions are recorded only in a log line

Both of these silently under-buy and neither leaves durable evidence. purchase.executeAndFinalize calls updatePlanProgress for side effect only and swallows any error into logging.Errorf, so once the Lambda log ages out there is nothing to find.

• A refused advance. CompletePlanStep refuses to advance when the completing step is more than one beyond CurrentStep, because the steps in between never completed and jumping would overstate what the plan bought. Reachable whenever a step fails terminally and a later step then succeeds. The plan silently stops advancing, next_execution_date stops moving, and plan_health can only infer "behind schedule" from wall-clock drift, which it reports identically for a plan that is merely late.
• A mis-stamped step. A purchase_executions row whose step_number predates fix(plans): retrying two failed accounts of one ramp step advances CurrentStep twice, so a later step silently never purchases #1669's convention correction, or that was written during the deploy overlap, completes a step the plan has already counted. The advance is a correct no-op, and equally invisible.

Wanted:

• Stamp the outcome on the execution row (an audit note alongside error) so a refused or no-op advance surfaces in History rather than only in CloudWatch.
• Give plan_health a distinct stalled-ramp factor, keyed on the recorded refusal instead of letting behind_schedule proxy for it.

Why this is P1 rather than P2

The failure direction is the same silent under-buy that #1669 was filed for: commitment the customer intended to buy that never gets bought, with nothing erroring and the plan reporting itself further along than it is. Multi-account plans are the normal shape for the customers who ramp, and per-account retry became the canonical recovery flow in #1655, so item 1 is reachable on the routine path rather than an edge case.

Related

• fix(plans): retrying two failed accounts of one ramp step advances CurrentStep twice, so a later step silently never purchases #1669 — ramp step counted twice; the fix that deliberately left these two open.
• fix(purchase): keep account scope on per-account retry successors #1655 / fix(purchase): retrying a per-account fan-out row re-fans-out and double-buys every account that already succeeded #1537 — per-account retry scope, which makes item 1 routine.
• fix(purchases): multi-account run mislabels partial success as failed; finalizeExecution coupling is fragile #1014 — multi-account partial-success classification.

[cristim]

Adding a third concrete failure to item 2, surfaced by CodeRabbit's review of #1862 and confirmed against the code.

A stale step_number takes the "already counted" branch, which is silent by construction and cannot be flagged at this layer.

CompletePlanStep returns nil when CurrentStep >= stepNumber, so a row stamped k on a plan already at CurrentStep = k produces no error and no log line at all. The other two paths in this issue at least log; this one does not.

It is indistinguishable from the designed case, which is why no guard in #1862 can catch it. A second per-account retry of a step a sibling already counted also arrives with CurrentStep == stepNumber, and that no-op is exactly what #1669 was filed to produce. Both look identical to the store, so flagging one flags the other and makes the normal recovery path noisy.

Why it matters more than "the plan is one step behind": the stall is not self-correcting. CompletePlanStep returns before its write, so next_execution_date stays stale; shouldNotifyPlan computes daysUntil := int(time.Until(*plan.NextExecutionDate).Hours() / config.HoursPerDay) and returns false once that is negative. The plan is then never notified again, getOrCreateExecution is never reached, and no further executions are created for it. On the notification-driven path the plan goes quiet permanently rather than retrying, so the remaining ramp steps never buy.

Reachable in two windows that #1862's migration deliberately does not close: instances still running the old writer during the deploy rollout, and retry successors of rows that were already terminal when the migration ran, since persistRetryExecution propagates failedExec.StepNumber and terminal rows are not rewritten.

This is the strongest argument for option 1 over option 2 in the original description. Deriving CurrentStep from the executions table separates the two cases naturally, because the derivation looks at what actually completed rather than at a counter plus a stored convention that can go stale. A sibling-completeness gate on the accumulated counter would not: it inherits the same ambiguity, since it still has to trust the row's step_number to know which step is being reported.

For the record, #1862 does now stamp the two detectable refusals on the execution row (recordRampAdvanceRefusal), so those surface in History rather than only in CloudWatch. This third path is the one that remains invisible, and it is invisible by construction rather than by omission.