dave/storkit
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 7 Wiki Activity

story-kit: merge 216_story_merge_quality_gates_should_use_project_toml_components_and_script_test_instead_of_hardcoded_frontend_pnpm

Browse Source
This commit is contained in:
Dave
2026-02-26 19:30:26 +00:00
parent 8de3591a5c
commit 8f0624f012
1 changed files with 95 additions and 169 deletions
Download Patch File Download Diff File Expand all files Collapse all files

264
server/src/agents.rs
View File

@@ -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<String> {
/// 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
);
}