fix: poll race, live poll interval, stuck-threshold fix, SSO hint profile

internal/config/config.go

@@ -8,6 +8,7 @@ import (
 	"path/filepath"
 	"slices"
 	"strings"
+	"time"
 
 	"github.com/BurntSushi/toml"
 )
@@ -57,6 +58,12 @@ func (p Project) IsEnabled() bool {
 	return p.Enabled == nil || *p.Enabled
 }
 
+// StuckThreshold is how long a resource stays in trouble before it counts as
+// Stuck: its webhook fires, and the fix key offers to cancel a stalled update.
+func (s Settings) StuckThreshold() time.Duration {
+	return time.Duration(s.StuckThresholdMinutes) * time.Minute
+}
+
 // Webhook is an endpoint that receives stuck-resource events. Secret is
 // optional; when set, requests are signed with HMAC-SHA256 over the body.
 type Webhook struct {

internal/fix/approval_test.go

@@ -3,6 +3,7 @@ package fix_test
 import (
 	"context"
 	"testing"
+	"time"
 
 	"github.com/ericdahl-dev/aws-green/internal/fix"
 	"github.com/ericdahl-dev/aws-green/internal/health"
@@ -62,7 +63,7 @@ func TestPlanApproval_reject(t *testing.T) {
 // Approvals are driven by their own keys; the smart fix must not approve
 // anything on the user's behalf.
 func TestPlan_doesNotApprove(t *testing.T) {
-	if plan := fix.Plan(approvalProject("tok-123")); plan != nil {
+	if plan := fix.Plan(approvalProject("tok-123"), 30*time.Minute); plan != nil {
 		t.Errorf("expected no smart fix for a pending approval, got %+v", plan)
 	}
 }

internal/fix/fix.go

@@ -78,14 +78,15 @@ type FixPlan struct {
 // Plan inspects a ProjectState and returns the highest-priority FixPlan, or nil if nothing to fix.
 // Precedence: rollback-failed stacks → pipeline → other stacks → ECS.
 // Rollback-failed stacks rank above pipeline because restarting the pipeline would fail
-// immediately until the stack is recovered.
-func Plan(proj state.ProjectState) *FixPlan {
+// immediately until the stack is recovered. A stack update counts as stalled once it
+// has run longer than stalledAfter — the same stuck threshold that raises its alert.
+func Plan(proj state.ProjectState, stalledAfter time.Duration) *FixPlan {
 	var plan *FixPlan
 
 	// 1. Rollback-failed stacks — must be resolved before any pipeline restart can succeed.
 	for _, s := range proj.Stacks {
 		if s.Status.RollbackFailed() {
-			if p := planStack(s); p != nil {
+			if p := planStack(s, stalledAfter); p != nil {
 				plan = p
 				break
 			}
@@ -100,7 +101,7 @@ func Plan(proj state.ProjectState) *FixPlan {
 	// 3. Other stacks (stalled in-progress)
 	if plan == nil {
 		for _, s := range proj.Stacks {
-			if p := planStack(s); p != nil {
+			if p := planStack(s, stalledAfter); p != nil {
 				plan = p
 				break
 			}
@@ -159,9 +160,7 @@ func PlanApproval(proj state.ProjectState, approve bool) *FixPlan {
 	}
 }
 
-const stalledStackThreshold = 30 * time.Minute
-
-func planStack(s state.StackState) *FixPlan {
+func planStack(s state.StackState, stalledAfter time.Duration) *FixPlan {
 	// Stuck rollback — ContinueUpdateRollback
 	if s.Status.RollbackFailed() {
 		return &FixPlan{
@@ -171,8 +170,8 @@ func planStack(s state.StackState) *FixPlan {
 		}
 	}
 
-	// In-progress > 30 min — CancelUpdateStack
-	if s.StartedAt != nil && s.Status.Cancellable() && time.Since(*s.StartedAt) > stalledStackThreshold {
+	// Update running past the stuck threshold — CancelUpdateStack
+	if s.StartedAt != nil && s.Status.Cancellable() && time.Since(*s.StartedAt) > stalledAfter {
 		elapsed := time.Since(*s.StartedAt).Round(time.Second)
 		return &FixPlan{
 			Kind:        KindCancelStackUpdate,

internal/fix/fix_test.go

@@ -20,23 +20,23 @@ func proj(pipeline health.Stoplight, stacks []state.StackState, ecs []state.ECSS
 
 func TestPlan_noActionWhenGreen(t *testing.T) {
 	p := proj(health.StoplightGreen, nil, nil)
-	plan := fix.Plan(p)
+	plan := fix.Plan(p, 30*time.Minute)
 	if plan != nil {
 		t.Errorf("expected nil plan for green project, got %v", plan)
 	}
 }
 
 func TestPlan_noActionWhenGrey(t *testing.T) {
 	p := proj(health.StoplightGrey, nil, nil)
-	plan := fix.Plan(p)
+	plan := fix.Plan(p, 30*time.Minute)
 	if plan != nil {
 		t.Errorf("expected nil plan for grey project, got %v", plan)
 	}
 }
 
 func TestPlan_restartFailedPipeline(t *testing.T) {
 	p := proj(health.StoplightRed, nil, nil)
-	plan := fix.Plan(p)
+	plan := fix.Plan(p, 30*time.Minute)
 	if plan == nil {
 		t.Fatal("expected plan for red pipeline")
 	}
@@ -52,7 +52,7 @@ func TestPlan_forceDeployDownECS(t *testing.T) {
 	p := proj(health.StoplightGreen, nil, []state.ECSServiceState{
 		{Name: "web", Cluster: "my-cluster", RunningCount: 0, DesiredCount: 3, Stoplight: health.StoplightRed},
 	})
-	plan := fix.Plan(p)
+	plan := fix.Plan(p, 30*time.Minute)
 	if plan == nil {
 		t.Fatal("expected plan for red ECS service")
 	}
@@ -65,7 +65,7 @@ func TestPlan_forceDeployStalledECS(t *testing.T) {
 	p := proj(health.StoplightGreen, nil, []state.ECSServiceState{
 		{Name: "web", Cluster: "my-cluster", ActiveDeployment: true, RunningCount: 2, DesiredCount: 2, Stoplight: health.StoplightYellow},
 	})
-	plan := fix.Plan(p)
+	plan := fix.Plan(p, 30*time.Minute)
 	if plan == nil {
 		t.Fatal("expected plan for yellow ECS service")
 	}
@@ -78,7 +78,7 @@ func TestPlan_continueRollbackStack(t *testing.T) {
 	p := proj(health.StoplightGreen, []state.StackState{
 		{Name: "my-stack", Status: "UPDATE_ROLLBACK_FAILED", Stoplight: health.StoplightRed},
 	}, nil)
-	plan := fix.Plan(p)
+	plan := fix.Plan(p, 30*time.Minute)
 	if plan == nil {
 		t.Fatal("expected plan for failed rollback stack")
 	}
@@ -92,7 +92,7 @@ func TestPlan_cancelStalledStack(t *testing.T) {
 	p := proj(health.StoplightGreen, []state.StackState{
 		{Name: "my-stack", Status: "UPDATE_IN_PROGRESS", StartedAt: &stale, Stoplight: health.StoplightYellow},
 	}, nil)
-	plan := fix.Plan(p)
+	plan := fix.Plan(p, 30*time.Minute)
 	if plan == nil {
 		t.Fatal("expected plan for stalled stack")
 	}
@@ -109,7 +109,7 @@ func TestPlan_noCancelForStalledNonUpdate(t *testing.T) {
 		p := proj(health.StoplightGreen, []state.StackState{
 			{Name: "my-stack", Status: status, StartedAt: &stale, Stoplight: status.Stoplight()},
 		}, nil)
-		if plan := fix.Plan(p); plan != nil {
+		if plan := fix.Plan(p, 30*time.Minute); plan != nil {
 			t.Errorf("%s: expected no plan, got %v", status, plan.Kind)
 		}
 	}
@@ -119,7 +119,7 @@ func TestPlan_pipelineTakesPrecedenceOverECS(t *testing.T) {
 	p := proj(health.StoplightRed, nil, []state.ECSServiceState{
 		{Name: "web", Cluster: "c", RunningCount: 0, DesiredCount: 1, Stoplight: health.StoplightRed},
 	})
-	plan := fix.Plan(p)
+	plan := fix.Plan(p, 30*time.Minute)
 	if plan == nil {
 		t.Fatal("expected plan")
 	}
@@ -138,7 +138,7 @@ func TestPlan_credentialsFromProject(t *testing.T) {
 			Stoplight: health.StoplightRed,
 		},
 	}
-	plan := fix.Plan(p)
+	plan := fix.Plan(p, 30*time.Minute)
 	if plan == nil {
 		t.Fatal("expected plan for red pipeline")
 	}
@@ -149,3 +149,18 @@ func TestPlan_credentialsFromProject(t *testing.T) {
 		t.Errorf("expected Region=us-east-1, got %q", plan.Region)
 	}
 }
+
+// The cancel fix is offered at the same threshold that makes the stack
+// Stuck — the user's stuck_threshold_minutes — not a fixed 30 minutes.
+func TestPlan_stalledStackUsesTheGivenThreshold(t *testing.T) {
+	started := time.Now().Add(-12 * time.Minute)
+	p := proj(health.StoplightGreen, []state.StackState{
+		{Name: "my-stack", Status: "UPDATE_IN_PROGRESS", StartedAt: &started, Stoplight: health.StoplightYellow},
+	}, nil)
+	if plan := fix.Plan(p, 10*time.Minute); plan == nil || plan.Kind != fix.KindCancelStackUpdate {
+		t.Errorf("12m into a 10m threshold: got %+v, want a cancel plan", plan)
+	}
+	if plan := fix.Plan(p, 15*time.Minute); plan != nil {
+		t.Errorf("12m into a 15m threshold: got %+v, want no plan", plan)
+	}
+}

internal/poller/poller.go

@@ -35,6 +35,14 @@ type Poller struct {
 	cfnFactory CFNClientFactory
 	ecsFactory ECSClientFactory
 	mu         sync.Mutex
+	// pollMu serializes poll cycles. A refresh or reload can ask for a
+	// cycle while the ticker's is in flight; run together, whichever
+	// finished last would win, so an older cycle could overwrite a newer
+	// one. It is separate from mu so Snapshot() isn't blocked by fetches.
+	pollMu sync.Mutex
+	// intervals hands a changed poll interval from ReloadConfig to the
+	// running ticker in Start.
+	intervals  chan time.Duration
 	current    []state.ProjectState
 	dispatcher *webhooks.Dispatcher
 	stuck      *stuckTracker
@@ -70,6 +78,7 @@ func New(cfg *config.Config, factory ClientFactory, cfnFactory CFNClientFactory,
 		dispatcher: webhooks.New(cfg.Webhooks),
 		stuck:      newStuckTracker(),
 		clients:    make(map[string]accountClients),
+		intervals:  make(chan time.Duration, 1),
 		now:        time.Now,
 	}
 }
@@ -99,6 +108,8 @@ func (p *Poller) Start(ctx context.Context) (<-chan state.Snapshot, context.Canc
 			select {
 			case <-ctx.Done():
 				return
+			case d := <-p.intervals:
+				ticker.Reset(d)
 			case <-ticker.C:
 				p.poll(ctx, ch)
 			}
@@ -119,6 +130,9 @@ func (p *Poller) ReloadConfig(cfg *config.Config, ctx context.Context, ch chan<-
 	cfg = cfg.Clone()
 	p.mu.Lock()
 	prev := p.current
+	if cfg.Settings.PollInterval != p.cfg.Settings.PollInterval {
+		p.setInterval(time.Duration(cfg.Settings.PollInterval) * time.Second)
+	}
 	p.cfg = cfg
 	p.dispatcher = webhooks.New(cfg.Webhooks)
 	enabled := cfg.EnabledProjects()
@@ -155,6 +169,17 @@ func (p *Poller) clientsFor(profile, region string) accountClients {
 	return c
 }
 
+// setInterval queues d for the running ticker, replacing any change it hasn't
+// picked up yet. Only a changed interval is sent: resetting on every reload
+// would let a run of edits postpone polling indefinitely.
+func (p *Poller) setInterval(d time.Duration) {
+	select {
+	case <-p.intervals:
+	default:
+	}
+	p.intervals <- d
+}
+
 // prevProject returns the last known state for a project, matched on the
 // account-qualified identity. Matching by identity rather than by index keeps
 // carried-forward state correct when a project is enabled or disabled and the
@@ -242,6 +267,9 @@ func configuredServices(carried []state.ECSServiceState, cfgECS []config.ECSConf
 }
 
 func (p *Poller) poll(ctx context.Context, ch chan<- state.Snapshot) {
+	p.pollMu.Lock()
+	defer p.pollMu.Unlock()
+
 	// Take cfg and current state under the lock. p.cfg is the poller's own copy,
 	// which ReloadConfig replaces wholesale rather than edits, so it is safe to
 	// read for the rest of the cycle.
@@ -276,8 +304,7 @@ func (p *Poller) poll(ctx context.Context, ch chan<- state.Snapshot) {
 
 	p.mu.Lock()
 	p.current = updated
-	threshold := time.Duration(cfg.Settings.StuckThresholdMinutes) * time.Minute
-	events := p.stuck.evaluate(updated, threshold, now)
+	events := p.stuck.evaluate(updated, cfg.Settings.StuckThreshold(), now)
 	dispatcher := p.dispatcher
 	p.mu.Unlock()
 

internal/poller/poller_test.go

@@ -732,3 +732,105 @@ func TestPollerOwnsItsConfig(t *testing.T) {
 		t.Errorf("poll saw an edit made outside ReloadConfig: fetched %v", got)
 	}
 }
+
+// firstCallGatedFetcher holds its first fetch until released and answers it
+// with a failure; later fetches answer at once with success. It stands in for
+// a slow poll cycle that a refresh overtakes.
+type firstCallGatedFetcher struct {
+	mu      sync.Mutex
+	calls   int
+	entered chan struct{}
+	release chan struct{}
+}
+
+func (f *firstCallGatedFetcher) FetchPipeline(_ context.Context, name string) (state.PipelineState, error) {
+	f.mu.Lock()
+	f.calls++
+	first := f.calls == 1
+	f.mu.Unlock()
+	if first {
+		close(f.entered)
+		<-f.release
+		return pipelineData(name, health.StatusFailed), nil
+	}
+	return pipelineData(name, health.StatusSucceeded), nil
+}
+
+// A refresh requested while a cycle is in flight must not race it: whatever
+// the refresh saw is newer, and the slow cycle finishing last must not
+// overwrite it.
+func TestRefreshDoesNotRaceAnInFlightPoll(t *testing.T) {
+	cfg := loadConfig(t, `
+[[projects]]
+name = "alpha"
+[projects.pipeline]
+name = "alpha-pipeline"
+`)
+	f := &firstCallGatedFetcher{entered: make(chan struct{}), release: make(chan struct{})}
+	p := New(cfg, pipelineFactory(f), nil, nil)
+	ctx := context.Background()
+	ch := make(chan state.Snapshot, 4)
+
+	go p.poll(ctx, ch) // slow cycle, will report red
+	<-f.entered
+	p.ForceRefresh(ctx, ch) // newer cycle, will report green
+	time.Sleep(50 * time.Millisecond)
+	close(f.release)
+	<-ch
+	<-ch
+
+	if got := p.Snapshot().Projects[0].Pipeline.Stoplight; got != health.StoplightGreen {
+		t.Errorf("final state = %v, want the refresh's green, not the slow cycle's red", got)
+	}
+}
+
+// A new poll interval from a config reload applies to the running ticker.
+func TestReloadConfigAppliesNewPollInterval(t *testing.T) {
+	cfg := loadConfig(t, twoProjectConfig)
+	cfg.Settings.PollInterval = 3600
+	p := New(cfg, pipelineFactory(&fakePipelineFetcher{}), nil, nil)
+
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	ch, stop := p.Start(ctx)
+	defer stop()
+	<-ch // the immediate first poll
+
+	faster := cfg.Clone()
+	faster.Settings.PollInterval = 1
+	p.ReloadConfig(faster, ctx, make(chan state.Snapshot, 4))
+
+	select {
+	case <-ch:
+	case <-time.After(3 * time.Second):
+		t.Fatal("no scheduled poll within 3s of switching to a 1s interval")
+	}
+}
+
+// Saving edits that leave the interval alone must not reset the ticker, or a
+// run of edits faster than the interval would stop scheduled polls entirely.
+func TestReloadConfigKeepsTickerWhenIntervalUnchanged(t *testing.T) {
+	cfg := loadConfig(t, twoProjectConfig)
+	cfg.Settings.PollInterval = 1
+	p := New(cfg, pipelineFactory(&fakePipelineFetcher{}), nil, nil)
+
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	ch, stop := p.Start(ctx)
+	defer stop()
+	<-ch
+
+	done := make(chan struct{})
+	go func() {
+		defer close(done)
+		for i := 0; i < 10; i++ {
+			p.ReloadConfig(cfg, ctx, make(chan state.Snapshot, 4))
+			time.Sleep(250 * time.Millisecond)
+		}
+	}()
+	select {
+	case <-ch:
+	case <-done:
+		t.Fatal("reloads with an unchanged interval held off every scheduled poll")
+	}
+}