Back to feed

sledtools/pika branch #131

ph-ci-head-guard

Guard ph against stale ci heads

Target branch: master

Merge Commit: cc8317faa8c7d4c142e79758a82a8db98ef88bf8

branch: merged tutorial: ready ci: success
Open CI Details

Continuous Integration

CI: success

Compact status on the review page, with full logs on the CI page.

Open CI Details

Latest run #166 success

2 passed

head 4c0b8d3b2782f236a6e388c16747427156d5f097 · queued 2026-03-27 00:29:08 · 2 lane(s)

queued 1m 12s · ran 28s

check-notifications · success check-agent-contracts · success

Summary

This branch adds a safety mechanism to the ph CLI tool that detects when CI results are stale — meaning the latest CI run was executed against a different commit than the current branch head. Without this guard, a user could mistakenly merge a branch whose green CI status was earned by an older commit, not the code that would actually be merged. The change introduces staleness checks into three key paths: the wait command (which polls until CI settles), the merge command (which lands branches), and the status renderer (which displays branch info). A pair of helper functions (latest_branch_run and stale_branch_ci_run) centralize the detection logic, and the polling snapshot format is extended to include head SHAs so that staleness transitions are observable. Four new tests cover the status warning, merge refusal, merge force-override, and wait failure scenarios.

Tutorial Steps

Add helper functions for detecting stale CI runs

Intent: Introduce two small utility functions that encapsulate the core staleness logic: `latest_branch_run` retrieves the first (most recent) CI run, and `stale_branch_ci_run` filters it to return `Some` only when the run's source head SHA differs from the branch's current head SHA. These are reused across all guard sites.

Affected files: crates/ph/src/commands.rs

Evidence
@@ -295,6 +319,14 @@ fn ensure_merge_ci_guard(branch: &BranchDetailResponse, force: bool) -> anyhow::
+fn latest_branch_run(branch: &BranchDetailResponse) -> Option<&crate::api::CiRun> {
+    branch.ci_runs.first()
+}
+
+fn stale_branch_ci_run(branch: &BranchDetailResponse) -> Option<&crate::api::CiRun> {
+    latest_branch_run(branch).filter(|run| run.source_head_sha != branch.branch.head_sha)
+}

Two new private functions are added at crates/ph/src/commands.rs:319-326:

  • latest_branch_run — returns branch.ci_runs.first(), relying on the API's convention that CI runs are ordered most-recent-first.
  • stale_branch_ci_run — chains a .filter() on latest_branch_run that checks whether the run's source_head_sha differs from branch.branch.head_sha. If they match, the CI is current and None is returned; if they differ, the stale run is returned for use in error messages.

This keeps the staleness predicate in one place, avoiding duplication across the wait, merge, and status code paths.

Guard the `cmd_wait` polling loop against stale CI heads

Intent: When `cmd_wait` detects that CI is no longer active, it previously checked only whether the overall status was success or failure. Now it first checks for staleness: if the latest run targeted a different commit than the branch head, the wait exits with a clear error instead of reporting a false success.

Affected files: crates/ph/src/commands.rs

