fix(config): refuse legacy writes to editor-managed plans

internal/api/handler_plans.go

@@ -388,6 +388,10 @@ func preserveRampProgress(existingPlan, plan *config.PurchasePlan) error {
 	if rampScheduleShapeUnchanged(existingPlan.RampSchedule, plan.RampSchedule) {
 		plan.RampSchedule.CurrentStep = existingPlan.RampSchedule.CurrentStep
 		plan.RampSchedule.StartDate = existingPlan.RampSchedule.StartDate
+		// Editor-owned markers the request cannot express; dropping them hands
+		// the plan back to the interval formula.
+		plan.RampSchedule.DatesManaged = existingPlan.RampSchedule.DatesManaged
+		plan.RampSchedule.AllocationMode = existingPlan.RampSchedule.AllocationMode
 		plan.NextExecutionDate = existingPlan.NextExecutionDate
 		return nil
 	}

internal/api/handler_plans_managed_rename_integration_test.go

@@ -0,0 +1,36 @@
+//go:build integration
+
+package api
+
+import (
+	"context"
+	"testing"
+
+	"github.com/aws/aws-lambda-go/events"
+	"github.com/stretchr/testify/require"
+)
+
+func TestUpdatePlanRenameKeepsManagedMarkers(t *testing.T) {
+	ctx := context.Background()
+	f := newCreateConcurrencyFixture(ctx, t)
+	_, err := f.pool.Exec(ctx, `UPDATE purchase_plans SET next_execution_date = now() + interval '3 days',
+		ramp_schedule = ramp_schedule || '{"dates_managed":true,"allocation_mode":"equal_count"}' WHERE id = $1`, f.planID)
+	require.NoError(t, err)
+	const snapshot = `SELECT jsonb_build_object('managed', ramp_schedule->'dates_managed', 'mode', ramp_schedule->'allocation_mode',
+		'step', ramp_schedule->'current_step', 'start', ramp_schedule->'start_date', 'next', next_execution_date)::text
+		FROM purchase_plans WHERE id = $1`
+	var before, after, name string
+	require.NoError(t, f.pool.QueryRow(ctx, snapshot, f.planID).Scan(&before))
+
+	_, err = f.handler.updatePlan(ctx, &events.LambdaFunctionURLRequest{
+		Headers: map[string]string{"Authorization": "Bearer admin-token", "Content-Type": "application/json"},
+		Body:    `{"name":"renamed","enabled":true,"notification_days_before":2,"ramp_schedule":"weekly-25pct"}`,
+	}, f.planID)
+	require.NoError(t, err)
+
+	require.NoError(t, f.pool.QueryRow(ctx, snapshot, f.planID).Scan(&after))
+	require.NoError(t, f.pool.QueryRow(ctx, `SELECT name FROM purchase_plans WHERE id = $1`, f.planID).Scan(&name))
+	require.Equal(t, "renamed", name)
+	require.JSONEq(t, before, after, "a rename must not touch the editor-owned markers, clock or progress")
+	require.Contains(t, after, `"managed": true`)
+}

internal/config/errors.go

@@ -15,6 +15,11 @@ var ErrEmptyPurchaseID = errors.New("purchase history record has no purchase id"
 // ErrPurchasePlanConflict means a plan changed after the caller read it.
 var ErrPurchasePlanConflict = errors.New("purchase plan changed")
 
+// ErrEditorReconciliationRequired means the plan is editor-managed and a legacy
+// writer tried to change what only the schedule editor may. Permanent for the
+// request: callers map it to 409; a background caller would skip, not retry.
+var ErrEditorReconciliationRequired = errors.New("plan is managed by the schedule editor")
+
 // ErrExecutionNotInExpectedStatus is returned by TransitionExecutionStatus
 // when the target execution exists but its current status is not in the
 // allowed `fromStatuses` set -- i.e. the atomic CAS rejected because some

internal/config/store_postgres.go

@@ -753,7 +753,7 @@ func (s *PostgresStore) UpdatePurchasePlan(ctx context.Context, plan *PurchasePl
 		if err != nil {
 			return err
 		}
-		if updErr := s.UpdatePurchasePlanTx(ctx, tx, &pending); updErr != nil {
+		if updErr := s.updatePurchasePlanGuardedTx(ctx, tx, old, &pending); updErr != nil {
 			return updErr
 		}
 		if old == nil || !rampBaselineStale(old, &pending) {
@@ -775,8 +775,24 @@ func (s *PostgresStore) UpdatePurchasePlan(ctx context.Context, plan *PurchasePl
 	return nil
 }
 
-// UpdatePurchasePlanTx rejects snapshots that lost a race with another plan writer.
+// UpdatePurchasePlanTx rejects snapshots that lost a race with another plan
+// writer and refuses changes to what the editor owns on a managed plan. It
+// re-locks the row, which every caller already holds, to judge the guard on
+// the locked version.
 func (s *PostgresStore) UpdatePurchasePlanTx(ctx context.Context, tx pgx.Tx, plan *PurchasePlan) error {
+	old, err := s.LockPurchasePlanTx(ctx, tx, plan.ID)
+	if err != nil {
+		return err
+	}
+	return s.updatePurchasePlanGuardedTx(ctx, tx, old, plan)
+}
+
+// updatePurchasePlanGuardedTx runs the managed-plan guard against old, the row
+// locked by the caller's transaction, and then the optimistic update.
+func (s *PostgresStore) updatePurchasePlanGuardedTx(ctx context.Context, tx pgx.Tx, old, plan *PurchasePlan) error {
+	if err := requireManagedPlanUpdateTx(old, plan); err != nil {
+		return err
+	}
 	// Marshal services and ramp_schedule to JSONB
 	servicesJSON, err := json.Marshal(plan.Services)
 	if err != nil {

internal/config/store_postgres_complete_step_test.go

@@ -163,6 +163,9 @@ func TestPGXMock_CompletePlanStep_LocksAndAdvances(t *testing.T) {
 		WithArgs("plan-123").
 		WillReturnRows(rampPlanRows(t, "plan-123", ramp, now, stale, sql.NullTime{Valid: false}))
 	expectFanOut(mock, "plan-123", 2, 3, 0)
+	mock.ExpectQuery(`SELECT[\s\S]*FROM purchase_plans[\s\S]*WHERE id = \$1 FOR UPDATE`).
+		WithArgs("plan-123").
+		WillReturnRows(rampPlanRows(t, "plan-123", ramp, now, stale, sql.NullTime{Valid: false}))
 	mock.ExpectQuery(`UPDATE purchase_plans`).
 		WithArgs(completeStepUpdateArgs(2, stale, false)...).
 		WillReturnRows(pgxmock.NewRows([]string{"updated_at"}).AddRow(now))
@@ -353,6 +356,10 @@ func TestPGXMock_CompletePlanStep_CompletedRampClearsNextDate(t *testing.T) {
 	ramp := RampSchedule{Type: "weekly", PercentPerStep: 25, StepIntervalDays: 7, CurrentStep: 4, TotalSteps: 4, StartDate: now}
 
 	mock.ExpectBegin()
+	mock.ExpectQuery(`SELECT[\s\S]*FROM purchase_plans[\s\S]*WHERE id = \$1 FOR UPDATE`).
+		WithArgs("plan-done").
+		WillReturnRows(rampPlanRows(t, "plan-done", ramp, now, stale, sql.NullTime{Valid: true, Time: now}))
+	// The managed-plan guard re-locks the row before the update.
 	mock.ExpectQuery(`SELECT[\s\S]*FROM purchase_plans[\s\S]*WHERE id = \$1 FOR UPDATE`).
 		WithArgs("plan-done").
 		WillReturnRows(rampPlanRows(t, "plan-done", ramp, now, stale, sql.NullTime{Valid: true, Time: now}))

internal/config/store_postgres_managed_guard.go

@@ -0,0 +1,60 @@
+package config
+
+import (
+	"bytes"
+	"encoding/json"
+	"fmt"
+	"time"
+)
+
+// requireManagedPlanUpdateTx refuses a legacy plan write that would change what
+// the schedule editor owns on an editor-managed plan. old is the row locked by
+// the caller's transaction; judging it, never a pre-lock copy, is what lets a
+// writer that waited behind the editor see the committed marker. Only the
+// editor command sets or clears dates_managed (it is part of ramp_schedule, so
+// adopting or stripping it is refused as a ramp change); it does not come
+// through here.
+func requireManagedPlanUpdateTx(old, next *PurchasePlan) error {
+	if old == nil || (!old.RampSchedule.DatesManaged && !next.RampSchedule.DatesManaged) {
+		return nil
+	}
+	if field := managedPlanSensitiveChange(old, next); field != "" {
+		return fmt.Errorf("%w: %s changed on plan %s", ErrEditorReconciliationRequired, field, old.ID)
+	}
+	return nil
+}
+
+// managedPlanSensitiveChange names the first editor-owned field whose
+// persisted value differs, or "" when only name, enabled, disarming, the cap
+// and the bookkeeping timestamps changed.
+func managedPlanSensitiveChange(old, next *PurchasePlan) string {
+	switch {
+	case !sameJSON(old.RampSchedule, next.RampSchedule):
+		return "ramp_schedule"
+	case !sameJSON(old.Services, next.Services):
+		return "services"
+	case !sameDate(old.NextExecutionDate, next.NextExecutionDate):
+		return "next_execution_date"
+	case old.NotificationDaysBefore != next.NotificationDaysBefore:
+		return "notification_days_before"
+	case !old.AutoPurchase && next.AutoPurchase:
+		return "auto_purchase"
+	}
+	return ""
+}
+
+// sameJSON compares what would be persisted: nil and empty slices marshal
+// equal under omitempty, so a rebuilt but identical value is not a change.
+func sameJSON(a, b any) bool {
+	aj, aErr := json.Marshal(a)
+	bj, bErr := json.Marshal(b)
+	return aErr == nil && bErr == nil && bytes.Equal(aj, bj)
+}
+
+// sameDate compares instants; both sides come from the database or a copy of it.
+func sameDate(a, b *time.Time) bool {
+	if a == nil || b == nil {
+		return a == b
+	}
+	return a.Equal(*b)
+}

internal/config/store_postgres_managed_guard_integration_test.go

@@ -0,0 +1,109 @@
+//go:build integration
+// +build integration
+
+package config
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/jackc/pgx/v5"
+	"github.com/stretchr/testify/require"
+)
+
+func storedPlanJSON(ctx context.Context, t *testing.T, f *rampFixture) string {
+	t.Helper()
+	var stored string
+	require.NoError(t, f.store.db.QueryRow(ctx, `SELECT to_jsonb(p)::text FROM purchase_plans p WHERE id = $1`, f.planID).Scan(&stored))
+	return stored
+}
+
+func TestManagedPlanUpdateGuard(t *testing.T) {
+	ctx := context.Background()
+	f := managedClockFixture(ctx, t, 1)
+	soon := time.Now().Add(48 * time.Hour)
+	refused := map[string]func(p *PurchasePlan){
+		"marker strip":        func(p *PurchasePlan) { p.RampSchedule.DatesManaged = false },
+		"allocation mode":     func(p *PurchasePlan) { p.RampSchedule.AllocationMode = RampAllocationEqualCount },
+		"ramp percent":        func(p *PurchasePlan) { p.RampSchedule.PercentPerStep = 50 },
+		"current step":        func(p *PurchasePlan) { p.RampSchedule.CurrentStep = 2 },
+		"service key":         func(p *PurchasePlan) { p.Services["ec2"] = ServiceConfig{Provider: "aws", Service: "ec2"} },
+		"service content":     func(p *PurchasePlan) { p.Services["future_service"] = ServiceConfig{Term: 3} },
+		"next date":           func(p *PurchasePlan) { p.NextExecutionDate = &soon },
+		"notice":              func(p *PurchasePlan) { p.NotificationDaysBefore += 3 },
+		"arming":              func(p *PurchasePlan) { p.AutoPurchase = true },
+		"arming with rename":  func(p *PurchasePlan) { p.AutoPurchase, p.Name = true, "armed" },
+		"notice with disable": func(p *PurchasePlan) { p.NotificationDaysBefore, p.Enabled = 9, false },
+	}
+	for name, mutate := range refused {
+		t.Run("refuses "+name, func(t *testing.T) {
+			plan, err := f.store.GetPurchasePlan(ctx, f.planID)
+			require.NoError(t, err)
+			before := storedPlanJSON(ctx, t, f)
+			mutate(plan)
+			require.ErrorIs(t, f.store.UpdatePurchasePlan(ctx, plan), ErrEditorReconciliationRequired)
+			require.Equal(t, before, storedPlanJSON(ctx, t, f))
+		})
+	}
+
+	t.Run("allows disarm, rename, cap and bookkeeping", func(t *testing.T) {
+		_, err := f.store.db.Exec(ctx, `UPDATE purchase_plans SET auto_purchase = true WHERE id = $1`, f.planID)
+		require.NoError(t, err)
+		plan, err := f.store.GetPurchasePlan(ctx, f.planID)
+		require.NoError(t, err)
+		cap, now := 5.0, time.Now().Truncate(time.Microsecond)
+		plan.Name, plan.Enabled, plan.AutoPurchase = "renamed", false, false
+		plan.MaxStepCommitment, plan.LastExecutionDate, plan.LastNotificationSent = &cap, &now, &now
+		require.NoError(t, f.store.UpdatePurchasePlan(ctx, plan))
+		var name string
+		var managed bool
+		require.NoError(t, f.store.db.QueryRow(ctx, `SELECT name, ramp_schedule->>'dates_managed' = 'true' FROM purchase_plans WHERE id = $1`, f.planID).Scan(&name, &managed))
+		require.Equal(t, "renamed", name)
+		require.True(t, managed)
+	})
+
+	t.Run("allows a rebuilt identical plan", func(t *testing.T) {
+		plan, err := f.store.GetPurchasePlan(ctx, f.planID)
+		require.NoError(t, err)
+		require.NoError(t, f.store.UpdatePurchasePlan(ctx, plan))
+	})
+}
+
+func TestUnmanagedPlanUpdateGuard(t *testing.T) {
+	ctx := context.Background()
+	f := newRampFixture(ctx, t, 0, 1, 3)
+	plan, err := f.store.GetPurchasePlan(ctx, f.planID)
+	require.NoError(t, err)
+	plan.Name, plan.NotificationDaysBefore, plan.AutoPurchase = "free", 7, true
+	plan.RampSchedule.PercentPerStep = 40
+	require.NoError(t, f.store.UpdatePurchasePlan(ctx, plan))
+
+	t.Run("refuses marker adoption", func(t *testing.T) {
+		plan.RampSchedule.DatesManaged = true
+		before := storedPlanJSON(ctx, t, f)
+		require.ErrorIs(t, f.store.UpdatePurchasePlan(ctx, plan), ErrEditorReconciliationRequired)
+		require.Equal(t, before, storedPlanJSON(ctx, t, f))
+	})
+
+	t.Run("judges the locked row, not the caller's stale copy", func(t *testing.T) {
+		stale, err := f.store.GetPurchasePlan(ctx, f.planID)
+		require.NoError(t, err)
+		_, err = f.store.db.Exec(ctx, `UPDATE purchase_plans SET ramp_schedule = ramp_schedule || '{"dates_managed":true}' WHERE id = $1`, f.planID)
+		require.NoError(t, err)
+		stale.Name = "stale rename"
+		require.ErrorIs(t, f.store.UpdatePurchasePlan(ctx, stale), ErrEditorReconciliationRequired)
+	})
+}
+
+func TestUpdatePurchasePlanTxGuardsDirectCallers(t *testing.T) {
+	ctx := context.Background()
+	f := managedClockFixture(ctx, t, 1)
+	plan, err := f.store.GetPurchasePlan(ctx, f.planID)
+	require.NoError(t, err)
+	before := storedPlanJSON(ctx, t, f)
+	plan.NotificationDaysBefore += 3
+	err = f.store.WithTx(ctx, func(tx pgx.Tx) error { return f.store.UpdatePurchasePlanTx(ctx, tx, plan) })
+	require.ErrorIs(t, err, ErrEditorReconciliationRequired)
+	require.Equal(t, before, storedPlanJSON(ctx, t, f))
+}

known_issues/resolved/34_legacy_rename_strips_dates_managed.md

@@ -0,0 +1,15 @@
+# Known Issues: legacy plan rename strips editor markers
+
+> **Audit status (2026-10-16):** `0 needs triage · 1 resolved`
+
+## HIGH: PUT rename dropped dates_managed and allocation_mode
+
+**Files:** `internal/api/handler_plans.go` (`preserveRampProgress`), `internal/config/store_postgres.go`
+
+**Description:** A legacy `PUT /plans/{id}` rebuilds the ramp schedule from the request, and `preserveRampProgress` carried over only the step, start date and next date. A bare rename of an editor-managed plan therefore wrote the plan back without `dates_managed` and `allocation_mode`, returning it to the interval formula (#631).
+
+**Why deferred:** Not deferred; found while planning the legacy-writer guards and fixed in the same change.
+
+**Issue:** #631
+
+**Status:** Resolved in the G1a guard PR. `preserveRampProgress` keeps both markers, and `UpdatePurchasePlanTx` refuses legacy writes that change editor-owned fields of a managed plan. Regression test: `TestUpdatePlanRenameKeepsManagedMarkers`.