Skip to content

feat(purchase): retry a failed first attempt that provably sent nothing (EC2, Azure savings plans) - #1870

Merged
cristim merged 7 commits into
mainfrom
feat/1803-nosubmission-retry
Nov 9, 2026
Merged

cristim merged 7 commits into
mainfrom
feat/1803-nosubmission-retry

Conversation

@cristim

@cristim cristim commented Nov 9, 2026 •

Copy link
Copy Markdown
Member

Closes #1803. Money path (double-buy risk): please review adversarially.

Problem

RedriveRefusalReason refuses Retry for EC2 and Azure savings-plan purchases whatever the row's NoSubmission says. That is right when a request may have reached the provider: neither API takes a client token. But a failed row that provably never reached the provider, such as a per-account skip row or a pre-execution refusal, could only be replaced by a brand-new purchase. (Redshift was handled separately in #1832.)

Change

Retry is allowed only for a failed first attempt (retry_attempt_n = 0) with no_submission set, and only for the two refusals that are about a prior submission (the AWS no-guard reason and the Azure savings-plan reason). Everything else keeps today's refusal, including unknown providers.

NoSubmission proves one row, not a lineage: retry successors reuse the lineage's idempotency key, and a successor can fail with NoSubmission while attempt 0 may have bought. So a retry of a retry is not lifted, and the proof is checked at three points:

  1. Retry time and History (purchase.RetryRefusalReason, used by the Retry gate and the History hint): the failed row is no_submission at attempt 0.
  2. Link (LinkUnsentRetryExecutionAtomic): when the lift applies, the successor is linked through the same compare-and-set as before plus AND no_submission AND retry_attempt_n = 0, so a stale read cannot link a submitted row. Other retries keep the existing method, unchanged.
  3. Execution time (armedRedriveRefusal): the claimed successor's own no_submission is always false, so the check reads its predecessor (PredecessorProvenUnsent: failed, no_submission, attempt 0). First purchases, and retries with no no-guard refusal (RDS, savings plans, GCP), keep today's path with no lookup. A lookup error refuses with a fixed text that tells the user to start a new purchase, and does not stamp no_submission.

RedriveRefusalReason itself is unchanged, so the reaper's in-place re-drive is unaffected.