Evidence
@@ -83,7 +83,14 @@ pub(crate) fn cmd_wait(
         if !branch_ci_active(&branch) {
-            return if branch.branch.ci_status.is_success() {
+            return if let Some(run) = stale_branch_ci_run(&branch) {
+                Err(anyhow!(
+                    "branch ci settled on stale head {} while branch head is {} (latest run #{})",
+                    short_sha(&run.source_head_sha),
+                    short_sha(&branch.branch.head_sha),
+                    run.id
+                ))
+            } else if branch.branch.ci_status.is_success() {

In the cmd_wait function (crates/ph/src/commands.rs:83), the conditional that fires when CI is no longer active now has a new first arm:

if let Some(run) = stale_branch_ci_run(&branch) {
    Err(anyhow!("branch ci settled on stale head ..."))
}

This takes priority over the existing success/failure checks. The error message includes the stale run's head SHA, the actual branch head SHA, and the run ID to help the user diagnose and rerun CI. The existing success and failure arms are preserved as else if / else branches for the non-stale case.

Extend the wait snapshot format to detect staleness transitions

Intent: The `branch_wait_snapshot` function produces a string that `cmd_wait` compares across polling iterations to decide whether to re-render status. Adding the branch head SHA and the latest run's head SHA to this snapshot ensures that a staleness change (e.g., a new push arriving mid-wait) triggers a re-render.

Affected files: crates/ph/src/commands.rs

Evidence
@@ -269,10 +276,15 @@ pub(crate) fn branch_wait_snapshot(branch: &BranchDetailResponse) -> String {
+    let latest_run_head = latest_branch_run(branch)
+        .map(|run| run.source_head_sha.as_str())
+        .unwrap_or_default();
     format!(
-        "{}|{}|{}",
+        "{}|{}|{}|{}|{}",
         branch.branch.ci_status,
         branch.ci_runs.len(),
+        branch.branch.head_sha,
+        latest_run_head,
         active

At crates/ph/src/commands.rs:276, the snapshot format string grows from three fields (ci_status|run_count|active_lanes) to five (ci_status|run_count|branch_head|latest_run_head|active_lanes). This means if the branch head changes (new push) or the latest CI run changes between polls, the snapshot string will differ and the wait loop will print updated status information.

latest_run_head is derived via latest_branch_run(branch) and falls back to an empty string when no runs exist.

Guard the merge command against stale CI heads

Intent: Prevent `ph merge` from landing a branch when the most recent CI run was against an outdated commit. The existing `ensure_merge_ci_guard` function is restructured to check for staleness before checking CI success, with `--force` as the only escape hatch.

Affected files: crates/ph/src/commands.rs

Evidence
@@ -285,7 +297,19 @@ pub(crate) fn branch_ci_active(branch: &BranchDetailResponse) -> bool {
 fn ensure_merge_ci_guard(branch: &BranchDetailResponse, force: bool) -> anyhow::Result<()> {
-    if force || branch.branch.ci_status.is_success() || branch_ci_active(branch) {
+    if force {
+        return Ok(());
+    }
+    if let Some(run) = stale_branch_ci_run(branch) {
+        return Err(anyhow!(
+            "refusing to merge branch #{} because the latest ci run #{} is for head {} while the branch head is {}. Rerun CI for the current head, or pass --force if absolutely necessary; --force is discouraged under normal circumstances.",
+            branch.branch.branch_id,
+            run.id,
+            short_sha(&run.source_head_sha),
+            short_sha(&branch.branch.head_sha)
+        ));
+    }
+    if branch.branch.ci_status.is_success() || branch_ci_active(branch) {

The ensure_merge_ci_guard function (crates/ph/src/commands.rs:297) is refactored from a single compound conditional into a sequence of early returns:

  1. --force check — if force is set, bypass all guards immediately.
  2. Staleness check — if the latest CI run targets a different commit, return a detailed error that names the branch ID, run ID, both SHAs, and advises rerunning CI or using --force.
  3. Success / active check — the original logic that allows merging when CI is green or still running.
  4. Fallthrough error — the existing message about CI not passing.

This ordering is important: force must come first so it always works, and staleness must come before the success check so a green-but-stale CI doesn't slip through.

Add a stale-head warning to the status renderer

Intent: When users run `ph status`, they should see an immediate visual warning if the latest CI run doesn't match the current branch head. This makes staleness visible even outside the merge/wait flows.

Affected files: crates/ph/src/commands.rs

Evidence
@@ -327,6 +359,13 @@ pub(crate) fn render_branch_status(branch: &BranchDetailResponse) -> String {
+        if run.source_head_sha != branch.branch.head_sha {
+            lines.push(format!(
+                "warning: latest ci run is for head {} but branch head is {}",
+                short_sha(&run.source_head_sha),
+                short_sha(&branch.branch.head_sha)
+            ));
+        }

In render_branch_status (crates/ph/src/commands.rs:359), right after printing the latest CI run's status line, a new conditional block compares run.source_head_sha against branch.branch.head_sha. When they differ, a warning line is appended:

warning: latest ci run is for head <stale> but branch head is <current>

This gives the user an at-a-glance indicator that their CI results may not reflect the code currently on the branch, prompting them to rerun CI before merging.

Add test for the status renderer stale-head warning

Intent: Verify that `render_branch_status` emits the expected warning text when the CI run's source head doesn't match the branch head.

Affected files: crates/ph/src/tests.rs

Evidence
@@ -306,6 +306,47 @@ fn branch_status_renders_waiting_and_failure_details() {
+#[test]
+fn branch_status_warns_when_latest_ci_run_is_for_stale_head() {
+    let branch = BranchDetailResponse {
+        branch: BranchSummary {
+            head_sha: "cafebabecafebabe".to_string(),
+            ...
+        },
+        ci_runs: vec![CiRun {
+            id: 12,
+            source_head_sha: "deadbeefdeadbeef".to_string(),
+            ...
+        }],
+    };
+    let rendered = render_branch_status(&branch);
+    assert!(rendered.contains("warning: latest ci run is for head deadbeefdead but branch head is cafebabecafe"));

A new unit test at crates/ph/src/tests.rs:308 constructs a BranchDetailResponse where the branch head is cafebabecafebabe but the sole CI run targets deadbeefdeadbeef. It calls render_branch_status and asserts that:

  1. The run summary line (run #12 success head deadbeefdead) is present.
  2. The warning line (warning: latest ci run is for head deadbeefdead but branch head is cafebabecafe) is present.

The SHAs are intentionally long to verify that short_sha truncates them correctly (to 12 hex characters).

Add integration test for merge refusal on stale CI

Intent: Ensure that `cmd_merge` returns an error (and never calls the merge endpoint) when the latest CI run is green but for a stale commit, and that `--force` is mentioned in the error.

Affected files: crates/ph/src/tests.rs

Evidence
@@ -733,6 +774,222 @@ fn merge_force_overrides_failed_ci_guard() {
+#[test]
+fn merge_refuses_stale_successful_ci_without_force() {
+    ...
+    let err = cmd_merge(&cli, Some("feature/stale-green"), false).expect_err("merge blocked");
+    assert!(err.to_string().contains("latest ci run #6 is for head deadbeef"));
+    assert!(err.to_string().contains("branch head is cafebabe"));
+    assert!(err.to_string().contains("--force"));
+    assert_eq!(merge_auth.load(Ordering::SeqCst), 0);

At crates/ph/src/tests.rs:776, a full integration test spins up a mock HTTP server that returns branch detail with head_sha: "cafebabe" but a CI run targeting source_head_sha: "deadbeef" with status success. It then invokes cmd_merge without --force and asserts:

  • The command returns an error.
  • The error mentions latest ci run #6 is for head deadbeef.
  • The error mentions branch head is cafebabe.
  • The error mentions --force as an escape hatch.
  • The merge endpoint was never called (merge_auth counter stays at 0).

This validates the full request path from CLI parsing through the guard to the error surface.

Add integration test for wait failure on stale CI

Intent: Verify that `cmd_wait` returns an error when CI settles as successful but the run was for a stale head, preventing the user from proceeding under false confidence.

Affected files: crates/ph/src/tests.rs

Evidence
@@ -733,6 +774,222 @@
+#[test]
+fn wait_returns_error_when_successful_ci_is_for_stale_head() {
+    ...
+    let err = cmd_wait(&cli, ..., 0).expect_err("stale green should fail wait");
+    assert!(err.to_string().contains("stale head deadbeef"));
+    assert!(err.to_string().contains("branch head is cafebabe"));

At crates/ph/src/tests.rs (within the new test block), another integration test sets up a mock server returning a settled, successful CI run for deadbeef while the branch head is cafebabe. It calls cmd_wait with --poll-secs 0 to avoid real delays and asserts:

  • The command returns an error (not success).
  • The error contains stale head deadbeef.
  • The error contains branch head is cafebabe.

This ensures the wait command doesn't silently report success when CI results are outdated, which would be dangerous in automated pipelines that chain ph wait && ph merge.

Diff