From afa982176f50308328ea8ecc76f1caff25e55b4f Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Mon, 11 May 2026 11:22:14 -0400 Subject: [PATCH] Fix MCP run search submitted ordering --- lib/crates/fabro-cli/tests/it/cmd/mcp.rs | 68 +++++++++++++++++++- lib/crates/fabro-mcp-server/src/run_tools.rs | 4 +- 2 files changed, 69 insertions(+), 3 deletions(-) diff --git a/lib/crates/fabro-cli/tests/it/cmd/mcp.rs b/lib/crates/fabro-cli/tests/it/cmd/mcp.rs index b2f162703..79b37aaf8 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/mcp.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/mcp.rs @@ -12,11 +12,12 @@ use std::io::{BufRead as _, Write as _}; use std::path::{Path, PathBuf}; use std::process::Stdio; -use chrono::{Duration as ChronoDuration, Utc}; +use chrono::{DateTime, Duration as ChronoDuration, Utc}; use fabro_client::{AuthEntry, AuthStore, DevTokenEntry, OAuthEntry, StoredSubject}; use fabro_mcp::client::McpClient; use fabro_mcp::config::{McpServerSettings, McpTransport}; use fabro_test::{fabro_json_snapshot, fabro_snapshot, test_context}; +use fabro_types::RunId; use httpmock::Method::{GET, POST}; use httpmock::MockServer; @@ -872,6 +873,64 @@ async fn mcp_search_orders_by_started_timestamp_before_created_timestamp() { .expect("MCP client should shut down"); } +#[tokio::test(flavor = "multi_thread")] +async fn mcp_search_orders_submitted_runs_by_created_timestamp_not_run_id_timestamp() { + let context = test_context!(); + let server = MockServer::start(); + let target_url = format!("{}/api/v1", server.base_url()); + let target: fabro_client::ServerTarget = target_url.parse().unwrap(); + seed_dev_token_auth(&context.home_dir, &target, TEST_DEV_TOKEN); + let newer_created_id = run_id_with_timestamp("2026-04-05T12:00:00Z", 1); + let older_created_id = run_id_with_timestamp("2026-04-05T12:40:00Z", 1); + let mut newer_created = remote_run_summary_json( + &newer_created_id, + "Simple", + "simple", + "Created later", + &serde_json::json!({ "kind": "submitted" }), + "2026-04-05T12:30:00Z", + ); + newer_created["timestamps"]["started_at"] = serde_json::Value::Null; + let mut older_created = remote_run_summary_json( + &older_created_id, + "Simple", + "simple", + "Created earlier", + &serde_json::json!({ "kind": "submitted" }), + "2026-04-05T12:10:00Z", + ); + older_created["timestamps"]["started_at"] = serde_json::Value::Null; + let list_runs = server.mock(|when, then| { + when.method(GET) + .path("/api/v1/runs") + .query_param("include_archived", "true") + .query_param("page[limit]", "100") + .query_param("page[offset]", "0"); + then.status(200) + .header("Content-Type", "application/json") + .json_body(serde_json::json!({ + "data": [newer_created, older_created], + "meta": { "has_more": false } + })); + }); + + let client = spawn_mcp_client(&context, &["--server", &target_url]).await; + let result = call_tool_json( + &client, + "fabro_run_search", + serde_json::json!({ "run_ids": [newer_created_id, older_created_id], "first": 2 }), + ) + .await; + + assert_eq!(result["runs"][0]["run_id"], newer_created_id); + assert_eq!(result["runs"][1]["run_id"], older_created_id); + list_runs.assert(); + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + #[tokio::test(flavor = "multi_thread")] async fn mcp_lifecycle_tools_manage_real_run() { let context = test_context!(); @@ -2054,3 +2113,10 @@ fn normalize_gather(mut value: serde_json::Value) -> serde_json::Value { } value } + +fn run_id_with_timestamp(timestamp: &str, sequence: u128) -> String { + let timestamp = DateTime::parse_from_rfc3339(timestamp) + .expect("test timestamp should parse") + .with_timezone(&Utc); + RunId::with_timestamp(timestamp, sequence).to_string() +} diff --git a/lib/crates/fabro-mcp-server/src/run_tools.rs b/lib/crates/fabro-mcp-server/src/run_tools.rs index 681f83c93..1b8a91e61 100644 --- a/lib/crates/fabro-mcp-server/src/run_tools.rs +++ b/lib/crates/fabro-mcp-server/src/run_tools.rs @@ -402,8 +402,8 @@ pub(crate) async fn search_runs( .await .map_err(|err| ToolError::from_anyhow(&err))?; runs.sort_by(|a, b| { - let a_sort_time = a.timestamps.started_at.unwrap_or_else(|| a.id.created_at()); - let b_sort_time = b.timestamps.started_at.unwrap_or_else(|| b.id.created_at()); + let a_sort_time = a.timestamps.started_at.unwrap_or(a.timestamps.created_at); + let b_sort_time = b.timestamps.started_at.unwrap_or(b.timestamps.created_at); b_sort_time.cmp(&a_sort_time).then_with(|| b.id.cmp(&a.id)) });