From 969846b2d5c872a33f6f85e924a992ff59ffa637 Mon Sep 17 00:00:00 2001 From: spud <92900806+jamespud@users.noreply.github.com> Date: Wed, 7 Oct 2026 15:42:56 +0800 Subject: [PATCH 1/7] feat(schema): persist artifact execution generations --- backend/adapter/model.go | 202 ++++++++++-------- backend/domain/entity/agent_run.go | 37 ++-- backend/domain/entity/claim.go | 19 +- backend/domain/entity/debate.go | 13 +- backend/domain/entity/evidence.go | 25 +-- backend/domain/entity/execution.go | 16 ++ backend/domain/entity/idmap.go | 11 +- backend/domain/entity/reflection.go | 2 + backend/domain/entity/resolution.go | 21 +- backend/domain/entity/tool_call.go | 27 +-- backend/domain/entity/vote.go | 25 +-- backend/domain/port/artifact.go | 6 +- backend/domain/port/repository.go | 29 +++ .../magi_s27_artifact_generation.sql | 62 ++++++ 14 files changed, 316 insertions(+), 179 deletions(-) create mode 100644 docker/atlas/migrations/magi_s27_artifact_generation.sql 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/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/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/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/docker/atlas/migrations/magi_s27_artifact_generation.sql b/docker/atlas/migrations/magi_s27_artifact_generation.sql new file mode 100644 index 00000000..2b6358fc --- /dev/null +++ b/docker/atlas/migrations/magi_s27_artifact_generation.sql @@ -0,0 +1,62 @@ +-- 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). +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; + +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. From f70f4314a3799ec6cedebf8cf1de6d6c3df91d80 Mon Sep 17 00:00:00 2001 From: spud <92900806+jamespud@users.noreply.github.com> Date: Wed, 7 Oct 2026 15:43:04 +0800 Subject: [PATCH 2/7] feat(runtime): fence artifacts and checkpoints by execution owner --- backend/adapter/repository.go | 475 ++++++++++++++++--- backend/application/decision/run_manager.go | 25 +- backend/domain/orchestration/dispatcher.go | 53 ++- backend/domain/orchestration/orchestrator.go | 237 ++++++--- backend/domain/orchestration/steps.go | 48 +- backend/domain/runtime/agent_loop.go | 30 +- backend/domain/runtime/checkpoint.go | 36 +- backend/domain/runtime/runtime.go | 13 +- 8 files changed, 735 insertions(+), 182 deletions(-) diff --git a/backend/adapter/repository.go b/backend/adapter/repository.go index 61a61fb5..f4434a11 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,23 @@ 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 directly attributed to it. +// S27 gives ToolCall, Reflection and Checkpoint direct case provenance, so +// deletion no longer depends on AgentRun joins or on checkpoint RunID shape. 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 { + // Legacy generation-0 checkpoints may predate direct case_id provenance. + // Remove both directly-scoped checkpoints and those linked by RunID to this + // Case's AgentRun before deleting the runs themselves. + 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 +746,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 +763,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 +781,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 +828,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 +864,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 +876,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 +907,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 +918,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 +938,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 +959,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 +970,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 +1078,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 +1095,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 +1106,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 +1119,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 +1141,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 +1156,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 +1314,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 +1328,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 +1337,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 +1350,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/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/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/orchestrator.go b/backend/domain/orchestration/orchestrator.go index e5b97cc9..2cc953ed 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 } @@ -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, 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, 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, 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, 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,35 @@ 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, 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) { +func (o *Orchestrator) persistArtifact(ctx context.Context, kind metrics.ArtifactKind, caseID string, 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 + } } + return nil } func (o *Orchestrator) configAt(i int) *entity.MagiConfig { diff --git a/backend/domain/orchestration/steps.go b/backend/domain/orchestration/steps.go index d0c665fe..67fa736a 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 @@ -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, 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, 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 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 From e80661522e563337374933dbe652735834ecf999 Mon Sep 17 00:00:00 2001 From: spud <92900806+jamespud@users.noreply.github.com> Date: Wed, 7 Oct 2026 15:43:10 +0800 Subject: [PATCH 3/7] feat(decision): scope current artifact views to generation --- backend/adapter/a2a_repository.go | 3 +- backend/application/decision/service.go | 140 +++++++++++++++++++++++- backend/bootstrap/container.go | 3 + 3 files changed, 140 insertions(+), 6 deletions(-) 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/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/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)) } From cb014244cd926762702a22576006b7b5d30d937a Mon Sep 17 00:00:00 2001 From: spud <92900806+jamespud@users.noreply.github.com> Date: Wed, 7 Oct 2026 15:43:15 +0800 Subject: [PATCH 4/7] test(t3): prove artifact generation fencing on MySQL --- .github/workflows/ci.yml | 10 + .../adapter/artifact_generation_mysql_test.go | 466 ++++++++++++++++++ backend/application/a2a/stream_test.go | 7 + backend/application/a2a/submission_test.go | 4 + backend/application/decision/cleaner_test.go | 46 +- .../decision/multi_instance_test.go | 12 + .../decision/run_manager_durable_test.go | 63 +++ .../application/decision/run_manager_test.go | 4 + backend/application/decision/service_test.go | 129 +++++ .../application/evaluation/service_test.go | 2 +- backend/bootstrap/migration_parity_test.go | 64 +++ .../mysql_artifact_generation_t3_test.go | 143 ++++++ .../mysql_execution_generation_claim_test.go | 9 + backend/bootstrap/mysql_migration_s27_test.go | 199 ++++++++ backend/domain/entity/idmap_test.go | 1 + .../orchestration/checkpoint_runid_test.go | 13 +- .../execution_generation_t3_test.go | 41 ++ .../runid_budget_internal_test.go | 4 +- 18 files changed, 1180 insertions(+), 37 deletions(-) create mode 100644 backend/adapter/artifact_generation_mysql_test.go create mode 100644 backend/bootstrap/mysql_artifact_generation_t3_test.go create mode 100644 backend/bootstrap/mysql_migration_s27_test.go create mode 100644 backend/domain/orchestration/execution_generation_t3_test.go 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/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/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_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_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/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..c673d711 --- /dev/null +++ b/backend/bootstrap/mysql_migration_s27_test.go @@ -0,0 +1,199 @@ +package bootstrap + +import ( + "strings" + "testing" + + magi "github.com/jamespud/magi/backend/adapter" + "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) + } +} + +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) + } + } + + for _, table := range []string{"reflection", "magi_tool_call"} { + var caseID string + if err := db.Raw("SELECT case_id FROM " + table + " LIMIT 1").Scan(&caseID).Error; err != nil { + t.Fatalf("read %s.case_id: %v", table, err) + } + if caseID != "case-legacy" { + t.Fatalf("%s legacy case_id = %q, want relational backfill case-legacy", table, 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") + } +} 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/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/execution_generation_t3_test.go b/backend/domain/orchestration/execution_generation_t3_test.go new file mode 100644 index 00000000..140a029b --- /dev/null +++ b/backend/domain/orchestration/execution_generation_t3_test.go @@ -0,0 +1,41 @@ +package orchestration + +import ( + "context" + "errors" + "testing" + + "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) + } +} 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} { From a5d5fa01b48cc700f0540942ffce958e7ace732b Mon Sep 17 00:00:00 2001 From: spud <92900806+jamespud@users.noreply.github.com> Date: Wed, 7 Oct 2026 17:56:19 +0800 Subject: [PATCH 5/7] fix(orchestration): fail closed on durable artifact persistence errors persistArtifact only escalated ErrLeaseLost, so any other repository or storage error on an authoritative artifact write was logged and counted while the run continued. T3 widened the authoritative generation domain past the terminal reference fence (AgentRun, ToolCall, Reflection, DebateRound), so a case could still reach RESOLVED while claiming artifacts that never landed. Route the durable owner through persistArtifact so any write error aborts the execution; the legacy generation-0 and best-effort paths keep the old log-and-count contract. A commit whose outcome is unknown is treated the same way: the generation is abandoned and a later one recovers, never guessed at. Also stop flattening the failure cause to a string in fail(), so a caller can classify it with errors.Is instead of only by message. Tests: unit classification of the durable/legacy split, durable in-memory orchestration failures for agent_run/evidence/claim/tool_call/vote, plus real-MySQL fault injection for an owner-verification lock timeout, an INSERT failure and an abandoned in-flight write against a resolving control. --- .../artifact_generation_fault_mysql_test.go | 207 +++++++++++ ...act_generation_orchestration_mysql_test.go | 336 ++++++++++++++++++ backend/application/metrics/registry.go | 6 +- .../durable_artifact_failure_test.go | 226 ++++++++++++ .../execution_generation_t3_test.go | 45 +++ backend/domain/orchestration/orchestrator.go | 53 ++- .../domain/orchestration/orchestrator_test.go | 10 +- backend/domain/orchestration/steps.go | 8 +- 8 files changed, 864 insertions(+), 27 deletions(-) create mode 100644 backend/adapter/artifact_generation_fault_mysql_test.go create mode 100644 backend/adapter/artifact_generation_orchestration_mysql_test.go create mode 100644 backend/domain/orchestration/durable_artifact_failure_test.go 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_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/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/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 index 140a029b..907d63a1 100644 --- a/backend/domain/orchestration/execution_generation_t3_test.go +++ b/backend/domain/orchestration/execution_generation_t3_test.go @@ -5,6 +5,7 @@ import ( "errors" "testing" + "github.com/jamespud/magi/backend/application/metrics" "github.com/jamespud/magi/backend/domain/entity" "github.com/jamespud/magi/backend/domain/port" ) @@ -39,3 +40,47 @@ func TestOrchestrateRejectsPositiveGenerationWithoutOwner(t *testing.T) { 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) + } + }) + } +} diff --git a/backend/domain/orchestration/orchestrator.go b/backend/domain/orchestration/orchestrator.go index 2cc953ed..4adffcd2 100644 --- a/backend/domain/orchestration/orchestrator.go +++ b/backend/domain/orchestration/orchestrator.go @@ -145,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) @@ -153,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) { @@ -630,7 +630,7 @@ 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) } - if err := o.persistArtifact(ctx, metrics.ArtifactAgentRun, case_.ID, func() error { + if err := o.persistArtifact(ctx, metrics.ArtifactAgentRun, case_.ID, execution, func() error { return o.persistArtifactValue(ctx, execution, run) }); err != nil { return remap, err @@ -649,7 +649,7 @@ func (o *Orchestrator) persistArtifacts(ctx context.Context, case_ *entity.Decis cp.CaseID = case_.ID cp.ExecutionGeneration = case_.ExecutionGeneration cp.AgentRunID = run.ID - if err := o.persistArtifact(ctx, metrics.ArtifactEvidence, case_.ID, func() error { + if err := o.persistArtifact(ctx, metrics.ArtifactEvidence, case_.ID, execution, func() error { return o.persistArtifactValue(ctx, execution, &cp) }); err != nil { return remap, err @@ -668,7 +668,7 @@ func (o *Orchestrator) persistArtifacts(ctx context.Context, case_ *entity.Decis cp.AgentRunID = run.ID cp.Supports = remapRefs(cl.Supports, remap.EvidenceMap()) cp.Contradicts = remapRefs(cl.Contradicts, remap.ClaimMap()) - if err := o.persistArtifact(ctx, metrics.ArtifactClaim, case_.ID, func() error { + if err := o.persistArtifact(ctx, metrics.ArtifactClaim, case_.ID, execution, func() error { return o.persistArtifactValue(ctx, execution, &cp) }); err != nil { return remap, err @@ -703,7 +703,7 @@ func (o *Orchestrator) persistArtifacts(ctx context.Context, case_ *entity.Decis DurationMs: tc.Duration.Milliseconds(), CreatedAt: now, } - if err := o.persistArtifact(ctx, metrics.ArtifactToolCall, case_.ID, func() error { + if err := o.persistArtifact(ctx, metrics.ArtifactToolCall, case_.ID, execution, func() error { return o.persistArtifactValue(ctx, execution, toolCall) }); err != nil { return remap, err @@ -731,7 +731,7 @@ func (o *Orchestrator) persistArtifacts(ctx context.Context, case_ *entity.Decis v.CaseID = case_.ID v.ExecutionGeneration = case_.ExecutionGeneration v.Round = round - if err := o.persistArtifact(ctx, metrics.ArtifactVote, case_.ID, func() error { + if err := o.persistArtifact(ctx, metrics.ArtifactVote, case_.ID, execution, func() error { return o.persistArtifactValue(ctx, execution, v) }); err != nil { return remap, err @@ -740,11 +740,24 @@ func (o *Orchestrator) persistArtifacts(ctx context.Context, case_ *entity.Decis 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) 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 nil } @@ -754,6 +767,9 @@ func (o *Orchestrator) persistArtifact(ctx context.Context, kind metrics.Artifac if errors.Is(err, port.ErrLeaseLost) { return port.ErrLeaseLost } + if owner != nil && owner.IsDurable() { + return err + } } return nil } @@ -932,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)}) @@ -949,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/steps.go b/backend/domain/orchestration/steps.go index 67fa736a..d5248fe5 100644 --- a/backend/domain/orchestration/steps.go +++ b/backend/domain/orchestration/steps.go @@ -105,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) }) } @@ -197,7 +197,7 @@ func (o *Orchestrator) stepDebate(ctx context.Context, case_ *entity.DecisionCas CaseID: case_.ID, ExecutionGeneration: case_.ExecutionGeneration, Round: st.Round, Packet: packet, StartedAt: time.Now(), } - if err := o.persistArtifact(ctx, metrics.ArtifactDebateRound, case_.ID, func() error { + 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 @@ -233,7 +233,7 @@ func (o *Orchestrator) stepRevote(ctx context.Context, case_ *entity.DecisionCas rf.AgentRunID = newVotes[idx].AgentRunID } remapReflection(rf, remap) - if err := o.persistArtifact(ctx, metrics.ArtifactReflection, case_.ID, func() error { + 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 @@ -287,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 }) From 4b4d2a046b44cceb0e1aa0b74ca94e62513b5d2b Mon Sep 17 00:00:00 2001 From: spud <92900806+jamespud@users.noreply.github.com> Date: Wed, 7 Oct 2026 18:06:47 +0800 Subject: [PATCH 6/7] fix(migration): define legacy checkpoint provenance retention S27 adds case_id to magi_agent_checkpoint, but the pre-S27 table recorded no Case at all: it stored only run_id plus snapshot state, and run_id is the logical working-memory identity rather than a declared reference to magi_agent_run. The migration comment now states that legacy rows keep case_id empty with execution_generation=0 instead of fabricating provenance, and that a reflection/tool-call row with a broken AgentRun relationship keeps unknown provenance rather than failing the upgrade. CaseRepo.Delete now documents the three buckets it actually has: direct case_id provenance, the pre-S27 containment rule that removes a checkpoint whose run_id names one of the Case's runs, and the unknown-provenance rows it deliberately retains. The containment rule stays because it is an existing cleanup contract; it is a deletion rule, not a provenance claim, and the generation-scoped loader cannot see retained rows. Tests: S27 upgrade assertions for unchanged checkpoint provenance, preserved orphan reflection/tool-call history and RunID collisions, plus the delete/retention contract on SQLite and real MySQL including generation-scoped loader isolation. --- backend/adapter/case_repository_test.go | 77 ++++++++ backend/adapter/repository.go | 21 ++- backend/bootstrap/mysql_migration_s27_test.go | 171 +++++++++++++++++- .../magi_s27_artifact_generation.sql | 18 +- 4 files changed, 276 insertions(+), 11 deletions(-) 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/repository.go b/backend/adapter/repository.go index f4434a11..e83ff8fb 100644 --- a/backend/adapter/repository.go +++ b/backend/adapter/repository.go @@ -674,17 +674,26 @@ 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 persisted row directly attributed to it. -// S27 gives ToolCall, Reflection and Checkpoint direct case provenance, so -// deletion no longer depends on AgentRun joins or on checkpoint RunID shape. +// 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 } - // Legacy generation-0 checkpoints may predate direct case_id provenance. - // Remove both directly-scoped checkpoints and those linked by RunID to this - // Case's AgentRun before deleting the runs themselves. 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 diff --git a/backend/bootstrap/mysql_migration_s27_test.go b/backend/bootstrap/mysql_migration_s27_test.go index c673d711..6b858093 100644 --- a/backend/bootstrap/mysql_migration_s27_test.go +++ b/backend/bootstrap/mysql_migration_s27_test.go @@ -1,10 +1,16 @@ 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" ) @@ -80,6 +86,19 @@ func seedPreS27Artifacts(t *testing.T, db *gorm.DB) { 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 { @@ -156,13 +175,51 @@ func TestMySQLMigration_S27ScopesArtifactsAndCheckpointsByGeneration(t *testing. } } - for _, table := range []string{"reflection", "magi_tool_call"} { + // 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 " + table + " LIMIT 1").Scan(&caseID).Error; err != nil { - t.Fatalf("read %s.case_id: %v", table, err) + 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 legacy case_id = %q, want relational backfill case-legacy", table, caseID) + 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) } } @@ -197,3 +254,109 @@ func TestMySQLMigration_S27ScopesArtifactsAndCheckpointsByGeneration(t *testing. 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/docker/atlas/migrations/magi_s27_artifact_generation.sql b/docker/atlas/migrations/magi_s27_artifact_generation.sql index 2b6358fc..f0ef04ff 100644 --- a/docker/atlas/migrations/magi_s27_artifact_generation.sql +++ b/docker/atlas/migrations/magi_s27_artifact_generation.sql @@ -31,7 +31,10 @@ ALTER TABLE reflection -- 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). +-- 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 @@ -50,6 +53,19 @@ 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, From 9883d75e4559b79397f58ddc36a009c9b2bc0c6d Mon Sep 17 00:00:00 2001 From: spud <92900806+jamespud@users.noreply.github.com> Date: Wed, 7 Oct 2026 18:38:04 +0800 Subject: [PATCH 7/7] test(orchestration): pin the ownerless positive-generation write guard persistArtifact only fails closed when it receives the durable owner, so the second defence line is persistArtifactValue refusing a positive-generation entity that arrives without one. That branch had no coverage: a future authoritative write that forgets to thread the owner would silently fall back to the legacy Create path and only the guard would stop it. The regression drives all seven authoritative artifact types through persistArtifact with a nil owner and a repository that does not implement OwnedArtifactRepository (its embedded interface is nil, so any storage call panics), and asserts ErrLeaseLost instead. --- .../execution_generation_t3_test.go | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/backend/domain/orchestration/execution_generation_t3_test.go b/backend/domain/orchestration/execution_generation_t3_test.go index 907d63a1..2127e7c9 100644 --- a/backend/domain/orchestration/execution_generation_t3_test.go +++ b/backend/domain/orchestration/execution_generation_t3_test.go @@ -84,3 +84,40 @@ func TestPersistArtifactFailsClosedOnlyForDurableOwner(t *testing.T) { }) } } + +// 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) + } + }) + } +}