From 8f0624f012f8d32cfdc3ea01fc53a45fca8df8ad Mon Sep 17 00:00:00 2001 From: Dave Date: Thu, 26 Feb 2026 19:30:26 +0000 Subject: [PATCH] story-kit: merge 216_story_merge_quality_gates_should_use_project_toml_components_and_script_test_instead_of_hardcoded_frontend_pnpm --- server/src/agents.rs | 264 ++++++++++++++++--------------------------- 1 file changed, 95 insertions(+), 169 deletions(-) diff --git a/server/src/agents.rs b/server/src/agents.rs index d33fd4d..6b96821 100644 --- a/server/src/agents.rs +++ b/server/src/agents.rs @@ -2803,42 +2803,39 @@ fn run_squash_merge( }); } - // ── Install frontend dependencies for quality gates ────────── - let frontend_dir = merge_wt_path.join("frontend"); - if frontend_dir.exists() { - // Ensure frontend/dist exists so RustEmbed (cargo clippy) doesn't fail - // even before pnpm build runs. - let dist_dir = frontend_dir.join("dist"); - let _ = std::fs::create_dir_all(&dist_dir); - - all_output.push_str("=== pnpm install (merge worktree) ===\n"); - let pnpm_install = Command::new("pnpm") - .args(["install"]) - .current_dir(&frontend_dir) - .output() - .map_err(|e| format!("Failed to run pnpm install: {e}"))?; - - let install_out = format!( - "{}{}", - String::from_utf8_lossy(&pnpm_install.stdout), - String::from_utf8_lossy(&pnpm_install.stderr) - ); - all_output.push_str(&install_out); - all_output.push('\n'); - - if !pnpm_install.status.success() { - all_output.push_str( - "=== pnpm install FAILED — aborting merge, master unchanged ===\n", - ); - cleanup_merge_workspace(project_root, &merge_wt_path, &merge_branch); - return Ok(SquashMergeResult { - success: false, - had_conflicts, - conflicts_resolved, - conflict_details, - output: all_output, - gates_passed: false, - }); + // ── Run component setup from project.toml (same as worktree creation) ────────── + { + let config = ProjectConfig::load(&merge_wt_path).unwrap_or_default(); + if !config.component.is_empty() { + all_output.push_str("=== component setup (merge worktree) ===\n"); + } + for component in &config.component { + let cmd_dir = merge_wt_path.join(&component.path); + for cmd in &component.setup { + all_output.push_str(&format!("--- {}: {cmd} ---\n", component.name)); + match Command::new("sh") + .args(["-c", cmd]) + .current_dir(&cmd_dir) + .output() + { + Ok(out) => { + all_output.push_str(&String::from_utf8_lossy(&out.stdout)); + all_output.push_str(&String::from_utf8_lossy(&out.stderr)); + all_output.push('\n'); + if !out.status.success() { + all_output.push_str(&format!( + "=== setup warning: '{}' failed: {cmd} ===\n", + component.name + )); + } + } + Err(e) => { + all_output.push_str(&format!( + "=== setup warning: failed to run '{cmd}': {e} ===\n" + )); + } + } + } } } @@ -3114,17 +3111,34 @@ fn resolve_simple_conflicts(content: &str) -> Option { /// Run quality gates in the project root after a successful merge. /// -/// Runs: cargo clippy and tests if Cargo.toml is present, script/test if present, and pnpm gates -/// if frontend/ exists. Sections are skipped when the corresponding project artefacts are absent. +/// Runs quality gates in the merge workspace. +/// +/// When `script/test` is present it is the single source of truth and is the +/// only gate that runs — it is expected to cover the full suite (clippy, unit +/// tests, frontend tests, etc.). When `script/test` is absent the function +/// falls back to `cargo clippy` + `cargo nextest`/`cargo test` for Rust +/// projects. No hardcoded references to pnpm or frontend/ are used. +/// /// Returns `(gates_passed, combined_output)`. fn run_merge_quality_gates(project_root: &Path) -> Result<(bool, String), String> { let mut all_output = String::new(); let mut all_passed = true; - let cargo_toml = project_root.join("Cargo.toml"); let script_test = project_root.join("script").join("test"); - // ── cargo clippy (only when a Rust project is present) ──────── + if script_test.exists() { + // Delegate entirely to script/test — it is the single source of truth + // for the full test suite (clippy, cargo tests, frontend builds, etc.). + let (success, output) = run_project_tests(project_root)?; + all_output.push_str(&output); + if !success { + all_passed = false; + } + return Ok((all_passed, all_output)); + } + + // No script/test — fall back to cargo gates for Rust projects. + let cargo_toml = project_root.join("Cargo.toml"); if cargo_toml.exists() { let clippy = Command::new("cargo") .args(["clippy", "--all-targets", "--all-features"]) @@ -3144,13 +3158,7 @@ fn run_merge_quality_gates(project_root: &Path) -> Result<(bool, String), String if !clippy.status.success() { all_passed = false; } - } - // ── tests (script/test if available, else cargo nextest/test) ─ - // Only run when there is something to run: a script/test entrypoint or a - // Rust project (Cargo.toml). Skipping avoids spurious failures in - // environments that have neither (e.g. test temp-dirs). - if script_test.exists() || cargo_toml.exists() { let (test_success, test_out) = run_project_tests(project_root)?; all_output.push_str(&test_out); if !test_success { @@ -3158,55 +3166,6 @@ fn run_merge_quality_gates(project_root: &Path) -> Result<(bool, String), String } } - // ── pnpm build (if frontend/ directory exists) ──────────────── - // pnpm test is handled by script/test when present; only run it here as - // a standalone fallback when there is no script/test. - let frontend_dir = project_root.join("frontend"); - if frontend_dir.exists() { - all_output.push_str("=== pnpm build ===\n"); - let pnpm_build = Command::new("pnpm") - .args(["run", "build"]) - .current_dir(&frontend_dir) - .output() - .map_err(|e| format!("Failed to run pnpm build: {e}"))?; - - let build_out = format!( - "{}{}", - String::from_utf8_lossy(&pnpm_build.stdout), - String::from_utf8_lossy(&pnpm_build.stderr) - ); - all_output.push_str(&build_out); - all_output.push('\n'); - - if !pnpm_build.status.success() { - all_passed = false; - } - - // Only run pnpm test separately when script/test is absent (it would - // already cover frontend tests in that case). - let script_test = project_root.join("script").join("test"); - if !script_test.exists() { - all_output.push_str("=== pnpm test ===\n"); - let pnpm_test = Command::new("pnpm") - .args(["test", "--run"]) - .current_dir(&frontend_dir) - .output() - .map_err(|e| format!("Failed to run pnpm test: {e}"))?; - - let pnpm_test_out = format!( - "{}{}", - String::from_utf8_lossy(&pnpm_test.stdout), - String::from_utf8_lossy(&pnpm_test.stderr) - ); - all_output.push_str(&pnpm_test_out); - all_output.push('\n'); - - if !pnpm_test.status.success() { - all_passed = false; - } - } - } - Ok((all_passed, all_output)) } @@ -6202,15 +6161,14 @@ theirs assert!(root.join(".story_kit/work/5_done/60_story_cleanup.md").exists()); } - // ── bug 154: merge worktree installs frontend deps ──────────────────── + // ── story 216: merge worktree uses project.toml component setup ─────────── - /// When the feature branch has a `frontend/` directory, `run_squash_merge` - /// must run `pnpm install` in the merge worktree before quality gates. - /// This test creates a repo with a `frontend/package.json` and verifies that - /// the merge output mentions the pnpm install step. + /// When the project has `[[component]]` entries in `.story_kit/project.toml`, + /// `run_squash_merge` must run their setup commands in the merge worktree + /// before quality gates — matching the behaviour of `create_worktree`. #[cfg(unix)] #[test] - fn squash_merge_runs_pnpm_install_when_frontend_exists() { + fn squash_merge_runs_component_setup_from_project_toml() { use std::fs; use tempfile::tempdir; @@ -6218,12 +6176,13 @@ theirs let repo = tmp.path(); init_git_repo(repo); - // Add a frontend/ directory with a minimal package.json on master. - let frontend = repo.join("frontend"); - fs::create_dir_all(&frontend).unwrap(); + // Add a .story_kit/project.toml with a component whose setup writes a + // sentinel file so we can confirm the command ran. + let sk_dir = repo.join(".story_kit"); + fs::create_dir_all(&sk_dir).unwrap(); fs::write( - frontend.join("package.json"), - r#"{"name":"test","version":"0.0.0","private":true}"#, + sk_dir.join("project.toml"), + "[[component]]\nname = \"sentinel\"\npath = \".\"\nsetup = [\"touch setup_ran.txt\"]\n", ) .unwrap(); Command::new("git") @@ -6232,14 +6191,14 @@ theirs .output() .unwrap(); Command::new("git") - .args(["commit", "-m", "add frontend dir"]) + .args(["commit", "-m", "add project.toml with component setup"]) .current_dir(repo) .output() .unwrap(); // Create feature branch with a change. Command::new("git") - .args(["checkout", "-b", "feature/story-154_test"]) + .args(["checkout", "-b", "feature/story-216_setup_test"]) .current_dir(repo) .output() .unwrap(); @@ -6263,21 +6222,28 @@ theirs .unwrap(); let result = - run_squash_merge(repo, "feature/story-154_test", "154_test").unwrap(); + run_squash_merge(repo, "feature/story-216_setup_test", "216_setup_test").unwrap(); - // The output must mention pnpm install, proving the new code path ran. + // The output must mention component setup, proving the new code path ran. assert!( - result.output.contains("pnpm install"), - "merge output must mention pnpm install when frontend/ exists, got:\n{}", + result.output.contains("component setup"), + "merge output must mention component setup when project.toml has components, got:\n{}", + result.output + ); + // The sentinel command must appear in the output. + assert!( + result.output.contains("sentinel"), + "merge output must name the component, got:\n{}", result.output ); } - /// When `pnpm install` fails in the merge worktree (e.g. no lockfile), - /// the merge must abort cleanly — success=false, workspace cleaned up. + /// When there are no `[[component]]` entries in project.toml (or no + /// project.toml at all), `run_squash_merge` must succeed without trying to + /// run any setup. No hardcoded pnpm or frontend/ references should appear. #[cfg(unix)] #[test] - fn squash_merge_aborts_when_pnpm_install_fails() { + fn squash_merge_succeeds_without_components_in_project_toml() { use std::fs; use tempfile::tempdir; @@ -6285,33 +6251,25 @@ theirs let repo = tmp.path(); init_git_repo(repo); - // Add a frontend/ directory with an invalid package.json that will - // cause pnpm install --frozen-lockfile to fail (no lockfile present). - let frontend = repo.join("frontend"); - fs::create_dir_all(&frontend).unwrap(); - fs::write( - frontend.join("package.json"), - r#"{"name":"test","version":"0.0.0","dependencies":{"nonexistent-pkg-xyz":"99.99.99"}}"#, - ) - .unwrap(); + // No .story_kit/project.toml — no component setup. + fs::write(repo.join("file.txt"), "initial").unwrap(); Command::new("git") .args(["add", "."]) .current_dir(repo) .output() .unwrap(); Command::new("git") - .args(["commit", "-m", "add frontend with bad deps"]) + .args(["commit", "-m", "initial commit"]) .current_dir(repo) .output() .unwrap(); - // Feature branch with a change. Command::new("git") - .args(["checkout", "-b", "feature/story-154_fail"]) + .args(["checkout", "-b", "feature/story-216_no_components"]) .current_dir(repo) .output() .unwrap(); - fs::write(repo.join("change.txt"), "feature").unwrap(); + fs::write(repo.join("change.txt"), "change").unwrap(); Command::new("git") .args(["add", "."]) .current_dir(repo) @@ -6323,58 +6281,26 @@ theirs .output() .unwrap(); - // Switch back to master, record HEAD. Command::new("git") .args(["checkout", "master"]) .current_dir(repo) .output() .unwrap(); - let head_before = String::from_utf8( - Command::new("git") - .args(["rev-parse", "HEAD"]) - .current_dir(repo) - .output() - .unwrap() - .stdout, - ) - .unwrap() - .trim() - .to_string(); let result = - run_squash_merge(repo, "feature/story-154_fail", "154_fail").unwrap(); + run_squash_merge(repo, "feature/story-216_no_components", "216_no_components") + .unwrap(); - // pnpm install --frozen-lockfile should fail (no lockfile), merge aborted. + // No pnpm or frontend references should appear in the output. assert!( - !result.success, - "merge should fail when pnpm install fails" + !result.output.contains("pnpm"), + "output must not mention pnpm, got:\n{}", + result.output ); assert!( - result.output.contains("pnpm install"), - "output should mention pnpm install" - ); - - // Master HEAD must not have moved. - let head_after = String::from_utf8( - Command::new("git") - .args(["rev-parse", "HEAD"]) - .current_dir(repo) - .output() - .unwrap() - .stdout, - ) - .unwrap() - .trim() - .to_string(); - assert_eq!( - head_before, head_after, - "master HEAD must not advance when pnpm install fails (bug 154)" - ); - - // Merge workspace should be cleaned up. - assert!( - !repo.join(".story_kit/merge_workspace").exists(), - "merge workspace should be cleaned up after failure" + !result.output.contains("frontend/"), + "output must not mention frontend/, got:\n{}", + result.output ); }