Commits (each at most 400 changed lines)

  1. retry predicate (internal/purchase/retry_unsent.go)
  2. PredecessorProvenUnsent, sharing one SQL fragment with StampFirstDispatchTx
  3. require a failed status on that predecessor (the no_submission => failed link is enforced only by the upsert's update branch). Because StampFirstDispatchTx shares that SQL fragment, the Redshift first-dispatch stamp rule now also requires the predecessor to be failed. This only makes the rule stricter, so it fails closed: a stamp is minted in a subset of the old cases, and when none is minted the Redshift gate falls back to its existing no-stamp refusal
  4. the execution-time check
  5. review fixes: the lookup-error refusal is pinned as not no_submission; docs after fix(purchase): allow Redshift re-drive behind the retry, lineage and journal gates #1832
  6. Retry handler, History hint, the guarded link, stale comments and openapi.yaml
  7. skipped-account note: Retry is offered only when the gate would allow it

What reviewers should attack

  • Any writer of no_submission = true that can run after, or concurrently with, a provider call for that row or its lineage (all four writers were audited: saveTerminalExecution with a typed before-submission error (manager.go:472), skippedAccountRows (execution.go:480), markFailedPlanStep (notifications.go:375), and markRefusedAccountRow (refusal_tally.go:51, which stamps a failed per-account row only when every rec was refused before submission and none holds a commitment)).
  • A retry successor, or a row that shares an idempotency key, being treated as proven unsent.
  • The execution-time lookup: every executor reaches it (approve, cron, fire, reaper), first purchases make no lookup, and a lookup error fails closed.
  • The link CAS: a stale read cannot link a submitted row; the settled clause is kept.
  • Skip rows copy RetryAttemptN from their root, so skip rows of a retried root stay refused and keep the "start a new purchase" text.

Tests

  • Unit: retry_unsent_test.go (table: first attempt allowed; submitted, attempt 1, not-failed and unknown-provider refused), armed_redrive_unsent_test.go (proven predecessor executes with exactly one provider call; unproven, attempt 2 and lookup error refuse with zero calls; first purchases and RDS make no lookup), handler_purchases_retry_unsent_test.go, execution_skipped_accounts_test.go.
  • Integration (real Postgres): TestPredecessorProvenUnsent (including a no-submission attempt-0 row that is not failed), TestLinkUnsentRetryExecutionAtomic (links; refuses not-no-submission, attempt 1, already linked, settled).
  • Each task went through a two-pass independent review with mutation probes: the shared predicate (10 probes), the execution-time check (8 probes plus fail-before on the parent), the Retry handler and link (10 probes), and the skip-row note (4 probes). Two probes survived along the way and were closed: the lookup-error refusal was not pinned as "not no_submission" (an assertion was added), and one reach-test assertion could not fail (it was removed as redundant).

Notes

  • No migration; no deploy-order constraint.
  • Known, pre-existing: a Redshift skip row from a root at attempt 1 or later shows the Retry note, but the Redshift gate refuses it after 24 hours (this PR does not change that; it cannot cause a double buy).
  • Not covered by this PR: per-account rows that fail at credential resolution or the shape pin are before submission but are not stamped no_submission, so they stay unretryable.
  • A successor that fails no_submission again is not retryable (attempt-0-only proof); the user starts a new purchase.

🤖 Generated with Claude Code

cristim and others added 7 commits November 9, 2026 17:02
…s for unsent first attempts

RetryRefusalReason lifts only the AWS no-guard and Azure savings-plan redrive reasons, and only for a failed first attempt proven to have sent nothing. RedriveRefusalReason is unchanged for the reaper.

Part of #1803

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…irst attempt

Part of #1803

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Only the upsert's update path ties no_submission to a failed status, so the shared predecessor check now states it explicitly. A redundant reach-test assertion that could not fail is dropped.

Part of #1803

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…xecute

At execution time the claimed successor's own no_submission is always false, so the check reads its predecessor: a first retry runs only when the predecessor is a failed, no_submission attempt 0. First attempts and retries without a no-guard refusal keep today's path with no lookup; a lookup error refuses and tells the user to start a new purchase.

Part of #1803

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ry docs after the Redshift lift

The lookup-error refusal must not stamp no_submission. #1832's Redshift guard test now calls the Manager method that replaced the free armedRedriveRefusal, and comments and the T1 table reflect that #1832 removed the Redshift no-guard refusal, so only EC2 and Azure savings plans are lifted.

Part of #1803

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…h a guarded link

Retry and the History hint use RetryRefusalReason. When the no-guard refusal is lifted, the successor links through LinkUnsentRetryExecutionAtomic, which also requires the failed row to be no_submission at attempt 0, so a stale read cannot link a submitted row; a lost link is the existing 409.

Part of #1803

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…y gate allows it

skippedAccountNote follows RetryRefusalReason, so skip rows of a first attempt on EC2 or Azure savings plans now say Retry, while skip rows of a retried root keep the start-a-new-purchase text.

Part of #1803

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cristim cristim added priority/p2 Backlog-worthy severity/medium Moderate harm urgency/this-quarter Within the quarter impact/few Limited audience effort/m Days type/feat New capability labels Nov 9, 2026
@coderabbitai

coderabbitai Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 5 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 9de6afd and f5e59e3.

Files selected for processing: 22

  • internal/api/handler_history.go
  • internal/api/handler_purchases.go
  • internal/api/handler_purchases_retry_unsent_test.go
  • internal/api/openapi.yaml
  • internal/config/interfaces.go
  • internal/config/store_postgres.go
  • internal/config/store_postgres_first_dispatch.go
  • internal/config/store_postgres_first_dispatch_integration_test.go
  • internal/config/store_postgres_link_unsent_integration_test.go
  • internal/config/store_postgres_ramp_step.go
  • internal/config/types.go
  • internal/mocks/stores.go
  • internal/purchase/armed_redrive_test.go
  • internal/purchase/armed_redrive_unsent_test.go
  • internal/purchase/aws_no_token_redrive_test.go
  • internal/purchase/errnotes.go
  • internal/purchase/errnotes_test.go
  • internal/purchase/execution.go
  • internal/purchase/execution_skipped_accounts_test.go
  • internal/purchase/manager.go
  • internal/purchase/retry_unsent.go
  • internal/purchase/retry_unsent_test.go
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@cristim

cristim commented Nov 9, 2026

Copy link
Copy Markdown
Member Author

APPROVED f5e59e3 by claude-fleet

Cold review of #1870 (Closes #1803, money path), on main 9de6afd. git merge-tree against the current main (bc5392d) is clean. CI at this head is green, including Unit Tests and Integration Tests (config, purchase and api): run 42653339479. Locally, on a fresh export of this head under a lock, with no cloud access:

  • The scoped unit tests in internal/purchase and internal/api (Retry, ArmedRedrive, ArmedRetry, SkippedAccount, ProvenUnsent) pass, and go vet and gofmt are clean.
  • I ran the two real-Postgres tests (TestPredecessorProvenUnsent, TestLinkUnsentRetryExecutionAtomic, -tags=integration): both pass.
  • I treated the safety argument as a claim and checked each part against the code. I found no way for a lifted retry to buy twice.

Probes: 16 of 16 killed, each on a fresh copy of this head:

Protection Probe Killed by
Predicate drop the failed status (retry_unsent.go:9) TestRetryRefusalReason/NS_attempt_0_not_failed_EC2
Predicate lift every reason, not just EC2 and Azure savings plans (retry_unsent.go:21) .../NS_unknown_provider, .../NS_mixed_EC2_and_unknown
Execution time predecessor result ignored (always unproven) TestArmedRetryOfProvenUnsentFirstAttemptExecutes/aws_ec2, /azure_savingsplans
Execution time always treated as proven TestArmedRetryRefusedWithoutProof/ec2_unproven, /azure_sp_unproven
Execution time a lookup error proceeds to buy (manager.go:598-601) TestArmedRetryRefusedWithoutProof/ec2_lookup_error, /azure_sp_lookup_error
Execution time a lookup error stamps no_submission the same two cases
Execution time first purchases also look up (manager.go:586) TestArmedRedriveRefusalDoesNotLookUpUnlessNeeded/ec2_attempt_0 and /azure_sp_at...
Link always the plain link CAS (handler_purchases.go:2908) TestRetryOfProvenUnsentRowLinksViaUnsentCAS
Link always the guarded link TestRetryPurchase_EmailFailureNotRecorded_Returns500
Link SQL drop no_submission AND retry_attempt_n = 0 (store_postgres.go:1301) TestLinkUnsentRetryExecutionAtomic/refuses_a_row_that_is_not_no_submission, .../refuses_a_no_submission_row_pa...
Predecessor SQL drop p.status = 'failed' (store_postgres_first_dispatch.go:25) TestPredecessorProvenUnsent/no_submission_attempt_0_but_running
Predecessor SQL drop p.retry_attempt_n = 0 .../no_submission_attempt_1
Predecessor SQL drop p.no_submission .../submitted_attempt_0
History use the reaper's predicate TestExecutionToHistoryRow_RetryHintForUnsentRows
Skip-row note use the reaper's predicate (execution.go:456) TestSkippedAccountNote_MatchesRetryEligibility

An independent property check of the predicate. Over a grid of 256 rows (8 provider and service pairs, 4 statuses, no_submission on and off, attempts 0 to 3), the Retry predicate lifts a refusal that the reaper's predicate keeps in exactly 2 cases: AWS EC2 and Azure savings plans, each as a failed attempt-0 row with no_submission. It never lifts any other combination.

What I attacked

  • A retry or History action racing the execution-time check.
    • History only reads and renders a hint.
    • The Retry creates the successor and links it in one transaction (writeRetrySuccessorTx).
    • The execution-time check on the successor finds its predecessor by that link (retry_execution_id = successor, store_postgres_first_dispatch.go:25-31), not by the idempotency key, so a row that merely shares the lineage key is never treated as proven.
    • Nothing clears retry_execution_id (the only write to it is the link, store_postgres.go:1315), so after one successor exists a second retry of the same row is refused for good.
  • The CAS under concurrent clicks. The link is one UPDATE conditional on status = 'failed', retry_execution_id IS NULL, the settled clause, and (for the lifted case) no_submission AND retry_attempt_n = 0. The loser matches zero rows and its whole transaction rolls back (errRetryLinkRaceLost), so no orphan successor remains. A stale read cannot link a row that stopped being proven.
  • The proof going stale between Retry and execution. The upsert keeps no_submission only while the row stays failed and clears it on any other status (store_postgres.go:1207). I searched for a transition that moves a failed row back to a running state without that upsert and found none. The reaper's in-place re-drive and allRecsSafeToRedrive still use the unchanged RedriveRefusalReason (stranded_redrive.go:17, manager.go:548), so EC2 rows are never re-driven automatically, and an interrupted stranded row is failed without no_submission.
  • Attempt 1 and deeper. The lookup runs only for attempt 1, and the predecessor query itself requires attempt 0. A successor that fails again, even with its own no_submission, is not retryable (NeverSubmitted requires attempt 0). Attempt 2 and above are refused whatever a store answers, and a lookup error refuses with fixed text and does not stamp no_submission (manager.go:598-602).
  • Writers of no_submission = true. I audited every site independently: saveTerminalExecution (manager.go:472, needs a failed row, no rec purchased and a typed pre-submission cause), markRefusedAccountRow (refusal_tally.go:51, same condition per account), skippedAccountRows (execution.go:481, an account whose purchase never started) and markFailedPlanStep (notifications.go:375, reached only through a compare-and-swap from pending or notified). None stamps the proof on a row that was submitted.
  • Other tenants. The Retry handler's access checks run before the gate, and the lookup and link are keyed by execution ID through the retry FK, so they cannot cross plans or tenants.
  • Skip-row wording against the real state. The note is computed after Status = failed and NoSubmission = true are set, and a skip row inherits its root's attempt number, so a retried root's skip rows stay on the "start a new purchase" text. TestSkippedAccountNote_MatchesRetryEligibility pins that the note and the gate agree.
  • Every caller of the changed predicates. The Retry gate (handler_purchases.go:2639), the link choice (:2908), History (handler_history.go:415) and the skip note use RetryRefusalReason. The reaper and stranded-approval paths (manager.go:548,589, stranded_redrive.go:17, redshift_lineage.go:202) keep RedriveRefusalReason, as the body says.

Informational notes, no change asked

  1. The body lists three writers of no_submission; there are four sites (markRefusedAccountRow in refusal_tally.go:51 is the fourth). It meets the same condition as the others, so this is a gap in the list only.
  2. StampFirstDispatchTx now shares the predecessor fragment, which adds p.status = 'failed' to the Redshift stamp rule (store_postgres_first_dispatch.go:25-31,51-55). A predecessor that is no_submission at attempt 0 but not failed no longer mints a first-dispatch stamp. That fails closed, since a row with no stamp is refused by the Redshift gate, but it is a small Redshift behaviour change that the body's Notes do not mention.
  3. A lifted retry is single-use. After the first successor is linked, the failed row cannot be linked again, so if that successor is canceled before it runs, the user must start a new purchase. This is the safe direction.

@cristim

cristim commented Nov 9, 2026

Copy link
Copy Markdown
Member Author

APPROVED f5e59e3 by claude-cc-platform

Head at start and at end: f5e59e3 (did not move). Reviewed on main 9de6afd.

Blocking findings

None.

Non-blocking

  1. Ramp interplay, checked further by me after the reviewer's draft. holdsCapacity (config/store_postgres_ramp_baseline.go:131-137) counts a failed no_submission row as holding no capacity. That only affects how the NEXT steps are sized. It does not let the same step fire a second time: a failed unit is not in RampStepSettledStatuses (config/types.go:545, which lists only completed, partially completed and canceled), so OccupiedRampStepsInRangeTx (config/store_postgres_ramp_step.go:196-205) counts it as a live unit and refuses to mint a second execution for that step, and the completeness gate holds the step open until the unit buys or the account is detached (store_postgres_ramp_step.go:19-49). That argues against a retry plus a re-fired step both buying for EC2 or Azure savings plans, which have no provider dedupe. What I did not trace is the scheduler's own claimPlanStep path (purchase/plan_step_attach.go:55). Non-blocking, but please add a test that pins it: a failed no_submission plan-step row that is now retryable does not let the same step be claimed again.
  2. StampFirstDispatchTx SQL now also requires p.status = 'failed' (store_postgres_first_dispatch.go:25-28). Redshift behavior is stricter in the safe direction only; a linked predecessor is failed by the link CAS, so no practical change.
  3. PR text says "all writers audited"; I agree with the three named writers plus markRefusedAccountRow (refusal_tally.go:49-53, uses the same tally), which the body folds under saveTerminalExecution.

Attack points

  1. Race with execution-time check: holds. A failed row is not claimable: claimAndExecuteWithin CAS only from approved/pending/notified (manager.go:230); reaper and stranded sweep use RedriveRefusalReason, unchanged, which still refuses EC2/Azure SP in place (manager.go:548, stranded_redrive.go:17). no_submission cannot clear while failed (store_postgres.go:1207-1209 is OR while failed), and retry_execution_id is never written by the upsert (L1228-1236). Execution-time check reads the predecessor via the successor's own id (first_dispatch.go:25-35), after the claim, so it re-verifies the row actually executing.
  2. Concurrent clicks: holds. One UPDATE with WHERE execution_id, status='failed', retry_execution_id IS NULL, not settled, plus (new) no_submission AND retry_attempt_n = 0 (store_postgres.go:1313-1321, 1301). Successor insert and link are in one WithTx (handler_purchases.go:2839, 2873-2893). Loser blocks on the row lock, then matches 0 rows, errRetryLinkRaceLost, rollback, 409 (L2851); per-creator submit lock also serializes the same user.
  3. attempt>0: holds. Handler lift requires NeverSubmitted = NoSubmission && attempt 0 (redshift_retry.go:26, retry_unsent.go:9); CAS pins attempt 0; execution-time only for RetryAttemptN == 1 (manager.go:596). Successor attempt = predecessor+1 is the only creator (handler_purchases.go:2796). Fan-out account rows copy the root's attempt (newAccountExecution), skip rows inherit it. Mixed recs: every rec is checked, only the two named reasons lift (retry_unsent.go:15-26).
  4. Tenant: holds. Retry path unchanged: requireExecutionAccess and authorizeSessionRetry precede the gates (handler_purchases.go:2536-2580); lookup is a boolean keyed on the successor id; CAS keyed on execution_id of an already-authorized row.
  5. Wording: holds. skippedAccountNote evaluated on the final failed/no_submission row (execution.go:453-482); retry note only when RetryRefusalReason is empty, matching the gate. The new armed lookup text is added with regexp.QuoteMeta to the anchored allowlist (errnotes.go:25) and carries no DB error (manager.go:598-600, test asserts "db down" absent).
  6. Provider scope: holds. Lift only for redriveReasonAWSNoGuard and redriveReasonAzureSavingsPlan; unknown providers and others refuse. no_submission is set only when every failed rec was refused with the beforeSubmission class, no ErrOutcomeUnknown and no rec purchased (refusal_tally.go:19-31, manager.go:466-472). A flat all-accounts-failed root is not stamped (errAllAccountsFailed has no beforeSubmission class, execution.go:~283), so a root plus its per-account rows cannot both be retried for the same account. Partial fan-out with a purchase is not stamped.
  7. Dimensions: lookup uses the pool outside any caller tx (called from executeAndFinalize via preExecutionRefusal, not in a tx); lookup error fails closed and is not no_submission. No migration, no jsonb concerns. SQL fragment shared with the Redshift stamp, one definition. Scope matches feat(purchase): allow Retry of proven-NoSubmission rows for EC2, Redshift and Azure savings plans #1803 plan (first attempt only, no chain walk).

Caller table (item 6)

Function Caller (file:line) Use Changed?
RetryRefusalReason (new) handler_purchases.go:2639 Retry gate, 409 yes (lifts proven attempt 0)
RetryRefusalReason handler_history.go:415 hides Retry, sets RedriveUnsafe yes, same input as gate
RetryRefusalReason handler_purchases.go:2908 picks link CAS new
RetryRefusalReason execution.go:457 skip-row note yes (was RedriveRefusalReason)
RedriveRefusalReason manager.go:548 (allRecsSafeToRedrive), manager.go:589 (armed), stranded_redrive.go:17, redshift_lineage.go:202, handler_purchases.go:2908 reaper/sweep re-drive, armed check, stranded text unchanged
armedRedriveRefusal (now method) manager.go:429 via preExecutionRefusal <- executeAndFinalize:348 execution-time gate for every executor new lookup only at attempt 1 with a no-guard reason
PredecessorProvenUnsent manager.go:598 only execution-time proof new
LinkUnsentRetryExecutionAtomic handler_purchases.go:2909 only link CAS new
LinkRetryExecutionAtomic handler_purchases.go:2911 other retries unchanged SQL
NeverSubmitted handler_purchases_redshift_retry.go:41, redshift_retry.go:37,52, retry_unsent.go:9 Redshift gate unchanged unchanged
StampFirstDispatchTx manager stampRedshiftDispatch SQL refactored, adds status = 'failed' stricter, safe
Nothing newly treats no_submission as proof outside Retry/History/link/armed; #1842's tally is read, not changed.

CI at f5e59e3 (run 42653339479 and siblings)

gh pr checks at review end: 42 pass, 1 skipping (CodeRabbit), 0 fail. Unit Tests pass 4m0s, Lint Code pass, Integration Tests api/config/purchase pass.

Mutation probes (inserted and asserted via diff stat; scoped runs; all reverted, git clean)

  • attempt-0 check dropped in RetryRefusalReason: KILLED, TestRetryRefusalReason, TestSkippedAccountNote_MatchesRetryEligibility
  • execution-time lookup dropped (proven=true): KILLED, TestArmedRetryRefusedWithoutProof
  • execution-time attempt == 1 to >= 1: KILLED, TestArmedRetryRefusedWithoutProof (AssertNotCalled)
  • lookup error treated as proven: KILLED, TestArmedRetryRefusedWithoutProof
  • unknown outcome counted as refused: KILLED, TestSingleAccount_MixedOutcomesAreNotAllRefused, TestMultiAccount_MixedOutcomesAreNotNoSubmission
  • skip-row wording condition flipped: KILLED, TestSkippedAccountNote_MatchesRetryEligibility, TestExecuteMultiAccount_RecordsAccountsSkippedAfterACommit
  • status=failed dropped from RetryRefusalReason: KILLED, TestRetryRefusalReason
  • handler always uses plain link CAS: KILLED, TestRetryOfProvenUnsentRowLinksViaUnsentCAS, ...LosingUnsentLinkIs409
  • CAS no_submission/attempt 0 predicate removed: KILLED, TestLinkUnsentRetryExecutionAtomic (integration, Docker)
  • CAS attempt-0 only removed: KILLED, TestLinkUnsentRetryExecutionAtomic
  • SQL fragment attempt-0 removed: KILLED, TestPredecessorProvenUnsent, TestStampFirstDispatch_RetryOfNoSubmissionAttempt1DoesNotMint
  • SQL fragment status=failed removed: KILLED, TestPredecessorProvenUnsent
  • History gate reverted to RedriveRefusalReason: KILLED, TestExecutionToHistoryRow_RetryHintForUnsentRows
    Tenant-scope probe: not applicable, the diff adds no scope logic (scope is the unchanged requireExecutionAccess before the gates). No survivors.
    Not run locally: full suites (CI covers).

@cristim

cristim commented Nov 9, 2026

Copy link
Copy Markdown
Member Author

APPROVE @ f5e59e3 by claude-nibblets

Adversarial review of #1870 (closes #1803, +641/-75, money path). I tried to find a route that lets a retry buy twice and did not find one.

Where it is enforced, and what I attacked:

  • Retry time (internal/purchase/retry_unsent.go, used by the Retry gate and the History hint): the two no-guard refusals (AWS EC2, Azure savings plans) are lifted only for a row that is failed, NoSubmission and attempt 0. A retry of a retry (attempt 1 or more) is not lifted, a non-failed row is not lifted, and every other reason (unknown provider, other providers' refusals) still refuses.
  • Link (internal/config/store_postgres.go, LinkUnsentRetryExecutionAtomic): the same CAS as before (status = 'failed' AND retry_execution_id IS NULL, so one successor per predecessor) plus no_submission AND retry_attempt_n = 0; the handler picks it from the same predicate (internal/api/handler_purchases.go, linkRetrySuccessor), so a row that stopped being a proof between gate and link is not linked.
  • Execution time (internal/purchase/manager.go, armedRedriveRefusal): the claimed successor's own no_submission is false, so it reads the predecessor through the link (PredecessorProvenUnsent: failed, no_submission, attempt 0, retry_execution_id = successor); only attempt 1 does the lookup, deeper attempts keep the refusal; a lookup error refuses with fixed text and stamps no beforeSubmission, so a failed lookup never reads as "nothing was sent".
  • RedriveRefusalReason itself is unchanged, so the reaper's in-place re-drive is unaffected; the stamp rule for first_dispatch_at now shares the same SQL fragment as the proof.

Scoped probes (go test -run, scratch worktree at this head, baselines for internal/purchase and internal/api ok), each killed: failed status not required (TestRetryRefusalReason); attempt 0 not required (TestRetryRefusalReason, TestSkippedAccountNote_MatchesRetryEligibility, TestRetryOfEC2RowWithoutAttemptZeroProofIsRefused); lifting every reason (TestRetryRefusalReason, TestRetryRefusalReasonLineageUnsentOverride); lookup error failing open (TestArmedRetryRefusedWithoutProof); armed check always lifted (five tests incl. TestApproveAndExecuteRefusesArmedAzureSavingsPlanRetry and TestAWSNoTokenRetrySuccessorIsRefusedAndFirstBuyStillExecutes); armed lookup skipped (TestArmedRetryOfProvenUnsentFirstAttemptExecutes, TestArmedRetryRefusedWithoutProof). The link CAS predicate is covered by the new integration test, which runs in CI (Integration Tests (config) passed at this head); I did not mutate it locally.

Residual risk, not new in this PR: the lift is only as good as no_submission. It is set for the beforeSubmission error class and for skipped-account rows (internal/purchase/execution.go:481), the same signal #1832 already uses for Redshift; I did not audit every beforeSubmission call site for a path that could set it after a request left, so a future one that does would now unlock an EC2 or Azure savings-plan retry.

CI at this head: 42 pass, 1 skipping (CodeRabbit); mergeStateStatus CLEAN.

@cristim
cristim merged commit 9608863 into main Nov 9, 2026
43 checks passed
@cristim
cristim deleted the feat/1803-nosubmission-retry branch November 9, 2026 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/m Days impact/few Limited audience priority/p2 Backlog-worthy severity/medium Moderate harm type/feat New capability urgency/this-quarter Within the quarter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(purchase): allow Retry of proven-NoSubmission rows for EC2, Redshift and Azure savings plans

1 participant