diff --git a/server/src/agents/gates.rs b/server/src/agents/gates.rs index e3ddbd26..23ada987 100644 --- a/server/src/agents/gates.rs +++ b/server/src/agents/gates.rs @@ -1,9 +1,109 @@ //! Acceptance gates — runs test suites and validation scripts in agent worktrees. +use serde::{Deserialize, Serialize}; use std::path::Path; use std::process::Command; use std::time::Duration; use wait_timeout::ChildExt; +/// Typed classification of a gate failure, produced at the gate execution boundary. +/// +/// Downstream decision logic (e.g. `is_self_evident_fix`) matches on the variant +/// rather than scanning the raw output string for patterns. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub enum GateFailureKind { + /// `cargo fmt --check` or `rustfmt --check` detected formatting drift. + Fmt, + /// `cargo clippy` produced warnings or errors (promoted via `-D warnings`). + Lint, + /// Test suite (`script/test`, `cargo nextest`, `cargo test`) failed. + Test, + /// `source-map-check` gate found missing or incomplete doc comments. + SourceMapCheck, + /// Git content conflict detected during squash-rebase. + ContentConflict, + /// Build-level failure (duplicate module files E0761, compile error). + Build, + /// Unclassified gate failure. + Other, +} + +impl GateFailureKind { + /// Classify a gate failure from its raw output at the gate execution boundary. + /// + /// Called once when a gate fails to produce a typed kind. Downstream code + /// matches on the variant and must not call this on subsequent reads. + pub fn classify(output: &str) -> Self { + if output.contains("CONFLICT (content):") || output.contains("Merge conflict:") { + GateFailureKind::ContentConflict + } else if output.contains("Diff in ") || output.contains("would reformat") { + GateFailureKind::Fmt + } else if output.contains("missing-docs direction") { + GateFailureKind::SourceMapCheck + } else if output.contains("error[clippy::") + || output.contains("warning[clippy::") + || output.contains("missing_doc_comments") + { + GateFailureKind::Lint + } else { + GateFailureKind::Test + } + } + + /// Whether this failure class is a self-evident fix that a short coder session + /// can resolve without human intervention (fmt drift, lint warnings, missing docs). + pub fn is_self_evident_fix(&self) -> bool { + matches!( + self, + GateFailureKind::Fmt | GateFailureKind::Lint | GateFailureKind::SourceMapCheck + ) + } +} + +/// Outcome of running quality gates, produced at the gate execution boundary. +/// +/// `failure_kind` drives routing decisions; `output` is retained for human-readable +/// display and injection into agent retry prompts only — it must not be used as a +/// decision source (story 986). +#[derive(Debug, Clone)] +pub struct GateOutcome { + /// Whether all gates passed. + pub passed: bool, + /// Typed failure classification; `None` when `passed` is `true`. + pub failure_kind: Option, + /// Human-readable combined gate output (display/prompt injection only). + pub output: String, +} + +impl GateOutcome { + /// Passing outcome. + pub(crate) fn pass(output: String) -> Self { + Self { + passed: true, + failure_kind: None, + output, + } + } + + /// Failing outcome — classifies `failure_kind` from the output at construction. + pub(crate) fn fail(output: String) -> Self { + let failure_kind = Some(GateFailureKind::classify(&output)); + Self { + passed: false, + failure_kind, + output, + } + } + + /// Failing outcome for a pre-classified build error (e.g. duplicate module files). + pub(crate) fn build_error(output: String) -> Self { + Self { + passed: false, + failure_kind: Some(GateFailureKind::Build), + output, + } + } +} + /// Maximum time any single test command is allowed to run before being killed. const TEST_TIMEOUT: Duration = Duration::from_secs(1200); // 20 minutes @@ -214,8 +314,8 @@ fn run_command_with_timeout( /// Run `cargo clippy` and the project test suite (via `script/test` if present, /// otherwise `cargo nextest run` / `cargo test`) in the given directory. -/// Returns `(gates_passed, combined_output)`. -pub(crate) fn run_acceptance_gates(path: &Path) -> Result<(bool, String), String> { +/// Returns a [`GateOutcome`] with a typed failure classification. +pub(crate) fn run_acceptance_gates(path: &Path) -> Result { // Pre-flight: detect duplicate module files (E0761) before running the // full test suite so the failure message is immediately actionable. let duplicates = find_duplicate_module_files(path); @@ -231,17 +331,17 @@ pub(crate) fn run_acceptance_gates(path: &Path) -> Result<(bool, String), String mod_path.display() )); } - return Ok((false, msg)); + return Ok(GateOutcome::build_error(msg)); } // Run script/test (or fallback to cargo test). Project-specific linting // and test commands belong in script/test. let (test_success, test_out) = run_project_tests(path)?; if !test_success { - return Ok((false, test_out)); + return Ok(GateOutcome::fail(test_out)); } - Ok((true, test_out)) + Ok(GateOutcome::pass(test_out)) } /// Scan `root` recursively for Rust source files where both `path/X.rs` and @@ -717,4 +817,135 @@ mod tests { "untracked file should be restored after cargo check" ); } + + // ── GateFailureKind::classify ───────────────────────────────────────────── + + #[test] + fn classify_fmt_from_diff_in() { + assert_eq!( + GateFailureKind::classify("Diff in server/src/lib.rs\n--- original\n+++ reformatted"), + GateFailureKind::Fmt + ); + } + + #[test] + fn classify_fmt_from_would_reformat() { + assert_eq!( + GateFailureKind::classify( + "Checking server/src/lib.rs\nwould reformat server/src/lib.rs" + ), + GateFailureKind::Fmt + ); + } + + #[test] + fn classify_lint_from_clippy_error() { + assert_eq!( + GateFailureKind::classify("error[clippy::unused_variable]: unused variable `x`"), + GateFailureKind::Lint + ); + } + + #[test] + fn classify_lint_from_clippy_warning() { + assert_eq!( + GateFailureKind::classify("warning[clippy::needless_return]: unneeded return"), + GateFailureKind::Lint + ); + } + + #[test] + fn classify_lint_from_missing_doc_comments() { + assert_eq!( + GateFailureKind::classify( + "error: missing_doc_comments: public item lacks documentation" + ), + GateFailureKind::Lint + ); + } + + #[test] + fn classify_source_map_check_from_missing_docs_direction() { + assert_eq!( + GateFailureKind::classify("missing-docs direction: server/src/foo.rs:42 pub fn bar"), + GateFailureKind::SourceMapCheck + ); + } + + #[test] + fn classify_content_conflict() { + assert_eq!( + GateFailureKind::classify("CONFLICT (content): Merge conflict in server/src/lib.rs"), + GateFailureKind::ContentConflict + ); + } + + #[test] + fn classify_test_failure_for_unrecognised_output() { + assert_eq!( + GateFailureKind::classify("test result: FAILED. 3 passed; 1 failed"), + GateFailureKind::Test + ); + } + + // ── GateFailureKind::is_self_evident_fix ───────────────────────────────── + + #[test] + fn fmt_is_self_evident_fix() { + assert!(GateFailureKind::Fmt.is_self_evident_fix()); + } + + #[test] + fn lint_is_self_evident_fix() { + assert!(GateFailureKind::Lint.is_self_evident_fix()); + } + + #[test] + fn source_map_check_is_self_evident_fix() { + assert!(GateFailureKind::SourceMapCheck.is_self_evident_fix()); + } + + #[test] + fn test_failure_is_not_self_evident_fix() { + assert!(!GateFailureKind::Test.is_self_evident_fix()); + } + + #[test] + fn content_conflict_is_not_self_evident_fix() { + assert!(!GateFailureKind::ContentConflict.is_self_evident_fix()); + } + + #[test] + fn build_error_is_not_self_evident_fix() { + assert!(!GateFailureKind::Build.is_self_evident_fix()); + } + + #[test] + fn other_is_not_self_evident_fix() { + assert!(!GateFailureKind::Other.is_self_evident_fix()); + } + + // ── GateOutcome constructors ────────────────────────────────────────────── + + #[test] + fn gate_outcome_pass_has_no_failure_kind() { + let outcome = GateOutcome::pass("all tests passed".to_string()); + assert!(outcome.passed); + assert!(outcome.failure_kind.is_none()); + assert_eq!(outcome.output, "all tests passed"); + } + + #[test] + fn gate_outcome_fail_classifies_kind() { + let outcome = GateOutcome::fail("Diff in server/src/lib.rs".to_string()); + assert!(!outcome.passed); + assert_eq!(outcome.failure_kind, Some(GateFailureKind::Fmt)); + } + + #[test] + fn gate_outcome_build_error_sets_build_kind() { + let outcome = GateOutcome::build_error("ERROR [E0761]: duplicate module files".to_string()); + assert!(!outcome.passed); + assert_eq!(outcome.failure_kind, Some(GateFailureKind::Build)); + } } diff --git a/server/src/agents/lifecycle.rs b/server/src/agents/lifecycle.rs index 658f52da..8835c1af 100644 --- a/server/src/agents/lifecycle.rs +++ b/server/src/agents/lifecycle.rs @@ -264,13 +264,21 @@ pub fn transition_to_merge_failure( ) -> Result { let display = kind.display_reason(); let gate_output = kind.to_gate_output(); + // Serialise the typed kind BEFORE it is moved into the event so both the + // JSON key and the legacy string key can be written after the transition. + let kind_json = serde_json::to_string(&kind).unwrap_or_default(); let fired = apply_transition(story_id, PipelineEvent::MergeFailed { kind }, None) .map_err(|e| e.to_string())?; - // Persist gate-output string so the CRDT projection can reconstruct the - // MergeFailureKind on server restart (display-only; scheduling uses the - // typed kind from the Stage variant). + // Persist the typed kind as JSON so the CRDT projection can reconstruct it + // without substring-scanning the gate output string (story 986). + crate::db::write_content( + crate::db::ContentKey::MergeFailureKind(story_id), + &kind_json, + ); + // Persist legacy gate-output string for human-readable display and + // backward-compatible fallback on pre-986 data. crate::db::write_content(crate::db::ContentKey::GateOutput(story_id), &gate_output); // Persist human-readable description on the MergeJob CRDT entry so display diff --git a/server/src/agents/merge/mod.rs b/server/src/agents/merge/mod.rs index 5108d703..0f735b2e 100644 --- a/server/src/agents/merge/mod.rs +++ b/server/src/agents/merge/mod.rs @@ -39,7 +39,15 @@ pub struct MergeReport { pub conflicts_resolved: bool, pub conflict_details: Option, pub gates_passed: bool, + /// Human-readable output from quality gates (display and retry-prompt injection only). pub gate_output: String, + /// Typed classification of the gate failure, produced at the gate boundary. + /// `None` when `gates_passed` is `true` or when there were no gate results. + #[serde(default)] + pub gate_failure_kind: Option, + /// `true` when the feature branch had zero commits ahead of the base branch. + #[serde(default)] + pub no_commits: bool, pub worktree_cleaned_up: bool, pub story_archived: bool, } @@ -56,4 +64,9 @@ pub(crate) struct SquashMergeResult { /// due to a gate failure; callers can use this to distinguish gate failures /// from merge/commit/FF failures in the `MergeReport`. pub(crate) gates_passed: bool, + /// Typed gate failure kind produced at the gate boundary. `None` when + /// `gates_passed` is `true` or when failure was not from the gate step. + pub(crate) gate_failure_kind: Option, + /// `true` when the feature branch had zero commits ahead of the base branch. + pub(crate) no_commits: bool, } diff --git a/server/src/agents/merge/squash/mod.rs b/server/src/agents/merge/squash/mod.rs index c0814628..1b7a1ebc 100644 --- a/server/src/agents/merge/squash/mod.rs +++ b/server/src/agents/merge/squash/mod.rs @@ -48,10 +48,19 @@ pub(crate) fn run_squash_merge( .parse() .unwrap_or(1); // parse failure → don't false-positive; let merge proceed if ahead == 0 { - return Err(format!( - "{story_id}: no commits to merge — feature branch '{branch}' \ - has 0 commits ahead of '{base_branch}'" - )); + return Ok(SquashMergeResult { + success: false, + had_conflicts: false, + conflicts_resolved: false, + conflict_details: None, + output: format!( + "{story_id}: no commits to merge — feature branch '{branch}' \ + has 0 commits ahead of '{base_branch}'" + ), + gates_passed: false, + gate_failure_kind: None, + no_commits: true, + }); } } @@ -125,6 +134,8 @@ pub(crate) fn run_squash_merge( conflict_details, output: all_output, gates_passed: false, + gate_failure_kind: None, + no_commits: false, }); } let had_conflicts = false; @@ -165,6 +176,8 @@ pub(crate) fn run_squash_merge( conflict_details: None, output: all_output, gates_passed: true, + gate_failure_kind: None, + no_commits: false, }); } cleanup_merge_workspace(project_root, &merge_wt_path, &merge_branch); @@ -175,6 +188,8 @@ pub(crate) fn run_squash_merge( conflict_details, output: all_output, gates_passed: false, + gate_failure_kind: None, + no_commits: false, }); } @@ -207,6 +222,8 @@ pub(crate) fn run_squash_merge( ), output: all_output, gates_passed: false, + gate_failure_kind: None, + no_commits: false, }); } } @@ -251,13 +268,13 @@ pub(crate) fn run_squash_merge( // Run gates in the merge worktree so that failures abort before master moves. all_output.push_str("=== Running quality gates before fast-forward ===\n"); match run_merge_quality_gates(&merge_wt_path) { - Ok((true, gate_out)) => { - all_output.push_str(&gate_out); + Ok(outcome) if outcome.passed => { + all_output.push_str(&outcome.output); all_output.push('\n'); all_output.push_str("=== Quality gates passed ===\n"); } - Ok((false, gate_out)) => { - all_output.push_str(&gate_out); + Ok(outcome) => { + all_output.push_str(&outcome.output); all_output.push('\n'); all_output.push_str( "=== Quality gates FAILED — aborting fast-forward, master unchanged ===\n", @@ -270,6 +287,8 @@ pub(crate) fn run_squash_merge( conflict_details, output: all_output, gates_passed: false, + gate_failure_kind: outcome.failure_kind, + no_commits: false, }); } Err(e) => { @@ -282,6 +301,8 @@ pub(crate) fn run_squash_merge( conflict_details, output: all_output, gates_passed: false, + gate_failure_kind: None, + no_commits: false, }); } } @@ -323,6 +344,8 @@ pub(crate) fn run_squash_merge( )), output: all_output, gates_passed: true, + gate_failure_kind: None, + no_commits: false, }); } @@ -358,6 +381,8 @@ pub(crate) fn run_squash_merge( )), output: all_output, gates_passed: true, + gate_failure_kind: None, + no_commits: false, }); } @@ -392,6 +417,8 @@ pub(crate) fn run_squash_merge( ), output: all_output, gates_passed: true, + gate_failure_kind: None, + no_commits: false, }); } @@ -410,6 +437,8 @@ pub(crate) fn run_squash_merge( conflict_details, output: all_output, gates_passed: true, + gate_failure_kind: None, + no_commits: false, }) } @@ -435,7 +464,11 @@ pub(crate) fn cleanup_merge_workspace( .output(); } -fn run_merge_quality_gates(project_root: &Path) -> Result<(bool, String), String> { +fn run_merge_quality_gates( + project_root: &Path, +) -> Result { + use crate::agents::gates::GateOutcome; + let mut all_output = String::new(); let mut all_passed = true; @@ -449,7 +482,11 @@ fn run_merge_quality_gates(project_root: &Path) -> Result<(bool, String), String if !success { all_passed = false; } - return Ok((all_passed, all_output)); + return if all_passed { + Ok(GateOutcome::pass(all_output)) + } else { + Ok(GateOutcome::fail(all_output)) + }; } // No script/test — fall back to cargo gates for Rust projects. @@ -481,7 +518,11 @@ fn run_merge_quality_gates(project_root: &Path) -> Result<(bool, String), String } } - Ok((all_passed, all_output)) + if all_passed { + Ok(GateOutcome::pass(all_output)) + } else { + Ok(GateOutcome::fail(all_output)) + } } #[cfg(test)] diff --git a/server/src/agents/pool/auto_assign/reconcile.rs b/server/src/agents/pool/auto_assign/reconcile.rs index cfc6c936..db509a00 100644 --- a/server/src/agents/pool/auto_assign/reconcile.rs +++ b/server/src/agents/pool/auto_assign/reconcile.rs @@ -109,7 +109,7 @@ impl AgentPool { .await; let (gates_passed, gate_output) = match gates_result { - Ok(Ok(pair)) => pair, + Ok(Ok(outcome)) => (outcome.passed, outcome.output), Ok(Err(e)) => { eprintln!("[startup:reconcile] Gate check error for '{story_id}': {e}"); let _ = progress_tx.send(ReconciliationEvent { diff --git a/server/src/agents/pool/pipeline/completion/legacy.rs b/server/src/agents/pool/pipeline/completion/legacy.rs index 75d428d0..fd699ca0 100644 --- a/server/src/agents/pool/pipeline/completion/legacy.rs +++ b/server/src/agents/pool/pipeline/completion/legacy.rs @@ -56,10 +56,10 @@ impl AgentPool { let path = worktree_path.clone(); // Run gate checks in a blocking thread to avoid stalling the async runtime. - let (gates_passed, gate_output) = tokio::task::spawn_blocking(move || { + let outcome = tokio::task::spawn_blocking(move || { // Step 1: Reject if worktree is dirty. crate::agents::gates::check_uncommitted_changes(&path)?; - // Step 2: Run clippy + tests and return (passed, output). + // Step 2: Run acceptance gates and return a typed GateOutcome. crate::agents::gates::run_acceptance_gates(&path) }) .await @@ -67,8 +67,8 @@ impl AgentPool { let report = CompletionReport { summary: summary.to_string(), - gates_passed, - gate_output, + gates_passed: outcome.passed, + gate_output: outcome.output, needs_commit_recovery: false, }; diff --git a/server/src/agents/pool/pipeline/completion/server.rs b/server/src/agents/pool/pipeline/completion/server.rs index 573592df..438e68fd 100644 --- a/server/src/agents/pool/pipeline/completion/server.rs +++ b/server/src/agents/pool/pipeline/completion/server.rs @@ -161,7 +161,7 @@ pub(in crate::agents::pool) async fn run_server_owned_completion( false, )); } - let (passed, output) = crate::agents::gates::run_acceptance_gates(&path)?; + let outcome = crate::agents::gates::run_acceptance_gates(&path)?; // Restore stashed uncommitted changes. if stashed { let _ = std::process::Command::new("git") @@ -169,7 +169,7 @@ pub(in crate::agents::pool) async fn run_server_owned_completion( .current_dir(&path) .output(); } - Ok((passed, output, false)) + Ok((outcome.passed, outcome.output, false)) }) .await { diff --git a/server/src/agents/pool/pipeline/merge/runner.rs b/server/src/agents/pool/pipeline/merge/runner.rs index 82def784..f1ffceb3 100644 --- a/server/src/agents/pool/pipeline/merge/runner.rs +++ b/server/src/agents/pool/pipeline/merge/runner.rs @@ -5,24 +5,6 @@ use crate::worktree; use std::path::Path; use std::sync::Arc; -/// Return `true` when `gate_output` matches a self-evident-fix class of failure -/// that a short fixup coder session can resolve without human intervention. -/// -/// Patterns covered: fmt drift (`cargo fmt --check`), clippy warnings promoted -/// to errors (`-D warnings`), and missing doc comments detected by clippy or -/// the source-map-check gate. -fn is_self_evident_fix(gate_output: &str) -> bool { - let patterns: &[&str] = &[ - "Diff in ", // cargo fmt --check output - "would reformat", // rustfmt --check output - "error[clippy::", // clippy error - "warning[clippy::", // clippy warning (treated as error via -D warnings) - "missing_doc_comments", // clippy missing-doc lint - "missing-docs direction", // source-map-check gate - ]; - patterns.iter().any(|p| gate_output.contains(p)) -} - use super::super::super::AgentPool; use super::time::{ decode_server_start_time, encode_server_start_time, server_start_time, unix_now, @@ -130,31 +112,28 @@ impl AgentPool { // On any failure: record merge_failure in CRDT and emit notification. if !success { let kind = match &report { + Ok(r) if r.no_commits => crate::pipeline_state::MergeFailureKind::NoCommits, + Ok(r) if r.had_conflicts => { + crate::pipeline_state::MergeFailureKind::ConflictDetected( + r.conflict_details.clone(), + ) + } Ok(r) => { - if r.had_conflicts { - crate::pipeline_state::MergeFailureKind::ConflictDetected( - r.conflict_details.clone(), - ) - } else { - crate::pipeline_state::MergeFailureKind::GatesFailed( - r.gate_output.clone(), - ) - } + crate::pipeline_state::MergeFailureKind::GatesFailed(r.gate_output.clone()) } Err(e) => crate::pipeline_state::MergeFailureKind::Other(e.clone()), }; - let is_no_commits = matches!( - &kind, - crate::pipeline_state::MergeFailureKind::Other(r) if r.contains("no commits to merge") - ); - // Self-evident fix: gate-only failure whose output matches a pattern - // a fixup coder can resolve in one short session (story 981). - let fixup_output = match &kind { - crate::pipeline_state::MergeFailureKind::GatesFailed(o) => o.as_str(), - _ => "", - }; - let is_fixup = - !is_no_commits && !fixup_output.is_empty() && is_self_evident_fix(fixup_output); + let is_no_commits = + matches!(&kind, crate::pipeline_state::MergeFailureKind::NoCommits); + // Self-evident fix: gate-only failure whose typed kind a fixup coder + // can resolve in one short session (story 981, 986). + let is_fixup = !is_no_commits + && report + .as_ref() + .ok() + .and_then(|r| r.gate_failure_kind.as_ref()) + .map(|k| k.is_self_evident_fix()) + .unwrap_or(false); if is_no_commits { let reason = kind.display_reason(); @@ -301,6 +280,8 @@ impl AgentPool { conflict_details: merge_result.conflict_details, gates_passed: merge_result.gates_passed, gate_output: merge_result.output, + gate_failure_kind: merge_result.gate_failure_kind, + no_commits: merge_result.no_commits, worktree_cleaned_up: false, story_archived: false, }); @@ -330,6 +311,8 @@ impl AgentPool { conflict_details: merge_result.conflict_details, gates_passed: true, gate_output: merge_result.output, + gate_failure_kind: None, + no_commits: false, worktree_cleaned_up, story_archived, }) diff --git a/server/src/agents/pool/pipeline/merge/status.rs b/server/src/agents/pool/pipeline/merge/status.rs index 34e5ee74..5e5a5c03 100644 --- a/server/src/agents/pool/pipeline/merge/status.rs +++ b/server/src/agents/pool/pipeline/merge/status.rs @@ -29,6 +29,8 @@ impl AgentPool { conflict_details: None, gates_passed: false, gate_output: String::new(), + gate_failure_kind: None, + no_commits: false, worktree_cleaned_up: false, story_archived: false, }); diff --git a/server/src/agents/pool/pipeline/merge/tests.rs b/server/src/agents/pool/pipeline/merge/tests.rs index 0be7ed86..3f853ee0 100644 --- a/server/src/agents/pool/pipeline/merge/tests.rs +++ b/server/src/agents/pool/pipeline/merge/tests.rs @@ -590,23 +590,27 @@ async fn merge_agent_work_zero_commits_ahead_stays_in_merge_stage() { let pool = Arc::new(AgentPool::new_test(3001)); let job = run_merge_to_completion(&pool, repo, "675_zero_commits").await; - // The job must have failed with a "no commits to merge" error. + // The job must have completed with success=false and no_commits=true. + // Since story 986 the "no commits" path returns Ok(result) not Err, so the + // job status is Completed (not Failed). match &job.status { - MergeJobStatus::Failed(e) => { + MergeJobStatus::Completed(report) => { assert!( - e.contains("no commits to merge"), - "error must contain 'no commits to merge', got: {e}" + !report.success, + "merge must not have succeeded when feature branch is empty" ); assert!( - e.contains("675_zero_commits"), - "error must name the story_id, got: {e}" + report.no_commits, + "report.no_commits must be true for a zero-ahead branch" + ); + assert!( + report.gate_output.contains("no commits to merge"), + "gate_output must contain 'no commits to merge', got: {}", + report.gate_output ); } - MergeJobStatus::Completed(report) => { - panic!( - "expected Failed status, got Completed with success={}: {}", - report.success, report.gate_output - ); + MergeJobStatus::Failed(e) => { + panic!("expected Completed(success=false) status, got Failed: {e}"); } MergeJobStatus::Running => panic!("should not still be running"), } diff --git a/server/src/crdt_state/read.rs b/server/src/crdt_state/read.rs index 62234c8b..bd522aa7 100644 --- a/server/src/crdt_state/read.rs +++ b/server/src/crdt_state/read.rs @@ -443,11 +443,18 @@ fn project_stage_for_view( commits_ahead: NonZeroU32::new(1).expect("1 is non-zero"), }), "merge_failure" => { - // Reconstruct the typed kind from ContentKey::GateOutput so the - // auto-assigner can match on the variant after a server restart. - // This is the sole persistence backing for MergeFailureKind. - let kind = crate::db::read_content(crate::db::ContentKey::GateOutput(story_id)) - .map(|s| crate::pipeline_state::MergeFailureKind::infer_from_gate_output(&s)) + // Story 986: read the typed kind directly from ContentKey::MergeFailureKind + // (written since 986) so no substring-scanning is needed. + // Fall back to infer_from_gate_output for data persisted pre-986. + let kind = crate::db::read_content(crate::db::ContentKey::MergeFailureKind(story_id)) + .and_then(|s| { + serde_json::from_str::(&s).ok() + }) + .or_else(|| { + crate::db::read_content(crate::db::ContentKey::GateOutput(story_id)).map(|s| { + crate::pipeline_state::MergeFailureKind::infer_from_gate_output(&s) + }) + }) .unwrap_or(crate::pipeline_state::MergeFailureKind::Other(String::new())); Some(Stage::MergeFailure { kind, diff --git a/server/src/db/content_store.rs b/server/src/db/content_store.rs index a12668f6..f9f0f6e0 100644 --- a/server/src/db/content_store.rs +++ b/server/src/db/content_store.rs @@ -33,6 +33,10 @@ pub enum ContentKey<'a> { /// can route the fixup coder's completion directly back to merge instead of /// through the normal QA path (story 981). MergeFixupPending(&'a str), + /// JSON-serialised `MergeFailureKind` written alongside `GateOutput` so the + /// CRDT projection layer can reconstruct the typed kind on server restart + /// without substring-scanning the gate output string (story 986). + MergeFailureKind(&'a str), } impl<'a> ContentKey<'a> { @@ -49,6 +53,7 @@ impl<'a> ContentKey<'a> { ContentKey::RunTestsOk(id) => format!("{id}:run_tests_ok"), ContentKey::CommitRecoveryPending(id) => format!("{id}:commit_recovery_pending"), ContentKey::MergeFixupPending(id) => format!("{id}:merge_fixup_pending"), + ContentKey::MergeFailureKind(id) => format!("{id}:merge_failure_kind"), } } } diff --git a/server/src/service/merge/status.rs b/server/src/service/merge/status.rs index bc854d3b..64113dca 100644 --- a/server/src/service/merge/status.rs +++ b/server/src/service/merge/status.rs @@ -44,6 +44,8 @@ mod tests { conflict_details: None, gates_passed, gate_output: String::new(), + gate_failure_kind: None, + no_commits: false, worktree_cleaned_up: false, story_archived: false, }