mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-06 08:18:58 +00:00
fix(mcp): bound search goal previews
This commit is contained in:
parent
8774abd786
commit
ab78422c35
2 changed files with 81 additions and 3 deletions
|
|
@ -18,13 +18,13 @@ None currently open.
|
|||
- **I10 — Archived runs not filtered from default search**: fixed on 2026-05-11 by aligning MCP search with the HTTP API. `fabro_run_search` now hides archived runs when `archived` is omitted, while `archived=true` still searches archived runs explicitly.
|
||||
- **I15 / I16 — yes/no answer flow**: re-tested on 2026-05-11 against `fabro server` `0.230.0-nightly.0` at `127.0.0.1:32276`. `answer=true` and `answer=false` both submit successfully for the bundled `interview` workflow's first `yes_no` question. `true` advanced the run to the next `confirmation` question.
|
||||
- **I22 — numeric answer local validation**: re-tested on 2026-05-11 against the same server. `answer=42` now returns `unsupported answer value: 42; expected boolean, string, or object` from the MCP layer before reaching the API.
|
||||
- **Section 2 side observation — Search payloads include full `goal` text**: fixed on 2026-05-11. `fabro_run_search` now returns bounded `goal_preview` plus `goal_truncated` instead of the full `goal`, keeping list responses compact while preserving full summaries on other run interactions.
|
||||
- **X6 — Cursor/filter ordering**: simplified on 2026-05-11 by applying search filters before sorting and applying the `after` cursor. This prevents unrelated runs outside the filtered result set from trimming the page. Pagination is explicitly not snapshot-isolated; a new matching run inserted before the cursor during traversal appears when the client starts a new search.
|
||||
|
||||
### UX / polish
|
||||
- **C12 — `cwd` errors don't distinguish "directory missing" from "workflow not in directory"**: both return `workflow not found: <slug>`.
|
||||
- **S9 (bonus) — Undocumented date format**: error message reveals `YYYY-MM-DD` is accepted alongside RFC3339, but the schema only says RFC3339.
|
||||
- **S17 — `run_ids` accepts more than IDs**: error message reveals it also matches ID prefixes and workflow names. Either rename the field or document.
|
||||
- **Section 2 side observation — Search payloads include full `goal` text**: a single long-goal run (e.g. `ImplementPlan`) inflates every search response by ~30 KB. Consider truncation or excluding `goal` from list responses.
|
||||
- **E4 — Events `search` is whole-envelope substring match**: search includes embedded payloads (workflow definitions, settings, sandbox dockerfile, etc.), so a search like `query="list_prs"` legitimately matches the `run.created` event because that event embeds the workflow JSON. Easy to misinterpret. Consider documenting or scoping search to event body only.
|
||||
|
||||
### Nice-to-haves
|
||||
|
|
|
|||
|
|
@ -10,6 +10,8 @@ use serde::{Deserialize, Serialize};
|
|||
use super::common;
|
||||
use super::common::{RunSummaryResult, ToolError, ToolResult};
|
||||
|
||||
const SEARCH_GOAL_PREVIEW_CHARS: usize = 240;
|
||||
|
||||
#[derive(Debug, Deserialize, JsonSchema)]
|
||||
pub(crate) struct FabroRunSearchParams {
|
||||
pub(crate) run_ids: Option<Vec<String>>,
|
||||
|
|
@ -68,10 +70,27 @@ impl TryFrom<FabroRunSearchParams> for ValidatedSearchRuns {
|
|||
|
||||
#[derive(Debug, Serialize, JsonSchema)]
|
||||
pub(crate) struct SearchRunsResult {
|
||||
pub(crate) runs: Vec<RunSummaryResult>,
|
||||
pub(crate) runs: Vec<SearchRunSummaryResult>,
|
||||
pub(crate) next_cursor: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Serialize, JsonSchema)]
|
||||
pub(crate) struct SearchRunSummaryResult {
|
||||
pub(crate) run_id: String,
|
||||
pub(crate) workflow_name: String,
|
||||
pub(crate) workflow_slug: Option<String>,
|
||||
pub(crate) status: String,
|
||||
pub(crate) archived: bool,
|
||||
pub(crate) created_at: String,
|
||||
pub(crate) started_at: Option<String>,
|
||||
pub(crate) completed_at: Option<String>,
|
||||
pub(crate) labels: HashMap<String, String>,
|
||||
pub(crate) source_directory: Option<String>,
|
||||
pub(crate) repo_origin_url: Option<String>,
|
||||
pub(crate) goal_preview: String,
|
||||
pub(crate) goal_truncated: bool,
|
||||
}
|
||||
|
||||
pub(crate) async fn search_runs(
|
||||
client: Arc<Client>,
|
||||
params: ValidatedSearchRuns,
|
||||
|
|
@ -89,11 +108,58 @@ pub(crate) async fn search_runs(
|
|||
let page = filter_sort_and_page_runs(runs, &raw, status.as_deref())?;
|
||||
|
||||
Ok(SearchRunsResult {
|
||||
runs: page.runs.iter().map(common::run_summary_result).collect(),
|
||||
runs: page.runs.iter().map(search_run_summary_result).collect(),
|
||||
next_cursor: page.next_cursor,
|
||||
})
|
||||
}
|
||||
|
||||
fn search_run_summary_result(run: &Run) -> SearchRunSummaryResult {
|
||||
let RunSummaryResult {
|
||||
run_id,
|
||||
workflow_name,
|
||||
workflow_slug,
|
||||
status,
|
||||
archived,
|
||||
created_at,
|
||||
started_at,
|
||||
completed_at,
|
||||
labels,
|
||||
source_directory,
|
||||
repo_origin_url,
|
||||
goal,
|
||||
} = common::run_summary_result(run);
|
||||
let (goal_preview, goal_truncated) = goal_preview(&goal);
|
||||
|
||||
SearchRunSummaryResult {
|
||||
run_id,
|
||||
workflow_name,
|
||||
workflow_slug,
|
||||
status,
|
||||
archived,
|
||||
created_at,
|
||||
started_at,
|
||||
completed_at,
|
||||
labels,
|
||||
source_directory,
|
||||
repo_origin_url,
|
||||
goal_preview,
|
||||
goal_truncated,
|
||||
}
|
||||
}
|
||||
|
||||
fn goal_preview(goal: &str) -> (String, bool) {
|
||||
let mut chars = goal.chars();
|
||||
let mut preview = chars
|
||||
.by_ref()
|
||||
.take(SEARCH_GOAL_PREVIEW_CHARS)
|
||||
.collect::<String>();
|
||||
let truncated = chars.next().is_some();
|
||||
if truncated {
|
||||
preview.push_str("...");
|
||||
}
|
||||
(preview, truncated)
|
||||
}
|
||||
|
||||
struct RunSearchPage {
|
||||
runs: Vec<Run>,
|
||||
next_cursor: Option<String>,
|
||||
|
|
@ -247,6 +313,18 @@ mod tests {
|
|||
assert_eq!(ids, vec![active.id]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn search_summary_uses_bounded_goal_preview() {
|
||||
let mut run = run("01KRBZW5C00000000000000001", "keep", 30);
|
||||
run.goal = format!("{}tail-marker", "a".repeat(300));
|
||||
|
||||
let summary = search_run_summary_result(&run);
|
||||
|
||||
assert!(summary.goal_truncated);
|
||||
assert!(summary.goal_preview.len() < run.goal.len());
|
||||
assert!(!summary.goal_preview.contains("tail-marker"));
|
||||
}
|
||||
|
||||
fn run(id: &str, group: &str, seconds: u32) -> Run {
|
||||
run_with_archived(id, group, seconds, false)
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue