From 6a86856ceebfd8f6ef9b3aa5da1416c6a2a0518f Mon Sep 17 00:00:00 2001
From: Steve Cliff <steve@devcloud.guru>
Date: Sun, 3 May 2026 22:43:57 +0100
Subject: [PATCH] agent/runner: ship repo.stats before job.finished in
 RunCheck/RunUnlock
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

RunCheck and RunUnlock were calling sendFinished before reportStats,
inverting the required job.started → log.stream → repo.stats →
job.finished envelope order. Move reportStats ahead of sendFinished in
both functions to match the pattern already correct in RunPrune.

Strengthen TestRunCheckShipsCheckStatus, TestRunCheckErrorsFoundShipsErrorsStatus,
and TestRunUnlockClearsLock with the same position-index ordering
assertions used by TestRunPruneShipsExpectedEnvelopes; these assertions
would have failed against the pre-fix code.
---
 internal/agent/runner/runner.go      |  6 ++-
 internal/agent/runner/runner_test.go | 79 ++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+), 2 deletions(-)

diff --git a/internal/agent/runner/runner.go b/internal/agent/runner/runner.go
index 423af45..9bf5ac6 100644
--- a/internal/agent/runner/runner.go
+++ b/internal/agent/runner/runner.go
@@ -276,7 +276,6 @@ func (r *Runner) RunCheck(ctx context.Context, jobID string, subsetPct int) erro
 	var seq atomic.Int64
 	res, err := env.RunCheck(ctx, subsetPct, r.streamHandler(jobID, &seq))
 	finishedAt := time.Now().UTC()
-	r.sendFinished(jobID, finishedAt, err, nil)
 
 	// Determine check status string.
 	checkStatus := "ok"
@@ -297,6 +296,8 @@ func (r *Runner) RunCheck(ctx context.Context, jobID string, subsetPct int) erro
 		slog.Warn("runner: stats.report after check failed", "job_id", jobID, "err", rerr)
 	}
 
+	r.sendFinished(jobID, finishedAt, err, nil)
+
 	if err != nil {
 		return fmt.Errorf("runner check: %w", err)
 	}
@@ -313,7 +314,6 @@ func (r *Runner) RunUnlock(ctx context.Context, jobID string) error {
 	var seq atomic.Int64
 	err := env.RunUnlock(ctx, r.streamHandler(jobID, &seq))
 	finishedAt := time.Now().UTC()
-	r.sendFinished(jobID, finishedAt, err, nil)
 
 	if err == nil {
 		lockFalse := false
@@ -323,6 +323,8 @@ func (r *Runner) RunUnlock(ctx context.Context, jobID string) error {
 		}
 	}
 
+	r.sendFinished(jobID, finishedAt, err, nil)
+
 	if err != nil {
 		return fmt.Errorf("runner unlock: %w", err)
 	}
diff --git a/internal/agent/runner/runner_test.go b/internal/agent/runner/runner_test.go
index 147b9ec..f32d6ee 100644
--- a/internal/agent/runner/runner_test.go
+++ b/internal/agent/runner/runner_test.go
@@ -143,6 +143,32 @@ esac
 		t.Fatalf("RunCheck: %v", err)
 	}
 
+	// Assert envelope ordering: job.started → log.stream → repo.stats → job.finished.
+	order := envelopeOrder(tx.envs)
+	wantTypes := []api.MessageType{api.MsgJobStarted, api.MsgLogStream, api.MsgRepoStats, api.MsgJobFinished}
+	positions := map[api.MessageType]int{}
+	for i, mt := range order {
+		if _, seen := positions[mt]; !seen {
+			positions[mt] = i
+		}
+	}
+	for i := 0; i < len(wantTypes)-1; i++ {
+		a, b := wantTypes[i], wantTypes[i+1]
+		pa, aOK := positions[a]
+		pb, bOK := positions[b]
+		if !aOK {
+			t.Errorf("envelope type %q not found in output %v", a, order)
+			continue
+		}
+		if !bOK {
+			t.Errorf("envelope type %q not found in output %v", b, order)
+			continue
+		}
+		if pa >= pb {
+			t.Errorf("expected %q before %q but positions are %d >= %d (order: %v)", a, b, pa, pb, order)
+		}
+	}
+
 	statsEnv := firstEnvOfType(t, tx.envs, api.MsgRepoStats)
 	var p api.RepoStatsPayload
 	if err := statsEnv.UnmarshalPayload(&p); err != nil {
@@ -180,6 +206,33 @@ esac
 		t.Fatalf("RunCheck: %v", err)
 	}
 
+	// Assert envelope ordering: job.started → repo.stats → job.finished.
+	// (No log.stream expected here because check exits immediately.)
+	order := envelopeOrder(tx.envs)
+	wantTypes := []api.MessageType{api.MsgJobStarted, api.MsgRepoStats, api.MsgJobFinished}
+	positions := map[api.MessageType]int{}
+	for i, mt := range order {
+		if _, seen := positions[mt]; !seen {
+			positions[mt] = i
+		}
+	}
+	for i := 0; i < len(wantTypes)-1; i++ {
+		a, b := wantTypes[i], wantTypes[i+1]
+		pa, aOK := positions[a]
+		pb, bOK := positions[b]
+		if !aOK {
+			t.Errorf("envelope type %q not found in output %v", a, order)
+			continue
+		}
+		if !bOK {
+			t.Errorf("envelope type %q not found in output %v", b, order)
+			continue
+		}
+		if pa >= pb {
+			t.Errorf("expected %q before %q but positions are %d >= %d (order: %v)", a, b, pa, pb, order)
+		}
+	}
+
 	statsEnv := firstEnvOfType(t, tx.envs, api.MsgRepoStats)
 	var p api.RepoStatsPayload
 	if err := statsEnv.UnmarshalPayload(&p); err != nil {
@@ -210,6 +263,32 @@ esac
 		t.Fatalf("RunUnlock: %v", err)
 	}
 
+	// Assert envelope ordering: job.started → log.stream → repo.stats → job.finished.
+	order := envelopeOrder(tx.envs)
+	wantTypes := []api.MessageType{api.MsgJobStarted, api.MsgLogStream, api.MsgRepoStats, api.MsgJobFinished}
+	positions := map[api.MessageType]int{}
+	for i, mt := range order {
+		if _, seen := positions[mt]; !seen {
+			positions[mt] = i
+		}
+	}
+	for i := 0; i < len(wantTypes)-1; i++ {
+		a, b := wantTypes[i], wantTypes[i+1]
+		pa, aOK := positions[a]
+		pb, bOK := positions[b]
+		if !aOK {
+			t.Errorf("envelope type %q not found in output %v", a, order)
+			continue
+		}
+		if !bOK {
+			t.Errorf("envelope type %q not found in output %v", b, order)
+			continue
+		}
+		if pa >= pb {
+			t.Errorf("expected %q before %q but positions are %d >= %d (order: %v)", a, b, pa, pb, order)
+		}
+	}
+
 	statsEnv := firstEnvOfType(t, tx.envs, api.MsgRepoStats)
 	var p api.RepoStatsPayload
 	if err := statsEnv.UnmarshalPayload(&p); err != nil {