From a9c5e4ad4e2137ef28dec3326f27efacd2f73eb9 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 29 Aug 2026 20:42:51 +0000 Subject: [PATCH] Recheck bounded prerequisites under lock --- internal/app/bounded_prerequisites_test.go | 54 ++++++++++++++++++++++ internal/app/runner.go | 7 +++ 2 files changed, 61 insertions(+) diff --git a/internal/app/bounded_prerequisites_test.go b/internal/app/bounded_prerequisites_test.go index 4a9b9e0..6b5eabb 100644 --- a/internal/app/bounded_prerequisites_test.go +++ b/internal/app/bounded_prerequisites_test.go @@ -116,6 +116,31 @@ func TestExecuteStagesRejectsBoundedPrerequisitesBeforePersistentMutation(t *tes } } +func TestExecuteStagesRechecksBoundedPrerequisitesUnderSessionLock(t *testing.T) { + cfg := testConfig(t) + plan := mustBoundedPlan(t, "render", "render") + m := manifest.New(cfg.Session.SessionID, time.Now().UTC()) + m.Campaign = cfg.Session.Campaign + markPrefixSucceeded(m, plan) + saveBoundedManifest(t, cfg, m) + + store := &prerequisiteChangingStore{local: &manifest.LocalStore{}} + runs := 0 + _, err := executeStages(context.Background(), cfg, []stage.Stage{countingStage{name: "render", runs: &runs}}, RunOptions{ + Plan: plan, + Env: &Env{ManifestStore: store}, + }) + if err == nil || !strings.Contains(err.Error(), `stage "prepare" has unusable status "running"`) { + t.Fatalf("executeStages() error = %v, want changed prerequisite rejection", err) + } + if store.loads != 2 { + t.Fatalf("manifest loads = %d, want preflight and locked reload", store.loads) + } + if runs != 0 || store.creates != 0 || store.saves != 0 { + t.Fatalf("runs=%d manifest creates=%d saves=%d, want no run or manifest mutation", runs, store.creates, store.saves) + } +} + func TestExecuteStagesBoundedCompositionUsesOnlySelectedCollaborators(t *testing.T) { for _, test := range []struct { name string @@ -246,6 +271,35 @@ type prerequisiteMutationSpy struct { saves int } +type prerequisiteChangingStore struct { + local *manifest.LocalStore + loads int + creates int + saves int +} + +func (s *prerequisiteChangingStore) Create(ctx context.Context, sessionID string) (*manifest.Manifest, error) { + s.creates++ + return s.local.Create(ctx, sessionID) +} + +func (s *prerequisiteChangingStore) Load(ctx context.Context, path string) (*manifest.Manifest, error) { + s.loads++ + m, err := s.local.Load(ctx, path) + if err != nil { + return nil, err + } + if s.loads == 2 { + m.MarkStageRunning("prepare", time.Now().UTC()) + } + return m, nil +} + +func (s *prerequisiteChangingStore) Save(ctx context.Context, path string, m *manifest.Manifest) error { + s.saves++ + return s.local.Save(ctx, path, m) +} + func (s *prerequisiteMutationSpy) Create(ctx context.Context, sessionID string) (*manifest.Manifest, error) { s.creates++ return s.local.Create(ctx, sessionID) diff --git a/internal/app/runner.go b/internal/app/runner.go index 1cbf923..e748b14 100644 --- a/internal/app/runner.go +++ b/internal/app/runner.go @@ -139,6 +139,13 @@ func executeStages(ctx context.Context, cfg *config.Config, stages []stage.Stage return nil, fmt.Errorf("create manifest: %w", err) } } + // The preflight prerequisite inspection avoids creating run state for an + // already-invalid request. Recheck the manifest protected by the session + // lock because another invocation may have changed prerequisite state while + // this invocation waited to acquire the lock. + if err := validateBoundedPrerequisites(opts.Plan, m); err != nil { + return nil, fmt.Errorf("validate bounded run prerequisites under session lock: %w", err) + } identity.applyToSessionManifest(m) if err := env.ManifestStore.Save(ctx, manifestPath, m); err != nil { return nil, fmt.Errorf("save manifest identity %q: %w", manifestPath, err)