diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index adcb15bf..e5844293 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -206,3 +206,13 @@ jobs: run: >- go test -p 1 -count=1 -v ./bootstrap/ -run 'TestMySQLDecisionJobClaim_|TestMySQLDecisionJobOwnership_|TestMySQLRunManager_ClaimReplyLoss' + + - name: Artifact generation fencing invariants + run: >- + go test -p 1 -count=1 -v ./bootstrap/ + -run 'TestMySQLArtifactGeneration_' + + - name: Artifact generation invariants + run: >- + go test -p 1 -count=1 -v ./adapter/ + -run '^TestArtifactGeneration_' diff --git a/backend/adapter/a2a_repository.go b/backend/adapter/a2a_repository.go index 0ce7525c..06219cdc 100644 --- a/backend/adapter/a2a_repository.go +++ b/backend/adapter/a2a_repository.go @@ -271,7 +271,8 @@ func loadConversationResolutions(tx *gorm.DB, messages []*entity.ConversationMes for i := range models { model := &models[i] out[model.CaseID] = &entity.Resolution{ - ID: model.ID, CaseID: model.CaseID, Consensus: fromJSON[entity.ConsensusResult](model.ConsensusJSON), + ID: model.ID, CaseID: model.CaseID, ExecutionGeneration: model.ExecutionGeneration, + Consensus: fromJSON[entity.ConsensusResult](model.ConsensusJSON), FinalDecision: entity.VoteDecision(model.FinalDecision), FinalReport: model.FinalReport, KeyEvidenceIDs: fromJSON[[]string](model.KeyEvidenceIDsJSON), KeyClaimIDs: fromJSON[[]string](model.KeyClaimIDsJSON), VoteIDs: fromJSON[[]string](model.VoteIDsJSON), Evaluation: fromJSON[*entity.Evaluation](model.EvaluationJSON), CreatedAt: model.CreatedAt, diff --git a/backend/adapter/artifact_generation_fault_mysql_test.go b/backend/adapter/artifact_generation_fault_mysql_test.go new file mode 100644 index 00000000..f6261751 --- /dev/null +++ b/backend/adapter/artifact_generation_fault_mysql_test.go @@ -0,0 +1,207 @@ +package magi_test + +import ( + "context" + "database/sql" + "errors" + "fmt" + "strings" + "testing" + "time" + + "github.com/google/uuid" + + magi "github.com/jamespud/magi/backend/adapter" + "github.com/jamespud/magi/backend/domain/entity" + "github.com/jamespud/magi/backend/domain/port" +) + +// singleConnPool pins one test's repository to a single MySQL connection so a +// session variable (innodb_lock_wait_timeout) applies to every statement the +// repository issues. Without it the pool may hand a different session to the +// verification query and the injected fault becomes non-deterministic. +func singleConnPool(t *testing.T, db interface{ DB() (*sql.DB, error) }) { + t.Helper() + sqlDB, err := db.DB() + if err != nil { + t.Fatalf("sql DB: %v", err) + } + sqlDB.SetMaxOpenConns(1) + sqlDB.SetMaxIdleConns(1) +} + +// P1: an error raised while verifying the durable owner (here: the +// SELECT ... FOR UPDATE against decision_job times out on a competing lock) is +// a storage failure, not a lost lease. It must surface as-is so the caller can +// abandon the execution instead of the old log-and-continue path. +// +// The competing lock is held on a separate pool because the repository pool is +// pinned to one connection for the session-scoped lock timeout. +func TestArtifactGeneration_OwnerVerificationQueryErrorIsNotLeaseLost(t *testing.T) { + db := openArtifactGenerationMySQL(t) + repo, jobs, job := seedArtifactGenerationJob(t, db) + owned := repo.(port.OwnedArtifactRepository) + singleConnPool(t, db) + if err := db.Exec("SET SESSION innodb_lock_wait_timeout = 1").Error; err != nil { + t.Fatalf("set lock wait timeout: %v", err) + } + + ctx := context.Background() + worker := "worker-owner-err-" + uuid.NewString() + _, owner := claimArtifactOwner(t, jobs, job, worker) + + // Competing lock on the exact row the owner predicate reads first. + blocker := openA2AMySQL(t) + blockerTx := blocker.Begin() + if blockerTx.Error != nil { + t.Fatalf("begin blocker tx: %v", blockerTx.Error) + } + defer func() { _ = blockerTx.Rollback().Error }() + var lockedID string + if err := blockerTx.Raw("SELECT id FROM decision_job WHERE id = ? FOR UPDATE", job.ID).Scan(&lockedID).Error; err != nil { + t.Fatalf("hold decision_job lock: %v", err) + } + + ev := &entity.EvidenceRecord{ID: "ev-owner-err-" + uuid.NewString(), CaseID: job.CaseID, CreatedAt: time.Now()} + start := time.Now() + err := owned.CreateEvidenceOwned(ctx, owner, ev) + if err == nil { + t.Fatal("owner verification lock timeout was reported as a successful write") + } + if errors.Is(err, port.ErrLeaseLost) { + t.Fatalf("owner verification query error was collapsed into ErrLeaseLost: %v", err) + } + if !strings.Contains(strings.ToLower(err.Error()), "lock wait timeout") { + t.Fatalf("error = %v, want the real MySQL lock wait timeout", err) + } + if elapsed := time.Since(start); elapsed < time.Second { + t.Fatalf("verification returned after %s; the query did not actually block on the lock", elapsed) + } + var rows int64 + if err := db.Model(&magi.EvidenceModel{}).Where("id = ?", ev.ID).Count(&rows).Error; err != nil { + t.Fatalf("count evidence: %v", err) + } + if rows != 0 { + t.Fatalf("evidence rows after a failed owner verification = %d, want 0", rows) + } + // The owner is still the active worker: the failure is transient storage + // trouble, not ownership loss, and the same generation may retry. + if err := blockerTx.Rollback().Error; err != nil { + t.Fatalf("release blocker: %v", err) + } + if err := owned.CreateEvidenceOwned(ctx, owner, ev); err != nil { + t.Fatalf("retry with a healthy lock: %v", err) + } +} + +// P1: an INSERT that fails inside the authoritative transaction must be +// reported as an error (never tolerated), and the transaction must leave no +// artifact behind. The trigger injects a real MySQL statement failure that is +// not a lease-loss signal. +func TestArtifactGeneration_ArtifactInsertErrorIsNotLeaseLost(t *testing.T) { + db := openArtifactGenerationMySQL(t) + repo, jobs, job := seedArtifactGenerationJob(t, db) + owned := repo.(port.OwnedArtifactRepository) + ctx := context.Background() + worker := "worker-insert-err-" + uuid.NewString() + _, owner := claimArtifactOwner(t, jobs, job, worker) + + runID := "run-insert-err-" + uuid.NewString() + trigger := "t3_insert_fail_" + strings.ReplaceAll(uuid.NewString(), "-", "")[:20] + ddl := fmt.Sprintf(`CREATE TRIGGER %s BEFORE INSERT ON magi_agent_run + FOR EACH ROW + BEGIN + IF NEW.id = '%s' THEN + SIGNAL SQLSTATE '45000' SET MESSAGE_TEXT = 't3 injected agent-run insert failure'; + END IF; + END`, trigger, runID) + if err := db.Exec(ddl).Error; err != nil { + t.Fatalf("create insert-failure trigger: %v", err) + } + defer func() { _ = db.Exec("DROP TRIGGER IF EXISTS " + trigger).Error }() + + err := owned.CreateAgentRunOwned(ctx, owner, &entity.AgentRun{ID: runID, CaseID: job.CaseID, StartedAt: time.Now()}) + if err == nil { + t.Fatal("injected INSERT failure was reported as a successful write") + } + if errors.Is(err, port.ErrLeaseLost) { + t.Fatalf("INSERT failure was collapsed into ErrLeaseLost: %v", err) + } + if !strings.Contains(err.Error(), "t3 injected agent-run insert failure") { + t.Fatalf("error = %v, want the injected MySQL statement failure", err) + } + var rows int64 + if err := db.Model(&magi.AgentRunModel{}).Where("id = ?", runID).Count(&rows).Error; err != nil { + t.Fatalf("count agent runs: %v", err) + } + if rows != 0 { + t.Fatalf("agent run rows after a failed INSERT = %d, want 0", rows) + } + + // The failed statement is recoverable: dropping the trigger frees the same + // owner/generation to write again. + if err := db.Exec("DROP TRIGGER IF EXISTS " + trigger).Error; err != nil { + t.Fatalf("drop trigger: %v", err) + } + if err := owned.CreateAgentRunOwned(ctx, owner, &entity.AgentRun{ID: runID, CaseID: job.CaseID, StartedAt: time.Now()}); err != nil { + t.Fatalf("retry after the INSERT failure: %v", err) + } +} + +// P1, unknown-outcome boundary: when the write is abandoned while the server +// statement is still in flight, the client cannot know whether the row landed. +// The repository must therefore never report success. Recovery is not "guess +// that it worked" -- the generation is abandoned and a later generation writes +// its own generation-scoped row. +// +// What this test does NOT prove: a literal post-COMMIT reply loss (the server +// commits, then the client never receives the acknowledgement). GORM/go-sql +// expose no seam to drop only the COMMIT reply, so that boundary stays an +// explicit, untested assumption rather than a claimed guarantee. +func TestArtifactGeneration_AbandonedInFlightWriteIsNotReportedCommitted(t *testing.T) { + db := openArtifactGenerationMySQL(t) + repo, jobs, job := seedArtifactGenerationJob(t, db) + owned := repo.(port.OwnedArtifactRepository) + worker := "worker-abandon-" + uuid.NewString() + first, owner1 := claimArtifactOwner(t, jobs, job, worker) + + runID := "run-abandon-" + uuid.NewString() + trigger := "t3_slow_insert_" + strings.ReplaceAll(uuid.NewString(), "-", "")[:19] + ddl := fmt.Sprintf(`CREATE TRIGGER %s BEFORE INSERT ON magi_agent_run + FOR EACH ROW + BEGIN + IF NEW.id = '%s' THEN + DO SLEEP(2); + END IF; + END`, trigger, runID) + if err := db.Exec(ddl).Error; err != nil { + t.Fatalf("create slow-insert trigger: %v", err) + } + defer func() { _ = db.Exec("DROP TRIGGER IF EXISTS " + trigger).Error }() + + ctx, cancel := context.WithTimeout(context.Background(), 250*time.Millisecond) + defer cancel() + err := owned.CreateAgentRunOwned(ctx, owner1, &entity.AgentRun{ID: runID, CaseID: job.CaseID, StartedAt: time.Now()}) + if err == nil { + t.Fatal("an abandoned in-flight authoritative write was reported as committed") + } + + // Whether the aborted statement eventually landed is exactly what the + // client cannot know; the observable contract is the abandoned generation. + // A later generation still owns the case, so requeue and recover. + if err := db.Exec("DROP TRIGGER IF EXISTS " + trigger).Error; err != nil { + t.Fatalf("drop trigger: %v", err) + } + requeueArtifactOwner(t, jobs, first, worker) + second, owner2 := claimArtifactOwner(t, jobs, job, worker) + if second.ExecutionGeneration == first.ExecutionGeneration { + t.Fatalf("recovery reused generation %d; the abandoned generation must be superseded", first.ExecutionGeneration) + } + if err := owned.CreateAgentRunOwned(context.Background(), owner2, &entity.AgentRun{ID: runID, CaseID: job.CaseID, StartedAt: time.Now()}); err != nil { + t.Fatalf("recover on new generation: %v", err) + } + gen2, err := owned.ListAgentRunsByGeneration(context.Background(), job.CaseID, second.ExecutionGeneration) + if err != nil || len(gen2) != 1 || gen2[0].ID != runID { + t.Fatalf("gen%d read = %+v err=%v", second.ExecutionGeneration, gen2, err) + } +} diff --git a/backend/adapter/artifact_generation_mysql_test.go b/backend/adapter/artifact_generation_mysql_test.go new file mode 100644 index 00000000..9382793d --- /dev/null +++ b/backend/adapter/artifact_generation_mysql_test.go @@ -0,0 +1,466 @@ +package magi_test + +import ( + "context" + "database/sql" + "errors" + "fmt" + "strings" + "testing" + "time" + + "github.com/google/uuid" + + magi "github.com/jamespud/magi/backend/adapter" + "github.com/jamespud/magi/backend/domain/entity" + "github.com/jamespud/magi/backend/domain/port" + "gorm.io/gorm" +) + +func openArtifactGenerationMySQL(t *testing.T) *gorm.DB { + t.Helper() + db := openA2AMySQL(t) + if err := db.AutoMigrate( + &magi.CaseModel{}, &magi.DecisionJobModel{}, &magi.DecisionJobClaimModel{}, + &magi.AgentRunModel{}, &magi.EvidenceModel{}, &magi.ClaimModel{}, &magi.VoteModel{}, + &magi.DebateRoundModel{}, &magi.ReflectionModel{}, &magi.ToolCallModel{}, + &magi.CheckpointModel{}, &magi.ResolutionModel{}, &magi.EventModel{}, &magi.EventCursorModel{}, + ); err != nil { + t.Fatalf("migrate T3 models: %v", err) + } + return db +} + +func seedArtifactGenerationJob(t *testing.T, db *gorm.DB) (port.Repository, port.DecisionJobRepository, *entity.DecisionJob) { + t.Helper() + ctx := context.Background() + caseID := "case-t3-" + uuid.NewString() + jobID := "job-t3-" + uuid.NewString() + repo := magi.NewRepository(db) + if err := repo.CaseRepo().Create(ctx, &entity.DecisionCase{ + ID: caseID, Status: entity.CaseStatusDraft, + }); err != nil { + t.Fatalf("create T3 case: %v", err) + } + if err := db.Create(&magi.DecisionJobModel{ + ID: jobID, CaseID: caseID, Status: string(entity.DecisionJobQueued), + MaxAttempts: 4, AvailableAt: time.Now().Add(-time.Second), + }).Error; err != nil { + t.Fatalf("create T3 job: %v", err) + } + t.Cleanup(func() { + // Order only matters for rows with application-level references; there + // are no SQL foreign keys in this persistence layer. + for _, table := range []string{ + "magi_tool_call", "reflection", "debate_round", "magi_vote", "claim", + "evidence_record", "magi_agent_run", "magi_agent_checkpoint", + "resolution", "magi_event", "magi_event_cursor", "decision_job_claim", + "decision_job", "decision_case", + } { + _ = db.Exec("DELETE FROM "+table+" WHERE "+cleanupPredicate(table), caseID).Error + } + }) + jobs := magi.NewDecisionJobRepository(db) + job, err := jobs.GetByCase(ctx, caseID) + if err != nil { + t.Fatalf("read T3 job: %v", err) + } + return repo, jobs, job +} + +func cleanupPredicate(table string) string { + switch table { + case "decision_job": + return "case_id = ?" + case "decision_case": + return "id = ?" + case "decision_job_claim": + return "case_id = ?" + case "magi_event_cursor": + return "case_id = ?" + default: + return "case_id = ?" + } +} + +func claimArtifactOwner(t *testing.T, jobs port.DecisionJobRepository, job *entity.DecisionJob, worker string) (*entity.DecisionJob, *entity.ExecutionContext) { + t.Helper() + claimed, ok, err := jobs.Claim(context.Background(), job.ID, worker, uuid.NewString(), time.Now().Add(time.Minute)) + if err != nil || !ok || claimed == nil { + t.Fatalf("claim T3 owner: job=%+v ok=%v err=%v", claimed, ok, err) + } + return claimed, &entity.ExecutionContext{ + CaseID: claimed.CaseID, JobID: claimed.ID, WorkerID: worker, + JobAttempt: claimed.Attempt, ExecutionGeneration: claimed.ExecutionGeneration, + } +} + +func requeueArtifactOwner(t *testing.T, jobs port.DecisionJobRepository, claimed *entity.DecisionJob, worker string) { + t.Helper() + retryAt := time.Now().Add(-time.Second) + if err := jobs.MarkFailed(context.Background(), claimed.ID, claimed.CaseID, worker, claimed.ExecutionGeneration, "t3 retry", &retryAt); err != nil { + t.Fatalf("requeue T3 owner: %v", err) + } +} + +func TestArtifactGeneration_StaleWriterFencedAndReadsScopedOnMySQL(t *testing.T) { + db := openArtifactGenerationMySQL(t) + repo, jobs, job := seedArtifactGenerationJob(t, db) + owned := repo.(port.OwnedArtifactRepository) + ctx := context.Background() + worker := "worker-t3-" + uuid.NewString() + first, owner1 := claimArtifactOwner(t, jobs, job, worker) + now := time.Now() + + active := []struct { + name string + run func() error + }{ + {"agent_run", func() error { + return owned.CreateAgentRunOwned(ctx, owner1, &entity.AgentRun{ID: "run-" + uuid.NewString(), CaseID: job.CaseID, StartedAt: now}) + }}, + {"evidence", func() error { + return owned.CreateEvidenceOwned(ctx, owner1, &entity.EvidenceRecord{ID: "ev-g1-" + uuid.NewString(), CaseID: job.CaseID, CreatedAt: now}) + }}, + {"claim", func() error { + return owned.CreateClaimOwned(ctx, owner1, &entity.Claim{ID: "cl-" + uuid.NewString(), CaseID: job.CaseID, CreatedAt: now}) + }}, + {"vote", func() error { + return owned.CreateVoteOwned(ctx, owner1, &entity.Vote{ID: "vote-" + uuid.NewString(), CaseID: job.CaseID, Decision: entity.VoteDecisionApprove, CreatedAt: now}) + }}, + {"debate", func() error { + return owned.CreateDebateRoundOwned(ctx, owner1, &entity.DebateRound{ID: "deb-" + uuid.NewString(), CaseID: job.CaseID, StartedAt: now}) + }}, + {"reflection", func() error { + return owned.CreateReflectionOwned(ctx, owner1, &entity.Reflection{ID: "refl-" + uuid.NewString(), CaseID: job.CaseID, CreatedAt: now}) + }}, + {"tool_call", func() error { + return owned.CreateToolCallOwned(ctx, owner1, &entity.ToolCall{ID: "tool-" + uuid.NewString(), CaseID: job.CaseID, CreatedAt: now}) + }}, + } + for _, tc := range active { + t.Run("active_"+tc.name, func(t *testing.T) { + if err := tc.run(); err != nil { + t.Fatalf("active write: %v", err) + } + }) + } + + // Intentionally do not call CleanupCaseArtifacts here. T3 correctness + // must come from generation fencing/read scoping; cleanup becomes GC in T5. + requeueArtifactOwner(t, jobs, first, worker) + second, owner2 := claimArtifactOwner(t, jobs, job, worker) + if second.ExecutionGeneration != first.ExecutionGeneration+1 { + t.Fatalf("generation = %d after %d, want +1", second.ExecutionGeneration, first.ExecutionGeneration) + } + + staleID := "ev-stale-" + uuid.NewString() + if err := owned.CreateEvidenceOwned(ctx, owner1, &entity.EvidenceRecord{ + ID: staleID, CaseID: job.CaseID, ExecutionGeneration: owner1.ExecutionGeneration, CreatedAt: now, + }); !errors.Is(err, port.ErrLeaseLost) { + t.Fatalf("stale writer error = %v, want ErrLeaseLost", err) + } + var staleRows int64 + if err := db.Model(&magi.EvidenceModel{}).Where("id = ?", staleID).Count(&staleRows).Error; err != nil { + t.Fatalf("count stale evidence: %v", err) + } + if staleRows != 0 { + t.Fatalf("stale writer inserted %d row(s)", staleRows) + } + + gen2ID := "ev-g2-" + uuid.NewString() + if err := owned.CreateEvidenceOwned(ctx, owner2, &entity.EvidenceRecord{ + ID: gen2ID, CaseID: job.CaseID, CreatedAt: now, + }); err != nil { + t.Fatalf("create gen2 evidence: %v", err) + } + gen1, err := owned.ListEvidenceByGeneration(ctx, job.CaseID, first.ExecutionGeneration) + if err != nil || len(gen1) != 1 { + t.Fatalf("gen1 evidence len=%d err=%v, want 1", len(gen1), err) + } + gen2, err := owned.ListEvidenceByGeneration(ctx, job.CaseID, second.ExecutionGeneration) + if err != nil || len(gen2) != 1 || gen2[0].ID != gen2ID { + t.Fatalf("gen2 evidence=%+v err=%v", gen2, err) + } + history, err := repo.EvidenceRepo().ListByCase(ctx, job.CaseID) + if err != nil || len(history) != 2 { + t.Fatalf("history evidence len=%d err=%v, want 2 generations", len(history), err) + } + + unfencedID := "ev-unfenced-" + uuid.NewString() + if err := repo.EvidenceRepo().Create(ctx, &entity.EvidenceRecord{ + ID: unfencedID, CaseID: job.CaseID, ExecutionGeneration: second.ExecutionGeneration, CreatedAt: now, + }); !errors.Is(err, port.ErrLeaseLost) { + t.Fatalf("legacy positive-generation Create error = %v, want ErrLeaseLost", err) + } +} + +func TestArtifactGeneration_HistoryPreservesLegacyGenerationZeroOnMySQL(t *testing.T) { + db := openArtifactGenerationMySQL(t) + repo, jobs, job := seedArtifactGenerationJob(t, db) + ctx := context.Background() + + legacyID := "ev-legacy-" + uuid.NewString() + if err := repo.EvidenceRepo().Create(ctx, &entity.EvidenceRecord{ + ID: legacyID, CaseID: job.CaseID, ExecutionGeneration: 0, CreatedAt: time.Now(), + }); err != nil { + t.Fatalf("create legacy evidence: %v", err) + } + + _, owner := claimArtifactOwner(t, jobs, job, "worker-history-"+uuid.NewString()) + currentID := "ev-current-" + uuid.NewString() + if err := repo.(port.OwnedArtifactRepository).CreateEvidenceOwned(ctx, owner, &entity.EvidenceRecord{ + ID: currentID, CaseID: job.CaseID, CreatedAt: time.Now(), + }); err != nil { + t.Fatalf("create current evidence: %v", err) + } + + history, err := repo.EvidenceRepo().ListByCase(ctx, job.CaseID) + if err != nil { + t.Fatalf("history read: %v", err) + } + if len(history) != 2 { + t.Fatalf("history len=%d, want legacy+current", len(history)) + } + generations := map[string]int64{} + for _, row := range history { + generations[row.ID] = row.ExecutionGeneration + } + if generations[legacyID] != 0 { + t.Fatalf("legacy generation=%d, want 0", generations[legacyID]) + } + if generations[currentID] != owner.ExecutionGeneration { + t.Fatalf("current generation=%d, want %d", generations[currentID], owner.ExecutionGeneration) + } +} + +func TestArtifactGeneration_CheckpointDoesNotCrossGenerationOnMySQL(t *testing.T) { + db := openArtifactGenerationMySQL(t) + repo, jobs, job := seedArtifactGenerationJob(t, db) + checkpoints := repo.(port.GenerationCheckpointRepository) + ctx := context.Background() + worker := "worker-cp-" + uuid.NewString() + first, owner1 := claimArtifactOwner(t, jobs, job, worker) + runID := "logical-run-" + uuid.NewString() + + if err := checkpoints.SaveForExecution(ctx, owner1, &entity.AgentState{ + CaseID: job.CaseID, RunID: runID, ExecutionGeneration: first.ExecutionGeneration, StepCount: 3, + }); err != nil { + t.Fatalf("save gen1 checkpoint: %v", err) + } + requeueArtifactOwner(t, jobs, first, worker) + second, owner2 := claimArtifactOwner(t, jobs, job, worker) + + if got, err := checkpoints.LoadForExecution(ctx, owner2, runID); err != nil || got != nil { + t.Fatalf("gen2 restored gen1 checkpoint: state=%+v err=%v", got, err) + } + if err := checkpoints.SaveForExecution(ctx, owner1, &entity.AgentState{ + CaseID: job.CaseID, RunID: runID, ExecutionGeneration: first.ExecutionGeneration, StepCount: 4, + }); !errors.Is(err, port.ErrLeaseLost) { + t.Fatalf("stale checkpoint save error = %v, want ErrLeaseLost", err) + } + if err := checkpoints.SaveForExecution(ctx, owner2, &entity.AgentState{ + CaseID: job.CaseID, RunID: runID, ExecutionGeneration: second.ExecutionGeneration, StepCount: 1, + }); err != nil { + t.Fatalf("save gen2 checkpoint: %v", err) + } + got, err := checkpoints.LoadForExecution(ctx, owner2, runID) + if err != nil || got == nil || got.ExecutionGeneration != second.ExecutionGeneration || got.StepCount != 1 { + t.Fatalf("gen2 checkpoint=%+v err=%v", got, err) + } + if err := db.Model(&magi.CheckpointModel{}). + Where("run_id = ? AND execution_generation = ?", runID, second.ExecutionGeneration). + Update("case_id", "case-corrupted-provenance").Error; err != nil { + t.Fatalf("corrupt checkpoint case provenance: %v", err) + } + if leaked, err := checkpoints.LoadForExecution(ctx, owner2, runID); err != nil || leaked != nil { + t.Fatalf("owner loaded checkpoint with mismatched case provenance: state=%+v err=%v", leaked, err) + } + if legacy, err := repo.CheckpointRepo().Load(ctx, runID); err != nil || legacy != nil { + t.Fatalf("legacy load exposed positive generation: state=%+v err=%v", legacy, err) + } + var rows int64 + if err := db.Model(&magi.CheckpointModel{}).Where("run_id = ?", runID).Count(&rows).Error; err != nil { + t.Fatalf("count checkpoints: %v", err) + } + if rows != 2 { + t.Fatalf("checkpoint rows = %d, want 2 generations", rows) + } +} + +func TestArtifactGeneration_TerminalReferencesRequireSameGenerationOnMySQL(t *testing.T) { + db := openArtifactGenerationMySQL(t) + repo, jobs, job := seedArtifactGenerationJob(t, db) + owned := repo.(port.OwnedArtifactRepository) + committer := repo.(port.TerminalCommitter) + ctx := context.Background() + worker := "worker-term-" + uuid.NewString() + first, owner1 := claimArtifactOwner(t, jobs, job, worker) + now := time.Now() + + voteOld := "vote-old-" + uuid.NewString() + evOld := "ev-old-" + uuid.NewString() + clOld := "cl-old-" + uuid.NewString() + if err := owned.CreateVoteOwned(ctx, owner1, &entity.Vote{ID: voteOld, CaseID: job.CaseID, Decision: entity.VoteDecisionApprove, CreatedAt: now}); err != nil { + t.Fatal(err) + } + if err := owned.CreateEvidenceOwned(ctx, owner1, &entity.EvidenceRecord{ID: evOld, CaseID: job.CaseID, CreatedAt: now}); err != nil { + t.Fatal(err) + } + if err := owned.CreateClaimOwned(ctx, owner1, &entity.Claim{ID: clOld, CaseID: job.CaseID, CreatedAt: now}); err != nil { + t.Fatal(err) + } + requeueArtifactOwner(t, jobs, first, worker) + second, owner2 := claimArtifactOwner(t, jobs, job, worker) + + for name, res := range map[string]*entity.Resolution{ + "vote": {ID: "res-v-" + uuid.NewString(), CaseID: job.CaseID, ExecutionGeneration: second.ExecutionGeneration, VoteIDs: []string{voteOld}}, + "evidence": {ID: "res-e-" + uuid.NewString(), CaseID: job.CaseID, ExecutionGeneration: second.ExecutionGeneration, KeyEvidenceIDs: []string{evOld}}, + "claim": {ID: "res-c-" + uuid.NewString(), CaseID: job.CaseID, ExecutionGeneration: second.ExecutionGeneration, KeyClaimIDs: []string{clOld}}, + } { + t.Run(name, func(t *testing.T) { + event := entity.NewEvent(job.CaseID, "", nil, entity.EventCaseCompleted, map[string]any{"kind": name}) + committed, err := committer.CommitTerminal(ctx, job.CaseID, entity.CaseStatusDraft, entity.CaseStatusResolved, res, &event) + if err == nil || committed { + t.Fatalf("cross-generation reference committed=%v err=%v", committed, err) + } + }) + } + + voteNew := "vote-new-" + uuid.NewString() + evNew := "ev-new-" + uuid.NewString() + clNew := "cl-new-" + uuid.NewString() + if err := owned.CreateVoteOwned(ctx, owner2, &entity.Vote{ID: voteNew, CaseID: job.CaseID, Decision: entity.VoteDecisionApprove, CreatedAt: now}); err != nil { + t.Fatal(err) + } + if err := owned.CreateEvidenceOwned(ctx, owner2, &entity.EvidenceRecord{ID: evNew, CaseID: job.CaseID, CreatedAt: now}); err != nil { + t.Fatal(err) + } + if err := owned.CreateClaimOwned(ctx, owner2, &entity.Claim{ID: clNew, CaseID: job.CaseID, CreatedAt: now}); err != nil { + t.Fatal(err) + } + res := &entity.Resolution{ + ID: "res-ok-" + uuid.NewString(), CaseID: job.CaseID, ExecutionGeneration: second.ExecutionGeneration, + VoteIDs: []string{voteNew}, KeyEvidenceIDs: []string{evNew}, KeyClaimIDs: []string{clNew}, + FinalDecision: entity.VoteDecisionApprove, CreatedAt: now, + } + event := entity.NewEvent(job.CaseID, "", nil, entity.EventCaseCompleted, nil) + committed, err := committer.CommitTerminal(ctx, job.CaseID, entity.CaseStatusDraft, entity.CaseStatusResolved, res, &event) + if err != nil || !committed { + t.Fatalf("same-generation terminal commit=%v err=%v", committed, err) + } + stored, err := repo.ResolutionRepo().Get(ctx, job.CaseID) + if err != nil || stored.ExecutionGeneration != second.ExecutionGeneration { + t.Fatalf("stored resolution=%+v err=%v", stored, err) + } +} + +func TestArtifactGeneration_OwnerCheckAndInsertShareLinearizationOnMySQL(t *testing.T) { + db := openArtifactGenerationMySQL(t) + repo, jobs, job := seedArtifactGenerationJob(t, db) + owned := repo.(port.OwnedArtifactRepository) + ctx := context.Background() + worker := "worker-linear-" + uuid.NewString() + first, owner := claimArtifactOwner(t, jobs, job, worker) + + sqlDB, err := db.DB() + if err != nil { + t.Fatalf("sql DB: %v", err) + } + blocker, err := sqlDB.Conn(ctx) + if err != nil { + t.Fatalf("blocker conn: %v", err) + } + defer blocker.Close() + + suffix := strings.ReplaceAll(uuid.NewString(), "-", "") + gateName := "t3_gate_" + suffix[:24] + enteredName := "t3_entered_" + suffix[:21] + triggerName := "t3_block_ev_" + suffix[:18] + var got int + if err := blocker.QueryRowContext(ctx, "SELECT GET_LOCK(?, 2)", gateName).Scan(&got); err != nil || got != 1 { + t.Fatalf("acquire gate: got=%d err=%v", got, err) + } + defer func() { + var released sql.NullInt64 + _ = blocker.QueryRowContext(context.Background(), "SELECT RELEASE_LOCK(?)", gateName).Scan(&released) + _ = db.Exec("DROP TRIGGER IF EXISTS " + triggerName).Error + }() + + trigger := fmt.Sprintf(`CREATE TRIGGER %s + BEFORE INSERT ON evidence_record + FOR EACH ROW + BEGIN + SET @t3_entered = GET_LOCK('%s', 0); + SET @t3_gate = GET_LOCK('%s', 10); + SET @t3_release_gate = RELEASE_LOCK('%s'); + SET @t3_release_entered = RELEASE_LOCK('%s'); + END`, triggerName, enteredName, gateName, gateName, enteredName) + if err := db.Exec(trigger).Error; err != nil { + t.Fatalf("create trigger: %v", err) + } + + evidenceID := "ev-linear-" + uuid.NewString() + writeDone := make(chan error, 1) + go func() { + writeDone <- owned.CreateEvidenceOwned(ctx, owner, &entity.EvidenceRecord{ + ID: evidenceID, CaseID: job.CaseID, CreatedAt: time.Now(), + }) + }() + + deadline := time.Now().Add(3 * time.Second) + entered := false + for time.Now().Before(deadline) { + var holder sql.NullInt64 + if err := db.Raw("SELECT IS_USED_LOCK(?)", enteredName).Scan(&holder).Error; err != nil { + t.Fatalf("observe trigger: %v", err) + } + if holder.Valid { + entered = true + break + } + time.Sleep(10 * time.Millisecond) + } + if !entered { + t.Fatal("artifact INSERT did not reach trigger barrier") + } + + transitionDone := make(chan error, 1) + retryAt := time.Now().Add(-time.Second) + go func() { + transitionDone <- jobs.MarkFailed(ctx, first.ID, first.CaseID, worker, first.ExecutionGeneration, "linearization", &retryAt) + }() + select { + case err := <-transitionDone: + t.Fatalf("owner transition completed while INSERT was blocked: %v", err) + case <-time.After(150 * time.Millisecond): + } + + var released sql.NullInt64 + if err := blocker.QueryRowContext(ctx, "SELECT RELEASE_LOCK(?)", gateName).Scan(&released); err != nil { + t.Fatalf("release gate: %v", err) + } + select { + case err := <-writeDone: + if err != nil { + t.Fatalf("artifact write: %v", err) + } + case <-time.After(3 * time.Second): + t.Fatal("artifact writer stayed blocked") + } + select { + case err := <-transitionDone: + if err != nil { + t.Fatalf("owner transition after write: %v", err) + } + case <-time.After(3 * time.Second): + t.Fatal("owner transition stayed blocked") + } + var rows int64 + if err := db.Model(&magi.EvidenceModel{}).Where("id = ?", evidenceID).Count(&rows).Error; err != nil { + t.Fatalf("count evidence: %v", err) + } + if rows != 1 { + t.Fatalf("evidence rows=%d, want 1", rows) + } +} diff --git a/backend/adapter/artifact_generation_orchestration_mysql_test.go b/backend/adapter/artifact_generation_orchestration_mysql_test.go new file mode 100644 index 00000000..9389b8d0 --- /dev/null +++ b/backend/adapter/artifact_generation_orchestration_mysql_test.go @@ -0,0 +1,336 @@ +package magi_test + +import ( + "context" + "errors" + "fmt" + "strings" + "sync" + "testing" + "time" + + "github.com/cloudwego/eino/components/model" + "github.com/cloudwego/eino/schema" + "github.com/google/uuid" + "gorm.io/gorm" + + magi "github.com/jamespud/magi/backend/adapter" + "github.com/jamespud/magi/backend/application/metrics" + "github.com/jamespud/magi/backend/domain/consensus" + "github.com/jamespud/magi/backend/domain/debate" + "github.com/jamespud/magi/backend/domain/entity" + "github.com/jamespud/magi/backend/domain/evidence" + "github.com/jamespud/magi/backend/domain/orchestration" + "github.com/jamespud/magi/backend/domain/port" + "github.com/jamespud/magi/backend/domain/runtime" + "github.com/jamespud/magi/backend/domain/service" + "github.com/jamespud/magi/backend/domain/validation" +) + +// --- minimal orchestration harness for the durable MySQL path --- + +type t3ScriptedModel struct { + mu sync.Mutex + responses []*schema.Message + calls int +} + +func (m *t3ScriptedModel) Generate(context.Context, []*schema.Message, ...model.Option) (*schema.Message, error) { + m.mu.Lock() + defer m.mu.Unlock() + if m.calls >= len(m.responses) { + return nil, fmt.Errorf("t3 scripted model exhausted after %d calls", m.calls) + } + msg := m.responses[m.calls] + m.calls++ + return msg, nil +} + +func (m *t3ScriptedModel) Stream(context.Context, []*schema.Message, ...model.Option) (*schema.StreamReader[*schema.Message], error) { + return nil, fmt.Errorf("stream not implemented") +} + +func (m *t3ScriptedModel) WithTools([]*schema.ToolInfo) (model.ToolCallingChatModel, error) { + return m, nil +} + +type t3ModelPort struct{ m model.ToolCallingChatModel } + +func (p t3ModelPort) Build(context.Context, entity.ModelRef) (model.ToolCallingChatModel, error) { + return p.m, nil +} + +// t3Runtime returns one approving ballot per agent without touching a model. +// The ledger and trace are non-empty so the orchestrator exercises the +// evidence/claim/tool-call writes as well as the agent run and vote. +type t3Runtime struct{} + +func (t3Runtime) Run(_ context.Context, cfg *entity.MagiConfig, actx *runtime.AgentContext) (*runtime.LoopResult, error) { + ledger := evidence.NewEvidenceLedger(actx.CaseID, "", cfg.Code) + ledger.Record("tc", "tool", "local", "", "observation", entity.ReliabilityScore{Final: 0.9}) + ledger.RecordClaim("claim by "+cfg.Code, nil, nil) + return &runtime.LoopResult{ + Vote: &entity.Vote{Decision: entity.VoteDecisionApprove, Confidence: 90, EvidenceIDs: []string{"EV-001"}}, + Status: runtime.LoopStatusCompleted, + Ledger: ledger, + Trace: &runtime.LoopTrace{Steps: []*runtime.Step{{ + IsFinal: true, + ToolCalls: []runtime.ToolCallRecord{{ + ToolCallID: "call-1", ToolName: "calc", Arguments: `{"a":1,"b":2}`, + Valid: true, Result: "3", Duration: 5 * time.Millisecond, + }}, + }}}, + Usage: &entity.Usage{TotalTokens: 100}, + }, nil +} + +func t3MagiConfig(code string) *entity.MagiConfig { + return &entity.MagiConfig{ + Code: code, Persona: code, + Objective: entity.ObjectiveFunction{Dimensions: []entity.UtilityDimension{ + {Code: "correctness", Weight: 0.5, Description: "be correct"}, + }}, + RiskTendency: entity.RiskTendencyNeutral, + EvidenceStandard: entity.EvidenceStandard{ + MinEvidenceCount: 1, MinQuantitativeCount: 1, MinReliability: 0, + RequireOwnCollected: true, + RequiredTypes: []entity.EvidenceTypeRequirement{{Type: "quantitative", MinCount: 1}}, + }, + Model: entity.ModelRef{ModelID: 1}, + Tools: []entity.ToolBinding{{Source: entity.ToolSourceLocal, ToolName: "calc"}}, + LoopPolicy: entity.LoopPolicy{MaxSteps: 12}, + } +} + +func t3Commander(t *testing.T) *service.Commander { + t.Helper() + script := &t3ScriptedModel{responses: []*schema.Message{ + schema.AssistantMessage(`{"canonical_question":"compute"}`, nil), + schema.AssistantMessage(`{"decision":"approve","summary":"decision report summary","key_reasons":["r1"],"risks":[],"next_steps":[],"key_evidence_ids":["EV-001"]}`, nil), + }} + cmd, err := service.NewCommander( + service.CommanderConfig{Model: entity.ModelRef{ModelID: 1}, Persona: "commander"}, + t3ModelPort{m: script}, + validation.NewReflectSchemaGenerator(), + validation.NewJSONSchemaValidator(), + ) + if err != nil { + t.Fatalf("commander: %v", err) + } + return cmd +} + +// t3DurableCase claims a real MySQL decision job and loads the Case row the +// claim just advanced to that execution generation, so the run satisfies the +// repository's active-owner predicate from the first authoritative write. +func t3DurableCase(t *testing.T, db *gorm.DB, repo port.Repository, jobs port.DecisionJobRepository, job *entity.DecisionJob, worker string) (*entity.DecisionCase, *entity.ExecutionContext) { + t.Helper() + if err := db.Model(&magi.CaseModel{}).Where("id = ?", job.CaseID). + Updates(map[string]any{"question": "compute", "max_debate_rounds": 1}).Error; err != nil { + t.Fatalf("seed case content: %v", err) + } + claimed, owner := claimArtifactOwner(t, jobs, job, worker) + case_, err := repo.CaseRepo().Get(context.Background(), job.CaseID) + if err != nil || case_ == nil { + t.Fatalf("load claimed case: %+v err=%v", case_, err) + } + if case_.ExecutionGeneration != owner.ExecutionGeneration { + t.Fatalf("case generation %d != owner generation %d", case_.ExecutionGeneration, owner.ExecutionGeneration) + } + case_.ExecutionAttempt = claimed.Attempt + return case_, owner +} + +func t3Orchestrator(t *testing.T, repo port.Repository, reg *metrics.Registry) *orchestration.Orchestrator { + t.Helper() + return orchestration.NewOrchestrator(orchestration.OrchestratorDeps{ + Repo: repo, + CaseRepo: repo.CaseRepo(), + AgentLoop: t3Runtime{}, + Consensus: consensus.NewConsensusEngine(), + Debate: debate.NewDebateEngine(nil), + Commander: t3Commander(t), + Configs: []*entity.MagiConfig{ + t3MagiConfig("melchior"), t3MagiConfig("balthasar"), t3MagiConfig("casper"), + }, + Policy: consensus.DefaultConsensusPolicy(), + Metrics: reg, + }) +} + +func t3IsSuccessTerminal(status entity.CaseStatus) bool { + switch status { + case entity.CaseStatusResolved, entity.CaseStatusMemoryIndexed, + entity.CaseStatusInsufficientEv, entity.CaseStatusDeadlocked: + return true + default: + return false + } +} + +// P1 control: the harness itself reaches a durable RESOLVED case on real MySQL. +// Without this, "the case never reaches success" would be vacuous. +func TestArtifactGeneration_DurableOrchestrationResolvesOnMySQL(t *testing.T) { + db := openArtifactGenerationMySQL(t) + repo, jobs, job := seedArtifactGenerationJob(t, db) + case_, owner := t3DurableCase(t, db, repo, jobs, job, "worker-orch-ok-"+uuid.NewString()) + reg := metrics.New() + + res, err := t3Orchestrator(t, repo, reg).OrchestrateForExecution(context.Background(), case_, owner) + if err != nil { + t.Fatalf("durable orchestration: %v", err) + } + if res == nil || res.FinalReport == "" { + t.Fatalf("resolution = %+v, want a persisted report", res) + } + stored, err := repo.CaseRepo().Get(context.Background(), case_.ID) + if err != nil || stored.Status != entity.CaseStatusResolved { + t.Fatalf("case status after success = %+v err=%v", stored, err) + } + runs, err := repo.(port.OwnedArtifactRepository).ListAgentRunsByGeneration(context.Background(), case_.ID, owner.ExecutionGeneration) + if err != nil || len(runs) != 3 { + t.Fatalf("gen%d agent runs = %d err=%v, want 3", owner.ExecutionGeneration, len(runs), err) + } + if got := reg.ArtifactPersistFailures(metrics.ArtifactAgentRun); got != 0 { + t.Fatalf("a successful run counted %d agent-run persist failures", got) + } +} + +// P1: a real MySQL INSERT failure on an authoritative artifact aborts the +// durable execution. The case must not reach a success terminal and must not +// persist a resolution. +func TestArtifactGeneration_DurableOrchestrationAbortsOnArtifactInsertError(t *testing.T) { + db := openArtifactGenerationMySQL(t) + repo, jobs, job := seedArtifactGenerationJob(t, db) + case_, owner := t3DurableCase(t, db, repo, jobs, job, "worker-orch-insert-"+uuid.NewString()) + reg := metrics.New() + + trigger := "t3_orch_fail_" + strings.ReplaceAll(uuid.NewString(), "-", "")[:20] + ddl := fmt.Sprintf(`CREATE TRIGGER %s BEFORE INSERT ON magi_agent_run + FOR EACH ROW + BEGIN + SIGNAL SQLSTATE '45000' SET MESSAGE_TEXT = 't3 orchestration artifact insert failure'; + END`, trigger) + if err := db.Exec(ddl).Error; err != nil { + t.Fatalf("create insert-failure trigger: %v", err) + } + defer func() { _ = db.Exec("DROP TRIGGER IF EXISTS " + trigger).Error }() + + res, err := t3Orchestrator(t, repo, reg).OrchestrateForExecution(context.Background(), case_, owner) + if err == nil { + t.Fatalf("durable orchestration swallowed an artifact INSERT failure: %+v", res) + } + if errors.Is(err, port.ErrLeaseLost) { + t.Fatalf("artifact INSERT failure was misreported as lease loss: %v", err) + } + if !strings.Contains(err.Error(), "t3 orchestration artifact insert failure") { + t.Fatalf("error = %v, want the injected MySQL failure", err) + } + stored, getErr := repo.CaseRepo().Get(context.Background(), case_.ID) + if getErr != nil { + t.Fatalf("reload case: %v", getErr) + } + if t3IsSuccessTerminal(stored.Status) { + t.Fatalf("case reached success terminal %s after an authoritative write failed", stored.Status) + } + if res != nil { + t.Fatalf("failed execution returned a resolution: %+v", res) + } + var resolutions int64 + if err := db.Model(&magi.ResolutionModel{}).Where("case_id = ?", case_.ID).Count(&resolutions).Error; err != nil { + t.Fatalf("count resolutions: %v", err) + } + if resolutions != 0 { + t.Fatalf("failed execution persisted %d resolution rows, want 0", resolutions) + } + if got := reg.ArtifactPersistFailures(metrics.ArtifactAgentRun); got == 0 { + t.Fatal("artifact persist failure was not counted") + } +} + +// P1: real MySQL storage error during owner verification (lock wait timeout on +// the decision_job row) aborts the durable execution instead of being logged +// away. Before this fix the run continued and reported a complete decision +// while the authoritative writes it claimed never landed. +func TestArtifactGeneration_DurableOrchestrationAbortsOnOwnerVerificationError(t *testing.T) { + db := openArtifactGenerationMySQL(t) + repo, jobs, job := seedArtifactGenerationJob(t, db) + case_, owner := t3DurableCase(t, db, repo, jobs, job, "worker-orch-owner-"+uuid.NewString()) + reg := metrics.New() + singleConnPool(t, db) + if err := db.Exec("SET SESSION innodb_lock_wait_timeout = 1").Error; err != nil { + t.Fatalf("set lock wait timeout: %v", err) + } + + blocker := openA2AMySQL(t) + blockerTx := blocker.Begin() + defer func() { _ = blockerTx.Rollback().Error }() + var lockedID string + if err := blockerTx.Raw("SELECT id FROM decision_job WHERE id = ? FOR UPDATE", job.ID).Scan(&lockedID).Error; err != nil { + t.Fatalf("hold decision_job lock: %v", err) + } + + res, err := t3Orchestrator(t, repo, reg).OrchestrateForExecution(context.Background(), case_, owner) + if err == nil { + t.Fatalf("durable orchestration swallowed an owner-verification error: %+v", res) + } + if errors.Is(err, port.ErrLeaseLost) { + t.Fatalf("owner-verification error was misreported as lease loss: %v", err) + } + if !strings.Contains(strings.ToLower(err.Error()), "lock wait timeout") { + t.Fatalf("error = %v, want the real MySQL lock wait timeout", err) + } + stored, getErr := repo.CaseRepo().Get(context.Background(), case_.ID) + if getErr != nil { + t.Fatalf("reload case: %v", getErr) + } + if t3IsSuccessTerminal(stored.Status) { + t.Fatalf("case reached success terminal %s after owner verification failed", stored.Status) + } + if res != nil { + t.Fatalf("failed execution returned a resolution: %+v", res) + } +} + +// P1, unknown-outcome boundary at the orchestration level: when an authoritative +// write is abandoned while the server statement is still running, the client +// cannot know whether it landed. The durable execution must not report a +// successful decision; recovery belongs to the next generation. +// +// Not covered, by construction: a literal post-COMMIT reply loss. GORM and +// go-sql-driver expose no seam to drop only the COMMIT acknowledgement, so this +// asserts the aborted-in-flight variant of the same "unknown outcome" class. +func TestArtifactGeneration_DurableOrchestrationAbandonsUnknownOutcome(t *testing.T) { + db := openArtifactGenerationMySQL(t) + repo, jobs, job := seedArtifactGenerationJob(t, db) + case_, owner := t3DurableCase(t, db, repo, jobs, job, "worker-orch-unknown-"+uuid.NewString()) + reg := metrics.New() + + trigger := "t3_orch_slow_" + strings.ReplaceAll(uuid.NewString(), "-", "")[:20] + ddl := fmt.Sprintf(`CREATE TRIGGER %s BEFORE INSERT ON magi_agent_run + FOR EACH ROW + BEGIN + DO SLEEP(2); + END`, trigger) + if err := db.Exec(ddl).Error; err != nil { + t.Fatalf("create slow-insert trigger: %v", err) + } + defer func() { _ = db.Exec("DROP TRIGGER IF EXISTS " + trigger).Error }() + + ctx, cancel := context.WithTimeout(context.Background(), 250*time.Millisecond) + defer cancel() + res, err := t3Orchestrator(t, repo, reg).OrchestrateForExecution(ctx, case_, owner) + if err == nil { + t.Fatalf("abandoned authoritative write produced a successful decision: %+v", res) + } + stored, getErr := repo.CaseRepo().Get(context.Background(), case_.ID) + if getErr != nil { + t.Fatalf("reload case: %v", getErr) + } + if t3IsSuccessTerminal(stored.Status) { + t.Fatalf("case reached success terminal %s after an abandoned write", stored.Status) + } + if res != nil { + t.Fatalf("abandoned execution returned a resolution: %+v", res) + } +} diff --git a/backend/adapter/case_repository_test.go b/backend/adapter/case_repository_test.go index 38896ae4..1d96d43d 100644 --- a/backend/adapter/case_repository_test.go +++ b/backend/adapter/case_repository_test.go @@ -208,3 +208,80 @@ func TestCaseRepository_DeleteRemovesAgentRunCheckpoint(t *testing.T) { t.Fatalf("keep checkpoint lost: %d", keepCheckpoint) } } + +// T3 / S27: magi_agent_checkpoint gained direct Case provenance, but the rows +// that predate it have none -- the legacy table stored no Case column, and +// run_id is the logical working-memory identity +// ("--r-"), not the magi_agent_run primary key. +// Delete therefore keeps two buckets: rows with direct provenance, and legacy +// rows whose run_id names one of the Case's runs (the pre-S27 containment rule). +// A legacy row that matches neither is retained as unknown-provenance history +// instead of being attributed to a Case by guesswork. +func TestCaseRepository_DeleteRetainsUnmatchedLegacyCheckpoints(t *testing.T) { + db := openCaseDB(t) + repo := magi.NewRepository(db) + cr := repo.CaseRepo() + ctx := context.Background() + + const caseID = "case-checkpoint-retention" + if err := cr.Create(ctx, &entity.DecisionCase{ID: caseID, Question: "q", CreatedAt: time.Now()}); err != nil { + t.Fatalf("create case: %v", err) + } + ownedRunID := caseID + "-melchior-g4-r1-investigate" + if err := repo.AgentRunRepo().Create(ctx, &entity.AgentRun{ID: ownedRunID, CaseID: caseID, StartedAt: time.Now()}); err != nil { + t.Fatalf("create agent run: %v", err) + } + // A legacy row whose run_id coincides with the AgentRun primary key: the + // pre-S27 deletion rule removes it with the Case. + matchedLegacyRunID := "run-victim-" + caseID + if err := repo.AgentRunRepo().Create(ctx, &entity.AgentRun{ID: matchedLegacyRunID, CaseID: caseID, StartedAt: time.Now()}); err != nil { + t.Fatalf("create matched legacy run: %v", err) + } + // A purely logical legacy identity matches no run and no Case column. + unmatchedRunID := caseID + "-melchior-r1-investigate" + + for _, m := range []magi.CheckpointModel{ + {RunID: ownedRunID, CaseID: caseID, ExecutionGeneration: 4}, + {RunID: matchedLegacyRunID}, + {RunID: unmatchedRunID}, + } { + row := m + if err := db.Create(&row).Error; err != nil { + t.Fatalf("seed checkpoint %s: %v", row.RunID, err) + } + } + + if err := cr.Delete(ctx, caseID); err != nil { + t.Fatalf("delete case: %v", err) + } + + for _, runID := range []string{ownedRunID, matchedLegacyRunID} { + var left int64 + if err := db.Model(&magi.CheckpointModel{}).Where("run_id = ?", runID).Count(&left).Error; err != nil { + t.Fatalf("count checkpoint %s: %v", runID, err) + } + if left != 0 { + t.Fatalf("checkpoint %s survived the delete", runID) + } + } + var runs int64 + if err := db.Model(&magi.AgentRunModel{}).Where("case_id = ?", caseID).Count(&runs).Error; err != nil { + t.Fatalf("count agent runs: %v", err) + } + if runs != 0 { + t.Fatalf("agent runs after delete = %d, want 0", runs) + } + + var retained struct { + CaseID string `gorm:"column:case_id"` + Generation int64 `gorm:"column:execution_generation"` + } + if err := db.Raw("SELECT case_id, execution_generation FROM magi_agent_checkpoint WHERE run_id = ?", unmatchedRunID). + Scan(&retained).Error; err != nil { + t.Fatalf("read retained checkpoint: %v", err) + } + if retained.CaseID != "" || retained.Generation != 0 { + t.Fatalf("retained checkpoint provenance = case %q generation %d, want unknown", + retained.CaseID, retained.Generation) + } +} diff --git a/backend/adapter/model.go b/backend/adapter/model.go index 24e9361c..bea42abf 100644 --- a/backend/adapter/model.go +++ b/backend/adapter/model.go @@ -29,19 +29,20 @@ type CaseModel struct { func (CaseModel) TableName() string { return "decision_case" } type AgentRunModel struct { - ID string `gorm:"primaryKey"` - CaseID string `gorm:"index"` - MagiConfigID string - MagiCode string - Round int - Status string - UsageJSON string `gorm:"type:text"` - EnvironmentJSON string `gorm:"type:text"` - Err string `gorm:"type:text"` - CheckpointJSON string `gorm:"type:text"` - SummaryJSON string `gorm:"type:text"` - StartedAt time.Time - CompletedAt *time.Time + ID string `gorm:"primaryKey"` + CaseID string `gorm:"index;index:idx_agent_run_case_generation,priority:1"` + ExecutionGeneration int64 `gorm:"not null;default:0;index:idx_agent_run_case_generation,priority:2"` + MagiConfigID string + MagiCode string + Round int + Status string + UsageJSON string `gorm:"type:text"` + EnvironmentJSON string `gorm:"type:text"` + Err string `gorm:"type:text"` + CheckpointJSON string `gorm:"type:text"` + SummaryJSON string `gorm:"type:text"` + StartedAt time.Time + CompletedAt *time.Time } func (AgentRunModel) TableName() string { return "magi_agent_run" } @@ -110,79 +111,85 @@ func (RagIndexJobModel) TableName() string { return "rag_index_job" } // own table: checkpoint writes are frequent and should not rewrite the // relatively stable AgentRun row. type CheckpointModel struct { - RunID string `gorm:"primaryKey"` - MessagesJSON string `gorm:"type:mediumtext"` - MessagesRefJSON string `gorm:"type:text"` - StepCount int - TokenUsed int - Phase string - SnapshotVersion int `gorm:"not null;default:1"` - SnapshotJSON string `gorm:"type:mediumtext"` - ManifestDigest string `gorm:"type:varchar(64);not null;default:''"` - UpdatedAt time.Time `gorm:"not null;type:datetime;default:CURRENT_TIMESTAMP"` + RunID string `gorm:"primaryKey;size:191"` + ExecutionGeneration int64 `gorm:"primaryKey;not null;default:0;index:idx_checkpoint_case_generation,priority:2"` + CaseID string `gorm:"size:64;not null;default:'';index:idx_checkpoint_case_generation,priority:1"` + MessagesJSON string `gorm:"type:mediumtext"` + MessagesRefJSON string `gorm:"type:text"` + StepCount int + TokenUsed int + Phase string + SnapshotVersion int `gorm:"not null;default:1"` + SnapshotJSON string `gorm:"type:mediumtext"` + ManifestDigest string `gorm:"type:varchar(64);not null;default:''"` + UpdatedAt time.Time `gorm:"not null;type:datetime;default:CURRENT_TIMESTAMP"` } func (CheckpointModel) TableName() string { return "magi_agent_checkpoint" } type EvidenceModel struct { - ID string `gorm:"primaryKey"` - CaseID string `gorm:"index"` - AgentRunID string `gorm:"index"` - ToolCallID string - ToolName string - SourceType string - SourceURI string `gorm:"type:varchar(512)"` - RawContent string `gorm:"type:text"` - Observation string `gorm:"type:text"` - ReliabilityJSON string `gorm:"type:text"` - CollectedBy string - CreatedAt time.Time + ID string `gorm:"primaryKey"` + CaseID string `gorm:"index;index:idx_evidence_case_generation,priority:1"` + ExecutionGeneration int64 `gorm:"not null;default:0;index:idx_evidence_case_generation,priority:2"` + AgentRunID string `gorm:"index"` + ToolCallID string + ToolName string + SourceType string + SourceURI string `gorm:"type:varchar(512)"` + RawContent string `gorm:"type:text"` + Observation string `gorm:"type:text"` + ReliabilityJSON string `gorm:"type:text"` + CollectedBy string + CreatedAt time.Time } func (EvidenceModel) TableName() string { return "evidence_record" } type ClaimModel struct { - ID string `gorm:"primaryKey"` - CaseID string `gorm:"index"` - AgentRunID string `gorm:"index"` - Statement string `gorm:"type:text"` - SupportsJSON string `gorm:"type:text"` - ContradictsJSON string `gorm:"type:text"` - Status string - CreatedBy string - CreatedAt time.Time + ID string `gorm:"primaryKey"` + CaseID string `gorm:"index;index:idx_claim_case_generation,priority:1"` + ExecutionGeneration int64 `gorm:"not null;default:0;index:idx_claim_case_generation,priority:2"` + AgentRunID string `gorm:"index"` + Statement string `gorm:"type:text"` + SupportsJSON string `gorm:"type:text"` + ContradictsJSON string `gorm:"type:text"` + Status string + CreatedBy string + CreatedAt time.Time } func (ClaimModel) TableName() string { return "claim" } type VoteModel struct { - ID string `gorm:"primaryKey"` - CaseID string `gorm:"index"` - AgentRunID string - Round int - Decision string - Confidence float64 - UtilityScoresJSON string `gorm:"type:text"` - KeyClaimIDsJSON string `gorm:"type:text"` - EvidenceIDsJSON string `gorm:"type:text"` - ReasoningSummary string `gorm:"type:text"` - ConditionsJSON string `gorm:"type:text"` - CreatedAt time.Time + ID string `gorm:"primaryKey"` + CaseID string `gorm:"index;index:idx_vote_case_generation,priority:1"` + ExecutionGeneration int64 `gorm:"not null;default:0;index:idx_vote_case_generation,priority:2"` + AgentRunID string + Round int + Decision string + Confidence float64 + UtilityScoresJSON string `gorm:"type:text"` + KeyClaimIDsJSON string `gorm:"type:text"` + EvidenceIDsJSON string `gorm:"type:text"` + ReasoningSummary string `gorm:"type:text"` + ConditionsJSON string `gorm:"type:text"` + CreatedAt time.Time } func (VoteModel) TableName() string { return "magi_vote" } type ResolutionModel struct { - ID string `gorm:"primaryKey"` - CaseID string `gorm:"uniqueIndex;size:64"` - ConsensusJSON string `gorm:"type:text"` - FinalDecision string - FinalReport string `gorm:"type:text"` - KeyEvidenceIDsJSON string `gorm:"type:text"` - KeyClaimIDsJSON string `gorm:"type:text"` - VoteIDsJSON string `gorm:"type:text"` - EvaluationJSON string `gorm:"type:text"` - CreatedAt time.Time + ID string `gorm:"primaryKey"` + CaseID string `gorm:"uniqueIndex;size:64"` + ExecutionGeneration int64 `gorm:"not null;default:0"` + ConsensusJSON string `gorm:"type:text"` + FinalDecision string + FinalReport string `gorm:"type:text"` + KeyEvidenceIDsJSON string `gorm:"type:text"` + KeyClaimIDsJSON string `gorm:"type:text"` + VoteIDsJSON string `gorm:"type:text"` + EvaluationJSON string `gorm:"type:text"` + CreatedAt time.Time } func (ResolutionModel) TableName() string { return "resolution" } @@ -208,28 +215,31 @@ type EventCursorModel struct { func (EventCursorModel) TableName() string { return "magi_event_cursor" } type DebateRoundModel struct { - ID string `gorm:"primaryKey"` - CaseID string `gorm:"index"` - Round int - PacketJSON string `gorm:"type:mediumtext"` - StartedAt time.Time - CompletedAt *time.Time + ID string `gorm:"primaryKey"` + CaseID string `gorm:"index;index:idx_debate_case_generation,priority:1"` + ExecutionGeneration int64 `gorm:"not null;default:0;index:idx_debate_case_generation,priority:2"` + Round int + PacketJSON string `gorm:"type:mediumtext"` + StartedAt time.Time + CompletedAt *time.Time } func (DebateRoundModel) TableName() string { return "debate_round" } type ReflectionModel struct { - ID string `gorm:"primaryKey"` - AgentRunID string `gorm:"index"` - Round int - PreviousVoteID string - PositionChange string - AcceptedClaimsJSON string `gorm:"type:text"` - RejectedClaimsJSON string `gorm:"type:text"` - NewEvidenceIDsJSON string `gorm:"type:text"` - Reasoning string `gorm:"type:text"` - ReadyToRevote bool - CreatedAt time.Time + ID string `gorm:"primaryKey"` + CaseID string `gorm:"size:64;not null;default:'';index:idx_reflection_case_generation,priority:1"` + ExecutionGeneration int64 `gorm:"not null;default:0;index:idx_reflection_case_generation,priority:2"` + AgentRunID string `gorm:"index"` + Round int + PreviousVoteID string + PositionChange string + AcceptedClaimsJSON string `gorm:"type:text"` + RejectedClaimsJSON string `gorm:"type:text"` + NewEvidenceIDsJSON string `gorm:"type:text"` + Reasoning string `gorm:"type:text"` + ReadyToRevote bool + CreatedAt time.Time } func (ReflectionModel) TableName() string { return "reflection" } @@ -251,18 +261,20 @@ type MemoryProjectionModel struct { func (MemoryProjectionModel) TableName() string { return "case_memory_projection" } type ToolCallModel struct { - ID string `gorm:"primaryKey"` - AgentRunID string `gorm:"index"` - ToolCallID string - ToolName string - Arguments string `gorm:"type:text"` - Valid bool - Result string `gorm:"type:text"` - Err string `gorm:"type:text"` - ApprovedBy string - EvidenceID string - DurationMs int64 - CreatedAt time.Time + ID string `gorm:"primaryKey"` + CaseID string `gorm:"size:64;not null;default:'';index:idx_tool_call_case_generation,priority:1"` + ExecutionGeneration int64 `gorm:"not null;default:0;index:idx_tool_call_case_generation,priority:2"` + AgentRunID string `gorm:"index"` + ToolCallID string + ToolName string + Arguments string `gorm:"type:text"` + Valid bool + Result string `gorm:"type:text"` + Err string `gorm:"type:text"` + ApprovedBy string + EvidenceID string + DurationMs int64 + CreatedAt time.Time } func (ToolCallModel) TableName() string { return "magi_tool_call" } diff --git a/backend/adapter/repository.go b/backend/adapter/repository.go index 61a61fb5..e83ff8fb 100644 --- a/backend/adapter/repository.go +++ b/backend/adapter/repository.go @@ -44,11 +44,9 @@ func NewRepository(db *gorm.DB) port.Repository { } // verifyResolutionReferences refuses a terminal commit whose resolution cites -// artifacts that are not in the database. It runs inside the same transaction -// that fences the terminal status, so a resolution can never be committed -// alongside a missing ballot, evidence record or claim. The cited ids embed the -// execution attempt, which keeps the check attempt-scoped: an older attempt's -// artifacts cannot satisfy a newer attempt's resolution. +// artifacts that are not in the same Case execution generation. It runs inside +// the same transaction that fences the terminal status. ID strings are never +// treated as ownership proof: case_id + execution_generation are authoritative. func verifyResolutionReferences(tx *gorm.DB, res *entity.Resolution) error { checks := []struct { kind string @@ -70,7 +68,7 @@ func verifyResolutionReferences(tx *gorm.DB, res *entity.Resolution) error { var n int64 // Scope by case: an id that exists but belongs to another case must not // satisfy this resolution's reference. - if err := tx.Model(check.model).Where("id IN ? AND case_id = ?", ids, res.CaseID).Count(&n).Error; err != nil { + if err := tx.Model(check.model).Where("id IN ? AND case_id = ? AND execution_generation = ?", ids, res.CaseID, res.ExecutionGeneration).Count(&n).Error; err != nil { return fmt.Errorf("terminal commit: verify %s references: %w", check.kind, err) } if int(n) != len(ids) { @@ -129,6 +127,315 @@ func (r *magiRepository) CleanupCaseArtifacts(ctx context.Context, caseID string }) } +func (r *magiRepository) withActiveExecution(ctx context.Context, owner *entity.ExecutionContext, write func(*gorm.DB) error) error { + if r.db == nil || owner == nil || !owner.IsDurable() || write == nil { + return port.ErrLeaseLost + } + return r.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { + now := time.Now() + var job DecisionJobModel + if err := tx.Clauses(clause.Locking{Strength: "UPDATE"}). + Where("id = ? AND case_id = ?", owner.JobID, owner.CaseID).First(&job).Error; err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + return port.ErrLeaseLost + } + return err + } + if job.Status != string(entity.DecisionJobRunning) || + job.WorkerID != owner.WorkerID || + job.ExecutionGeneration != owner.ExecutionGeneration || + job.LeaseUntil == nil || !job.LeaseUntil.After(now) { + return port.ErrLeaseLost + } + var caseModel CaseModel + if err := tx.Clauses(clause.Locking{Strength: "UPDATE"}). + Where("id = ?", owner.CaseID).First(&caseModel).Error; err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + return port.ErrLeaseLost + } + return err + } + if caseModel.ExecutionGeneration != owner.ExecutionGeneration { + return port.ErrLeaseLost + } + return write(tx) + }) +} + +func validateArtifactOwner(owner *entity.ExecutionContext, caseID string, generation int64) error { + if owner == nil || !owner.IsDurable() || caseID != owner.CaseID { + return port.ErrLeaseLost + } + if generation != 0 && generation != owner.ExecutionGeneration { + return port.ErrLeaseLost + } + return nil +} + +func (r *magiRepository) CreateAgentRunOwned(ctx context.Context, owner *entity.ExecutionContext, a *entity.AgentRun) error { + if a == nil || validateArtifactOwner(owner, a.CaseID, a.ExecutionGeneration) != nil { + return port.ErrLeaseLost + } + a.ExecutionGeneration = owner.ExecutionGeneration + m := AgentRunModel{ + ID: a.ID, CaseID: a.CaseID, ExecutionGeneration: a.ExecutionGeneration, + MagiConfigID: a.MagiConfigID, MagiCode: string(a.MagiCode), Round: a.Round, + Status: string(a.Status), UsageJSON: toJSON(a.Usage), EnvironmentJSON: toJSON(a.Environment), + Err: a.Err, StartedAt: a.StartedAt, CompletedAt: a.CompletedAt, + } + return r.withActiveExecution(ctx, owner, func(tx *gorm.DB) error { return tx.Create(&m).Error }) +} + +func (r *magiRepository) CreateEvidenceOwned(ctx context.Context, owner *entity.ExecutionContext, e *entity.EvidenceRecord) error { + if e == nil || validateArtifactOwner(owner, e.CaseID, e.ExecutionGeneration) != nil { + return port.ErrLeaseLost + } + e.ExecutionGeneration = owner.ExecutionGeneration + uri := "" + if e.SourceURI != nil { + uri = *e.SourceURI + } + m := EvidenceModel{ + ID: e.ID, CaseID: e.CaseID, ExecutionGeneration: e.ExecutionGeneration, + AgentRunID: e.AgentRunID, ToolCallID: e.ToolCallID, ToolName: e.ToolName, + SourceType: string(e.SourceType), SourceURI: uri, RawContent: e.RawContent, + Observation: e.Observation, ReliabilityJSON: toJSON(e.Reliability), + CollectedBy: string(e.CollectedBy), CreatedAt: e.CreatedAt, + } + return r.withActiveExecution(ctx, owner, func(tx *gorm.DB) error { return tx.Create(&m).Error }) +} + +func (r *magiRepository) CreateClaimOwned(ctx context.Context, owner *entity.ExecutionContext, c *entity.Claim) error { + if c == nil || validateArtifactOwner(owner, c.CaseID, c.ExecutionGeneration) != nil { + return port.ErrLeaseLost + } + c.ExecutionGeneration = owner.ExecutionGeneration + m := ClaimModel{ + ID: c.ID, CaseID: c.CaseID, ExecutionGeneration: c.ExecutionGeneration, + AgentRunID: c.AgentRunID, Statement: c.Statement, SupportsJSON: toJSON(c.Supports), + ContradictsJSON: toJSON(c.Contradicts), Status: string(c.Status), + CreatedBy: string(c.CreatedBy), CreatedAt: c.CreatedAt, + } + return r.withActiveExecution(ctx, owner, func(tx *gorm.DB) error { return tx.Create(&m).Error }) +} + +func (r *magiRepository) CreateVoteOwned(ctx context.Context, owner *entity.ExecutionContext, v *entity.Vote) error { + if v == nil || validateArtifactOwner(owner, v.CaseID, v.ExecutionGeneration) != nil { + return port.ErrLeaseLost + } + v.ExecutionGeneration = owner.ExecutionGeneration + m := VoteModel{ + ID: v.ID, CaseID: v.CaseID, ExecutionGeneration: v.ExecutionGeneration, + AgentRunID: v.AgentRunID, Round: v.Round, Decision: string(v.Decision), + Confidence: v.Confidence, UtilityScoresJSON: toJSON(v.UtilityScores), + KeyClaimIDsJSON: toJSON(v.KeyClaimIDs), EvidenceIDsJSON: toJSON(v.EvidenceIDs), + ReasoningSummary: v.ReasoningSummary, ConditionsJSON: toJSON(v.Conditions), CreatedAt: v.CreatedAt, + } + return r.withActiveExecution(ctx, owner, func(tx *gorm.DB) error { return tx.Create(&m).Error }) +} + +func (r *magiRepository) CreateDebateRoundOwned(ctx context.Context, owner *entity.ExecutionContext, d *entity.DebateRound) error { + if d == nil || validateArtifactOwner(owner, d.CaseID, d.ExecutionGeneration) != nil { + return port.ErrLeaseLost + } + d.ExecutionGeneration = owner.ExecutionGeneration + m := DebateRoundModel{ + ID: d.ID, CaseID: d.CaseID, ExecutionGeneration: d.ExecutionGeneration, + Round: d.Round, PacketJSON: toJSON(d.Packet), StartedAt: d.StartedAt, CompletedAt: d.CompletedAt, + } + return r.withActiveExecution(ctx, owner, func(tx *gorm.DB) error { return tx.Create(&m).Error }) +} + +func (r *magiRepository) CreateReflectionOwned(ctx context.Context, owner *entity.ExecutionContext, rf *entity.Reflection) error { + if rf == nil || validateArtifactOwner(owner, rf.CaseID, rf.ExecutionGeneration) != nil { + return port.ErrLeaseLost + } + rf.ExecutionGeneration = owner.ExecutionGeneration + m := ReflectionModel{ + ID: rf.ID, CaseID: rf.CaseID, ExecutionGeneration: rf.ExecutionGeneration, + AgentRunID: rf.AgentRunID, Round: rf.Round, PreviousVoteID: rf.PreviousVoteID, + PositionChange: string(rf.PositionChange), AcceptedClaimsJSON: toJSON(rf.AcceptedClaims), + RejectedClaimsJSON: toJSON(rf.RejectedClaims), NewEvidenceIDsJSON: toJSON(rf.NewEvidenceIDs), + Reasoning: rf.Reasoning, ReadyToRevote: rf.ReadyToRevote, CreatedAt: rf.CreatedAt, + } + return r.withActiveExecution(ctx, owner, func(tx *gorm.DB) error { return tx.Create(&m).Error }) +} + +func (r *magiRepository) CreateToolCallOwned(ctx context.Context, owner *entity.ExecutionContext, t *entity.ToolCall) error { + if t == nil || validateArtifactOwner(owner, t.CaseID, t.ExecutionGeneration) != nil { + return port.ErrLeaseLost + } + t.ExecutionGeneration = owner.ExecutionGeneration + m := ToolCallModel{ + ID: t.ID, CaseID: t.CaseID, ExecutionGeneration: t.ExecutionGeneration, + AgentRunID: t.AgentRunID, ToolCallID: t.ToolCallID, ToolName: t.ToolName, + Arguments: t.Arguments, Valid: t.Valid, Result: t.Result, Err: t.Err, + ApprovedBy: t.ApprovedBy, EvidenceID: t.EvidenceID, DurationMs: t.DurationMs, CreatedAt: t.CreatedAt, + } + return r.withActiveExecution(ctx, owner, func(tx *gorm.DB) error { return tx.Create(&m).Error }) +} + +func (r *magiRepository) SaveForExecution(ctx context.Context, owner *entity.ExecutionContext, state *entity.AgentState) error { + if state == nil || state.RunID == "" || validateArtifactOwner(owner, state.CaseID, state.ExecutionGeneration) != nil { + return port.ErrLeaseLost + } + state.ExecutionGeneration = owner.ExecutionGeneration + m := checkpointModel(state) + return r.withActiveExecution(ctx, owner, func(tx *gorm.DB) error { + return tx.Clauses(clause.OnConflict{ + Columns: []clause.Column{{Name: "run_id"}, {Name: "execution_generation"}}, + UpdateAll: true, + }).Create(&m).Error + }) +} + +func (r *magiRepository) LoadForExecution(ctx context.Context, owner *entity.ExecutionContext, runID string) (*entity.AgentState, error) { + if runID == "" || owner == nil || !owner.IsDurable() { + return nil, port.ErrLeaseLost + } + var out *entity.AgentState + err := r.withActiveExecution(ctx, owner, func(tx *gorm.DB) error { + var m CheckpointModel + if err := tx.First(&m, "run_id = ? AND case_id = ? AND execution_generation = ?", runID, owner.CaseID, owner.ExecutionGeneration).Error; err != nil { + if errors.Is(err, gorm.ErrRecordNotFound) { + out = nil + return nil + } + return err + } + out = checkpointFromModel(&m) + return nil + }) + return out, err +} + +func (r *magiRepository) ListAgentRunsByGeneration(ctx context.Context, caseID string, generation int64) ([]*entity.AgentRun, error) { + var models []AgentRunModel + if err := r.db.WithContext(ctx).Where("case_id = ? AND execution_generation = ?", caseID, generation). + Order("magi_code asc, round asc, started_at asc, id asc").Find(&models).Error; err != nil { + return nil, err + } + out := make([]*entity.AgentRun, len(models)) + for i, m := range models { + out[i] = &entity.AgentRun{ + ID: m.ID, CaseID: m.CaseID, ExecutionGeneration: m.ExecutionGeneration, + MagiConfigID: m.MagiConfigID, MagiCode: entity.MagiCode(m.MagiCode), Round: m.Round, + Status: entity.AgentRunStatus(m.Status), Usage: fromJSON[*entity.Usage](m.UsageJSON), + Environment: fromJSON[*entity.RunEnvironment](m.EnvironmentJSON), Err: m.Err, + StartedAt: m.StartedAt, CompletedAt: m.CompletedAt, + } + } + return out, nil +} + +func (r *magiRepository) ListEvidenceByGeneration(ctx context.Context, caseID string, generation int64) ([]*entity.EvidenceRecord, error) { + var models []EvidenceModel + if err := r.db.WithContext(ctx).Where("case_id = ? AND execution_generation = ?", caseID, generation).Find(&models).Error; err != nil { + return nil, err + } + out := make([]*entity.EvidenceRecord, len(models)) + for i := range models { + out[i] = evidenceFromModel(&models[i]) + } + return out, nil +} + +func (r *magiRepository) ListClaimsByGeneration(ctx context.Context, caseID string, generation int64) ([]*entity.Claim, error) { + var models []ClaimModel + if err := r.db.WithContext(ctx).Where("case_id = ? AND execution_generation = ?", caseID, generation).Find(&models).Error; err != nil { + return nil, err + } + out := make([]*entity.Claim, len(models)) + for i := range models { + out[i] = claimFromModel(&models[i]) + } + return out, nil +} + +func voteArtifactFromModel(m *VoteModel) *entity.Vote { + return &entity.Vote{ + ID: m.ID, CaseID: m.CaseID, ExecutionGeneration: m.ExecutionGeneration, + AgentRunID: m.AgentRunID, Round: m.Round, Decision: entity.VoteDecision(m.Decision), + Confidence: m.Confidence, UtilityScores: fromJSON[[]entity.UtilityDimensionScore](m.UtilityScoresJSON), + KeyClaimIDs: fromJSON[[]string](m.KeyClaimIDsJSON), EvidenceIDs: fromJSON[[]string](m.EvidenceIDsJSON), + ReasoningSummary: m.ReasoningSummary, Conditions: fromJSON[[]entity.DecisionCondition](m.ConditionsJSON), + CreatedAt: m.CreatedAt, + } +} + +func (r *magiRepository) ListVotesByGeneration(ctx context.Context, caseID string, generation int64) ([]*entity.Vote, error) { + var models []VoteModel + if err := r.db.WithContext(ctx).Where("case_id = ? AND execution_generation = ?", caseID, generation).Find(&models).Error; err != nil { + return nil, err + } + out := make([]*entity.Vote, len(models)) + for i := range models { + out[i] = voteArtifactFromModel(&models[i]) + } + return out, nil +} + +func (r *magiRepository) ListDebateRoundsByGeneration(ctx context.Context, caseID string, generation int64) ([]*entity.DebateRound, error) { + var models []DebateRoundModel + if err := r.db.WithContext(ctx).Where("case_id = ? AND execution_generation = ?", caseID, generation).Find(&models).Error; err != nil { + return nil, err + } + out := make([]*entity.DebateRound, len(models)) + for i, m := range models { + out[i] = &entity.DebateRound{ + ID: m.ID, CaseID: m.CaseID, ExecutionGeneration: m.ExecutionGeneration, Round: m.Round, + Packet: fromJSON[entity.DebatePacket](m.PacketJSON), StartedAt: m.StartedAt, CompletedAt: m.CompletedAt, + } + } + return out, nil +} + +func reflectionFromModel(m *ReflectionModel) *entity.Reflection { + return &entity.Reflection{ + ID: m.ID, CaseID: m.CaseID, ExecutionGeneration: m.ExecutionGeneration, + AgentRunID: m.AgentRunID, Round: m.Round, PreviousVoteID: m.PreviousVoteID, + PositionChange: entity.PositionChange(m.PositionChange), + AcceptedClaims: fromJSON[[]string](m.AcceptedClaimsJSON), + RejectedClaims: fromJSON[[]string](m.RejectedClaimsJSON), + NewEvidenceIDs: fromJSON[[]string](m.NewEvidenceIDsJSON), + Reasoning: m.Reasoning, ReadyToRevote: m.ReadyToRevote, CreatedAt: m.CreatedAt, + } +} + +func (r *magiRepository) ListReflectionsByGeneration(ctx context.Context, caseID string, generation int64) ([]*entity.Reflection, error) { + var models []ReflectionModel + if err := r.db.WithContext(ctx).Where("case_id = ? AND execution_generation = ?", caseID, generation).Find(&models).Error; err != nil { + return nil, err + } + out := make([]*entity.Reflection, len(models)) + for i := range models { + out[i] = reflectionFromModel(&models[i]) + } + return out, nil +} + +func toolCallFromModel(m *ToolCallModel) *entity.ToolCall { + return &entity.ToolCall{ + ID: m.ID, CaseID: m.CaseID, ExecutionGeneration: m.ExecutionGeneration, + AgentRunID: m.AgentRunID, ToolCallID: m.ToolCallID, ToolName: m.ToolName, + Arguments: m.Arguments, Valid: m.Valid, Result: m.Result, Err: m.Err, + ApprovedBy: m.ApprovedBy, EvidenceID: m.EvidenceID, DurationMs: m.DurationMs, CreatedAt: m.CreatedAt, + } +} + +func (r *magiRepository) ListToolCallsByGeneration(ctx context.Context, caseID string, generation int64) ([]*entity.ToolCall, error) { + var models []ToolCallModel + if err := r.db.WithContext(ctx).Where("case_id = ? AND execution_generation = ?", caseID, generation). + Order("created_at ASC, id ASC").Find(&models).Error; err != nil { + return nil, err + } + out := make([]*entity.ToolCall, len(models)) + for i := range models { + out[i] = toolCallFromModel(&models[i]) + } + return out, nil +} + func (r *magiRepository) CaseRepo() port.CaseRepository { return &caseRepo{db: r.db} } func (r *magiRepository) AgentRunRepo() port.AgentRunRepository { return &agentRunRepo{db: r.db} } func (r *magiRepository) EvidenceRepo() port.EvidenceRepository { return &evidenceRepo{db: r.db} } @@ -220,6 +527,8 @@ func (r *magiRepository) CommitStatusTransition(ctx context.Context, caseID stri return committed, nil } +var _ port.OwnedArtifactRepository = (*magiRepository)(nil) +var _ port.GenerationCheckpointRepository = (*magiRepository)(nil) var _ port.Repository = (*magiRepository)(nil) var _ port.TerminalCommitter = (*magiRepository)(nil) var _ port.StatusTransitionCommitter = (*magiRepository)(nil) @@ -365,39 +674,32 @@ func (r *caseRepo) UpdateFlags(ctx context.Context, id string, pinned, archived return r.db.WithContext(ctx).Model(&CaseModel{}).Where("id = ?", id).Updates(updates).Error } -// Delete removes a case and every artifact keyed by case_id (P2 D16). Tool -// calls and reflections are keyed by agent_run_id, so they are removed via a -// subquery against magi_agent_run. +// Delete removes a Case and every persisted row attributed to it. S27 gave +// ToolCall, Reflection and Checkpoint direct case_id provenance, so those rows +// are removed by column. +// +// Checkpoints additionally keep the pre-S27 containment rule: magi_agent_run +// rows still exist at this point, so a legacy checkpoint whose run_id names one +// of this Case's runs is removed with it. That join is a deletion rule, not a +// provenance claim -- the legacy checkpoint table recorded no Case at all, and +// its run_id is the logical working-memory identity ("--r-") +// that only coincides with an AgentRun primary key for unqualified runs. +// +// A legacy checkpoint that matches neither column is left in place: it has +// unknown provenance, so it is retained as legacy history instead of being +// attributed to a Case by guesswork. Positive-generation readers cannot see it +// because LoadForExecution requires an explicit case_id. func (r *caseRepo) Delete(ctx context.Context, id string) error { tx := r.db.WithContext(ctx).Begin() if tx.Error != nil { return tx.Error } - // Tool calls and reflections are keyed by agent_run_id, so they must be - // removed while magi_agent_run still exists. Deleting AgentRunModel first - // would make this subquery see an empty table and silently orphan them. - if err := tx.Where("agent_run_id IN (?)", - tx.Model(&AgentRunModel{}).Select("id").Where("case_id = ?", id), - ).Delete(&ToolCallModel{}).Error; err != nil { - _ = tx.Rollback() - return err - } - if err := tx.Where("agent_run_id IN (?)", - tx.Model(&AgentRunModel{}).Select("id").Where("case_id = ?", id), - ).Delete(&ReflectionModel{}).Error; err != nil { - _ = tx.Rollback() - return err - } - // Working-memory checkpoints are keyed by agent_run_id, not case_id; they - // must be removed while magi_agent_run still exists, before the run rows - // are deleted below. - if err := tx.Where("run_id IN (?)", - tx.Model(&AgentRunModel{}).Select("id").Where("case_id = ?", id), - ).Delete(&CheckpointModel{}).Error; err != nil { + if err := tx.Where("case_id = ? OR run_id IN (SELECT id FROM magi_agent_run WHERE case_id = ?)", id, id).Delete(&CheckpointModel{}).Error; err != nil { _ = tx.Rollback() return err } tables := []any{ + &ToolCallModel{}, &ReflectionModel{}, &AgentRunModel{}, &EvidenceModel{}, &ClaimModel{}, &VoteModel{}, &ResolutionModel{}, &EventModel{}, &EventCursorModel{}, &DebateRoundModel{}, &MemoryProjectionModel{}, &DecisionJobModel{}, &ApprovalModel{}, &JudgeModel{}, @@ -453,8 +755,11 @@ func caseFromModel(m *CaseModel) *entity.DecisionCase { type agentRunRepo struct{ db *gorm.DB } func (r *agentRunRepo) Create(ctx context.Context, a *entity.AgentRun) error { + if a == nil || a.ExecutionGeneration > 0 { + return port.ErrLeaseLost + } m := AgentRunModel{ - ID: a.ID, CaseID: a.CaseID, MagiConfigID: a.MagiConfigID, MagiCode: string(a.MagiCode), + ID: a.ID, CaseID: a.CaseID, ExecutionGeneration: a.ExecutionGeneration, MagiConfigID: a.MagiConfigID, MagiCode: string(a.MagiCode), Round: a.Round, Status: string(a.Status), UsageJSON: toJSON(a.Usage), EnvironmentJSON: toJSON(a.Environment), Err: a.Err, StartedAt: a.StartedAt, CompletedAt: a.CompletedAt, @@ -467,7 +772,7 @@ func (r *agentRunRepo) Get(ctx context.Context, id string) (*entity.AgentRun, er return nil, err } return &entity.AgentRun{ - ID: m.ID, CaseID: m.CaseID, MagiConfigID: m.MagiConfigID, MagiCode: entity.MagiCode(m.MagiCode), + ID: m.ID, CaseID: m.CaseID, ExecutionGeneration: m.ExecutionGeneration, MagiConfigID: m.MagiConfigID, MagiCode: entity.MagiCode(m.MagiCode), Round: m.Round, Status: entity.AgentRunStatus(m.Status), Usage: fromJSON[*entity.Usage](m.UsageJSON), Environment: fromJSON[*entity.RunEnvironment](m.EnvironmentJSON), Err: m.Err, StartedAt: m.StartedAt, CompletedAt: m.CompletedAt, @@ -485,7 +790,7 @@ func (r *agentRunRepo) ListByCase(ctx context.Context, caseID string) ([]*entity out := make([]*entity.AgentRun, len(models)) for i, m := range models { out[i] = &entity.AgentRun{ - ID: m.ID, CaseID: m.CaseID, MagiConfigID: m.MagiConfigID, MagiCode: entity.MagiCode(m.MagiCode), + ID: m.ID, CaseID: m.CaseID, ExecutionGeneration: m.ExecutionGeneration, MagiConfigID: m.MagiConfigID, MagiCode: entity.MagiCode(m.MagiCode), Round: m.Round, Status: entity.AgentRunStatus(m.Status), Usage: fromJSON[*entity.Usage](m.UsageJSON), Environment: fromJSON[*entity.RunEnvironment](m.EnvironmentJSON), Err: m.Err, StartedAt: m.StartedAt, CompletedAt: m.CompletedAt, @@ -532,12 +837,15 @@ func (r *agentRunRepo) SumUsageByUser(ctx context.Context, userID int64) (int64, type evidenceRepo struct{ db *gorm.DB } func (r *evidenceRepo) Create(ctx context.Context, e *entity.EvidenceRecord) error { + if e == nil || e.ExecutionGeneration > 0 { + return port.ErrLeaseLost + } uri := "" if e.SourceURI != nil { uri = *e.SourceURI } m := EvidenceModel{ - ID: e.ID, CaseID: e.CaseID, AgentRunID: e.AgentRunID, ToolCallID: e.ToolCallID, ToolName: e.ToolName, + ID: e.ID, CaseID: e.CaseID, ExecutionGeneration: e.ExecutionGeneration, AgentRunID: e.AgentRunID, ToolCallID: e.ToolCallID, ToolName: e.ToolName, SourceType: string(e.SourceType), SourceURI: uri, RawContent: e.RawContent, Observation: e.Observation, ReliabilityJSON: toJSON(e.Reliability), CollectedBy: string(e.CollectedBy), CreatedAt: e.CreatedAt, } @@ -565,7 +873,7 @@ func (r *evidenceRepo) ListByCase(ctx context.Context, caseID string) ([]*entity func evidenceFromModel(m *EvidenceModel) *entity.EvidenceRecord { uri := m.SourceURI return &entity.EvidenceRecord{ - ID: m.ID, CaseID: m.CaseID, AgentRunID: m.AgentRunID, ToolCallID: m.ToolCallID, ToolName: m.ToolName, + ID: m.ID, CaseID: m.CaseID, ExecutionGeneration: m.ExecutionGeneration, AgentRunID: m.AgentRunID, ToolCallID: m.ToolCallID, ToolName: m.ToolName, SourceType: entity.EvidenceSourceType(m.SourceType), SourceURI: &uri, RawContent: m.RawContent, Observation: m.Observation, Reliability: fromJSON[entity.ReliabilityScore](m.ReliabilityJSON), CollectedBy: entity.MagiCode(m.CollectedBy), CreatedAt: m.CreatedAt, @@ -577,8 +885,11 @@ func evidenceFromModel(m *EvidenceModel) *entity.EvidenceRecord { type claimRepo struct{ db *gorm.DB } func (r *claimRepo) Create(ctx context.Context, c *entity.Claim) error { + if c == nil || c.ExecutionGeneration > 0 { + return port.ErrLeaseLost + } m := ClaimModel{ - ID: c.ID, CaseID: c.CaseID, AgentRunID: c.AgentRunID, Statement: c.Statement, + ID: c.ID, CaseID: c.CaseID, ExecutionGeneration: c.ExecutionGeneration, AgentRunID: c.AgentRunID, Statement: c.Statement, SupportsJSON: toJSON(c.Supports), ContradictsJSON: toJSON(c.Contradicts), Status: string(c.Status), CreatedBy: string(c.CreatedBy), CreatedAt: c.CreatedAt, } @@ -605,7 +916,7 @@ func (r *claimRepo) ListByCase(ctx context.Context, caseID string) ([]*entity.Cl func claimFromModel(m *ClaimModel) *entity.Claim { return &entity.Claim{ - ID: m.ID, CaseID: m.CaseID, AgentRunID: m.AgentRunID, Statement: m.Statement, + ID: m.ID, CaseID: m.CaseID, ExecutionGeneration: m.ExecutionGeneration, AgentRunID: m.AgentRunID, Statement: m.Statement, Supports: fromJSON[[]string](m.SupportsJSON), Contradicts: fromJSON[[]string](m.ContradictsJSON), Status: entity.ClaimStatus(m.Status), CreatedBy: entity.MagiCode(m.CreatedBy), CreatedAt: m.CreatedAt, } @@ -616,8 +927,11 @@ func claimFromModel(m *ClaimModel) *entity.Claim { type voteRepo struct{ db *gorm.DB } func (r *voteRepo) Create(ctx context.Context, v *entity.Vote) error { + if v == nil || v.ExecutionGeneration > 0 { + return port.ErrLeaseLost + } m := VoteModel{ - ID: v.ID, CaseID: v.CaseID, AgentRunID: v.AgentRunID, Round: v.Round, Decision: string(v.Decision), + ID: v.ID, CaseID: v.CaseID, ExecutionGeneration: v.ExecutionGeneration, AgentRunID: v.AgentRunID, Round: v.Round, Decision: string(v.Decision), Confidence: v.Confidence, UtilityScoresJSON: toJSON(v.UtilityScores), KeyClaimIDsJSON: toJSON(v.KeyClaimIDs), EvidenceIDsJSON: toJSON(v.EvidenceIDs), ReasoningSummary: v.ReasoningSummary, ConditionsJSON: toJSON(v.Conditions), CreatedAt: v.CreatedAt, @@ -633,7 +947,7 @@ func (r *voteRepo) ListByCase(ctx context.Context, caseID string) ([]*entity.Vot for i := range models { m := &models[i] out[i] = &entity.Vote{ - ID: m.ID, CaseID: m.CaseID, AgentRunID: m.AgentRunID, Round: m.Round, Decision: entity.VoteDecision(m.Decision), + ID: m.ID, CaseID: m.CaseID, ExecutionGeneration: m.ExecutionGeneration, AgentRunID: m.AgentRunID, Round: m.Round, Decision: entity.VoteDecision(m.Decision), Confidence: m.Confidence, UtilityScores: fromJSON[[]entity.UtilityDimensionScore](m.UtilityScoresJSON), KeyClaimIDs: fromJSON[[]string](m.KeyClaimIDsJSON), EvidenceIDs: fromJSON[[]string](m.EvidenceIDsJSON), ReasoningSummary: m.ReasoningSummary, Conditions: fromJSON[[]entity.DecisionCondition](m.ConditionsJSON), @@ -654,7 +968,7 @@ func (r *resolutionRepo) Create(ctx context.Context, res *entity.Resolution) err func resolutionModel(res *entity.Resolution) ResolutionModel { return ResolutionModel{ - ID: res.ID, CaseID: res.CaseID, ConsensusJSON: toJSON(res.Consensus), FinalDecision: string(res.FinalDecision), + ID: res.ID, CaseID: res.CaseID, ExecutionGeneration: res.ExecutionGeneration, ConsensusJSON: toJSON(res.Consensus), FinalDecision: string(res.FinalDecision), FinalReport: res.FinalReport, KeyEvidenceIDsJSON: toJSON(res.KeyEvidenceIDs), KeyClaimIDsJSON: toJSON(res.KeyClaimIDs), VoteIDsJSON: toJSON(res.VoteIDs), EvaluationJSON: toJSON(res.Evaluation), CreatedAt: res.CreatedAt, } @@ -665,7 +979,7 @@ func (r *resolutionRepo) Get(ctx context.Context, caseID string) (*entity.Resolu return nil, err } return &entity.Resolution{ - ID: m.ID, CaseID: m.CaseID, Consensus: fromJSON[entity.ConsensusResult](m.ConsensusJSON), + ID: m.ID, CaseID: m.CaseID, ExecutionGeneration: m.ExecutionGeneration, Consensus: fromJSON[entity.ConsensusResult](m.ConsensusJSON), FinalDecision: entity.VoteDecision(m.FinalDecision), FinalReport: m.FinalReport, KeyEvidenceIDs: fromJSON[[]string](m.KeyEvidenceIDsJSON), KeyClaimIDs: fromJSON[[]string](m.KeyClaimIDsJSON), VoteIDs: fromJSON[[]string](m.VoteIDsJSON), Evaluation: fromJSON[*entity.Evaluation](m.EvaluationJSON), CreatedAt: m.CreatedAt, @@ -773,8 +1087,11 @@ func (r *eventRepo) ListAfterSeq(ctx context.Context, caseID string, afterSeq ui type debateRepo struct{ db *gorm.DB } func (r *debateRepo) Create(ctx context.Context, d *entity.DebateRound) error { + if d == nil || d.ExecutionGeneration > 0 { + return port.ErrLeaseLost + } m := DebateRoundModel{ - ID: d.ID, CaseID: d.CaseID, Round: d.Round, PacketJSON: toJSON(d.Packet), + ID: d.ID, CaseID: d.CaseID, ExecutionGeneration: d.ExecutionGeneration, Round: d.Round, PacketJSON: toJSON(d.Packet), StartedAt: d.StartedAt, CompletedAt: d.CompletedAt, } return r.db.WithContext(ctx).Create(&m).Error @@ -787,7 +1104,7 @@ func (r *debateRepo) ListByCase(ctx context.Context, caseID string) ([]*entity.D out := make([]*entity.DebateRound, len(models)) for i := range models { m := &models[i] - out[i] = &entity.DebateRound{ID: m.ID, CaseID: m.CaseID, Round: m.Round, + out[i] = &entity.DebateRound{ID: m.ID, CaseID: m.CaseID, ExecutionGeneration: m.ExecutionGeneration, Round: m.Round, Packet: fromJSON[entity.DebatePacket](m.PacketJSON), StartedAt: m.StartedAt, CompletedAt: m.CompletedAt} } return out, nil @@ -798,8 +1115,11 @@ func (r *debateRepo) ListByCase(ctx context.Context, caseID string) ([]*entity.D type reflectionRepo struct{ db *gorm.DB } func (r *reflectionRepo) Create(ctx context.Context, rf *entity.Reflection) error { + if rf == nil || rf.ExecutionGeneration > 0 { + return port.ErrLeaseLost + } m := ReflectionModel{ - ID: rf.ID, AgentRunID: rf.AgentRunID, Round: rf.Round, PreviousVoteID: rf.PreviousVoteID, + ID: rf.ID, CaseID: rf.CaseID, ExecutionGeneration: rf.ExecutionGeneration, AgentRunID: rf.AgentRunID, Round: rf.Round, PreviousVoteID: rf.PreviousVoteID, PositionChange: string(rf.PositionChange), AcceptedClaimsJSON: toJSON(rf.AcceptedClaims), RejectedClaimsJSON: toJSON(rf.RejectedClaims), NewEvidenceIDsJSON: toJSON(rf.NewEvidenceIDs), Reasoning: rf.Reasoning, ReadyToRevote: rf.ReadyToRevote, CreatedAt: rf.CreatedAt, @@ -808,13 +1128,13 @@ func (r *reflectionRepo) Create(ctx context.Context, rf *entity.Reflection) erro } func (r *reflectionRepo) ListByCase(ctx context.Context, caseID string) ([]*entity.Reflection, error) { var models []ReflectionModel - if err := r.db.WithContext(ctx).Joins("JOIN magi_agent_run ON magi_agent_run.id = reflection.agent_run_id").Where("magi_agent_run.case_id = ?", caseID).Find(&models).Error; err != nil { + if err := r.db.WithContext(ctx).Where("case_id = ?", caseID).Find(&models).Error; err != nil { return nil, err } out := make([]*entity.Reflection, len(models)) for i := range models { m := &models[i] - out[i] = &entity.Reflection{ID: m.ID, AgentRunID: m.AgentRunID, Round: m.Round, + out[i] = &entity.Reflection{ID: m.ID, CaseID: m.CaseID, ExecutionGeneration: m.ExecutionGeneration, AgentRunID: m.AgentRunID, Round: m.Round, PreviousVoteID: m.PreviousVoteID, PositionChange: entity.PositionChange(m.PositionChange), AcceptedClaims: fromJSON[[]string](m.AcceptedClaimsJSON), RejectedClaims: fromJSON[[]string](m.RejectedClaimsJSON), NewEvidenceIDs: fromJSON[[]string](m.NewEvidenceIDsJSON), Reasoning: m.Reasoning, ReadyToRevote: m.ReadyToRevote, CreatedAt: m.CreatedAt} @@ -830,19 +1150,14 @@ func (r *checkpointRepo) Save(ctx context.Context, state *entity.AgentState) err if state == nil || state.RunID == "" { return nil } - m := CheckpointModel{ - RunID: state.RunID, - MessagesJSON: state.MessagesJSON, - MessagesRefJSON: toJSON(state.Messages), - StepCount: state.StepCount, - TokenUsed: state.TokenUsed, - Phase: state.Phase, - SnapshotVersion: state.SnapshotVersion, - SnapshotJSON: state.SnapshotJSON, - ManifestDigest: state.ManifestDigest, - } - // Save is an upsert so every loop step has one durable snapshot per run. - return r.db.WithContext(ctx).Save(&m).Error + if state.ExecutionGeneration > 0 { + return port.ErrLeaseLost + } + m := checkpointModel(state) + return r.db.WithContext(ctx).Clauses(clause.OnConflict{ + Columns: []clause.Column{{Name: "run_id"}, {Name: "execution_generation"}}, + UpdateAll: true, + }).Create(&m).Error } func (r *checkpointRepo) Load(ctx context.Context, runID string) (*entity.AgentState, error) { @@ -850,23 +1165,41 @@ func (r *checkpointRepo) Load(ctx context.Context, runID string) (*entity.AgentS return nil, nil } var m CheckpointModel - if err := r.db.WithContext(ctx).First(&m, "run_id = ?", runID).Error; err != nil { + if err := r.db.WithContext(ctx).First(&m, "run_id = ? AND execution_generation = 0", runID).Error; err != nil { if errors.Is(err, gorm.ErrRecordNotFound) { return nil, nil } return nil, err } + return checkpointFromModel(&m), nil +} + +func (r *checkpointRepo) SaveForExecution(ctx context.Context, owner *entity.ExecutionContext, state *entity.AgentState) error { + return (&magiRepository{db: r.db}).SaveForExecution(ctx, owner, state) +} + +func (r *checkpointRepo) LoadForExecution(ctx context.Context, owner *entity.ExecutionContext, runID string) (*entity.AgentState, error) { + return (&magiRepository{db: r.db}).LoadForExecution(ctx, owner, runID) +} + +func checkpointModel(state *entity.AgentState) CheckpointModel { + return CheckpointModel{ + RunID: state.RunID, CaseID: state.CaseID, ExecutionGeneration: state.ExecutionGeneration, + MessagesJSON: state.MessagesJSON, MessagesRefJSON: toJSON(state.Messages), + StepCount: state.StepCount, TokenUsed: state.TokenUsed, Phase: state.Phase, + SnapshotVersion: state.SnapshotVersion, SnapshotJSON: state.SnapshotJSON, + ManifestDigest: state.ManifestDigest, + } +} + +func checkpointFromModel(m *CheckpointModel) *entity.AgentState { return &entity.AgentState{ - RunID: m.RunID, - Messages: fromJSON[[]entity.MessageRef](m.MessagesRefJSON), - MessagesJSON: m.MessagesJSON, - StepCount: m.StepCount, - TokenUsed: m.TokenUsed, - Phase: m.Phase, - SnapshotVersion: m.SnapshotVersion, - SnapshotJSON: m.SnapshotJSON, - ManifestDigest: m.ManifestDigest, - }, nil + RunID: m.RunID, CaseID: m.CaseID, ExecutionGeneration: m.ExecutionGeneration, + Messages: fromJSON[[]entity.MessageRef](m.MessagesRefJSON), MessagesJSON: m.MessagesJSON, + StepCount: m.StepCount, TokenUsed: m.TokenUsed, Phase: m.Phase, + SnapshotVersion: m.SnapshotVersion, SnapshotJSON: m.SnapshotJSON, + ManifestDigest: m.ManifestDigest, + } } // --- MemoryRepository (DB) --- @@ -990,8 +1323,11 @@ func (r *memoryRepo) Search(ctx context.Context, query string, limit int) ([]*en type toolCallRepo struct{ db *gorm.DB } func (r *toolCallRepo) Create(ctx context.Context, t *entity.ToolCall) error { + if t == nil || t.ExecutionGeneration > 0 { + return port.ErrLeaseLost + } m := ToolCallModel{ - ID: t.ID, AgentRunID: t.AgentRunID, ToolCallID: t.ToolCallID, ToolName: t.ToolName, + ID: t.ID, CaseID: t.CaseID, ExecutionGeneration: t.ExecutionGeneration, AgentRunID: t.AgentRunID, ToolCallID: t.ToolCallID, ToolName: t.ToolName, Arguments: t.Arguments, Valid: t.Valid, Result: t.Result, Err: t.Err, ApprovedBy: t.ApprovedBy, EvidenceID: t.EvidenceID, DurationMs: t.DurationMs, CreatedAt: t.CreatedAt, } @@ -1001,8 +1337,7 @@ func (r *toolCallRepo) Create(ctx context.Context, t *entity.ToolCall) error { func (r *toolCallRepo) ListByCase(ctx context.Context, caseID string) ([]*entity.ToolCall, error) { var models []ToolCallModel err := r.db.WithContext(ctx). - Joins("JOIN magi_agent_run ON magi_agent_run.id = magi_tool_call.agent_run_id"). - Where("magi_agent_run.case_id = ?", caseID). + Where("case_id = ?", caseID). Order("magi_tool_call.created_at ASC, magi_tool_call.id ASC"). Find(&models).Error if err != nil { @@ -1011,7 +1346,7 @@ func (r *toolCallRepo) ListByCase(ctx context.Context, caseID string) ([]*entity out := make([]*entity.ToolCall, len(models)) for i, m := range models { out[i] = &entity.ToolCall{ - ID: m.ID, AgentRunID: m.AgentRunID, ToolCallID: m.ToolCallID, ToolName: m.ToolName, + ID: m.ID, CaseID: m.CaseID, ExecutionGeneration: m.ExecutionGeneration, AgentRunID: m.AgentRunID, ToolCallID: m.ToolCallID, ToolName: m.ToolName, Arguments: m.Arguments, Valid: m.Valid, Result: m.Result, Err: m.Err, ApprovedBy: m.ApprovedBy, EvidenceID: m.EvidenceID, DurationMs: m.DurationMs, CreatedAt: m.CreatedAt, } @@ -1024,6 +1359,7 @@ var _ port.ConditionalCaseStatusWriter = (*caseRepo)(nil) var _ port.CaseListFilter = (*caseRepo)(nil) var _ port.EventRepository = (*eventRepo)(nil) var _ port.ToolCallRepository = (*toolCallRepo)(nil) +var _ port.GenerationCheckpointRepository = (*checkpointRepo)(nil) // CountCasesByUser returns the number of cases per owner (SQL GROUP BY). It // backs admin.Usage so a large case history is never fully loaded into memory. diff --git a/backend/application/a2a/stream_test.go b/backend/application/a2a/stream_test.go index b5f96bce..0a35e759 100644 --- a/backend/application/a2a/stream_test.go +++ b/backend/application/a2a/stream_test.go @@ -216,6 +216,13 @@ type retryTerminalOrchestrator struct { calls atomic.Int32 } +func (o *retryTerminalOrchestrator) OrchestrateForExecution(ctx context.Context, c *entity.DecisionCase, execution *entity.ExecutionContext) (*entity.Resolution, error) { + if execution == nil || !execution.IsDurable() || execution.CaseID != c.ID || execution.ExecutionGeneration != c.ExecutionGeneration { + return nil, port.ErrLeaseLost + } + return o.Orchestrate(ctx, c) +} + func (o *retryTerminalOrchestrator) Orchestrate(ctx context.Context, c *entity.DecisionCase) (*entity.Resolution, error) { if o.calls.Add(1) == 1 { return nil, errors.New("transient") diff --git a/backend/application/a2a/submission_test.go b/backend/application/a2a/submission_test.go index e88ccda5..4ae4b55c 100644 --- a/backend/application/a2a/submission_test.go +++ b/backend/application/a2a/submission_test.go @@ -58,6 +58,10 @@ func (b *blockingOrch) Orchestrate(ctx context.Context, c *entity.DecisionCase) return nil, ctx.Err() } +func (b *blockingOrch) OrchestrateForExecution(ctx context.Context, c *entity.DecisionCase, _ *entity.ExecutionContext) (*entity.Resolution, error) { + return b.Orchestrate(ctx, c) +} + // fakeJobRepo is a deterministic in-memory DecisionJobRepository for // SubmissionService tests. type fakeJobRepo struct { diff --git a/backend/application/decision/cleaner_test.go b/backend/application/decision/cleaner_test.go index 2385aecb..108680fc 100644 --- a/backend/application/decision/cleaner_test.go +++ b/backend/application/decision/cleaner_test.go @@ -3,7 +3,6 @@ package decision_test import ( "context" "errors" - "strings" "sync" "sync/atomic" "testing" @@ -26,7 +25,7 @@ func (c *recordingCleaner) CleanupCaseArtifacts(_ context.Context, caseID string return nil } -func TestRunManager_RetryCleansPreviousAttemptArtifacts(t *testing.T) { +func TestRunManager_GenerationRetryDoesNotUseCaseWideCleanup(t *testing.T) { db := openJobDB(t) repo := magi.NewRepository(db) if err := repo.CaseRepo().Create(context.Background(), &entity.DecisionCase{ID: "case-clean"}); err != nil { @@ -48,8 +47,8 @@ func TestRunManager_RetryCleansPreviousAttemptArtifacts(t *testing.T) { } cleaner.mu.Lock() defer cleaner.mu.Unlock() - if len(cleaner.calls) != 1 || cleaner.calls[0] != "case-clean" { - t.Fatalf("cleaner calls: %v", cleaner.calls) + if len(cleaner.calls) != 0 { + t.Fatalf("generation-aware retry must not invoke case-wide cleaner: %v", cleaner.calls) } } @@ -65,9 +64,10 @@ func (c *failingCleaner) CleanupCaseArtifacts(_ context.Context, caseID string) return errors.New("cleanup unavailable") } -// TestRunManager_RetryAbortsWhenCleanupFails proves the retry cleanup is a -// barrier: if attempt N-1 artifacts cannot be removed, attempt N must not start. -func TestRunManager_RetryAbortsWhenCleanupFails(t *testing.T) { +// TestRunManager_GenerationRetryIgnoresLegacyCleanupFailure proves T3 +// correctness no longer depends on case-wide cleanup. The cleaner is unsafe for +// positive generations and is reserved for generation-0 compatibility until T5. +func TestRunManager_GenerationRetryIgnoresLegacyCleanupFailure(t *testing.T) { db := openJobDB(t) repo := magi.NewRepository(db) if err := repo.CaseRepo().Create(context.Background(), &entity.DecisionCase{ID: "case-bad-clean"}); err != nil { @@ -83,34 +83,16 @@ func TestRunManager_RetryAbortsWhenCleanupFails(t *testing.T) { if err := rm.Start(context.Background(), &entity.DecisionCase{ID: "case-bad-clean"}); err != nil { t.Fatalf("start: %v", err) } - // The job must reach a terminal FAILED state via the fenced settlement, not - // sit in RUNNING until its lease expires. - job := waitJobStatus(t, jobs, "case-bad-clean", entity.DecisionJobFailed) - if job.Status != entity.DecisionJobFailed { - t.Fatalf("job status = %s, want %s", job.Status, entity.DecisionJobFailed) + job := waitJobStatus(t, jobs, "case-bad-clean", entity.DecisionJobSucceeded) + if job.Attempt != 2 || job.ExecutionGeneration != 2 { + t.Fatalf("retry result = %+v, want attempt=2 generation=2", job) } - if !strings.Contains(job.LastError, "retry cleanup failed") { - t.Fatalf("last error = %q, want it to record the cleanup failure", job.LastError) - } - if job.WorkerID != "" { - t.Fatalf("worker id = %q, want it cleared on settlement", job.WorkerID) - } - // resetCaseForRetry already moved the persisted Case to DRAFT before the - // cleanup failed, so the abort must settle Case + Job together rather than - // leaving Case=DRAFT next to Job=FAILED. - cs, err := repo.CaseRepo().Get(context.Background(), "case-bad-clean") - if err != nil { - t.Fatalf("reload case: %v", err) - } - if cs.Status != entity.CaseStatusFailed { - t.Fatalf("case status = %s, want %s (must not be left DRAFT)", cs.Status, entity.CaseStatusFailed) - } - if got := atomic.LoadInt32(&orch.calls); got != 1 { - t.Fatalf("orchestrator calls = %d, want 1 (retry must not run after a failed cleanup)", got) + if got := atomic.LoadInt32(&orch.calls); got != 2 { + t.Fatalf("orchestrator calls = %d, want 2", got) } cleaner.mu.Lock() defer cleaner.mu.Unlock() - if len(cleaner.calls) != 1 { - t.Fatalf("cleaner calls = %v, want exactly one", cleaner.calls) + if len(cleaner.calls) != 0 { + t.Fatalf("generation-aware retry invoked legacy cleaner: %v", cleaner.calls) } } diff --git a/backend/application/decision/multi_instance_test.go b/backend/application/decision/multi_instance_test.go index 494ad5ef..be0faf69 100644 --- a/backend/application/decision/multi_instance_test.go +++ b/backend/application/decision/multi_instance_test.go @@ -347,3 +347,15 @@ func TestRunManager_DBLimitAcrossInstances(t *testing.T) { close(orch.release) rm.Cancel("c1") } + +func (o *retryResetOrchestrator) OrchestrateForExecution(ctx context.Context, c *entity.DecisionCase, _ *entity.ExecutionContext) (*entity.Resolution, error) { + return o.Orchestrate(ctx, c) +} + +func (o *failThenSucceedCaseOrchestrator) OrchestrateForExecution(ctx context.Context, c *entity.DecisionCase, _ *entity.ExecutionContext) (*entity.Resolution, error) { + return o.Orchestrate(ctx, c) +} + +func (o *remoteCancelOrchestrator) OrchestrateForExecution(ctx context.Context, c *entity.DecisionCase, _ *entity.ExecutionContext) (*entity.Resolution, error) { + return o.Orchestrate(ctx, c) +} diff --git a/backend/application/decision/run_manager.go b/backend/application/decision/run_manager.go index 1ae86d7d..92091d4e 100644 --- a/backend/application/decision/run_manager.go +++ b/backend/application/decision/run_manager.go @@ -386,16 +386,17 @@ func (m *RunManager) execute(ctx context.Context, c *entity.DecisionCase, job *e m.releaseRejectedRetryClaim(claimed) return } - if m.cleaner != nil { - // Cleanup is a correctness barrier for the retry: a partial - // cleanup would leave attempt N-1 artifacts (votes, evidence, - // claims) mixed with attempt N, so a failure aborts the retry - // rather than being silently ignored. + if c.ExecutionGeneration == 0 && m.cleaner != nil { + // Legacy generation-0 retries still depend on the historical + // case-wide cleanup. Positive generations are isolated by T3 + // read/write fencing and MUST NOT invoke this unsafe delete: + // a delayed case-wide cleaner could erase a newer generation. + // T5 replaces the legacy cleaner with generation-scoped GC. cleanupCtx, cancelCleanup := detachedContext(ctx) cleanupErr := m.cleaner.CleanupCaseArtifacts(cleanupCtx, c.ID) cancelCleanup() if cleanupErr != nil { - log.Printf("run manager: retry cleanup for case %s failed, aborting retry: %v", c.ID, cleanupErr) + log.Printf("run manager: legacy retry cleanup for case %s failed, aborting retry: %v", c.ID, cleanupErr) m.settleRetryCleanupFailure(claimed, c, cleanupErr) return } @@ -406,7 +407,17 @@ func (m *RunManager) execute(ctx context.Context, c *entity.DecisionCase, job *e runStart := time.Now() m.metrics.RunStart() m.metrics.RunStartForUser(userIDString(c.UserID)) - _, runErr := m.orch.Orchestrate(attemptCtx, c) + executionOrch, ownerAware := m.orch.(ExecutionOrchestrator) + var runErr error + if !ownerAware { + runErr = fmt.Errorf("run manager: durable orchestration requires execution-owner capability") + } else { + owner := &entity.ExecutionContext{ + CaseID: c.ID, JobID: claimed.ID, WorkerID: m.workerID, + JobAttempt: claimed.Attempt, ExecutionGeneration: claimed.ExecutionGeneration, + } + _, runErr = executionOrch.OrchestrateForExecution(attemptCtx, c, owner) + } stopHeartbeat() finishMetrics := func(ok bool) { m.metrics.RunFinish(ok) diff --git a/backend/application/decision/run_manager_durable_test.go b/backend/application/decision/run_manager_durable_test.go index 1ac3d24f..24086bcb 100644 --- a/backend/application/decision/run_manager_durable_test.go +++ b/backend/application/decision/run_manager_durable_test.go @@ -25,6 +25,15 @@ type durableRetryOrchestrator struct { calls int32 } +type countingArtifactCleaner struct { + calls atomic.Int32 +} + +func (c *countingArtifactCleaner) CleanupCaseArtifacts(context.Context, string) error { + c.calls.Add(1) + return nil +} + type terminalCaseErrorOrchestrator struct{} func (terminalCaseErrorOrchestrator) Orchestrate(context.Context, *entity.DecisionCase) (*entity.Resolution, error) { @@ -237,6 +246,28 @@ func TestRunManager_DurableRetry(t *testing.T) { } } +func TestRunManager_GenerationAwareRetryDoesNotCallCaseWideCleaner(t *testing.T) { + db := openJobDB(t) + seedDecisionCase(t, db, "case-retry-no-cleanup", 0) + jobs := magi.NewDecisionJobRepository(db) + orch := &durableRetryOrchestrator{} + cleaner := &countingArtifactCleaner{} + rm := decision.NewRunManager(orch, decision.RunManagerDeps{ + JobRepo: jobs, WorkerID: "worker-no-cleanup", MaxAttempts: 2, + RetryBase: 10 * time.Millisecond, Cleaner: cleaner, + }) + if err := rm.Start(context.Background(), &entity.DecisionCase{ID: "case-retry-no-cleanup"}); err != nil { + t.Fatalf("start: %v", err) + } + job := waitJobStatus(t, jobs, "case-retry-no-cleanup", entity.DecisionJobSucceeded) + if job.Attempt != 2 || atomic.LoadInt32(&orch.calls) != 2 { + t.Fatalf("retry result: job=%+v calls=%d", job, orch.calls) + } + if got := cleaner.calls.Load(); got != 0 { + t.Fatalf("case-wide cleanup calls = %d, want 0 for generation-aware retry", got) + } +} + func TestRunManager_BlockingHeartbeatCancelsAttemptAtLeaseExpiry(t *testing.T) { db := openJobDB(t) seedDecisionCase(t, db, "case-heartbeat-blocked", 0) @@ -774,3 +805,35 @@ func TestRunManager_RetryPreservesExecutionAttemptAcrossReload(t *testing.T) { t.Fatalf("observed ExecutionAttempts = %v, want [1 2]", got) } } + +func (o *durableRetryOrchestrator) OrchestrateForExecution(ctx context.Context, c *entity.DecisionCase, _ *entity.ExecutionContext) (*entity.Resolution, error) { + return o.Orchestrate(ctx, c) +} + +func (o terminalCaseErrorOrchestrator) OrchestrateForExecution(ctx context.Context, c *entity.DecisionCase, _ *entity.ExecutionContext) (*entity.Resolution, error) { + return o.Orchestrate(ctx, c) +} + +func (o *successThenObserveCancellationOrchestrator) OrchestrateForExecution(ctx context.Context, c *entity.DecisionCase, _ *entity.ExecutionContext) (*entity.Resolution, error) { + return o.Orchestrate(ctx, c) +} + +func (o *lateSuccessAfterLeaseLossOrchestrator) OrchestrateForExecution(ctx context.Context, c *entity.DecisionCase, _ *entity.ExecutionContext) (*entity.Resolution, error) { + return o.Orchestrate(ctx, c) +} + +func (o *blockingUserOrchestrator) OrchestrateForExecution(ctx context.Context, c *entity.DecisionCase, _ *entity.ExecutionContext) (*entity.Resolution, error) { + return o.Orchestrate(ctx, c) +} + +func (o *countingRecoveryOrchestrator) OrchestrateForExecution(ctx context.Context, c *entity.DecisionCase, _ *entity.ExecutionContext) (*entity.Resolution, error) { + return o.Orchestrate(ctx, c) +} + +func (o *attemptRecordingOrchestrator) OrchestrateForExecution(ctx context.Context, c *entity.DecisionCase, _ *entity.ExecutionContext) (*entity.Resolution, error) { + return o.Orchestrate(ctx, c) +} + +func (o *shutdownBlockingOrchestrator) OrchestrateForExecution(ctx context.Context, c *entity.DecisionCase, _ *entity.ExecutionContext) (*entity.Resolution, error) { + return o.Orchestrate(ctx, c) +} diff --git a/backend/application/decision/run_manager_test.go b/backend/application/decision/run_manager_test.go index ab310245..6d008180 100644 --- a/backend/application/decision/run_manager_test.go +++ b/backend/application/decision/run_manager_test.go @@ -384,3 +384,7 @@ func TestService_ReportLoadsResolution(t *testing.T) { t.Fatal("Report for unknown case should be empty string") } } + +func (b *blockingOrchestrator) OrchestrateForExecution(ctx context.Context, c *entity.DecisionCase, _ *entity.ExecutionContext) (*entity.Resolution, error) { + return b.Orchestrate(ctx, c) +} diff --git a/backend/application/decision/service.go b/backend/application/decision/service.go index 2aa73494..99401810 100644 --- a/backend/application/decision/service.go +++ b/backend/application/decision/service.go @@ -25,6 +25,13 @@ type Orchestrator interface { Orchestrate(ctx context.Context, c *entity.DecisionCase) (*entity.Resolution, error) } +// ExecutionOrchestrator is the durable-run capability that carries the exact +// Claim owner into the domain/runtime persistence path. A positive-generation +// worker must use this capability; generation alone is not authorization. +type ExecutionOrchestrator interface { + OrchestrateForExecution(ctx context.Context, c *entity.DecisionCase, execution *entity.ExecutionContext) (*entity.Resolution, error) +} + // ServiceConfig holds application-level config for DecisionService. type ServiceConfig struct { MaxDebateRounds int @@ -102,6 +109,13 @@ func WithToolCallRepo(repo port.ToolCallRepository) Option { return func(s *Service) { s.toolCallRepo = repo } } +// WithOwnedArtifactRepository injects the generation-scoped authoritative read +// capability. Production should provide it; lightweight fakes may omit it and +// use the compatibility ListByCase+filter path. +func WithOwnedArtifactRepository(repo port.OwnedArtifactRepository) Option { + return func(s *Service) { s.ownedArtifacts = repo } +} + // Service is the application-layer service for decision cases. type Service struct { orch Orchestrator @@ -113,6 +127,7 @@ type Service struct { voteRepo port.VoteRepository agentRunRepo port.AgentRunRepository toolCallRepo port.ToolCallRepository + ownedArtifacts port.OwnedArtifactRepository runs RunController taskTreeCleaner port.TaskTreeCleaner } @@ -341,12 +356,47 @@ func (s *Service) Resolution(ctx context.Context, caseID string) (*entity.Resolu return res, nil } +func (s *Service) currentExecutionGeneration(ctx context.Context, caseID string) (int64, bool, error) { + if s.caseRepo == nil { + // Standalone/legacy mode has no durable Case generation to scope by. + return 0, false, nil + } + c, err := s.caseRepo.Get(ctx, caseID) + if err != nil { + return 0, true, fmt.Errorf("load current execution generation for case %s: %w", caseID, err) + } + if c == nil { + return 0, true, fmt.Errorf("load current execution generation for case %s: case not found", caseID) + } + return c.ExecutionGeneration, true, nil +} + // Evidence returns all evidence records for a case. func (s *Service) Evidence(ctx context.Context, caseID string) ([]*entity.EvidenceRecord, error) { if s.evidenceRepo == nil { return nil, nil } - return s.evidenceRepo.ListByCase(ctx, caseID) + generation, scoped, err := s.currentExecutionGeneration(ctx, caseID) + if err != nil { + return nil, err + } + if scoped && s.ownedArtifacts != nil { + return s.ownedArtifacts.ListEvidenceByGeneration(ctx, caseID, generation) + } + rows, err := s.evidenceRepo.ListByCase(ctx, caseID) + if err != nil { + return nil, err + } + if scoped { + filtered := rows[:0] + for _, row := range rows { + if row.ExecutionGeneration == generation { + filtered = append(filtered, row) + } + } + return filtered, nil + } + return rows, nil } // Claims returns all claims for a case. @@ -354,7 +404,27 @@ func (s *Service) Claims(ctx context.Context, caseID string) ([]*entity.Claim, e if s.claimRepo == nil { return nil, nil } - return s.claimRepo.ListByCase(ctx, caseID) + generation, scoped, err := s.currentExecutionGeneration(ctx, caseID) + if err != nil { + return nil, err + } + if scoped && s.ownedArtifacts != nil { + return s.ownedArtifacts.ListClaimsByGeneration(ctx, caseID, generation) + } + rows, err := s.claimRepo.ListByCase(ctx, caseID) + if err != nil { + return nil, err + } + if scoped { + filtered := rows[:0] + for _, row := range rows { + if row.ExecutionGeneration == generation { + filtered = append(filtered, row) + } + } + return filtered, nil + } + return rows, nil } // Votes returns all votes for a case. @@ -362,7 +432,27 @@ func (s *Service) Votes(ctx context.Context, caseID string) ([]*entity.Vote, err if s.voteRepo == nil { return nil, nil } - return s.voteRepo.ListByCase(ctx, caseID) + generation, scoped, err := s.currentExecutionGeneration(ctx, caseID) + if err != nil { + return nil, err + } + if scoped && s.ownedArtifacts != nil { + return s.ownedArtifacts.ListVotesByGeneration(ctx, caseID, generation) + } + rows, err := s.voteRepo.ListByCase(ctx, caseID) + if err != nil { + return nil, err + } + if scoped { + filtered := rows[:0] + for _, row := range rows { + if row.ExecutionGeneration == generation { + filtered = append(filtered, row) + } + } + return filtered, nil + } + return rows, nil } // AgentRuns returns all agent runs for a case. @@ -370,7 +460,27 @@ func (s *Service) AgentRuns(ctx context.Context, caseID string) ([]*entity.Agent if s.agentRunRepo == nil { return nil, nil } - return s.agentRunRepo.ListByCase(ctx, caseID) + generation, scoped, err := s.currentExecutionGeneration(ctx, caseID) + if err != nil { + return nil, err + } + if scoped && s.ownedArtifacts != nil { + return s.ownedArtifacts.ListAgentRunsByGeneration(ctx, caseID, generation) + } + rows, err := s.agentRunRepo.ListByCase(ctx, caseID) + if err != nil { + return nil, err + } + if scoped { + filtered := rows[:0] + for _, row := range rows { + if row.ExecutionGeneration == generation { + filtered = append(filtered, row) + } + } + return filtered, nil + } + return rows, nil } // ToolCalls returns all tool-call records for a case. @@ -378,7 +488,27 @@ func (s *Service) ToolCalls(ctx context.Context, caseID string) ([]*entity.ToolC if s.toolCallRepo == nil { return nil, nil } - return s.toolCallRepo.ListByCase(ctx, caseID) + generation, scoped, err := s.currentExecutionGeneration(ctx, caseID) + if err != nil { + return nil, err + } + if scoped && s.ownedArtifacts != nil { + return s.ownedArtifacts.ListToolCallsByGeneration(ctx, caseID, generation) + } + rows, err := s.toolCallRepo.ListByCase(ctx, caseID) + if err != nil { + return nil, err + } + if scoped { + filtered := rows[:0] + for _, row := range rows { + if row.ExecutionGeneration == generation { + filtered = append(filtered, row) + } + } + return filtered, nil + } + return rows, nil } // Cancel cancels a DecisionCase by setting its status to CANCELLED. diff --git a/backend/application/decision/service_test.go b/backend/application/decision/service_test.go index c85ee383..e4aa02ce 100644 --- a/backend/application/decision/service_test.go +++ b/backend/application/decision/service_test.go @@ -3,6 +3,7 @@ package decision_test import ( "context" "errors" + "fmt" "testing" magi "github.com/jamespud/magi/backend/adapter" @@ -434,3 +435,131 @@ func TestService_ListScoped_DelegatesToRepo(t *testing.T) { t.Fatalf("ListForUser not called with expected args: %d %d %d", repo.lastUserID, repo.lastLimit, repo.lastOffset) } } + +type failingGenerationCaseRepo struct { + port.CaseRepository +} + +func (f failingGenerationCaseRepo) Get(ctx context.Context, id string) (*entity.DecisionCase, error) { + return nil, errors.New("generation lookup unavailable") +} + +func TestService_CurrentArtifactReadsAreGenerationScoped(t *testing.T) { + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + if err := db.AutoMigrate( + &magi.CaseModel{}, &magi.AgentRunModel{}, &magi.EvidenceModel{}, &magi.ClaimModel{}, + &magi.VoteModel{}, &magi.ToolCallModel{}, + ); err != nil { + t.Fatalf("migrate: %v", err) + } + repo := magi.NewRepository(db) + ctx := context.Background() + const caseID = "case-current-generation-read" + if err := repo.CaseRepo().Create(ctx, &entity.DecisionCase{ + ID: caseID, Status: entity.CaseStatusDraft, ExecutionGeneration: 2, + }); err != nil { + t.Fatalf("create case: %v", err) + } + for _, generation := range []int64{1, 2} { + suffix := fmt.Sprintf("g%d", generation) + if err := db.Create(&magi.AgentRunModel{ + ID: "run-" + suffix, CaseID: caseID, ExecutionGeneration: generation, + }).Error; err != nil { + t.Fatalf("seed agent run %s: %v", suffix, err) + } + if err := db.Create(&magi.EvidenceModel{ + ID: "ev-" + suffix, CaseID: caseID, ExecutionGeneration: generation, + }).Error; err != nil { + t.Fatalf("seed evidence %s: %v", suffix, err) + } + if err := db.Create(&magi.ClaimModel{ + ID: "cl-" + suffix, CaseID: caseID, ExecutionGeneration: generation, + }).Error; err != nil { + t.Fatalf("seed claim %s: %v", suffix, err) + } + if err := db.Create(&magi.VoteModel{ + ID: "vote-" + suffix, CaseID: caseID, ExecutionGeneration: generation, + }).Error; err != nil { + t.Fatalf("seed vote %s: %v", suffix, err) + } + if err := db.Create(&magi.ToolCallModel{ + ID: "tool-" + suffix, CaseID: caseID, ExecutionGeneration: generation, + }).Error; err != nil { + t.Fatalf("seed tool call %s: %v", suffix, err) + } + } + + svc := decision.NewService(&stubOrchestrator{}, decision.ServiceConfig{}, + decision.WithCaseRepo(repo.CaseRepo()), + decision.WithAgentRunRepo(repo.AgentRunRepo()), + decision.WithEvidenceRepo(repo.EvidenceRepo()), + decision.WithClaimRepo(repo.ClaimRepo()), + decision.WithVoteRepo(repo.VoteRepo()), + decision.WithToolCallRepo(repo.ToolCallRepo()), + decision.WithOwnedArtifactRepository(repo.(port.OwnedArtifactRepository)), + ) + + runs, err := svc.AgentRuns(ctx, caseID) + if err != nil || len(runs) != 1 || runs[0].ExecutionGeneration != 2 { + t.Fatalf("current agent runs = %+v err=%v", runs, err) + } + evidence, err := svc.Evidence(ctx, caseID) + if err != nil || len(evidence) != 1 || evidence[0].ExecutionGeneration != 2 { + t.Fatalf("current evidence = %+v err=%v", evidence, err) + } + claims, err := svc.Claims(ctx, caseID) + if err != nil || len(claims) != 1 || claims[0].ExecutionGeneration != 2 { + t.Fatalf("current claims = %+v err=%v", claims, err) + } + votes, err := svc.Votes(ctx, caseID) + if err != nil || len(votes) != 1 || votes[0].ExecutionGeneration != 2 { + t.Fatalf("current votes = %+v err=%v", votes, err) + } + tools, err := svc.ToolCalls(ctx, caseID) + if err != nil || len(tools) != 1 || tools[0].ExecutionGeneration != 2 { + t.Fatalf("current tool calls = %+v err=%v", tools, err) + } +} + +func TestService_CurrentArtifactReadsFailClosedWhenGenerationLookupFails(t *testing.T) { + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + if err := db.AutoMigrate(&magi.CaseModel{}, &magi.EvidenceModel{}); err != nil { + t.Fatalf("migrate: %v", err) + } + repo := magi.NewRepository(db) + ctx := context.Background() + const caseID = "case-generation-read-failure" + if err := repo.CaseRepo().Create(ctx, &entity.DecisionCase{ + ID: caseID, ExecutionGeneration: 2, + }); err != nil { + t.Fatalf("create case: %v", err) + } + if err := db.Create(&magi.EvidenceModel{ + ID: "ev-old", CaseID: caseID, ExecutionGeneration: 1, + }).Error; err != nil { + t.Fatalf("seed old evidence: %v", err) + } + if err := db.Create(&magi.EvidenceModel{ + ID: "ev-current", CaseID: caseID, ExecutionGeneration: 2, + }).Error; err != nil { + t.Fatalf("seed current evidence: %v", err) + } + + svc := decision.NewService(&stubOrchestrator{}, decision.ServiceConfig{}, + decision.WithCaseRepo(failingGenerationCaseRepo{CaseRepository: repo.CaseRepo()}), + decision.WithEvidenceRepo(repo.EvidenceRepo()), + ) + rows, err := svc.Evidence(ctx, caseID) + if err == nil { + t.Fatalf("generation lookup failure returned rows instead of failing closed: %+v", rows) + } + if len(rows) != 0 { + t.Fatalf("generation lookup failure leaked history rows: %+v", rows) + } +} diff --git a/backend/application/evaluation/service_test.go b/backend/application/evaluation/service_test.go index 7ec8f8a6..753f097d 100644 --- a/backend/application/evaluation/service_test.go +++ b/backend/application/evaluation/service_test.go @@ -52,7 +52,7 @@ func TestEvaluationService_EvaluateCaseFromRepository(t *testing.T) { t.Fatalf("evidence: %v", err) } if err := repo.ToolCallRepo().Create(context.Background(), &entity.ToolCall{ - ID: "tc-1", AgentRunID: "run-1", Valid: true, + ID: "tc-1", CaseID: "case-1", AgentRunID: "run-1", Valid: true, }); err != nil { t.Fatalf("tool call: %v", err) } diff --git a/backend/application/metrics/registry.go b/backend/application/metrics/registry.go index e70af4a9..f40b2042 100644 --- a/backend/application/metrics/registry.go +++ b/backend/application/metrics/registry.go @@ -166,8 +166,10 @@ var artifactKinds = [...]ArtifactKind{ ArtifactDebateRound, ArtifactReflection, ArtifactTaskSnapshot, ArtifactMemoryProjection, } -// IncArtifactPersistFailure records a durable write that failed and was -// tolerated. The kind must be one of the fixed constants above. +// IncArtifactPersistFailure records one artifact write that returned an error. +// On a durable owner path the caller escalates that error and aborts the +// execution; on the legacy/best-effort path the write is tolerated and the +// counter is the only trace. The kind must be one of the fixed constants above. func (r *Registry) IncArtifactPersistFailure(kind ArtifactKind) { if r == nil { return diff --git a/backend/bootstrap/container.go b/backend/bootstrap/container.go index 9046917b..317b21a5 100644 --- a/backend/bootstrap/container.go +++ b/backend/bootstrap/container.go @@ -822,6 +822,9 @@ func provideDecisionService( decision.WithToolCallRepo(repo.ToolCallRepo()), decision.WithRunManager(rm), } + if owned, ok := repo.(port.OwnedArtifactRepository); ok { + opts = append(opts, decision.WithOwnedArtifactRepository(owned)) + } if tt, ok := ttRepo.(port.TaskTreeCleaner); ok { opts = append(opts, decision.WithTaskTreeCleaner(tt)) } diff --git a/backend/bootstrap/migration_parity_test.go b/backend/bootstrap/migration_parity_test.go index 60a247d0..4e3a1a38 100644 --- a/backend/bootstrap/migration_parity_test.go +++ b/backend/bootstrap/migration_parity_test.go @@ -17,6 +17,7 @@ const s21MigrationPath = "../../docker/atlas/migrations/magi_s21_runtime_kernel. const s24MigrationPath = "../../docker/atlas/migrations/magi_s24_approval_intent_binding.sql" const s25MigrationPath = "../../docker/atlas/migrations/magi_s25_execution_generation.sql" const s26MigrationPath = "../../docker/atlas/migrations/magi_s26_claim_token.sql" +const s27MigrationPath = "../../docker/atlas/migrations/magi_s27_artifact_generation.sql" // T1 (#19): the Case execution generation is persisted on both the Case and the // job that claims it. The Atlas migration and the GORM models must agree on the @@ -88,6 +89,69 @@ func TestS26ClaimTokenMigrationMatchesModel(t *testing.T) { } } +// T3 (#21): authoritative artifacts and checkpoints carry explicit Case +// execution generation. Atlas and AutoMigrate must agree on BIGINT/zero legacy +// semantics; checkpoint authority is keyed by logical RunID + generation. +func TestS27ArtifactGenerationMigrationMatchesModel(t *testing.T) { + raw, err := os.ReadFile(s27MigrationPath) + if err != nil { + t.Fatalf("read s27 migration: %v", err) + } + sql := string(raw) + for _, table := range []string{"reflection", "magi_tool_call", "magi_agent_checkpoint"} { + pattern := regexp.MustCompile(`(?i)ALTER TABLE\s+` + table + `[^;]*ADD COLUMN\s+case_id\s+VARCHAR\(64\)\s+NOT NULL\s+DEFAULT\s+''`) + if pattern.FindString(sql) == "" { + t.Fatalf("S27 must add direct case_id provenance to %s", table) + } + } + + for _, table := range []string{ + "magi_agent_run", "evidence_record", "claim", "magi_vote", + "debate_round", "reflection", "magi_tool_call", "resolution", + "magi_agent_checkpoint", + } { + pattern := regexp.MustCompile(`(?i)ALTER TABLE\s+` + table + `[^;]*ADD COLUMN\s+execution_generation\s+BIGINT\s+NOT NULL\s+DEFAULT\s+0`) + if pattern.FindString(sql) == "" { + t.Fatalf("S27 must add execution_generation BIGINT NOT NULL DEFAULT 0 to %s", table) + } + } + if regexp.MustCompile(`(?is)ALTER TABLE\s+magi_agent_checkpoint[^;]*ADD PRIMARY KEY\s*\(\s*run_id\s*,\s*execution_generation\s*\)`).FindString(sql) == "" { + t.Fatal("S27 must make checkpoint primary key (run_id, execution_generation)") + } + + for _, tc := range []struct { + model any + name string + }{ + {&magi.AgentRunModel{}, "AgentRunModel"}, + {&magi.EvidenceModel{}, "EvidenceModel"}, + {&magi.ClaimModel{}, "ClaimModel"}, + {&magi.VoteModel{}, "VoteModel"}, + {&magi.DebateRoundModel{}, "DebateRoundModel"}, + {&magi.ReflectionModel{}, "ReflectionModel"}, + {&magi.ToolCallModel{}, "ToolCallModel"}, + {&magi.ResolutionModel{}, "ResolutionModel"}, + {&magi.CheckpointModel{}, "CheckpointModel"}, + } { + field, ok := reflect.TypeOf(tc.model).Elem().FieldByName("ExecutionGeneration") + if !ok { + t.Fatalf("%s is missing ExecutionGeneration", tc.name) + } + tag := field.Tag.Get("gorm") + if !strings.Contains(tag, "not null") || !strings.Contains(tag, "default:0") { + t.Fatalf("%s.ExecutionGeneration tag = %q, want not null and default:0", tc.name, tag) + } + } + runID, ok := reflect.TypeOf(magi.CheckpointModel{}).FieldByName("RunID") + if !ok || !strings.Contains(runID.Tag.Get("gorm"), "primaryKey") { + t.Fatal("CheckpointModel.RunID must remain part of the primary key") + } + generation, _ := reflect.TypeOf(magi.CheckpointModel{}).FieldByName("ExecutionGeneration") + if !strings.Contains(generation.Tag.Get("gorm"), "primaryKey") { + t.Fatal("CheckpointModel.ExecutionGeneration must be part of the primary key") + } +} + // Issue #9: the approval identity became the logical invocation, pinned by an // intent digest. The Atlas script and the GORM model must agree on the column // widths and on the unique key, so a deployment that applies the SQL by hand diff --git a/backend/bootstrap/mysql_artifact_generation_t3_test.go b/backend/bootstrap/mysql_artifact_generation_t3_test.go new file mode 100644 index 00000000..c6539c61 --- /dev/null +++ b/backend/bootstrap/mysql_artifact_generation_t3_test.go @@ -0,0 +1,143 @@ +package bootstrap + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/google/uuid" + + magi "github.com/jamespud/magi/backend/adapter" + "github.com/jamespud/magi/backend/domain/entity" + "github.com/jamespud/magi/backend/domain/port" +) + +func TestMySQLArtifactGeneration_StaleWriterFencedAndReadsIsolated(t *testing.T) { + db := provideDBForTest(t, newMySQLSchema(t)) + repo, jobs, job := seedMySQLDecisionJob(t, db, "case-t3-artifact-fence", 3) + owned, ok := repo.(port.OwnedArtifactRepository) + if !ok { + t.Fatal("repository missing OwnedArtifactRepository") + } + ctx := context.Background() + worker := "worker-t3" + first, claimed, err := jobs.Claim(ctx, job.ID, worker, uuid.NewString(), time.Now().Add(time.Minute)) + if err != nil || !claimed { + t.Fatalf("claim gen1: job=%+v claimed=%v err=%v", first, claimed, err) + } + owner1 := &entity.ExecutionContext{ + CaseID: job.CaseID, JobID: job.ID, WorkerID: worker, + JobAttempt: first.Attempt, ExecutionGeneration: first.ExecutionGeneration, + } + ev1 := &entity.EvidenceRecord{ID: "ev-g1", CaseID: job.CaseID, ExecutionGeneration: first.ExecutionGeneration} + if err := owned.CreateEvidenceOwned(ctx, owner1, ev1); err != nil { + t.Fatalf("write gen1 evidence: %v", err) + } + + retryAt := time.Now().Add(-time.Second) + if err := jobs.MarkFailed(ctx, job.ID, job.CaseID, worker, first.ExecutionGeneration, "retry", &retryAt); err != nil { + t.Fatalf("requeue gen1: %v", err) + } + second, claimed, err := jobs.Claim(ctx, job.ID, worker, uuid.NewString(), time.Now().Add(time.Minute)) + if err != nil || !claimed || second.ExecutionGeneration != first.ExecutionGeneration+1 { + t.Fatalf("claim gen2: job=%+v claimed=%v err=%v", second, claimed, err) + } + owner2 := &entity.ExecutionContext{ + CaseID: job.CaseID, JobID: job.ID, WorkerID: worker, + JobAttempt: second.Attempt, ExecutionGeneration: second.ExecutionGeneration, + } + + stale := &entity.EvidenceRecord{ID: "ev-late-g1", CaseID: job.CaseID, ExecutionGeneration: first.ExecutionGeneration} + if err := owned.CreateEvidenceOwned(ctx, owner1, stale); !errors.Is(err, port.ErrLeaseLost) { + t.Fatalf("late gen1 write error = %v, want ErrLeaseLost", err) + } + var staleRows int64 + if err := db.Model(&magi.EvidenceModel{}).Where("id = ?", stale.ID).Count(&staleRows).Error; err != nil { + t.Fatalf("count stale evidence: %v", err) + } + if staleRows != 0 { + t.Fatalf("late gen1 evidence rows = %d, want 0", staleRows) + } + + ev2 := &entity.EvidenceRecord{ID: "ev-g2", CaseID: job.CaseID, ExecutionGeneration: second.ExecutionGeneration} + if err := owned.CreateEvidenceOwned(ctx, owner2, ev2); err != nil { + t.Fatalf("write gen2 evidence: %v", err) + } + gen1, err := owned.ListEvidenceByGeneration(ctx, job.CaseID, first.ExecutionGeneration) + if err != nil || len(gen1) != 1 || gen1[0].ID != ev1.ID { + t.Fatalf("gen1 read = %+v err=%v", gen1, err) + } + gen2, err := owned.ListEvidenceByGeneration(ctx, job.CaseID, second.ExecutionGeneration) + if err != nil || len(gen2) != 1 || gen2[0].ID != ev2.ID { + t.Fatalf("gen2 read = %+v err=%v", gen2, err) + } + history, err := repo.EvidenceRepo().ListByCase(ctx, job.CaseID) + if err != nil || len(history) != 2 { + t.Fatalf("history read = %+v err=%v", history, err) + } +} + +func TestMySQLArtifactGeneration_CheckpointDoesNotCrossGeneration(t *testing.T) { + db := provideDBForTest(t, newMySQLSchema(t)) + repo, jobs, job := seedMySQLDecisionJob(t, db, "case-t3-checkpoint-fence", 3) + checkpoints, ok := repo.CheckpointRepo().(port.GenerationCheckpointRepository) + if !ok { + t.Fatal("checkpoint repository missing GenerationCheckpointRepository") + } + ctx := context.Background() + worker := "worker-t3-checkpoint" + first, claimed, err := jobs.Claim(ctx, job.ID, worker, uuid.NewString(), time.Now().Add(time.Minute)) + if err != nil || !claimed { + t.Fatalf("claim gen1: job=%+v claimed=%v err=%v", first, claimed, err) + } + owner1 := &entity.ExecutionContext{ + CaseID: job.CaseID, JobID: job.ID, WorkerID: worker, + JobAttempt: first.Attempt, ExecutionGeneration: first.ExecutionGeneration, + } + const runID = "logical-agent-run" + state1 := &entity.AgentState{ + RunID: runID, CaseID: job.CaseID, ExecutionGeneration: first.ExecutionGeneration, + StepCount: 3, Phase: "gather", + } + if err := checkpoints.SaveForExecution(ctx, owner1, state1); err != nil { + t.Fatalf("save gen1 checkpoint: %v", err) + } + retryAt := time.Now().Add(-time.Second) + if err := jobs.MarkFailed(ctx, job.ID, job.CaseID, worker, first.ExecutionGeneration, "retry", &retryAt); err != nil { + t.Fatalf("requeue gen1: %v", err) + } + second, claimed, err := jobs.Claim(ctx, job.ID, worker, uuid.NewString(), time.Now().Add(time.Minute)) + if err != nil || !claimed { + t.Fatalf("claim gen2: job=%+v claimed=%v err=%v", second, claimed, err) + } + owner2 := &entity.ExecutionContext{ + CaseID: job.CaseID, JobID: job.ID, WorkerID: worker, + JobAttempt: second.Attempt, ExecutionGeneration: second.ExecutionGeneration, + } + if got, err := checkpoints.LoadForExecution(ctx, owner2, runID); err != nil || got != nil { + t.Fatalf("gen2 implicitly loaded gen1 checkpoint: got=%+v err=%v", got, err) + } + state1.StepCount = 4 + if err := checkpoints.SaveForExecution(ctx, owner1, state1); !errors.Is(err, port.ErrLeaseLost) { + t.Fatalf("late gen1 checkpoint save = %v, want ErrLeaseLost", err) + } + state2 := &entity.AgentState{ + RunID: runID, CaseID: job.CaseID, ExecutionGeneration: second.ExecutionGeneration, + StepCount: 1, Phase: "gather", + } + if err := checkpoints.SaveForExecution(ctx, owner2, state2); err != nil { + t.Fatalf("save gen2 checkpoint: %v", err) + } + got2, err := checkpoints.LoadForExecution(ctx, owner2, runID) + if err != nil || got2 == nil || got2.StepCount != 1 || got2.ExecutionGeneration != second.ExecutionGeneration { + t.Fatalf("load gen2 checkpoint = %+v err=%v", got2, err) + } + var rows int64 + if err := db.Model(&magi.CheckpointModel{}).Where("run_id = ?", runID).Count(&rows).Error; err != nil { + t.Fatalf("count checkpoint generations: %v", err) + } + if rows != 2 { + t.Fatalf("checkpoint generations = %d, want 2", rows) + } +} diff --git a/backend/bootstrap/mysql_execution_generation_claim_test.go b/backend/bootstrap/mysql_execution_generation_claim_test.go index 20766342..d4977ab4 100644 --- a/backend/bootstrap/mysql_execution_generation_claim_test.go +++ b/backend/bootstrap/mysql_execution_generation_claim_test.go @@ -245,6 +245,15 @@ func (o *mysqlSuccessOrchestrator) Orchestrate(context.Context, *entity.Decision return &entity.Resolution{FinalDecision: entity.VoteDecisionApprove}, nil } +func (o *mysqlSuccessOrchestrator) OrchestrateForExecution(_ context.Context, c *entity.DecisionCase, owner *entity.ExecutionContext) (*entity.Resolution, error) { + if owner == nil || !owner.IsDurable() || c == nil || owner.CaseID != c.ID || + owner.ExecutionGeneration != c.ExecutionGeneration { + return nil, port.ErrLeaseLost + } + o.calls.Add(1) + return &entity.Resolution{CaseID: c.ID, ExecutionGeneration: c.ExecutionGeneration, FinalDecision: entity.VoteDecisionApprove}, nil +} + func waitMySQLDecisionJobStatus(t *testing.T, jobs port.DecisionJobRepository, caseID string, want entity.DecisionJobStatus) *entity.DecisionJob { t.Helper() deadline := time.Now().Add(3 * time.Second) diff --git a/backend/bootstrap/mysql_migration_s27_test.go b/backend/bootstrap/mysql_migration_s27_test.go new file mode 100644 index 00000000..6b858093 --- /dev/null +++ b/backend/bootstrap/mysql_migration_s27_test.go @@ -0,0 +1,362 @@ +package bootstrap + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/google/uuid" + + magi "github.com/jamespud/magi/backend/adapter" + "github.com/jamespud/magi/backend/domain/entity" + "github.com/jamespud/magi/backend/domain/port" + "gorm.io/gorm" +) + +const preS27ArtifactDDL = ` +CREATE TABLE magi_agent_run ( + id VARCHAR(191) NOT NULL PRIMARY KEY, + case_id VARCHAR(64) NOT NULL +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; +CREATE TABLE evidence_record ( + id VARCHAR(191) NOT NULL PRIMARY KEY, + case_id VARCHAR(64) NOT NULL +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; +CREATE TABLE claim ( + id VARCHAR(191) NOT NULL PRIMARY KEY, + case_id VARCHAR(64) NOT NULL +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; +CREATE TABLE magi_vote ( + id VARCHAR(191) NOT NULL PRIMARY KEY, + case_id VARCHAR(64) NOT NULL +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; +CREATE TABLE debate_round ( + id VARCHAR(191) NOT NULL PRIMARY KEY, + case_id VARCHAR(64) NOT NULL +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; +CREATE TABLE reflection ( + id VARCHAR(191) NOT NULL PRIMARY KEY, + agent_run_id VARCHAR(191) NOT NULL +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; +CREATE TABLE magi_tool_call ( + id VARCHAR(191) NOT NULL PRIMARY KEY, + agent_run_id VARCHAR(191) NOT NULL +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; +CREATE TABLE resolution ( + id VARCHAR(191) NOT NULL PRIMARY KEY, + case_id VARCHAR(64) NOT NULL +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; +CREATE TABLE magi_agent_checkpoint ( + run_id VARCHAR(191) NOT NULL PRIMARY KEY +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; +` + +func seedPreS27Artifacts(t *testing.T, db *gorm.DB) { + t.Helper() + for _, statement := range splitS27SQLStatements(preS27ArtifactDDL) { + if err := db.Exec(statement).Error; err != nil { + t.Fatalf("seed pre-S27 schema: %v\n%s", err, statement) + } + } + if err := db.Exec(`INSERT INTO magi_agent_run (id, case_id) VALUES ('run-legacy', 'case-legacy')`).Error; err != nil { + t.Fatalf("seed legacy agent run: %v", err) + } + if err := db.Exec(`INSERT INTO evidence_record (id, case_id) VALUES ('ev-legacy', 'case-legacy')`).Error; err != nil { + t.Fatalf("seed legacy evidence: %v", err) + } + if err := db.Exec(`INSERT INTO claim (id, case_id) VALUES ('cl-legacy', 'case-legacy')`).Error; err != nil { + t.Fatalf("seed legacy claim: %v", err) + } + if err := db.Exec(`INSERT INTO magi_vote (id, case_id) VALUES ('vote-legacy', 'case-legacy')`).Error; err != nil { + t.Fatalf("seed legacy vote: %v", err) + } + if err := db.Exec(`INSERT INTO debate_round (id, case_id) VALUES ('deb-legacy', 'case-legacy')`).Error; err != nil { + t.Fatalf("seed legacy debate: %v", err) + } + if err := db.Exec(`INSERT INTO reflection (id, agent_run_id) VALUES ('refl-legacy', 'run-legacy')`).Error; err != nil { + t.Fatalf("seed legacy reflection: %v", err) + } + if err := db.Exec(`INSERT INTO magi_tool_call (id, agent_run_id) VALUES ('tool-legacy', 'run-legacy')`).Error; err != nil { + t.Fatalf("seed legacy tool call: %v", err) + } + if err := db.Exec(`INSERT INTO resolution (id, case_id) VALUES ('res-legacy', 'case-legacy')`).Error; err != nil { + t.Fatalf("seed legacy resolution: %v", err) + } + if err := db.Exec(`INSERT INTO magi_agent_checkpoint (run_id) VALUES ('checkpoint-legacy')`).Error; err != nil { + t.Fatalf("seed legacy checkpoint: %v", err) + } + // A legacy checkpoint whose RunID happens to equal a magi_agent_run primary + // key. Pre-S27 checkpoints have no Case column at all, so this coincidence + // must not be read as provenance by the migration. + if err := db.Exec(`INSERT INTO magi_agent_checkpoint (run_id) VALUES ('run-legacy')`).Error; err != nil { + t.Fatalf("seed RunID-colliding legacy checkpoint: %v", err) + } + // Pre-existing orphan history: no magi_agent_run row ever recorded. + if err := db.Exec(`INSERT INTO reflection (id, agent_run_id) VALUES ('refl-orphan', 'run-missing')`).Error; err != nil { + t.Fatalf("seed orphan reflection: %v", err) + } + if err := db.Exec(`INSERT INTO magi_tool_call (id, agent_run_id) VALUES ('tool-orphan', 'run-missing')`).Error; err != nil { + t.Fatalf("seed orphan tool call: %v", err) + } +} + +func splitS27SQLStatements(script string) []string { + var out []string + for _, part := range strings.Split(script, ";") { + part = strings.TrimSpace(part) + if part != "" { + out = append(out, part) + } + } + return out +} + +func assertS27GenerationColumn(t *testing.T, db *gorm.DB, table string) { + t.Helper() + var row struct { + DataType string `gorm:"column:data_type"` + IsNullable string `gorm:"column:is_nullable"` + DefaultVal *string `gorm:"column:column_default"` + } + if err := db.Raw(`SELECT DATA_TYPE AS data_type, IS_NULLABLE AS is_nullable, + COLUMN_DEFAULT AS column_default + FROM information_schema.COLUMNS + WHERE TABLE_SCHEMA = DATABASE() AND TABLE_NAME = ? AND COLUMN_NAME = 'execution_generation'`, table). + Scan(&row).Error; err != nil { + t.Fatalf("read %s.execution_generation: %v", table, err) + } + if row.DataType != "bigint" || row.IsNullable != "NO" || row.DefaultVal == nil || *row.DefaultVal != "0" { + t.Fatalf("%s.execution_generation shape = %+v, want BIGINT NOT NULL DEFAULT 0", table, row) + } +} + +func checkpointPrimaryKeyColumns(t *testing.T, db *gorm.DB) []string { + t.Helper() + var rows []struct { + ColumnName string `gorm:"column:column_name"` + } + if err := db.Raw(`SELECT COLUMN_NAME AS column_name + FROM information_schema.STATISTICS + WHERE TABLE_SCHEMA = DATABASE() AND TABLE_NAME = 'magi_agent_checkpoint' + AND INDEX_NAME = 'PRIMARY' + ORDER BY SEQ_IN_INDEX`).Scan(&rows).Error; err != nil { + t.Fatalf("read checkpoint primary key: %v", err) + } + out := make([]string, len(rows)) + for i := range rows { + out[i] = rows[i].ColumnName + } + return out +} + +func TestMySQLMigration_S27ScopesArtifactsAndCheckpointsByGeneration(t *testing.T) { + dsn := newMySQLSchema(t) + db := openSchema(t, dsn) + seedPreS27Artifacts(t, db) + applyMySQLScript(t, db, s27MigrationPath) + + tables := []string{ + "magi_agent_run", "evidence_record", "claim", "magi_vote", + "debate_round", "reflection", "magi_tool_call", "resolution", + "magi_agent_checkpoint", + } + for _, table := range tables { + assertS27GenerationColumn(t, db, table) + } + + for _, table := range []string{"magi_agent_run", "evidence_record", "claim", "magi_vote", "debate_round", "reflection", "magi_tool_call", "resolution", "magi_agent_checkpoint"} { + var positives int64 + if err := db.Raw("SELECT COUNT(*) FROM " + table + " WHERE execution_generation <> 0").Scan(&positives).Error; err != nil { + t.Fatalf("count legacy generations in %s: %v", table, err) + } + if positives != 0 { + t.Fatalf("%s contains %d legacy rows with fabricated positive generation", table, positives) + } + } + + // Relational backfill: the linked legacy row takes the Case of its AgentRun. + for _, tc := range []struct{ table, id string }{ + {"reflection", "refl-legacy"}, + {"magi_tool_call", "tool-legacy"}, + } { + var caseID string + if err := db.Raw("SELECT case_id FROM "+tc.table+" WHERE id = ?", tc.id).Scan(&caseID).Error; err != nil { + t.Fatalf("read %s.case_id: %v", tc.table, err) + } + if caseID != "case-legacy" { + t.Fatalf("%s %s case_id = %q, want relational backfill case-legacy", tc.table, tc.id, caseID) + } + } + + // Broken relationships and checkpoints keep unknown provenance rather than + // failing the upgrade or being attributed to a Case by guesswork. + for _, tc := range []struct{ table, id string }{ + {"reflection", "refl-orphan"}, + {"magi_tool_call", "tool-orphan"}, + } { + var caseID string + if err := db.Raw("SELECT case_id FROM "+tc.table+" WHERE id = ?", tc.id).Scan(&caseID).Error; err != nil { + t.Fatalf("read orphan %s.case_id: %v", tc.table, err) + } + if caseID != "" { + t.Fatalf("orphan %s %s case_id = %q, want unknown provenance", tc.table, tc.id, caseID) + } + } + + // Every legacy checkpoint keeps case_id empty, including the one whose RunID + // collides with a magi_agent_run primary key: the pre-S27 table recorded no + // Case, and RunID is a logical identity, not a reference. + var checkpointCases []struct { + RunID string `gorm:"column:run_id"` + CaseID string `gorm:"column:case_id"` + } + if err := db.Raw("SELECT run_id, case_id FROM magi_agent_checkpoint ORDER BY run_id").Scan(&checkpointCases).Error; err != nil { + t.Fatalf("read checkpoint provenance: %v", err) + } + if len(checkpointCases) != 2 { + t.Fatalf("legacy checkpoints = %d, want 2", len(checkpointCases)) + } + for _, row := range checkpointCases { + if row.CaseID != "" { + t.Fatalf("legacy checkpoint %s case_id = %q, want unknown provenance", row.RunID, row.CaseID) + } + } + + pk := checkpointPrimaryKeyColumns(t, db) + if len(pk) != 2 || pk[0] != "run_id" || pk[1] != "execution_generation" { + t.Fatalf("checkpoint primary key = %v, want [run_id execution_generation]", pk) + } + + // Startup AutoMigrate over the upgraded schema must preserve the same + // generation/key contract. + upgraded := provideDBForTest(t, dsn) + for _, table := range tables { + assertS27GenerationColumn(t, upgraded, table) + } + pk = checkpointPrimaryKeyColumns(t, upgraded) + if len(pk) != 2 || pk[0] != "run_id" || pk[1] != "execution_generation" { + t.Fatalf("upgraded AutoMigrate checkpoint primary key = %v", pk) + } + + // A fresh schema created only by AutoMigrate reaches the same contract. + fresh := provideDBForTest(t, newMySQLSchema(t)) + for _, table := range tables { + assertS27GenerationColumn(t, fresh, table) + } + pk = checkpointPrimaryKeyColumns(t, fresh) + if len(pk) != 2 || pk[0] != "run_id" || pk[1] != "execution_generation" { + t.Fatalf("fresh checkpoint primary key = %v", pk) + } + + if !fresh.Migrator().HasColumn(&magi.ReflectionModel{}, "case_id") || + !fresh.Migrator().HasColumn(&magi.ToolCallModel{}, "case_id") { + t.Fatal("fresh schema must persist direct case provenance for reflection/tool calls") + } +} + +// T3 / S27 legacy checkpoints carry no recorded Case provenance: the pre-S27 +// table stored only run_id and snapshot state, and run_id is the logical +// working-memory identity rather than a declared reference to magi_agent_run. +// This pins the downstream contract on real MySQL: the generation-scoped loader +// never serves a legacy row, Delete removes the rows it can attribute (direct +// case_id, or the pre-S27 run_id containment rule), and a legacy row that +// matches neither is retained as unknown-provenance history instead of being +// guessed into a Case. +func TestMySQLMigration_S27LegacyCheckpointRetention(t *testing.T) { + db := provideDBForTest(t, newMySQLSchema(t)) + const caseID = "case-s27-retention" + repo, jobs, job := seedMySQLDecisionJob(t, db, caseID, 3) + checkpoints, ok := repo.CheckpointRepo().(port.GenerationCheckpointRepository) + if !ok { + t.Fatal("checkpoint repository missing GenerationCheckpointRepository") + } + ctx := context.Background() + worker := "worker-s27-retention" + claimed, ok, err := jobs.Claim(ctx, job.ID, worker, uuid.NewString(), time.Now().Add(time.Minute)) + if err != nil || !ok || claimed == nil { + t.Fatalf("claim: job=%+v ok=%v err=%v", claimed, ok, err) + } + owner := &entity.ExecutionContext{ + CaseID: claimed.CaseID, JobID: claimed.ID, WorkerID: worker, + JobAttempt: claimed.Attempt, ExecutionGeneration: claimed.ExecutionGeneration, + } + + // Two upgraded pre-S27 rows: one keeps the purely logical identity (matches + // nothing), the other equals a magi_agent_run primary key of this Case and + // is therefore inside the pre-S27 deletion containment rule. + logicalRunID := caseID + "-melchior-r1-investigate" + matchedRunID := "run-s27-matched" + if err := db.Create(&magi.AgentRunModel{ + ID: matchedRunID, CaseID: caseID, ExecutionGeneration: owner.ExecutionGeneration, + StartedAt: time.Now(), + }).Error; err != nil { + t.Fatalf("seed matched agent run: %v", err) + } + for _, runID := range []string{logicalRunID, matchedRunID} { + if err := db.Create(&magi.CheckpointModel{RunID: runID, ExecutionGeneration: 0, CaseID: ""}).Error; err != nil { + t.Fatalf("seed legacy checkpoint %s: %v", runID, err) + } + } + positiveRunID := caseID + "-casper-g1-r1-investigate" + if err := checkpoints.SaveForExecution(ctx, owner, &entity.AgentState{ + RunID: positiveRunID, CaseID: caseID, + ExecutionGeneration: owner.ExecutionGeneration, StepCount: 1, Phase: "gather", + }); err != nil { + t.Fatalf("save generation-scoped checkpoint: %v", err) + } + + // The current API is generation-scoped and must not answer a legacy row, + // even while the Case and its owner are alive. + for _, runID := range []string{logicalRunID, matchedRunID} { + if got, err := checkpoints.LoadForExecution(ctx, owner, runID); err != nil || got != nil { + t.Fatalf("LoadForExecution(%s) = %+v err=%v, want empty", runID, got, err) + } + legacy, err := repo.CheckpointRepo().Load(ctx, runID) + if err != nil || legacy == nil { + t.Fatalf("legacy Load(%s) = %+v err=%v, want the generation-0 row", runID, legacy, err) + } + if legacy.CaseID != "" || legacy.ExecutionGeneration != 0 { + t.Fatalf("legacy checkpoint %s provenance = case %q generation %d, want unknown", + runID, legacy.CaseID, legacy.ExecutionGeneration) + } + } + + if err := repo.CaseRepo().Delete(ctx, caseID); err != nil { + t.Fatalf("delete case: %v", err) + } + + var positiveRows int64 + if err := db.Model(&magi.CheckpointModel{}).Where("run_id = ?", positiveRunID).Count(&positiveRows).Error; err != nil { + t.Fatalf("count generation-scoped checkpoint: %v", err) + } + if positiveRows != 0 { + t.Fatalf("generation-scoped checkpoint rows after delete = %d, want 0", positiveRows) + } + var matchedRows int64 + if err := db.Model(&magi.CheckpointModel{}).Where("run_id = ?", matchedRunID).Count(&matchedRows).Error; err != nil { + t.Fatalf("count matched legacy checkpoint: %v", err) + } + if matchedRows != 0 { + t.Fatalf("checkpoint matching a Case AgentRun survived the delete: %d", matchedRows) + } + var retained struct { + CaseID string `gorm:"column:case_id"` + Generation int64 `gorm:"column:execution_generation"` + } + if err := db.Raw("SELECT case_id, execution_generation FROM magi_agent_checkpoint WHERE run_id = ?", logicalRunID). + Scan(&retained).Error; err != nil { + t.Fatalf("read retained legacy checkpoint: %v", err) + } + if retained.CaseID != "" || retained.Generation != 0 { + t.Fatalf("retained legacy checkpoint provenance = case %q generation %d, want unknown", + retained.CaseID, retained.Generation) + } + var runs int64 + if err := db.Model(&magi.AgentRunModel{}).Where("case_id = ?", caseID).Count(&runs).Error; err != nil { + t.Fatalf("count agent runs: %v", err) + } + if runs != 0 { + t.Fatalf("agent runs after delete = %d, want 0", runs) + } +} diff --git a/backend/domain/entity/agent_run.go b/backend/domain/entity/agent_run.go index 81fc3561..4025923a 100644 --- a/backend/domain/entity/agent_run.go +++ b/backend/domain/entity/agent_run.go @@ -4,17 +4,18 @@ import "time" // AgentRun is one Magi's run within a case (one round). type AgentRun struct { - ID string - CaseID string - MagiConfigID string - MagiCode MagiCode - Round int - Status AgentRunStatus - StartedAt time.Time - CompletedAt *time.Time - Usage *Usage - Err string - Environment *RunEnvironment + ID string + CaseID string + ExecutionGeneration int64 + MagiConfigID string + MagiCode MagiCode + Round int + Status AgentRunStatus + StartedAt time.Time + CompletedAt *time.Time + Usage *Usage + Err string + Environment *RunEnvironment } // RunEnvironment snapshots the configuration a Magi run executed under, so @@ -53,12 +54,14 @@ const ( // AgentState is a working-memory snapshot for checkpoint/resume. type AgentState struct { - RunID string - Messages []MessageRef - MessagesJSON string // full []*schema.Message JSON for non-lossy resume - StepCount int - TokenUsed int - Phase string + RunID string + CaseID string + ExecutionGeneration int64 + Messages []MessageRef + MessagesJSON string // full []*schema.Message JSON for non-lossy resume + StepCount int + TokenUsed int + Phase string // Snapshot V2 is the authoritative checkpoint payload. The legacy fields // remain available while existing checkpoint rows are migrated. diff --git a/backend/domain/entity/claim.go b/backend/domain/entity/claim.go index 3807d9e6..84a5afbd 100644 --- a/backend/domain/entity/claim.go +++ b/backend/domain/entity/claim.go @@ -5,15 +5,16 @@ import "time" // Claim is an agent's interpretation about the world, grounded in evidence // (ADR-005). Supports reference EV-IDs; Contradicts reference other Claim-IDs. type Claim struct { - ID string - CaseID string - AgentRunID string - Statement string - Supports []string // EV-IDs - Contradicts []string // Claim-IDs - Status ClaimStatus - CreatedBy MagiCode - CreatedAt time.Time + ID string + CaseID string + ExecutionGeneration int64 + AgentRunID string + Statement string + Supports []string // EV-IDs + Contradicts []string // Claim-IDs + Status ClaimStatus + CreatedBy MagiCode + CreatedAt time.Time } type ClaimStatus string diff --git a/backend/domain/entity/debate.go b/backend/domain/entity/debate.go index 2ce0e77f..ca7a8429 100644 --- a/backend/domain/entity/debate.go +++ b/backend/domain/entity/debate.go @@ -4,12 +4,13 @@ import "time" // DebateRound is one round of structured debate over conflicting claims. type DebateRound struct { - ID string - CaseID string - Round int - Packet DebatePacket - StartedAt time.Time - CompletedAt *time.Time + ID string + CaseID string + ExecutionGeneration int64 + Round int + Packet DebatePacket + StartedAt time.Time + CompletedAt *time.Time } // DebatePacket is sent to all three Magi (minority AND majority). diff --git a/backend/domain/entity/evidence.go b/backend/domain/entity/evidence.go index 923a6abe..8430c55e 100644 --- a/backend/domain/entity/evidence.go +++ b/backend/domain/entity/evidence.go @@ -5,18 +5,19 @@ import "time" // EvidenceRecord is an observation collected by a tool (ADR-005). It records // what the tool returned, NOT the agent's interpretation (which lives in Claim). type EvidenceRecord struct { - ID string - CaseID string - AgentRunID string - ToolCallID string - ToolName string - SourceType EvidenceSourceType - SourceURI *string - RawContent string - Observation string - Reliability ReliabilityScore - CollectedBy MagiCode - CreatedAt time.Time + ID string + CaseID string + ExecutionGeneration int64 + AgentRunID string + ToolCallID string + ToolName string + SourceType EvidenceSourceType + SourceURI *string + RawContent string + Observation string + Reliability ReliabilityScore + CollectedBy MagiCode + CreatedAt time.Time } type EvidenceSourceType string diff --git a/backend/domain/entity/execution.go b/backend/domain/entity/execution.go index 83d1d2a1..8b78acf0 100644 --- a/backend/domain/entity/execution.go +++ b/backend/domain/entity/execution.go @@ -13,6 +13,22 @@ type ExecutionIdentity struct { AttemptID string } +// ExecutionContext is the explicit durable owner identity carried by one +// execution. JobAttempt and RunID are diagnostic/runtime identities; fencing +// uses CaseID + JobID + WorkerID + ExecutionGeneration and the durable lease. +type ExecutionContext struct { + CaseID string + JobID string + RunID string + WorkerID string + JobAttempt int + ExecutionGeneration int64 +} + +func (e *ExecutionContext) IsDurable() bool { + return e != nil && e.CaseID != "" && e.JobID != "" && e.WorkerID != "" && e.ExecutionGeneration > 0 +} + type InvocationStatus string const ( diff --git a/backend/domain/entity/idmap.go b/backend/domain/entity/idmap.go index 8f8fdba2..65c6e2a2 100644 --- a/backend/domain/entity/idmap.go +++ b/backend/domain/entity/idmap.go @@ -110,7 +110,7 @@ func (m *ArtifactRemap) lookup(id string) string { } // CodeOfRun extracts the MagiCode from a persisted agent run ID. Run IDs are -// deterministic: "-[-a]-r-". +// deterministic: "-[-g|-a]-r-". func CodeOfRun(runID string) MagiCode { if runID == "" { return "" @@ -124,7 +124,7 @@ func CodeOfRun(runID string) MagiCode { if strings.HasPrefix(p, "r") && len(p) > 1 && isDigits(p[1:]) { // Skip the trailing phase segment if present. codeIdx := i - 1 - if codeIdx-1 >= 0 && strings.HasPrefix(parts[codeIdx], "a") && isDigits(parts[codeIdx][1:]) { + if codeIdx-1 >= 0 && isExecutionMarker(parts[codeIdx]) { codeIdx-- } if codeIdx >= 0 { @@ -135,6 +135,13 @@ func CodeOfRun(runID string) MagiCode { return "" } +func isExecutionMarker(s string) bool { + if len(s) < 2 || (s[0] != 'a' && s[0] != 'g') { + return false + } + return isDigits(s[1:]) +} + func isDigits(s string) bool { if s == "" { return false diff --git a/backend/domain/entity/idmap_test.go b/backend/domain/entity/idmap_test.go index 4ebe413f..190c6d9e 100644 --- a/backend/domain/entity/idmap_test.go +++ b/backend/domain/entity/idmap_test.go @@ -40,6 +40,7 @@ func TestCodeOfRun(t *testing.T) { }{ {"case-abc-melchior-r1-investigate", MagiCodeMelchior}, {"case-abc-balthasar-a2-r1-reconsider", MagiCodeBalthasar}, + {"case-abc-balthasar-g7-r1-reconsider", MagiCodeBalthasar}, {"case-abc-casper-r2-investigate", MagiCodeCasper}, {"", ""}, {"case-abc", ""}, diff --git a/backend/domain/entity/reflection.go b/backend/domain/entity/reflection.go index 9b73df78..70a37691 100644 --- a/backend/domain/entity/reflection.go +++ b/backend/domain/entity/reflection.go @@ -5,6 +5,8 @@ import "time" // Reflection is a Magi's reconsideration output after a debate round. type Reflection struct { ID string `json:"id,omitempty"` + CaseID string `json:"case_id,omitempty"` + ExecutionGeneration int64 `json:"execution_generation,omitempty"` AgentRunID string `json:"agent_run_id,omitempty"` Round int `json:"round,omitempty"` PreviousVoteID string `json:"previous_vote_id,omitempty"` diff --git a/backend/domain/entity/resolution.go b/backend/domain/entity/resolution.go index 1e48f338..f6a1d1a2 100644 --- a/backend/domain/entity/resolution.go +++ b/backend/domain/entity/resolution.go @@ -4,16 +4,17 @@ import "time" // Resolution is the final outcome of a DecisionCase. type Resolution struct { - ID string - CaseID string - Consensus ConsensusResult - FinalDecision VoteDecision - FinalReport string - KeyEvidenceIDs []string - KeyClaimIDs []string - VoteIDs []string - Evaluation *Evaluation - CreatedAt time.Time + ID string + CaseID string + ExecutionGeneration int64 + Consensus ConsensusResult + FinalDecision VoteDecision + FinalReport string + KeyEvidenceIDs []string + KeyClaimIDs []string + VoteIDs []string + Evaluation *Evaluation + CreatedAt time.Time } // Dissent is a structured minority position recorded for auditability. diff --git a/backend/domain/entity/tool_call.go b/backend/domain/entity/tool_call.go index 919a42bd..a3606a39 100644 --- a/backend/domain/entity/tool_call.go +++ b/backend/domain/entity/tool_call.go @@ -5,17 +5,18 @@ import "time" // ToolCall is one tool invocation by an agent during a run (persisted to // magi_tool_call, S8). Mirrors runtime.ToolCallRecord plus case/run linkage. type ToolCall struct { - ID string - CaseID string - AgentRunID string - ToolCallID string // the LLM-assigned tool-call id - ToolName string - Arguments string - Valid bool - Result string - Err string - ApprovedBy string - EvidenceID string // namespaced persisted EV-ID this call produced (may be empty) - DurationMs int64 - CreatedAt time.Time + ID string + CaseID string + ExecutionGeneration int64 + AgentRunID string + ToolCallID string // the LLM-assigned tool-call id + ToolName string + Arguments string + Valid bool + Result string + Err string + ApprovedBy string + EvidenceID string // namespaced persisted EV-ID this call produced (may be empty) + DurationMs int64 + CreatedAt time.Time } diff --git a/backend/domain/entity/vote.go b/backend/domain/entity/vote.go index 84a702ad..82bb403b 100644 --- a/backend/domain/entity/vote.go +++ b/backend/domain/entity/vote.go @@ -32,18 +32,19 @@ type ClaimSubmission struct { // Vote is a Magi's structured final decision for a round. type Vote struct { - ID string `json:"id,omitempty"` - CaseID string `json:"case_id,omitempty"` - AgentRunID string `json:"agent_run_id,omitempty"` - Round int `json:"round,omitempty"` - Decision VoteDecision `json:"decision"` - Confidence float64 `json:"confidence"` - UtilityScores []UtilityDimensionScore `json:"utility_scores"` - KeyClaimIDs []string `json:"key_claim_ids,omitempty"` - EvidenceIDs []string `json:"evidence_ids"` - ReasoningSummary string `json:"reasoning_summary,omitempty"` - Conditions []DecisionCondition `json:"conditions,omitempty"` - CreatedAt time.Time `json:"created_at,omitempty"` + ID string `json:"id,omitempty"` + CaseID string `json:"case_id,omitempty"` + ExecutionGeneration int64 `json:"execution_generation,omitempty"` + AgentRunID string `json:"agent_run_id,omitempty"` + Round int `json:"round,omitempty"` + Decision VoteDecision `json:"decision"` + Confidence float64 `json:"confidence"` + UtilityScores []UtilityDimensionScore `json:"utility_scores"` + KeyClaimIDs []string `json:"key_claim_ids,omitempty"` + EvidenceIDs []string `json:"evidence_ids"` + ReasoningSummary string `json:"reasoning_summary,omitempty"` + Conditions []DecisionCondition `json:"conditions,omitempty"` + CreatedAt time.Time `json:"created_at,omitempty"` } // NormalizeConfidence coerces a model-provided confidence value to a 0-100 diff --git a/backend/domain/orchestration/checkpoint_runid_test.go b/backend/domain/orchestration/checkpoint_runid_test.go index 9b7b05a7..8160cb37 100644 --- a/backend/domain/orchestration/checkpoint_runid_test.go +++ b/backend/domain/orchestration/checkpoint_runid_test.go @@ -8,11 +8,11 @@ import ( "github.com/jamespud/magi/backend/domain/orchestration" ) -func TestDispatch_CheckpointRunIDIgnoresExecutionAttempt(t *testing.T) { +func TestDispatch_CheckpointRunIDRemainsLogicalAcrossExecutionGenerations(t *testing.T) { cr := &captureRuntime{} d := orchestration.NewDispatcher(cr, nil) cfg := &entity.MagiConfig{Code: "melchior"} - case_ := &entity.DecisionCase{ID: "c1", ExecutionAttempt: 2} + case_ := &entity.DecisionCase{ID: "c1", ExecutionAttempt: 2, ExecutionGeneration: 7} d.Dispatch(context.Background(), case_, nil, []*entity.MagiConfig{cfg}, 1) if cr.actx == nil { t.Fatal("no actx captured") @@ -21,4 +21,13 @@ func TestDispatch_CheckpointRunIDIgnoresExecutionAttempt(t *testing.T) { if cr.actx.RunID != want { t.Fatalf("RunID=%q want=%q", cr.actx.RunID, want) } + + cr2 := &captureRuntime{} + d2 := orchestration.NewDispatcher(cr2, nil) + case_.ExecutionAttempt = 99 + case_.ExecutionGeneration = 8 + d2.Dispatch(context.Background(), case_, nil, []*entity.MagiConfig{cfg}, 1) + if cr2.actx == nil || cr2.actx.RunID != want { + t.Fatalf("next generation RunID=%q want logical RunID %q", cr2.actx.RunID, want) + } } diff --git a/backend/domain/orchestration/dispatcher.go b/backend/domain/orchestration/dispatcher.go index 9b41b32c..aa085ad0 100644 --- a/backend/domain/orchestration/dispatcher.go +++ b/backend/domain/orchestration/dispatcher.go @@ -84,9 +84,21 @@ func (d *Dispatcher) Dispatch( task *entity.DecisionTask, configs []*entity.MagiConfig, round int, +) []*runtime.LoopResult { + return d.DispatchForExecution(ctx, case_, task, configs, round, nil) +} + +func (d *Dispatcher) DispatchForExecution( + ctx context.Context, + case_ *entity.DecisionCase, + task *entity.DecisionTask, + configs []*entity.MagiConfig, + round int, + execution *entity.ExecutionContext, ) []*runtime.LoopResult { results := make([]*runtime.LoopResult, len(configs)) base := d.buildBase(ctx, case_, task) + base.Execution = execution var wg sync.WaitGroup wg.Add(len(configs)) for i, cfg := range configs { @@ -118,9 +130,23 @@ func (d *Dispatcher) DispatchReconsider( prevResults []*runtime.LoopResult, configs []*entity.MagiConfig, round int, +) []*runtime.LoopResult { + return d.DispatchReconsiderForExecution(ctx, case_, task, packet, prevResults, configs, round, nil) +} + +func (d *Dispatcher) DispatchReconsiderForExecution( + ctx context.Context, + case_ *entity.DecisionCase, + task *entity.DecisionTask, + packet entity.DebatePacket, + prevResults []*runtime.LoopResult, + configs []*entity.MagiConfig, + round int, + execution *entity.ExecutionContext, ) []*runtime.LoopResult { results := make([]*runtime.LoopResult, len(configs)) base := d.buildBase(ctx, case_, task) + base.Execution = execution var wg sync.WaitGroup wg.Add(len(configs)) for i, cfg := range configs { @@ -155,8 +181,21 @@ func (d *Dispatcher) RetryAgent( cfg *entity.MagiConfig, round, attempt int, phase string, +) *runtime.LoopResult { + return d.RetryAgentForExecution(ctx, case_, task, cfg, round, attempt, phase, nil) +} + +func (d *Dispatcher) RetryAgentForExecution( + ctx context.Context, + case_ *entity.DecisionCase, + task *entity.DecisionTask, + cfg *entity.MagiConfig, + round, attempt int, + phase string, + execution *entity.ExecutionContext, ) *runtime.LoopResult { base := d.buildBase(ctx, case_, task) + base.Execution = execution actx := *base actx.RunID = checkpointRunID(case_.ID, entity.MagiCode(cfg.Code), round, phase) + fmt.Sprintf("-retry%d", attempt) r, _ := d.agentLoop.Run(ctx, cfg, &actx) @@ -172,19 +211,15 @@ func isCompleted(r *runtime.LoopResult) bool { return r != nil && r.Status == runtime.LoopStatusCompleted && r.Err == nil && r.Vote != nil } -// checkpointRunID is the stable identity of an agent's working memory for a -// (case, agent, round, phase). Unlike executionRunID it deliberately excludes -// the execution attempt so durable retries resume the same checkpoint. +// checkpointRunID is the stable logical identity of an agent's working memory +// for a (case, agent, round, phase). Durable storage scopes that logical ID by +// execution_generation, so a newer owner never resumes an older generation. func checkpointRunID(caseID string, code entity.MagiCode, round int, phase string) string { return fmt.Sprintf("%s-%s-r%d-%s", caseID, code, round, phase) } -func executionRunID(caseID string, code entity.MagiCode, attempt, round int, phase string) string { - attemptPart := "" - if attempt > 0 { - attemptPart = fmt.Sprintf("-a%d", attempt) - } - return fmt.Sprintf("%s-%s%s-r%d-%s", caseID, code, attemptPart, round, phase) +func executionRunID(caseID string, code entity.MagiCode, generation int64, round int, phase string) string { + return fmt.Sprintf("%s-%s-g%d-r%d-%s", caseID, code, generation, round, phase) } func derefTask(t *entity.DecisionTask) entity.DecisionTask { diff --git a/backend/domain/orchestration/durable_artifact_failure_test.go b/backend/domain/orchestration/durable_artifact_failure_test.go new file mode 100644 index 00000000..89c84d2a --- /dev/null +++ b/backend/domain/orchestration/durable_artifact_failure_test.go @@ -0,0 +1,226 @@ +package orchestration_test + +import ( + "context" + "errors" + "testing" + + "github.com/jamespud/magi/backend/application/metrics" + "github.com/jamespud/magi/backend/domain/consensus" + "github.com/jamespud/magi/backend/domain/debate" + "github.com/jamespud/magi/backend/domain/entity" + "github.com/jamespud/magi/backend/domain/orchestration" + "github.com/jamespud/magi/backend/domain/port" +) + +// durableArtifactRepo upgrades the in-memory stubRepo with the capability T3 +// requires of a durable worker: generation-scoped authoritative writes behind +// the repository's active-owner predicate. One artifact kind can be made to +// return a non-lease storage error, modelling the failures that used to be +// logged and swallowed (deadlock, lock-wait timeout, INSERT error, an +// unknown commit outcome). +type durableArtifactRepo struct { + *stubRepo + failKind string + failErr error + writes []string +} + +func (r *durableArtifactRepo) recordOwned(kind string) error { + r.mu.Lock() + defer r.mu.Unlock() + r.writes = append(r.writes, kind) + if kind == r.failKind && r.failErr != nil { + return r.failErr + } + return nil +} + +func (r *durableArtifactRepo) CreateAgentRunOwned(_ context.Context, _ *entity.ExecutionContext, a *entity.AgentRun) error { + if err := r.recordOwned("agent_run"); err != nil { + return err + } + r.mu.Lock() + defer r.mu.Unlock() + r.agentRuns = append(r.agentRuns, a) + return nil +} + +func (r *durableArtifactRepo) CreateEvidenceOwned(_ context.Context, _ *entity.ExecutionContext, e *entity.EvidenceRecord) error { + if err := r.recordOwned("evidence"); err != nil { + return err + } + r.mu.Lock() + defer r.mu.Unlock() + r.evidence = append(r.evidence, e) + return nil +} + +func (r *durableArtifactRepo) CreateClaimOwned(_ context.Context, _ *entity.ExecutionContext, c *entity.Claim) error { + if err := r.recordOwned("claim"); err != nil { + return err + } + r.mu.Lock() + defer r.mu.Unlock() + r.claims = append(r.claims, c) + return nil +} + +func (r *durableArtifactRepo) CreateVoteOwned(_ context.Context, _ *entity.ExecutionContext, v *entity.Vote) error { + if err := r.recordOwned("vote"); err != nil { + return err + } + r.mu.Lock() + defer r.mu.Unlock() + r.votes = append(r.votes, v) + return nil +} + +func (r *durableArtifactRepo) CreateDebateRoundOwned(_ context.Context, _ *entity.ExecutionContext, _ *entity.DebateRound) error { + return r.recordOwned("debate_round") +} + +func (r *durableArtifactRepo) CreateReflectionOwned(_ context.Context, _ *entity.ExecutionContext, _ *entity.Reflection) error { + return r.recordOwned("reflection") +} + +func (r *durableArtifactRepo) CreateToolCallOwned(_ context.Context, _ *entity.ExecutionContext, tc *entity.ToolCall) error { + if err := r.recordOwned("tool_call"); err != nil { + return err + } + r.mu.Lock() + defer r.mu.Unlock() + r.toolCalls = append(r.toolCalls, tc) + return nil +} + +func (r *durableArtifactRepo) ListAgentRunsByGeneration(context.Context, string, int64) ([]*entity.AgentRun, error) { + return nil, nil +} +func (r *durableArtifactRepo) ListEvidenceByGeneration(context.Context, string, int64) ([]*entity.EvidenceRecord, error) { + return nil, nil +} +func (r *durableArtifactRepo) ListClaimsByGeneration(context.Context, string, int64) ([]*entity.Claim, error) { + return nil, nil +} +func (r *durableArtifactRepo) ListVotesByGeneration(context.Context, string, int64) ([]*entity.Vote, error) { + return nil, nil +} +func (r *durableArtifactRepo) ListDebateRoundsByGeneration(context.Context, string, int64) ([]*entity.DebateRound, error) { + return nil, nil +} +func (r *durableArtifactRepo) ListReflectionsByGeneration(context.Context, string, int64) ([]*entity.Reflection, error) { + return nil, nil +} +func (r *durableArtifactRepo) ListToolCallsByGeneration(context.Context, string, int64) ([]*entity.ToolCall, error) { + return nil, nil +} + +var _ port.OwnedArtifactRepository = (*durableArtifactRepo)(nil) + +// P1: on the positive-generation durable path a repository/storage error is not +// a tolerated metric, it aborts the execution. Before this fix every tier below +// the vote/evidence/claim reference fence (AgentRun, ToolCall, Reflection, +// DebateRound) could fail and the case still reported success. +func TestOrchestrateForExecution_DurableArtifactFailureAbortsExecution(t *testing.T) { + storageErr := errors.New("mysql: transient storage failure") + for _, kind := range []string{"agent_run", "evidence", "claim", "tool_call", "vote"} { + t.Run(kind, func(t *testing.T) { + repo := &durableArtifactRepo{stubRepo: newStubRepo(), failKind: kind, failErr: storageErr} + reg := metrics.New() + mrt := newMockMagiRuntime() + mrt.votes["melchior"] = []*entity.Vote{approve(), approve()} + mrt.votes["balthasar"] = []*entity.Vote{approve(), approve()} + mrt.votes["casper"] = []*entity.Vote{approve(), approve()} + + orch := orchestration.NewOrchestrator(orchestration.OrchestratorDeps{ + Repo: repo, + CaseRepo: repo.CaseRepo(), + AgentLoop: mrt, + Consensus: consensus.NewConsensusEngine(), + Debate: debate.NewDebateEngine(nil), + Commander: newCommander(t), + Configs: []*entity.MagiConfig{ + magiCfg("melchior"), magiCfg("balthasar"), magiCfg("casper"), + }, + Policy: consensus.DefaultConsensusPolicy(), + Metrics: reg, + }) + + case_ := &entity.DecisionCase{ + ID: "c-durable-fail", Question: "compute", MaxDebateRounds: 1, + ExecutionGeneration: 5, ExecutionAttempt: 2, + } + owner := &entity.ExecutionContext{ + CaseID: case_.ID, JobID: "job-1", WorkerID: "worker-1", + JobAttempt: 2, ExecutionGeneration: 5, + } + + res, err := orch.OrchestrateForExecution(context.Background(), case_, owner) + if err == nil { + t.Fatalf("durable %s write failure was swallowed; resolution=%+v", kind, res) + } + if errors.Is(err, port.ErrLeaseLost) { + t.Fatalf("storage error was misreported as lease loss: %v", err) + } + if !errors.Is(err, storageErr) { + t.Fatalf("error = %v, want the injected storage error to survive", err) + } + if got := reg.ArtifactPersistFailures(metrics.ArtifactKind(kind)); got == 0 { + t.Fatalf("artifact persist failure for %s was not counted", kind) + } + // The execution must not advertise a successful terminal outcome. + switch repo.statuses[case_.ID] { + case entity.CaseStatusResolved, entity.CaseStatusMemoryIndexed, + entity.CaseStatusInsufficientEv, entity.CaseStatusDeadlocked: + t.Fatalf("failed durable execution reached success terminal %s", repo.statuses[case_.ID]) + } + repo.mu.Lock() + defer repo.mu.Unlock() + if len(repo.resolutions) != 0 { + t.Fatalf("failed durable execution persisted %d resolutions", len(repo.resolutions)) + } + }) + } +} + +// A durable execution whose authoritative write fails must stop at the failing +// artifact, not keep writing later artifacts that would then look like a +// complete generation. +func TestOrchestrateForExecution_DurableArtifactFailureStopsGeneration(t *testing.T) { + storageErr := errors.New("mysql: insert into magi_agent_run failed") + repo := &durableArtifactRepo{stubRepo: newStubRepo(), failKind: "agent_run", failErr: storageErr} + mrt := newMockMagiRuntime() + mrt.votes["melchior"] = []*entity.Vote{approve(), approve()} + mrt.votes["balthasar"] = []*entity.Vote{approve(), approve()} + mrt.votes["casper"] = []*entity.Vote{approve(), approve()} + + orch := orchestration.NewOrchestrator(orchestration.OrchestratorDeps{ + Repo: repo, CaseRepo: repo.CaseRepo(), AgentLoop: mrt, + Consensus: consensus.NewConsensusEngine(), Debate: debate.NewDebateEngine(nil), + Commander: newCommander(t), + Configs: []*entity.MagiConfig{magiCfg("melchior"), magiCfg("balthasar"), magiCfg("casper")}, + Policy: consensus.DefaultConsensusPolicy(), + Metrics: metrics.New(), + }) + case_ := &entity.DecisionCase{ + ID: "c-durable-stop", Question: "compute", MaxDebateRounds: 1, + ExecutionGeneration: 3, ExecutionAttempt: 1, + } + owner := &entity.ExecutionContext{ + CaseID: case_.ID, JobID: "job-1", WorkerID: "worker-1", ExecutionGeneration: 3, + } + + if _, err := orch.OrchestrateForExecution(context.Background(), case_, owner); !errors.Is(err, storageErr) { + t.Fatalf("error = %v, want the injected agent-run storage error", err) + } + repo.mu.Lock() + defer repo.mu.Unlock() + if len(repo.writes) != 1 || repo.writes[0] != "agent_run" { + t.Fatalf("authoritative writes after the failure = %v, want exactly one agent_run attempt", repo.writes) + } + if len(repo.votes) != 0 || len(repo.evidence) != 0 || len(repo.claims) != 0 { + t.Fatalf("generation continued past the failed write: votes=%d evidence=%d claims=%d", + len(repo.votes), len(repo.evidence), len(repo.claims)) + } +} diff --git a/backend/domain/orchestration/execution_generation_t3_test.go b/backend/domain/orchestration/execution_generation_t3_test.go new file mode 100644 index 00000000..2127e7c9 --- /dev/null +++ b/backend/domain/orchestration/execution_generation_t3_test.go @@ -0,0 +1,123 @@ +package orchestration + +import ( + "context" + "errors" + "testing" + + "github.com/jamespud/magi/backend/application/metrics" + "github.com/jamespud/magi/backend/domain/entity" + "github.com/jamespud/magi/backend/domain/port" +) + +// T3 / #21: persisted artifact IDs use the durable Case execution generation, +// never the resettable ExecutionAttempt retry ordinal. +func TestExecutionRunIDUsesGenerationNamespace(t *testing.T) { + got := executionRunID("case-1", "melchior", 7, 2, "investigate") + want := "case-1-melchior-g7-r2-investigate" + if got != want { + t.Fatalf("executionRunID = %q, want %q", got, want) + } +} + +func TestOrchestrateForExecutionRequiresOwnedArtifactCapability(t *testing.T) { + o := &Orchestrator{} + c := &entity.DecisionCase{ID: "case-1", ExecutionGeneration: 3} + owner := &entity.ExecutionContext{ + CaseID: "case-1", JobID: "job-1", WorkerID: "worker-1", ExecutionGeneration: 3, + } + if _, err := o.OrchestrateForExecution(context.Background(), c, owner); !errors.Is(err, port.ErrLeaseLost) { + t.Fatalf("OrchestrateForExecution error = %v, want ErrLeaseLost", err) + } +} + +func TestOrchestrateRejectsPositiveGenerationWithoutOwner(t *testing.T) { + o := &Orchestrator{} + _, err := o.Orchestrate(context.Background(), &entity.DecisionCase{ + ID: "case-owned", ExecutionGeneration: 1, + }) + if !errors.Is(err, port.ErrLeaseLost) { + t.Fatalf("legacy Orchestrate error = %v, want ErrLeaseLost", err) + } +} + +// persistArtifact's failure contract differs by authority: a durable owner +// makes every authoritative write fail-closed, while the legacy/non-durable +// path keeps the historical tolerate-and-count behaviour (ErrLeaseLost still +// escalates on both). +func TestPersistArtifactFailsClosedOnlyForDurableOwner(t *testing.T) { + storageErr := errors.New("mysql: lock wait timeout exceeded") + durable := &entity.ExecutionContext{ + CaseID: "case-1", JobID: "job-1", WorkerID: "worker-1", ExecutionGeneration: 4, + } + // Missing worker/generation: not a durable owner, so writes stay best effort. + legacy := &entity.ExecutionContext{CaseID: "case-1", JobID: "job-1"} + + cases := []struct { + name string + owner *entity.ExecutionContext + write error + want error + wantNil bool + }{ + {name: "durable storage error", owner: durable, write: storageErr, want: storageErr}, + {name: "durable lease lost", owner: durable, write: port.ErrLeaseLost, want: port.ErrLeaseLost}, + {name: "legacy storage error", owner: nil, write: storageErr, wantNil: true}, + {name: "legacy lease lost", owner: nil, write: port.ErrLeaseLost, want: port.ErrLeaseLost}, + {name: "non-durable owner storage error", owner: legacy, write: storageErr, wantNil: true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + o := &Orchestrator{} + err := o.persistArtifact(context.Background(), metrics.ArtifactAgentRun, "case-1", tc.owner, func() error { + return tc.write + }) + if tc.wantNil { + if err != nil { + t.Fatalf("persistArtifact = %v, want tolerated (nil)", err) + } + return + } + if !errors.Is(err, tc.want) { + t.Fatalf("persistArtifact = %v, want %v", err, tc.want) + } + }) + } +} + +// legacyOnlyRepo is a port.Repository that deliberately does NOT implement +// OwnedArtifactRepository. The embedded interface is nil, so any repository +// method call would panic: the legacy generation guard has to reject the write +// before touching storage. +type legacyOnlyRepo struct{ port.Repository } + +// A caller that threads the durable owner through persistArtifact is only half +// the defence. If a future positive-generation call site forgets to pass it, the +// write still lands in persistArtifactValue with a nil execution, so the entity +// generation must refuse the legacy Create path instead of degrading to the +// log-and-continue contract. +func TestPersistArtifactRefusesPositiveGenerationWithoutOwner(t *testing.T) { + o := &Orchestrator{repo: legacyOnlyRepo{}} + artifacts := []struct { + name string + value any + }{ + {"agent run", &entity.AgentRun{ID: "run-g1", CaseID: "case-g1", ExecutionGeneration: 1}}, + {"evidence", &entity.EvidenceRecord{ID: "ev-g1", CaseID: "case-g1", ExecutionGeneration: 1}}, + {"claim", &entity.Claim{ID: "cl-g1", CaseID: "case-g1", ExecutionGeneration: 1}}, + {"vote", &entity.Vote{ID: "vote-g1", CaseID: "case-g1", ExecutionGeneration: 1}}, + {"debate round", &entity.DebateRound{ID: "deb-g1", CaseID: "case-g1", ExecutionGeneration: 1}}, + {"reflection", &entity.Reflection{ID: "refl-g1", CaseID: "case-g1", ExecutionGeneration: 1}}, + {"tool call", &entity.ToolCall{ID: "tc-g1", CaseID: "case-g1", ExecutionGeneration: 1}}, + } + for _, tc := range artifacts { + t.Run(tc.name, func(t *testing.T) { + err := o.persistArtifact(context.Background(), metrics.ArtifactAgentRun, "case-g1", nil, func() error { + return o.persistArtifactValue(context.Background(), nil, tc.value) + }) + if !errors.Is(err, port.ErrLeaseLost) { + t.Fatalf("ownerless positive-generation %T write = %v, want ErrLeaseLost", tc.value, err) + } + }) + } +} diff --git a/backend/domain/orchestration/orchestrator.go b/backend/domain/orchestration/orchestrator.go index e5b97cc9..4adffcd2 100644 --- a/backend/domain/orchestration/orchestrator.go +++ b/backend/domain/orchestration/orchestrator.go @@ -90,10 +90,34 @@ func NewOrchestrator(d OrchestratorDeps) *Orchestrator { } func (o *Orchestrator) Orchestrate(ctx context.Context, case_ *entity.DecisionCase) (*entity.Resolution, error) { + if case_ != nil && case_.ExecutionGeneration > 0 { + return nil, port.ErrLeaseLost + } + return o.orchestrate(ctx, case_, nil) +} + +// OrchestrateForExecution carries the exact durable Claim owner through every +// authoritative artifact/checkpoint write. A positive generation without this +// identity is not sufficient to authorize persistence. +func (o *Orchestrator) OrchestrateForExecution(ctx context.Context, case_ *entity.DecisionCase, execution *entity.ExecutionContext) (*entity.Resolution, error) { + if case_ == nil || execution == nil || !execution.IsDurable() || + execution.CaseID != case_.ID || execution.ExecutionGeneration != case_.ExecutionGeneration { + return nil, port.ErrLeaseLost + } + if o.repo == nil { + return nil, port.ErrLeaseLost + } + if _, ok := o.repo.(port.OwnedArtifactRepository); !ok { + return nil, port.ErrLeaseLost + } + return o.orchestrate(ctx, case_, execution) +} + +func (o *Orchestrator) orchestrate(ctx context.Context, case_ *entity.DecisionCase, execution *entity.ExecutionContext) (*entity.Resolution, error) { if case_ == nil { return nil, fmt.Errorf("nil case") } - st := &State{MaxDebate: case_.MaxDebateRounds, Round: 1} + st := &State{MaxDebate: case_.MaxDebateRounds, Round: 1, Execution: execution} if st.MaxDebate == 0 { st.MaxDebate = 1 } @@ -121,7 +145,7 @@ func (o *Orchestrator) Orchestrate(ctx context.Context, case_ *entity.DecisionCa for { if o.blueprint != nil && prevStatus != "" && prevStatus != status { if violations := o.blueprint.ValidatePath([]string{string(prevStatus), string(status)}); len(violations) > 0 { - return o.fail(ctx, case_, fmt.Sprintf("fsm blueprint violation: %s", violations[0])) + return o.fail(ctx, case_, fmt.Errorf("fsm blueprint violation: %s", violations[0])) } } next, done, err := o.dispatch(ctx, case_, prevStatus, status, st) @@ -129,7 +153,7 @@ func (o *Orchestrator) Orchestrate(ctx context.Context, case_ *entity.DecisionCa if errors.Is(err, port.ErrLeaseLost) { return nil, err } - return o.fail(ctx, case_, err.Error()) + return o.fail(ctx, case_, err) } prevStatus = status if isNormalTerminal(next) { @@ -258,10 +282,10 @@ func (o *Orchestrator) commitTerminal(ctx context.Context, case_ *entity.Decisio // evidence decided the case"; citing a row that was never persisted would make // that claim false, so the terminal commit must not happen. // -// The cited ids embed the execution attempt, so checking them for existence is -// also what keeps a retry from validating against artifacts of an older -// attempt. This is the fallback path's guard; the database-backed -// TerminalCommitter runs the same check inside its terminal transaction. +// Artifact ID strings are only stable identities. Authority comes from the +// persisted Case + execution_generation provenance: the fallback filters each +// cited row to the Resolution generation, while the database-backed +// TerminalCommitter performs the same check inside its terminal transaction. func (o *Orchestrator) verifyResolutionArtifacts(ctx context.Context, res *entity.Resolution) error { if res == nil { return nil @@ -273,7 +297,9 @@ func (o *Orchestrator) verifyResolutionArtifacts(ctx context.Context, res *entit } present := make(map[string]struct{}, len(votes)) for _, v := range votes { - present[v.ID] = struct{}{} + if v.ExecutionGeneration == res.ExecutionGeneration { + present[v.ID] = struct{}{} + } } if err := requireAllPresent("vote", res.VoteIDs, present); err != nil { return err @@ -286,7 +312,9 @@ func (o *Orchestrator) verifyResolutionArtifacts(ctx context.Context, res *entit } present := make(map[string]struct{}, len(evidence)) for _, e := range evidence { - present[e.ID] = struct{}{} + if e.ExecutionGeneration == res.ExecutionGeneration { + present[e.ID] = struct{}{} + } } if err := requireAllPresent("evidence", res.KeyEvidenceIDs, present); err != nil { return err @@ -299,7 +327,9 @@ func (o *Orchestrator) verifyResolutionArtifacts(ctx context.Context, res *entit } present := make(map[string]struct{}, len(claims)) for _, c := range claims { - present[c.ID] = struct{}{} + if c.ExecutionGeneration == res.ExecutionGeneration { + present[c.ID] = struct{}{} + } } if err := requireAllPresent("claim", res.KeyClaimIDs, present); err != nil { return err @@ -453,6 +483,7 @@ func (o *Orchestrator) retryFailedAgents( results []*runtime.LoopResult, round int, phase string, + execution *entity.ExecutionContext, ) []*runtime.LoopResult { limit := o.failPolicy.RetryLimit if limit <= 0 || len(results) == 0 { @@ -466,7 +497,7 @@ func (o *Orchestrator) retryFailedAgents( continue } for attempt := 1; attempt <= limit; attempt++ { - rr := o.dispatcher.RetryAgent(ctx, case_, task, o.configs[i], round, attempt, phase) + rr := o.dispatcher.RetryAgentForExecution(ctx, case_, task, o.configs[i], round, attempt, phase, execution) results[i] = rr if isCompleted(rr) { break @@ -476,6 +507,80 @@ func (o *Orchestrator) retryFailedAgents( return results } +// persistArtifactValue routes positive-generation writes through the durable +// active-owner repository. Legacy generation-0 callers retain the old +// sub-repository path for standalone/tests only. +func (o *Orchestrator) persistArtifactValue(ctx context.Context, execution *entity.ExecutionContext, value any) error { + if o.repo == nil { + return nil + } + if execution != nil && execution.IsDurable() { + owned, ok := o.repo.(port.OwnedArtifactRepository) + if !ok { + return port.ErrLeaseLost + } + switch v := value.(type) { + case *entity.AgentRun: + return owned.CreateAgentRunOwned(ctx, execution, v) + case *entity.EvidenceRecord: + return owned.CreateEvidenceOwned(ctx, execution, v) + case *entity.Claim: + return owned.CreateClaimOwned(ctx, execution, v) + case *entity.Vote: + return owned.CreateVoteOwned(ctx, execution, v) + case *entity.DebateRound: + return owned.CreateDebateRoundOwned(ctx, execution, v) + case *entity.Reflection: + return owned.CreateReflectionOwned(ctx, execution, v) + case *entity.ToolCall: + return owned.CreateToolCallOwned(ctx, execution, v) + default: + return fmt.Errorf("unsupported authoritative artifact type %T", value) + } + } + // A positive generation without a complete durable owner would otherwise + // bypass T2's worker/job/lease predicate through a legacy Create method. + switch v := value.(type) { + case *entity.AgentRun: + if v.ExecutionGeneration > 0 { + return port.ErrLeaseLost + } + return o.repo.AgentRunRepo().Create(ctx, v) + case *entity.EvidenceRecord: + if v.ExecutionGeneration > 0 { + return port.ErrLeaseLost + } + return o.repo.EvidenceRepo().Create(ctx, v) + case *entity.Claim: + if v.ExecutionGeneration > 0 { + return port.ErrLeaseLost + } + return o.repo.ClaimRepo().Create(ctx, v) + case *entity.Vote: + if v.ExecutionGeneration > 0 { + return port.ErrLeaseLost + } + return o.repo.VoteRepo().Create(ctx, v) + case *entity.DebateRound: + if v.ExecutionGeneration > 0 { + return port.ErrLeaseLost + } + return o.repo.DebateRepo().Create(ctx, v) + case *entity.Reflection: + if v.ExecutionGeneration > 0 { + return port.ErrLeaseLost + } + return o.repo.ReflectionRepo().Create(ctx, v) + case *entity.ToolCall: + if v.ExecutionGeneration > 0 { + return port.ErrLeaseLost + } + return o.repo.ToolCallRepo().Create(ctx, v) + default: + return fmt.Errorf("unsupported authoritative artifact type %T", value) + } +} + // persistArtifacts writes agent runs, evidence, claims, and votes for one // dispatch round. Nil-safe: no-op when Repo is unset (back-compat for tests). // @@ -488,25 +593,33 @@ func (o *Orchestrator) retryFailedAgents( // which would collide on the shared table's primary key. Supports/contradicts/ // evidence_ids references are rewritten to the namespaced IDs so intra-agent // links stay consistent. The in-memory ledger is not mutated (copies persisted). -func (o *Orchestrator) persistArtifacts(ctx context.Context, case_ *entity.DecisionCase, results []*runtime.LoopResult, votes []*entity.Vote, round int, phase string) *ArtifactRemap { +func (o *Orchestrator) persistArtifacts(ctx context.Context, case_ *entity.DecisionCase, results []*runtime.LoopResult, votes []*entity.Vote, round int, phase string, execution *entity.ExecutionContext) (*ArtifactRemap, error) { remap := newArtifactRemap() if o.repo == nil { - return remap + if case_ != nil && case_.ExecutionGeneration > 0 { + return remap, port.ErrLeaseLost + } + return remap, nil + } + if case_.ExecutionGeneration > 0 && (execution == nil || !execution.IsDurable() || + execution.CaseID != case_.ID || execution.ExecutionGeneration != case_.ExecutionGeneration) { + return remap, port.ErrLeaseLost } now := time.Now() for i, r := range results { cfg := o.configAt(i) code := codeOf(cfg) run := &entity.AgentRun{ - ID: executionRunID(case_.ID, code, case_.ExecutionAttempt, round, phase), - CaseID: case_.ID, - MagiCode: code, - Round: round, - Status: agentRunStatus(r), - StartedAt: now, - CompletedAt: &now, - Usage: r.Usage, - Err: errStr(r.Err), + ID: executionRunID(case_.ID, code, case_.ExecutionGeneration, round, phase), + CaseID: case_.ID, + ExecutionGeneration: case_.ExecutionGeneration, + MagiCode: code, + Round: round, + Status: agentRunStatus(r), + StartedAt: now, + CompletedAt: &now, + Usage: r.Usage, + Err: errStr(r.Err), } run.Environment = &entity.RunEnvironment{ ModelName: cfg.Model.ModelName, @@ -517,13 +630,15 @@ func (o *Orchestrator) persistArtifacts(ctx context.Context, case_ *entity.Decis for _, tb := range cfg.Tools { run.Environment.Tools = append(run.Environment.Tools, string(tb.Source)+":"+tb.ToolName) } - o.persistArtifact(ctx, metrics.ArtifactAgentRun, case_.ID, func() error { - return o.repo.AgentRunRepo().Create(ctx, run) - }) + if err := o.persistArtifact(ctx, metrics.ArtifactAgentRun, case_.ID, execution, func() error { + return o.persistArtifactValue(ctx, execution, run) + }); err != nil { + return remap, err + } // Build the ID remap (old in-memory ID -> namespaced persisted ID) and // persist copies so the ledger is left untouched for any later use. - prefix := executionRunID(case_.ID, code, case_.ExecutionAttempt, round, phase) + prefix := executionRunID(case_.ID, code, case_.ExecutionGeneration, round, phase) if r.Ledger != nil { evidence := r.Ledger.List() for _, ev := range evidence { @@ -532,10 +647,13 @@ func (o *Orchestrator) persistArtifacts(ctx context.Context, case_ *entity.Decis cp := *ev cp.ID = newID cp.CaseID = case_.ID + cp.ExecutionGeneration = case_.ExecutionGeneration cp.AgentRunID = run.ID - o.persistArtifact(ctx, metrics.ArtifactEvidence, case_.ID, func() error { - return o.repo.EvidenceRepo().Create(ctx, &cp) - }) + if err := o.persistArtifact(ctx, metrics.ArtifactEvidence, case_.ID, execution, func() error { + return o.persistArtifactValue(ctx, execution, &cp) + }); err != nil { + return remap, err + } } claims := r.Ledger.ListClaims() for _, cl := range claims { @@ -546,12 +664,15 @@ func (o *Orchestrator) persistArtifacts(ctx context.Context, case_ *entity.Decis cp := *cl cp.ID = remap.ClaimMap()[cl.ID] cp.CaseID = case_.ID + cp.ExecutionGeneration = case_.ExecutionGeneration cp.AgentRunID = run.ID cp.Supports = remapRefs(cl.Supports, remap.EvidenceMap()) cp.Contradicts = remapRefs(cl.Contradicts, remap.ClaimMap()) - o.persistArtifact(ctx, metrics.ArtifactClaim, case_.ID, func() error { - return o.repo.ClaimRepo().Create(ctx, &cp) - }) + if err := o.persistArtifact(ctx, metrics.ArtifactClaim, case_.ID, execution, func() error { + return o.persistArtifactValue(ctx, execution, &cp) + }); err != nil { + return remap, err + } } } // Persist tool-call records from the run trace. The PK is a namespaced @@ -567,23 +688,26 @@ func (o *Orchestrator) persistArtifacts(ctx context.Context, case_ *entity.Decis evID = remapped } toolCall := &entity.ToolCall{ - ID: fmt.Sprintf("%s-tc%d", prefix, toolIdx), - CaseID: case_.ID, - AgentRunID: run.ID, - ToolCallID: tc.ToolCallID, - ToolName: tc.ToolName, - Arguments: tc.Arguments, - Valid: tc.Valid, - Result: tc.Result, - Err: tc.Err, - ApprovedBy: tc.ApprovedBy, - EvidenceID: evID, - DurationMs: tc.Duration.Milliseconds(), - CreatedAt: now, + ID: fmt.Sprintf("%s-tc%d", prefix, toolIdx), + CaseID: case_.ID, + ExecutionGeneration: case_.ExecutionGeneration, + AgentRunID: run.ID, + ToolCallID: tc.ToolCallID, + ToolName: tc.ToolName, + Arguments: tc.Arguments, + Valid: tc.Valid, + Result: tc.Result, + Err: tc.Err, + ApprovedBy: tc.ApprovedBy, + EvidenceID: evID, + DurationMs: tc.Duration.Milliseconds(), + CreatedAt: now, + } + if err := o.persistArtifact(ctx, metrics.ArtifactToolCall, case_.ID, execution, func() error { + return o.persistArtifactValue(ctx, execution, toolCall) + }); err != nil { + return remap, err } - o.persistArtifact(ctx, metrics.ArtifactToolCall, case_.ID, func() error { - return o.repo.ToolCallRepo().Create(ctx, toolCall) - }) } } } @@ -603,28 +727,51 @@ func (o *Orchestrator) persistArtifacts(ctx context.Context, case_ *entity.Decis // Regenerate the vote ID unconditionally: a reconsider result may reuse // the investigate Vote pointer, which would otherwise re-insert the old // ID and violate the primary key (observed on real runs). - v.ID = "vote-" + executionRunID(case_.ID, codeOf(cfg), case_.ExecutionAttempt, round, phase) + v.ID = "vote-" + executionRunID(case_.ID, codeOf(cfg), case_.ExecutionGeneration, round, phase) v.CaseID = case_.ID + v.ExecutionGeneration = case_.ExecutionGeneration v.Round = round - o.persistArtifact(ctx, metrics.ArtifactVote, case_.ID, func() error { - return o.repo.VoteRepo().Create(ctx, v) - }) + if err := o.persistArtifact(ctx, metrics.ArtifactVote, case_.ID, execution, func() error { + return o.persistArtifactValue(ctx, execution, v) + }); err != nil { + return remap, err + } } - return remap + return remap, nil } -// persistArtifact runs one durable artifact write. Failures are counted and -// logged rather than silently dropped: a missing vote/evidence/tool-call row is -// an audit hole, and the stock-mcp incident showed it can show up as a complete -// case with no trace of the tool call. -func (o *Orchestrator) persistArtifact(ctx context.Context, kind metrics.ArtifactKind, caseID string, write func() error) { +// persistArtifact runs one artifact write and classifies its failure by the +// authority of the write. +// +// owner carries the durable execution that authorizes an authoritative, +// generation-scoped artifact (AgentRun/Evidence/Claim/Vote/DebateRound/ +// Reflection/ToolCall). On that path any repository or storage error is +// fail-closed: once the database cannot prove the authoritative write landed, +// the execution must not continue and report success. A commit whose result is +// unknown is treated the same way: the generation is abandoned and a later +// generation recovers, rather than continuing on a guessed outcome. +// +// owner == nil keeps the historical best-effort contract for non-authoritative +// writes (task-snapshot cache, memory projection) and for legacy generation-0 +// storage: the failure is logged and counted, and ErrLeaseLost still +// propagates. Omitting the owner cannot smuggle a positive-generation artifact +// past the fence: persistArtifactValue refuses that combination with +// ErrLeaseLost, which this function escalates on either path. +func (o *Orchestrator) persistArtifact(ctx context.Context, kind metrics.ArtifactKind, caseID string, owner *entity.ExecutionContext, write func() error) error { if write == nil { - return + return nil } if err := write(); err != nil { log.Printf("orchestrator: persist %s for case %s failed: %v", kind, caseID, err) o.metrics.IncArtifactPersistFailure(kind) + if errors.Is(err, port.ErrLeaseLost) { + return port.ErrLeaseLost + } + if owner != nil && owner.IsDurable() { + return err + } } + return nil } func (o *Orchestrator) configAt(i int) *entity.MagiConfig { @@ -801,16 +948,19 @@ func (o *Orchestrator) publish(ctx context.Context, case_ *entity.DecisionCase, return o.eventPub.Publish(ctx, entity.NewEvent(case_.ID, "", nil, et, payload)) } -func (o *Orchestrator) fail(ctx context.Context, case_ *entity.DecisionCase, msg string) (*entity.Resolution, error) { - runErr := fmt.Errorf("%s", msg) +// fail records a synchronous orchestration failure. The cause is returned +// unchanged rather than flattened to a string so a caller can still classify it +// (for example with errors.Is), which is what lets a durable worker distinguish +// a storage failure from a lease loss. +func (o *Orchestrator) fail(ctx context.Context, case_ *entity.DecisionCase, cause error) (*entity.Resolution, error) { if isPublicTerminalCaseStatus(case_.Status) { if case_.ExecutionAttempt > 0 { - return nil, runErr + return nil, cause } return nil, port.ErrLeaseLost } if case_.ExecutionAttempt > 0 { - return nil, runErr + return nil, cause } event := entity.NewEvent(case_.ID, "", nil, entity.EventCaseFailed, map[string]any{"status": string(entity.CaseStatusFailed)}) @@ -818,7 +968,7 @@ func (o *Orchestrator) fail(ctx context.Context, case_ *entity.DecisionCase, msg return nil, err } case_.Status = entity.CaseStatusFailed - return nil, runErr + return nil, cause } func isPublicTerminalCaseStatus(status entity.CaseStatus) bool { diff --git a/backend/domain/orchestration/orchestrator_test.go b/backend/domain/orchestration/orchestrator_test.go index 47d95892..4161d1dc 100644 --- a/backend/domain/orchestration/orchestrator_test.go +++ b/backend/domain/orchestration/orchestrator_test.go @@ -611,13 +611,15 @@ func TestOrchestrate_CountsDebatePersistFailure(t *testing.T) { // A tolerated artifact write failure must be counted instead of vanishing: the // stock-mcp incident produced complete-looking cases whose tool-call/evidence // rows never persisted. +// +// This exercises the legacy/non-durable generation-0 path, where writes stay +// best effort and the counter is the only trace. On the durable path T3 makes +// tool calls an authoritative generation-scoped artifact, so the same failure +// aborts the execution instead of being tolerated; that contract is pinned by +// TestOrchestrateForExecution_DurableArtifactFailureAbortsExecution. func TestOrchestrate_CountsToleratedPersistFailure(t *testing.T) { reg := metrics.New() repo := newStubRepo() - // A tool-call trace is operational telemetry, not the decision's basis, so a - // failed write stays tolerated and only the counter records it. The - // authoritative artifacts (votes, evidence, claims) are covered by - // TestOrchestrate_RequiredArtifactFailureBlocksTerminalSuccess instead. repo.toolCallErr = errors.New("data too long for column 'result'") orch := orchestration.NewOrchestrator(orchestration.OrchestratorDeps{ diff --git a/backend/domain/orchestration/runid_budget_internal_test.go b/backend/domain/orchestration/runid_budget_internal_test.go index 74478446..2b3bc4aa 100644 --- a/backend/domain/orchestration/runid_budget_internal_test.go +++ b/backend/domain/orchestration/runid_budget_internal_test.go @@ -19,10 +19,10 @@ func TestRunIDsFitInvocationColumns(t *testing.T) { for _, code := range codes { for _, phase := range phases { - for attempt := 0; attempt <= 9; attempt++ { + for generation := int64(0); generation <= 9; generation++ { for round := 1; round <= 3; round++ { checkpoint := checkpointRunID(caseID, code, round, phase) - execution := executionRunID(caseID, code, attempt, round, phase) + execution := executionRunID(caseID, code, generation, round, phase) // The dispatcher retry path appends -retryN to the checkpoint ID. retry := checkpoint + "-retry9" for _, id := range []string{checkpoint, execution, retry} { diff --git a/backend/domain/orchestration/steps.go b/backend/domain/orchestration/steps.go index d0c665fe..d5248fe5 100644 --- a/backend/domain/orchestration/steps.go +++ b/backend/domain/orchestration/steps.go @@ -20,6 +20,7 @@ import ( // (and later the table-driven action dispatch) operates on one explicit state. type State struct { MaxDebate int + Execution *entity.ExecutionContext Task *entity.DecisionTask Results []*runtime.LoopResult Votes []*entity.Vote @@ -104,7 +105,7 @@ func (o *Orchestrator) stepNormalize(ctx context.Context, case_ *entity.Decision if o.caseRepo != nil { // task_json is a cache of the normalized task, not an audit artifact: // losing it is tolerable, but it must still be visible. - o.persistArtifact(ctx, metrics.ArtifactTaskSnapshot, case_.ID, func() error { + o.persistArtifact(ctx, metrics.ArtifactTaskSnapshot, case_.ID, nil, func() error { return o.caseRepo.UpdateTask(ctx, case_.ID, t) }) } @@ -123,8 +124,8 @@ func (o *Orchestrator) stepRetrieveMemory(ctx context.Context, case_ *entity.Dec func (o *Orchestrator) stepInvestigate(ctx context.Context, case_ *entity.DecisionCase, st *State) (entity.CaseStatus, bool, error) { o.publish(ctx, case_, entity.EventAgentStarted, map[string]any{"round": st.Round}) - st.Results = o.dispatcher.Dispatch(ctx, case_, st.Task, o.configs, st.Round) - st.Results = o.retryFailedAgents(ctx, case_, st.Task, st.Results, st.Round, "investigate") + st.Results = o.dispatcher.DispatchForExecution(ctx, case_, st.Task, o.configs, st.Round, st.Execution) + st.Results = o.retryFailedAgents(ctx, case_, st.Task, st.Results, st.Round, "investigate", st.Execution) return entity.CaseStatusEvidenceGating, false, nil } @@ -141,7 +142,10 @@ func (o *Orchestrator) stepCollectVotes(ctx context.Context, case_ *entity.Decis if o.failPolicy.Mode == "fail_case" && len(st.Absences) > 0 { return "", false, fmt.Errorf("%w: %s", ErrAgentFailed, absencesMsg(st.Absences)) } - remap := o.persistArtifacts(ctx, case_, st.Results, st.Votes, st.Round, "investigate") + remap, err := o.persistArtifacts(ctx, case_, st.Results, st.Votes, st.Round, "investigate", st.Execution) + if err != nil { + return "", false, err + } st.AllRemaps = append(st.AllRemaps, remap) payload := map[string]any{"round": st.Round, "votes": ballotCount(st.Votes)} if len(st.Absences) > 0 { @@ -188,15 +192,19 @@ func (o *Orchestrator) stepDebate(ctx context.Context, case_ *entity.DecisionCas allEvidence := o.collectEvidence(st.Results) packet := o.debate.BuildPacket(derefVotes(st.Votes), allClaims, st.Round, allEvidence) if o.repo != nil { - o.persistArtifact(ctx, metrics.ArtifactDebateRound, case_.ID, func() error { - return o.repo.DebateRepo().Create(ctx, &entity.DebateRound{ - ID: fmt.Sprintf("deb-%s-r%d", case_.ID, st.Round), CaseID: case_.ID, Round: st.Round, - Packet: packet, StartedAt: time.Now(), - }) - }) + debateRound := &entity.DebateRound{ + ID: fmt.Sprintf("deb-%s-g%d-r%d", case_.ID, case_.ExecutionGeneration, st.Round), + CaseID: case_.ID, ExecutionGeneration: case_.ExecutionGeneration, Round: st.Round, + Packet: packet, StartedAt: time.Now(), + } + if err := o.persistArtifact(ctx, metrics.ArtifactDebateRound, case_.ID, st.Execution, func() error { + return o.persistArtifactValue(ctx, st.Execution, debateRound) + }); err != nil { + return "", false, err + } } - st.Results = o.dispatcher.DispatchReconsider(ctx, case_, st.Task, packet, st.Results, o.configs, st.Round) - st.Results = o.retryFailedAgents(ctx, case_, st.Task, st.Results, st.Round, "reconsider") + st.Results = o.dispatcher.DispatchReconsiderForExecution(ctx, case_, st.Task, packet, st.Results, o.configs, st.Round, st.Execution) + st.Results = o.retryFailedAgents(ctx, case_, st.Task, st.Results, st.Round, "reconsider", st.Execution) return entity.CaseStatusReflecting, false, nil } @@ -208,21 +216,28 @@ func (o *Orchestrator) stepReflect(ctx context.Context, case_ *entity.DecisionCa func (o *Orchestrator) stepRevote(ctx context.Context, case_ *entity.DecisionCase, st *State) (entity.CaseStatus, bool, error) { o.publish(ctx, case_, entity.EventRevoteSubmitted, map[string]any{"round": st.Round}) newVotes, newAbsences := o.collectBallots(st.Results) - remap := o.persistArtifacts(ctx, case_, st.Results, newVotes, st.Round, "reconsider") + remap, err := o.persistArtifacts(ctx, case_, st.Results, newVotes, st.Round, "reconsider", st.Execution) + if err != nil { + return "", false, err + } st.AllRemaps = append(st.AllRemaps, remap) reflections := EnforceReflectionRule(st.Votes, newVotes, st.Results, o.configs, st.Round) if o.repo != nil { for idx, rf := range reflections { // IDs must be unique per case/attempt/round/agent: inferred // reflections are re-created on every retry attempt. - rf.ID = fmt.Sprintf("refl-%s-a%d-r%d-%d", case_.ID, case_.ExecutionAttempt, rf.Round, idx) + rf.ID = fmt.Sprintf("refl-%s-g%d-r%d-%d", case_.ID, case_.ExecutionGeneration, rf.Round, idx) + rf.CaseID = case_.ID + rf.ExecutionGeneration = case_.ExecutionGeneration if rf.AgentRunID == "" && idx < len(newVotes) && newVotes[idx] != nil { rf.AgentRunID = newVotes[idx].AgentRunID } remapReflection(rf, remap) - o.persistArtifact(ctx, metrics.ArtifactReflection, case_.ID, func() error { - return o.repo.ReflectionRepo().Create(ctx, rf) - }) + if err := o.persistArtifact(ctx, metrics.ArtifactReflection, case_.ID, st.Execution, func() error { + return o.persistArtifactValue(ctx, st.Execution, rf) + }); err != nil { + return "", false, err + } } } st.Votes = newVotes @@ -245,6 +260,7 @@ func (o *Orchestrator) stepResolve(ctx context.Context, case_ *entity.DecisionCa } res.ID = fmt.Sprintf("res-%s", case_.ID) res.CaseID = case_.ID + res.ExecutionGeneration = case_.ExecutionGeneration res.CreatedAt = time.Now() st.Resolution = res return entity.CaseStatusGeneratingReport, false, nil @@ -271,7 +287,7 @@ func (o *Orchestrator) stepSaveMemory(ctx context.Context, case_ *entity.Decisio // after indexing actually completes (sync) or by the async worker. // The projection is best-effort (the background indexer retries), but a // persistent failure should be visible. - o.persistArtifact(ctx, metrics.ArtifactMemoryProjection, case_.ID, func() error { + o.persistArtifact(ctx, metrics.ArtifactMemoryProjection, case_.ID, nil, func() error { _, err := o.knowledge.Store(ctx, proj) return err }) diff --git a/backend/domain/port/artifact.go b/backend/domain/port/artifact.go index 7ba150d0..22607778 100644 --- a/backend/domain/port/artifact.go +++ b/backend/domain/port/artifact.go @@ -2,9 +2,9 @@ package port import "context" -// ArtifactCleaner removes the persisted artifacts of a previous execution -// attempt so a durable retry that resumes from checkpoints does not duplicate -// agent runs, evidence, claims, votes or tool calls. +// ArtifactCleaner is the legacy generation-0 cleanup hook. Generation-aware +// retries must not depend on or invoke case-wide deletion for correctness; +// T5 replaces this surface with generation-scoped retention/GC. type ArtifactCleaner interface { CleanupCaseArtifacts(ctx context.Context, caseID string) error } diff --git a/backend/domain/port/repository.go b/backend/domain/port/repository.go index c5be7b4b..78cae6cc 100644 --- a/backend/domain/port/repository.go +++ b/backend/domain/port/repository.go @@ -161,6 +161,35 @@ type CaseListFilter interface { ListForUser(ctx context.Context, userID int64, limit, offset int) ([]*entity.DecisionCase, error) } +// OwnedArtifactRepository is the generation-aware authoritative artifact +// capability. Durable workers must use these methods; the legacy sub-repository +// Create/ListByCase methods remain generation-0/history compatibility APIs. +type OwnedArtifactRepository interface { + CreateAgentRunOwned(ctx context.Context, owner *entity.ExecutionContext, r *entity.AgentRun) error + CreateEvidenceOwned(ctx context.Context, owner *entity.ExecutionContext, e *entity.EvidenceRecord) error + CreateClaimOwned(ctx context.Context, owner *entity.ExecutionContext, c *entity.Claim) error + CreateVoteOwned(ctx context.Context, owner *entity.ExecutionContext, v *entity.Vote) error + CreateDebateRoundOwned(ctx context.Context, owner *entity.ExecutionContext, d *entity.DebateRound) error + CreateReflectionOwned(ctx context.Context, owner *entity.ExecutionContext, r *entity.Reflection) error + CreateToolCallOwned(ctx context.Context, owner *entity.ExecutionContext, t *entity.ToolCall) error + + ListAgentRunsByGeneration(ctx context.Context, caseID string, generation int64) ([]*entity.AgentRun, error) + ListEvidenceByGeneration(ctx context.Context, caseID string, generation int64) ([]*entity.EvidenceRecord, error) + ListClaimsByGeneration(ctx context.Context, caseID string, generation int64) ([]*entity.Claim, error) + ListVotesByGeneration(ctx context.Context, caseID string, generation int64) ([]*entity.Vote, error) + ListDebateRoundsByGeneration(ctx context.Context, caseID string, generation int64) ([]*entity.DebateRound, error) + ListReflectionsByGeneration(ctx context.Context, caseID string, generation int64) ([]*entity.Reflection, error) + ListToolCallsByGeneration(ctx context.Context, caseID string, generation int64) ([]*entity.ToolCall, error) +} + +// GenerationCheckpointRepository keeps the logical RunID stable while scoping +// durable checkpoint authority to one execution generation. Positive-generation +// runtime code must not fall back to the legacy CheckpointRepository API. +type GenerationCheckpointRepository interface { + SaveForExecution(ctx context.Context, owner *entity.ExecutionContext, s *entity.AgentState) error + LoadForExecution(ctx context.Context, owner *entity.ExecutionContext, runID string) (*entity.AgentState, error) +} + type AgentRunRepository interface { Create(ctx context.Context, r *entity.AgentRun) error Get(ctx context.Context, id string) (*entity.AgentRun, error) diff --git a/backend/domain/runtime/agent_loop.go b/backend/domain/runtime/agent_loop.go index 8c6c4528..c4a2ed6e 100644 --- a/backend/domain/runtime/agent_loop.go +++ b/backend/domain/runtime/agent_loop.go @@ -218,18 +218,22 @@ func (l *AgentLoop) leaseLost(ctx context.Context) bool { // A successful Save against a configured repository records CHECKPOINT_COMMITTED // and fails closed if that event cannot be persisted. A nil repository or empty // run ID stays stateless and only records nothing. -func (l *AgentLoop) commitCheckpoint(ctx context.Context, caseID, runID string, agentCode entity.MagiCode, messages []*schema.Message, nextStep int, ts *TerminationState, phase string, result *LoopResult, compacted bool, ledger *evidence.EvidenceLedger, manifestDigest, lastCommittedInvocationID string, pendingResponse *schema.Message, pendingToolIndex int) error { - if runID == "" { +func (l *AgentLoop) commitCheckpoint(ctx context.Context, actx *AgentContext, agentCode entity.MagiCode, messages []*schema.Message, nextStep int, ts *TerminationState, phase string, result *LoopResult, compacted bool, ledger *evidence.EvidenceLedger, manifestDigest, lastCommittedInvocationID string, pendingResponse *schema.Message, pendingToolIndex int) error { + if actx == nil || actx.RunID == "" { return nil } if errors.Is(context.Cause(ctx), port.ErrLeaseLost) { return port.ErrLeaseLost } - state, err := l.buildCheckpointState(runID, messages, nextStep, ts, phase, result, compacted, ledger, manifestDigest, lastCommittedInvocationID, pendingResponse, pendingToolIndex) + state, err := l.buildCheckpointState(actx.RunID, messages, nextStep, ts, phase, result, compacted, ledger, manifestDigest, lastCommittedInvocationID, pendingResponse, pendingToolIndex) if err != nil { return err } - return l.checkpoints.Commit(ctx, caseID, runID, agentCode, state) + state.CaseID = actx.CaseID + if actx.Execution != nil && actx.Execution.IsDurable() { + state.ExecutionGeneration = actx.Execution.ExecutionGeneration + } + return l.checkpoints.CommitForExecution(ctx, actx.Execution, actx.CaseID, actx.RunID, agentCode, state) } // traceIdentity keeps logical IDs stable across the dispatcher retry convention @@ -475,7 +479,7 @@ func (l *AgentLoop) run(ctx context.Context, cfg *entity.MagiConfig, actx *Agent var checkpoint *execution.AgentSnapshotV2 var legacyCheckpoint *entity.AgentState if actx.RunID != "" { - state, err := l.checkpoints.Load(ctx, actx.RunID) + state, err := l.checkpoints.LoadForExecution(ctx, actx.Execution, actx.RunID) if err != nil { return nil, fmt.Errorf("agent loop: load checkpoint: %w", err) } @@ -620,7 +624,7 @@ func (l *AgentLoop) run(ctx context.Context, cfg *entity.MagiConfig, actx *Agent } else { // Persist the pre-compaction state before the compactor can invoke the // model, then persist again if compaction changed the next invocation. - if err := l.commitCheckpoint(ctx, actx.CaseID, actx.RunID, agentCode, messages, step, ts, phase, result, compacted, ledger, manifestDigest, lastCommittedInvocationID, nil, -1); err != nil { + if err := l.commitCheckpoint(ctx, actx, agentCode, messages, step, ts, phase, result, compacted, ledger, manifestDigest, lastCommittedInvocationID, nil, -1); err != nil { result.Status = LoopStatusError result.Err = err finalizeTrace(trace, result.Status) @@ -637,7 +641,7 @@ func (l *AgentLoop) run(ctx context.Context, cfg *entity.MagiConfig, actx *Agent ts.TokenUsed += usage.TotalTokens } l.publish(ctx, actx.CaseID, actx.RunID, agentCode, entity.EventContextCompacted, map[string]any{"step": step, "tokens_used": ts.TokenUsed}) - if err := l.commitCheckpoint(ctx, actx.CaseID, actx.RunID, agentCode, messages, step, ts, phase, result, compacted, ledger, manifestDigest, lastCommittedInvocationID, nil, -1); err != nil { + if err := l.commitCheckpoint(ctx, actx, agentCode, messages, step, ts, phase, result, compacted, ledger, manifestDigest, lastCommittedInvocationID, nil, -1); err != nil { result.Status = LoopStatusError result.Err = err finalizeTrace(trace, result.Status) @@ -716,7 +720,7 @@ func (l *AgentLoop) run(ctx context.Context, cfg *entity.MagiConfig, actx *Agent st.ModelUsage = extractUsage(resp) result.Usage = addUsage(result.Usage, st.ModelUsage) ts.TokenUsed += st.ModelUsage.TotalTokens - if err := l.commitCheckpoint(ctx, actx.CaseID, actx.RunID, agentCode, messages, step, ts, phase, result, compacted, ledger, manifestDigest, lastCommittedInvocationID, resp, -1); err != nil { + if err := l.commitCheckpoint(ctx, actx, agentCode, messages, step, ts, phase, result, compacted, ledger, manifestDigest, lastCommittedInvocationID, resp, -1); err != nil { result.Status = LoopStatusError result.Err = err finalizeTrace(trace, result.Status) @@ -753,7 +757,7 @@ func (l *AgentLoop) run(ctx context.Context, cfg *entity.MagiConfig, actx *Agent messages = append(messages, schema.UserMessage(fmt.Sprintf( "You have reached the tool-call limit (%d). Stop calling tools and output your EvidenceSummary JSON now, citing the EV-IDs you have gathered.", cfg.LoopPolicy.MaxToolCalls))) - if err := l.commitCheckpoint(ctx, actx.CaseID, actx.RunID, agentCode, messages, step+1, ts, phase, result, compacted, ledger, manifestDigest, lastCommittedInvocationID, nil, -1); err != nil { + if err := l.commitCheckpoint(ctx, actx, agentCode, messages, step+1, ts, phase, result, compacted, ledger, manifestDigest, lastCommittedInvocationID, nil, -1); err != nil { result.Status = LoopStatusError result.Err = err finalizeTrace(trace, result.Status) @@ -778,7 +782,7 @@ func (l *AgentLoop) run(ctx context.Context, cfg *entity.MagiConfig, actx *Agent } else { ts.ToolCalls++ } - if err := l.commitCheckpoint(ctx, actx.CaseID, actx.RunID, agentCode, messages, step, ts, phase, result, compacted, ledger, manifestDigest, lastCommittedInvocationID, resp, ordinal); err != nil { + if err := l.commitCheckpoint(ctx, actx, agentCode, messages, step, ts, phase, result, compacted, ledger, manifestDigest, lastCommittedInvocationID, resp, ordinal); err != nil { result.Status = LoopStatusError result.Err = err finalizeTrace(trace, result.Status) @@ -914,7 +918,7 @@ func (l *AgentLoop) run(ctx context.Context, cfg *entity.MagiConfig, actx *Agent lastCommittedInvocationID = execution.NewInvocationID(st.ID, execution.InvocationTool, len(resp.ToolCalls)-1) } pendingToolIndex = -1 - if err := l.commitCheckpoint(ctx, actx.CaseID, actx.RunID, agentCode, messages, step+1, ts, phase, result, compacted, ledger, manifestDigest, lastCommittedInvocationID, nil, -1); err != nil { + if err := l.commitCheckpoint(ctx, actx, agentCode, messages, step+1, ts, phase, result, compacted, ledger, manifestDigest, lastCommittedInvocationID, nil, -1); err != nil { result.Status = LoopStatusError result.Err = err finalizeTrace(trace, result.Status) @@ -993,7 +997,7 @@ func (l *AgentLoop) run(ctx context.Context, cfg *entity.MagiConfig, actx *Agent messages = append(messages, schema.UserMessage("Evidence gate passed. Now output the Vote JSON.")) phase = "vote" } - if err := l.commitCheckpoint(ctx, actx.CaseID, actx.RunID, agentCode, messages, step+1, ts, phase, result, compacted, ledger, manifestDigest, lastCommittedInvocationID, nil, -1); err != nil { + if err := l.commitCheckpoint(ctx, actx, agentCode, messages, step+1, ts, phase, result, compacted, ledger, manifestDigest, lastCommittedInvocationID, nil, -1); err != nil { result.Status = LoopStatusError result.Err = err finalizeTrace(trace, result.Status) @@ -1007,7 +1011,7 @@ func (l *AgentLoop) run(ctx context.Context, cfg *entity.MagiConfig, actx *Agent messages = append(messages, resp) messages = append(messages, schema.UserMessage("Reflection recorded. Now output the Vote JSON.")) phase = "vote" - if err := l.commitCheckpoint(ctx, actx.CaseID, actx.RunID, agentCode, messages, step+1, ts, phase, result, compacted, ledger, manifestDigest, lastCommittedInvocationID, nil, -1); err != nil { + if err := l.commitCheckpoint(ctx, actx, agentCode, messages, step+1, ts, phase, result, compacted, ledger, manifestDigest, lastCommittedInvocationID, nil, -1); err != nil { result.Status = LoopStatusError result.Err = err finalizeTrace(trace, result.Status) diff --git a/backend/domain/runtime/checkpoint.go b/backend/domain/runtime/checkpoint.go index d479f8d1..8c04a8ba 100644 --- a/backend/domain/runtime/checkpoint.go +++ b/backend/domain/runtime/checkpoint.go @@ -32,15 +32,49 @@ func (s *CheckpointService) Load(ctx context.Context, runID string) (*entity.Age return s.repo.Load(ctx, runID) } +// LoadForExecution loads only the checkpoint owned by the supplied durable +// generation. Positive-generation callers may not fall back to legacy RunID-only +// lookup because that would resurrect a superseded worker snapshot. +func (s *CheckpointService) LoadForExecution(ctx context.Context, owner *entity.ExecutionContext, runID string) (*entity.AgentState, error) { + if s == nil || s.repo == nil { + return nil, nil + } + if owner == nil || !owner.IsDurable() { + return s.repo.Load(ctx, runID) + } + generationRepo, ok := s.repo.(port.GenerationCheckpointRepository) + if !ok { + return nil, port.ErrLeaseLost + } + return generationRepo.LoadForExecution(ctx, owner, runID) +} + // Commit persists a checkpoint snapshot and records a durable execution-history // event. A configured repository Save error or a CHECKPOINT_COMMITTED record // failure stops the caller (fail closed). A nil repository is stateless. func (s *CheckpointService) Commit(ctx context.Context, caseID, runID string, agentCode entity.MagiCode, state *entity.AgentState) error { + return s.CommitForExecution(ctx, nil, caseID, runID, agentCode, state) +} + +// CommitForExecution persists one generation-scoped checkpoint through the +// same active-owner predicate as authoritative artifacts. +func (s *CheckpointService) CommitForExecution(ctx context.Context, owner *entity.ExecutionContext, caseID, runID string, agentCode entity.MagiCode, state *entity.AgentState) error { if s == nil || s.repo == nil || state == nil { return nil } ac := agentCode - if err := s.repo.Save(ctx, state); err != nil { + var err error + if owner != nil && owner.IsDurable() { + generationRepo, ok := s.repo.(port.GenerationCheckpointRepository) + if !ok { + err = port.ErrLeaseLost + } else { + err = generationRepo.SaveForExecution(ctx, owner, state) + } + } else { + err = s.repo.Save(ctx, state) + } + if err != nil { if s.recorder != nil { _ = s.recorder.Critical(ctx, entity.NewEvent(caseID, runID, &ac, entity.EventCheckpointFailed, map[string]any{"step": state.StepCount + 1, "error": err.Error()})) } diff --git a/backend/domain/runtime/runtime.go b/backend/domain/runtime/runtime.go index 93637b23..52437399 100644 --- a/backend/domain/runtime/runtime.go +++ b/backend/domain/runtime/runtime.go @@ -64,15 +64,10 @@ type AgentContext struct { Execution *ExecutionContext } -// ExecutionContext identifies a worker-owned job attempt. JobID and RunID are -// the durable identities; WorkerID and JobAttempt are the physical worker/lease -// fencing fields used to reject late commits after a lease loss. -type ExecutionContext struct { - JobID string - RunID string - WorkerID string - JobAttempt int -} +// ExecutionContext is kept as a runtime alias so existing runtime callers use +// the same owner value while repository ports can depend on the lower entity +// layer without a package cycle. +type ExecutionContext = entity.ExecutionContext type DebateContext struct { Packet entity.DebatePacket diff --git a/docker/atlas/migrations/magi_s27_artifact_generation.sql b/docker/atlas/migrations/magi_s27_artifact_generation.sql new file mode 100644 index 00000000..f0ef04ff --- /dev/null +++ b/docker/atlas/migrations/magi_s27_artifact_generation.sql @@ -0,0 +1,78 @@ +-- MAGI S27: bind authoritative decision artifacts and checkpoints to the +-- durable Case execution generation introduced by S25/T2. +-- +-- generation 0 means legacy/unknown provenance. This migration never infers +-- provenance from -aN- identifiers and never backfills legacy rows to a +-- positive generation. +ALTER TABLE magi_agent_run + ADD COLUMN execution_generation BIGINT NOT NULL DEFAULT 0, + ADD INDEX idx_agent_run_case_generation (case_id, execution_generation); + +ALTER TABLE evidence_record + ADD COLUMN execution_generation BIGINT NOT NULL DEFAULT 0, + ADD INDEX idx_evidence_case_generation (case_id, execution_generation); + +ALTER TABLE claim + ADD COLUMN execution_generation BIGINT NOT NULL DEFAULT 0, + ADD INDEX idx_claim_case_generation (case_id, execution_generation); + +ALTER TABLE magi_vote + ADD COLUMN execution_generation BIGINT NOT NULL DEFAULT 0, + ADD INDEX idx_vote_case_generation (case_id, execution_generation); + +ALTER TABLE debate_round + ADD COLUMN execution_generation BIGINT NOT NULL DEFAULT 0, + ADD INDEX idx_debate_case_generation (case_id, execution_generation); + +ALTER TABLE reflection + ADD COLUMN case_id VARCHAR(64) NOT NULL DEFAULT '', + ADD COLUMN execution_generation BIGINT NOT NULL DEFAULT 0, + ADD INDEX idx_reflection_case_generation (case_id, execution_generation); + +-- Legacy reflection/tool-call rows already have a relational AgentRun link. +-- Backfill only Case provenance from that relationship; execution_generation +-- deliberately remains 0 (legacy/unknown). A row whose agent_run_id has no +-- matching magi_agent_run keeps case_id='' instead of failing the upgrade: +-- pre-existing orphan history is preserved as unknown provenance, and is never +-- attributed to a Case by guesswork. +UPDATE reflection AS r +JOIN magi_agent_run AS ar ON ar.id = r.agent_run_id +SET r.case_id = ar.case_id +WHERE r.case_id = ''; + +ALTER TABLE magi_tool_call + ADD COLUMN case_id VARCHAR(64) NOT NULL DEFAULT '', + ADD COLUMN execution_generation BIGINT NOT NULL DEFAULT 0, + ADD INDEX idx_tool_call_case_generation (case_id, execution_generation); + +UPDATE magi_tool_call AS tc +JOIN magi_agent_run AS ar ON ar.id = tc.agent_run_id +SET tc.case_id = ar.case_id +WHERE tc.case_id = ''; + +ALTER TABLE resolution + ADD COLUMN execution_generation BIGINT NOT NULL DEFAULT 0; + +-- Checkpoints are migrated to the generation contract but deliberately NOT +-- backfilled. The pre-S27 table stores only run_id plus snapshot state: it has +-- no case_id and no agent_run_id, and run_id is the logical working-memory +-- identity ("--r-"), not a declared reference to +-- magi_agent_run.id. Deriving a Case from that string would fabricate +-- provenance, so legacy rows keep case_id='' with execution_generation=0 +-- (legacy/unknown). +-- +-- CaseRepo.Delete still applies its pre-S27 containment rule (remove rows whose +-- run_id names one of the Case's AgentRun rows) as a deletion rule, not as a +-- provenance claim. A legacy checkpoint that matches neither that rule nor a +-- recorded case_id is retained as unknown-provenance history rather than being +-- attributed to a Case, and the generation-scoped loader cannot return it. +ALTER TABLE magi_agent_checkpoint + ADD COLUMN case_id VARCHAR(64) NOT NULL DEFAULT '', + ADD COLUMN execution_generation BIGINT NOT NULL DEFAULT 0, + DROP PRIMARY KEY, + ADD PRIMARY KEY (run_id, execution_generation), + ADD INDEX idx_checkpoint_case_generation (case_id, execution_generation); + +-- Rollback warning: once positive-generation artifact/checkpoint rows exist, +-- a writer that ignores execution_generation can mix authority across retries. +-- Downgrade to a generation-unaware writer is unsafe.