Repository navigation
fix(plans): open step rows orphaned before #1046 and scheduled steps of deleted plans #1048
Description
#1046 cancels a plan's open step rows (pending, notified, paused) inside DeletePurchasePlan, so deleting a plan no longer leaves approvable plan-less rows. Two cases are left over:
1. Rows orphaned before #1046
On existing deployments, plans deleted before #1046 left their open step rows with plan_id NULL (purchase_executions.plan_id is ON DELETE SET NULL). Token approval skips requirePlanStepCap and the approve-own plan-step rule when PlanID == "", so those rows can still be approved with no cap.
A backfill needs a reliable way to tell an orphaned plan step from an ad-hoc execution. No current column does that: step_number defaults to 1 for both. Options:
- add a marker (for example
sourceor a nullabledeleted_plan_id) going forward, plus a one-off best-effort backfill; - cancel every open plan-less row older than a cutoff (this risks canceling legitimate ad-hoc rows);
- leave them and document a manual cleanup query.
Any migration should take the next free number: 000126 is the highest on main, and the open #1020 and #1022 add 000127 and 000128.
2. scheduled rows of a deleted plan
#1046 deliberately leaves scheduled rows alone, because they were approved while the plan existed. They still fire at their scheduled time after the plan is deleted. Whether deleting a plan should also revoke its scheduled steps is a product decision.
No frontend change is implied either way.
Activity
IAmJSD commentedon Oct 27, 2026
Status update (2026-10-27, checked against main at 7b86e38): Waiting on an owner decision.
Unchanged on main. DeletePurchasePlan cancels only pending/notified/paused steps (cancelOpenPlanExecutionsTx), plan-less rows still skip requirePlanStepCap on token approval (handler_purchases.go PlanID == "" branches), and no column tells an orphaned plan step from an ad-hoc execution.
Left / blocked on: Owner decision: how should open step rows orphaned before #1046 be handled, and should deleting a plan also cancel its already-scheduled steps? Recommendation: add a nullable deleted_plan_id marker set by DeletePurchasePlan going forward, ship a documented manual cleanup query for pre-#1046 rows instead of auto-cancelling (auto-cancel risks real ad-hoc rows), and cancel scheduled steps on plan delete too, since deleting a plan signals stop buying.
cristim commentedon Nov 5, 2026
Triaged (p2). Severity medium: an orphaned open plan step can still be approved without the step cap on deployments that deleted plans before #1046. Few deployments are affected, and a backfill needs a reliable orphan marker first. Effort m: one migration plus the scheduled-step cleanup.
cristim commentedon Nov 10, 2026
Claimed by claude-fleet (coordinator assignment). Premise check against current main first (does any column or field now tell an orphaned plan step from an ad-hoc execution, and what do the approve paths do with plan-less rows), then a decision comment with a recommended default before any code.
cristim commentedon Nov 10, 2026
Claimed by oc-fleet (taking over claude-fleet's assigned lane, coordinator handoff). Premise check against current main first, then the guard + a failing test in
~/.claude/worktrees/platform-1048-orphaned-steps(branchfix/1048-orphaned-step-approval).cristim commentedon Nov 12, 2026
Owner go (2026-11-12, decide-now list): claude-platform-2 is implementing this item now per the recorded decision; if another lane holds an uncommitted claim on it, please hand over via the bus instead of duplicating.
fix(plans): refuse to fire plan steps whose plan is gone
cristim commentedon Nov 12, 2026
Decided (owner go 2026-11-12) by claude-platform-2 and landed as two commits on main: 2d06d7fd (fire-time guard) and db4da994 (retry refusal + real-Postgres test); tip 879c621; two adversarial review passes (round 1 blocked on a red unit suite, a money bypass via run-now/direct-execute and a retry bypass, all fixed), local verify (unit suites for internal/purchase, api, server, scheduler, mocks, config; scoped integration probes on real Postgres; golangci-lint 0 issues, v2.11.4 because v2.10.1 is not installed; pre-commit), CI green on the commit (CI - Build & Test, pre-commit, frontend-build-sentinel, Deploy to AWS Lambda). Owner decision: refuse to fire any step whose plan is gone, no deletes, with a test. Implementation: preExecutionRefusal, which every fire path funnels through (scheduled sweep, approve, claim, run-now, direct-execute, redrive), now refuses a plan step whose plan is gone: the row ends failed with NoSubmission, a visible reason ('refusing to execute: this plan step's plan no longer exists, so nothing was purchased; start a new purchase if it is still wanted'), an Error log and the operator failure email; nothing is deleted, no migration. A plan step is recognised from STORED fields only: plan_id empty and step_number >= 1 (the store writes step_number explicitly, so the DB default of 1 never applies; direct-execute and ladder rows store 0; migration 000098 only touched rows joined to a plan). It deliberately ignores Source, ExecutedByUserID and PreApprovalSkipReason because run-now and direct-execute stamp those in memory before the guard runs; it reads nothing from the store (a missing plan row with a plan_id is still refused by executePurchase). retryPurchase refuses (409, same text) retrying a refused orphan before writing anything; ad-hoc rows stay retryable. Upgrade impact: existing open orphans become visible failed rows with an email the first time anything tries to fire or approve them, never purchases. Known limits: a step-0 orphan from before migration 000098 is not recognised; a legacy ad-hoc row stored with plan_id NULL and step_number >= 1 would be refused. OWNER CHECK (read-only, production database; I cannot run it): SELECT count(*) FROM purchase_executions WHERE plan_id IS NULL AND step_number >= 1 AND status IN ('pending','notified','paused','scheduled'); every hit is either an orphan (correct to refuse) or a legacy ad-hoc row to inspect before relying on this.