fix(server): order AzDO synthetic steps and skip the job timeline record

Two defects on the timeline path, both instances of fixing only the path
I was looking at.

execution_key fell back to u32::MAX whenever runner_number was absent.
TimelineRecord carries no ordinal, so every synthetic step reported over
AzDO sorted after all declared steps — Set up job last, which is the
exact defect the ordering commit claims to prevent. The key now has three
tiers: the runner's reported number, then observed start time, then the
declared manifest position for steps that have not run.

The reconciliation loop also accepted the job's own record. Its id is a
UUID while job_id.0 is the workflow job key, so the existing guard never
matched and the job appeared as an extra step named after itself, both in
run projections and in the persisted rows. Only Task records are steps.

Reported on #212 by Cubic (ordering, P2 blocker) and CodeRabbit (job
record).

Author: Bnjoroge1

crates/preloop-runner-server/src/lib_tests.rs

@@ -2843,6 +2843,83 @@ async fn log_run_logs_unknown_job_is_404_not_whole_run() {
     );
 }
 
+/// The AzDO timeline path orders synthetic steps and ignores the job record.
+///
+/// `TimelineRecord` carries no ordinal, so a synthetic step reported this way
+/// has no `runner_number` and must be ordered by when it started — otherwise
+/// `Set up job` sorts after every declared step. The PATCH also carries the
+/// job's own record, whose UUID never equals the workflow job key, so it was
+/// reconciled in as an extra step named after the job.
+#[tokio::test]
+async fn timeline_path_orders_synthetic_steps_and_skips_the_job_record() {
+    let temp = tempfile::tempdir().unwrap();
+    let state = AppState::new(temp.path().to_path_buf()).await.unwrap();
+    let app = app(state.clone(), CancellationToken::new());
+    let (run_id, _) = three_step_run_for_log_filters(&app, &state).await;
+    let ids = workflow_step_ids(&state, run_id, "build").await;
+    let (plan_id, timeline_id) = {
+        let inner = state.inner.lock().await;
+        let request = inner
+            .job_requests
+            .values()
+            .find(|request| request.run_id == run_id)
+            .expect("dispatched request");
+        (request.plan_id.clone(), request.timeline_id.to_string())
+    };
+
+    // Setup started before the declared steps, and the job record is sent
+    // alongside them exactly as the official runner does.
+    let setup_id = uuid::Uuid::new_v4().to_string();
+    let record = |id: &str, kind: &str, name: &str, start: &str| {
+        json!({
+            "id": id,
+            "type": kind,
+            "name": name,
+            "displayName": name,
+            "state": "completed",
+            "result": "succeeded",
+            "startTime": start,
+        })
+    };
+    let response = request_json(
+        &app,
+        Method::PATCH,
+        &format!("/_apis/v1/Timeline/scope/actions/{plan_id}/{timeline_id}"),
+        json!({"count": 5, "value": [
+            record(&uuid::Uuid::new_v4().to_string(), "Job", "build", "2026-01-01T00:00:00Z"),
+            record(&ids[0], "Task", "Run echo one", "2026-01-01T00:00:02Z"),
+            record(&ids[1], "Task", "Run echo two", "2026-01-01T00:00:03Z"),
+            record(&ids[2], "Task", "Run echo three", "2026-01-01T00:00:04Z"),
+            record(&setup_id, "Task", "Set up job", "2026-01-01T00:00:01Z"),
+        ]}),
+    )
+    .await;
+    assert_eq!(response["count"], 5);
+
+    let run = get_run_json(&app, &run_id.to_string()).await;
+    let names: Vec<&str> = run["jobs_list"]
+        .as_array()
+        .unwrap()
+        .iter()
+        .find(|detail| detail["job_id"] == "build")
+        .expect("build detail")["steps"]
+        .as_array()
+        .unwrap()
+        .iter()
+        .map(|step| step["name"].as_str().unwrap_or_default())
+        .collect();
+    assert_eq!(
+        names,
+        vec![
+            "Set up job",
+            "Run echo one",
+            "Run echo two",
+            "Run echo three"
+        ],
+        "synthetic setup must lead, and the job record must not become a step"
+    );
+}
+
 /// Expansion does not leave the placeholder's manifest behind.
 ///
 /// A deferred-matrix node is dispatched as a placeholder, gets a manifest

crates/preloop-runner-server/src/models.rs

@@ -145,17 +145,33 @@ impl StepRecord {
 
     /// Sort key placing a step in execution order.
     ///
-    /// `runner_number` is the runner's own 1-based timeline position and counts
-    /// every step it ran, so it is the truth once reported. Until then a
-    /// declared step falls back to its manifest position, biased past any
-    /// reported number so freshly seeded steps trail what has actually run.
-    /// A step id is a v4 UUID and sorts randomly, so it is only the final
+    /// Three tiers, because the two reporting paths carry different evidence:
+    ///
+    /// 1. `runner_number` — the runner's own 1-based timeline position, which
+    ///    counts every step it ran. The broker path reports it, and it is the
+    ///    truth once present.
+    /// 2. `started_at` — when the server first saw the step run. The AzDO
+    ///    timeline path carries no ordinal (`TimelineRecord` has no `order`
+    ///    field), so a synthetic step there has no number; without this tier
+    ///    `Set up job` sorted after every declared step on that path, which is
+    ///    the defect this ordering exists to prevent.
+    /// 3. `workflow_index` — a declared step that has not run yet, ordered as
+    ///    the workflow declares and placed after everything that has run.
+    ///
+    /// A step id is a v4 UUID and sorts randomly, so it is only ever the final
     /// tie-break for determinism.
-    fn execution_key(&self) -> (u32, usize, &str) {
-        match self.runner_number {
-            Some(number) => (number, 0, self.id.as_str()),
-            None => (
-                u32::MAX,
+    fn execution_key(&self) -> (u8, i64, usize, &str) {
+        match (self.runner_number, self.started_at) {
+            (Some(number), _) => (0, i64::from(number), 0, self.id.as_str()),
+            (None, Some(started_at)) => (
+                1,
+                started_at.timestamp_micros(),
+                self.workflow_index.unwrap_or(usize::MAX),
+                self.id.as_str(),
+            ),
+            (None, None) => (
+                2,
+                0,
                 self.workflow_index.unwrap_or(usize::MAX),
                 self.id.as_str(),
             ),

crates/preloop-runner-server/src/timeline_logs.rs

@@ -156,6 +156,15 @@ pub(crate) async fn patch_timeline_records(
                     let Some(name) = &record.display_name else {
                         continue;
                     };
+                    // A PATCH carries the job's own record alongside its
+                    // steps. Its id is a UUID and `job_id.0` is the workflow
+                    // job key, so the id comparison below never matched it and
+                    // the job was reconciled in as a synthetic step — showing
+                    // up in run projections as an extra step named after the
+                    // job, and persisted there. Only task records are steps.
+                    if !matches!(record.record_type, Some(azdo::TimelineRecordType::Task)) {
+                        continue;
+                    }
                     if record.id.to_string() == job_id.0 {
                         continue;
                     }