diff --git a/.fabro/workflows/daytona-medium/workflow.fabro b/.fabro/workflows/daytona-medium/workflow.fabro new file mode 100644 index 000000000..1b801c763 --- /dev/null +++ b/.fabro/workflows/daytona-medium/workflow.fabro @@ -0,0 +1,11 @@ +digraph DaytonaMedium { + graph [goal="Verify the Daytona daytona-medium sandbox starts with standard tooling", retry_target=exit] + rankdir=LR + + start [shape=Mdiamond, label="Start"] + exit [shape=Msquare, label="Exit"] + + inspect [label="Inspect Sandbox", shape=parallelogram, goal_gate=true, script="set -e\nprintf 'cwd: '; pwd\nprintf 'user: '; whoami\nprintf 'git: '; git --version\nif command -v python3 >/dev/null; then printf 'python: '; python3 --version; else echo 'python: not installed'; fi\nif command -v node >/dev/null; then printf 'node: '; node --version; else echo 'node: not installed'; fi\nprintf 'top-level files:\\n'; ls -la | sed -n '1,40p'"] + + start -> inspect -> exit +} diff --git a/.fabro/workflows/daytona-medium/workflow.toml b/.fabro/workflows/daytona-medium/workflow.toml new file mode 100644 index 000000000..ec86fbdba --- /dev/null +++ b/.fabro/workflows/daytona-medium/workflow.toml @@ -0,0 +1,10 @@ +_version = 1 + +[workflow] +graph = "workflow.fabro" + +[run.sandbox] +provider = "daytona" + +[run.sandbox.daytona.snapshot] +name = "daytona-medium" diff --git a/Cargo.lock b/Cargo.lock index a013f06ec..05a03f41b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1741,7 +1741,9 @@ dependencies = [ "fabro-interview", "fabro-llm", "fabro-macros", + "fabro-manifest", "fabro-mcp", + "fabro-mcp-server", "fabro-model", "fabro-oauth", "fabro-proc", @@ -2066,6 +2068,24 @@ dependencies = [ "syn 2.0.117", ] +[[package]] +name = "fabro-manifest" +version = "0.230.0-nightly.0" +dependencies = [ + "anyhow", + "fabro-api", + "fabro-config", + "fabro-github", + "fabro-graphviz", + "fabro-template", + "fabro-types", + "fabro-workflow", + "git2", + "temp-env", + "tempfile", + "toml 0.8.23", +] + [[package]] name = "fabro-mcp" version = "0.230.0-nightly.0" @@ -2082,6 +2102,28 @@ dependencies = [ "tracing", ] +[[package]] +name = "fabro-mcp-server" +version = "0.230.0-nightly.0" +dependencies = [ + "anyhow", + "chrono", + "fabro-api", + "fabro-client", + "fabro-config", + "fabro-manifest", + "fabro-server", + "fabro-types", + "fabro-util", + "futures", + "rmcp", + "schemars 1.2.1", + "serde", + "serde_json", + "tokio", + "toml 0.8.23", +] + [[package]] name = "fabro-model" version = "0.230.0-nightly.0" @@ -2218,6 +2260,7 @@ dependencies = [ "fabro-interview", "fabro-llm", "fabro-macros", + "fabro-manifest", "fabro-model", "fabro-proc", "fabro-redact", diff --git a/docs/internal/mcp-server-qa-test-plan.md b/docs/internal/mcp-server-qa-test-plan.md new file mode 100644 index 000000000..c3f6a6b5e --- /dev/null +++ b/docs/internal/mcp-server-qa-test-plan.md @@ -0,0 +1,246 @@ +# Fabro MCP Server — QA Test Plan + +One-time manual QA pass for the 5 tools exposed by `fabro-mcp-server`. Source of truth: `lib/crates/fabro-mcp-server/src/run_tools/`. + +This plan is **not** a template for adding automated test coverage — it exists to drive a single hands-on sweep against a real running server. Tick boxes as scenarios pass; add notes inline for failures or surprising behavior. Open bugs/PRs for issues found; do not port these scenarios into the Rust test suite. + +## Findings rollup + +Live list of bugs and notable observations surfaced during the sweep. Each entry links back to the scenario where it was found. + +### Bugs / mismatches +None currently open. + +### Rechecked / no longer open +- **C4 — `inputs` schema/runtime mismatch**: fixed by narrowing MCP input values to scalar JSON (`string`, `boolean`, `integer`, `number`) and rejecting arrays/objects locally with scalar-only errors. Re-tested on 2026-05-11 against `127.0.0.1:32276`; `tools/list` now advertises scalar-only `inputs.additionalProperties`. +- **C5 — Misleading null-input error message**: fixed. Re-tested on 2026-05-11; null now returns ``input `maybe` cannot be null; use a string, boolean, or number``. +- **I7 / I9 — Misleading "Run not found." on terminal runs**: fixed on 2026-05-11 in the server API layer. `message`/steer against a durable terminal run that no longer has a live managed engine now returns `409` with `run_not_steerable`; `cancel` returns `409` with `Run is already terminal and cannot be cancelled.` True missing runs still return `404`. +- **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: `. +- **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. +- **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 +- **C16 — Helpful error**: unknown workflow lists available workflows. Keep. + +## Pre-flight (all tools) + +- [ ] **P1** Server unreachable — stop `fabro server`, call any tool, expect a clear connection-error message (not a panic, not a hang). +- [ ] **P2** Schema discovery — list tools through an MCP client; verify each tool has a complete JSON schema and the documented `anyOf` for `AnswerValue`. + +--- + +## 1. `fabro_run_create` + +Source: `run_tools/create.rs:124` + +### Happy path +- [x] **C1** Create one run from an existing workflow (e.g. `gh-list`); default `start=true` → expect `started=true`, `status` in `{queued, starting, running}`. — **PASS**. `status=queued`. +- [x] **C2** Create with `start=false` → expect `started=false`, `status=submitted`. — **PASS**. Run `01KRC4MP2NEQS9GJDE9FJ0EECH` kept as fixture for I3. +- [x] **C3** Batch create 5 runs in one call → all return; result preserves array order. — **PASS**. ULIDs monotonically increasing. + +### Inputs / manifest +- [x] **C4** Pass `inputs` with string / number / boolean / nested object / array → **PASS** after 2026-05-11 recheck. Scalar values are accepted. Arrays and objects are rejected locally with scalar-only errors, and the MCP schema now advertises scalar-only `inputs` values. +- [x] **C5** `inputs` containing `null` → **PASS** after 2026-05-11 recheck. Returns ``input `maybe` cannot be null; use a string, boolean, or number``. +- [x] **C6** `labels={"team": "qa"}` round-trip via search. — **PASS**. All 5 C3 runs returned with labels intact. +- [x] **C7** Optional flags: `goal`, `model+provider`, `sandbox`, `preserve_sandbox+auto_approve+dry_run`. — **PASS** all accepted; `goal` override round-tripped via search. +- [x] **C8** Custom `run_id`: valid ULID accepted (`01KRC500000000C8TEST00000A`); wrong length → `invalid length`; invalid Crockford char (e.g. `U`) → `invalid character`. — **PASS**. + +### `cwd` +- [x] **C9** Omit `cwd` → uses base CWD. — **PASS** (covered by every prior scenario). +- [x] **C10** `cwd` to repo root resolves workflow. — **PASS**. +- [x] **C11** `cwd=/tmp` (no `.fabro/workflows`) → `workflow not found: gh-list`. — **PASS**. +- [x] **C12** `cwd=/this/path/does/not/exist/xyz123` → same generic `workflow not found: gh-list`. — **PASS but note**: error doesn't distinguish "directory missing" from "workflow not in directory". Minor UX gap. + +### Validation +- [x] **C13** Empty `runs: []` → `runs must contain at least 1 item(s)`. — **PASS**. +- [x] **C14** 51 entries → `runs must contain no more than 50 item(s)`. — **PASS**. +- [x] **C15** Missing required `workflow` → MCP layer `-32602: missing field 'workflow'`. — **PASS**. +- [x] **C16** Unknown workflow slug → `Unknown workflow 'X'\n\nAvailable workflows: ...`. — **PASS** (very helpful — lists available workflows). + +### Failure semantics +- [x] **C17** Invalid sandbox name → `failed to resolve manifest settings: run.sandbox.provider: invalid value - unknown sandbox provider: this-sandbox-does-not-exist`. — **PASS**. Error raised at manifest-resolve time before any run record is created (no orphaned submitted run). + +--- + +## 2. `fabro_run_search` + +Source: `run_tools/search.rs:75` + +### Happy path +- [x] **S1** No params → returns up to 20 runs, sorted by `started_at OR created_at` desc. — **PASS**. Mixed-timestamp ordering correct (succeeded run at pos 8 sorts by its `started_at` between two `created_at`-only runs). +- [x] **S2** `first=5` → exactly 5; `next_cursor` is the last run's ID. — **PASS**. +- [x] **S3** `first=100` → all 17 runs, `next_cursor=null`. — **PASS**. +- [x] **S4** Cursor follow-through: page 1 IDs `[A, B]`, page 2 with `after=B` returns `[C, D]`. No overlap. — **PASS**. Note: cursors are run IDs, not opaque tokens. + +### Filters +- [x] **S5** `workflow="smoke"` (slug) and `workflow="Smoke"` (name) both match same run. — **PASS**. +- [x] **S6** `status=["succeeded"]` → 4; `["failed","dead"]` → 1; `["submitted"]` → 5. — **PASS**. +- [x] **S7** Labels round-trip. — **PASS** (verified via C6). +- [x] **S8** `archived=false` → all unarchived runs; `archived=true` → `[]` (no archived runs yet). Re-verify after I10. — **PARTIAL** (no archived fixtures yet). +- [x] **S9** `created_after`/`created_before` (RFC3339) bound results correctly; tight window `17:00–18:00` returns only old runs. — **PASS**. **Bonus**: error message reveals `YYYY-MM-DD` is also accepted — undocumented in the schema. +- [x] **S10** `run_ids=[A,B,A]` → 2 deduped runs. — **PASS**. +- [x] **S11** Combined `workflow + status + labels + archived` → returns exactly the 5 batch=c3 runs. — **PASS**. + +### Validation +- [x] **S12** `first=101` → `first must be <= 100`. — **PASS**. +- [x] **S13** `run_ids=[]` → `run_ids must contain at least 1 item(s)`. — **PASS**. +- [x] **S14** `run_ids` length 101 → `run_ids must contain no more than 100 item(s)`. — **PASS**. +- [x] **S15** `status=["bogus"]` → `unknown run status 'bogus'`. — **PASS**. +- [x] **S16** `created_after="not-a-date"` → `created_after must be RFC3339 or YYYY-MM-DD: input contains invalid characters`. — **PASS**. + +### Edge cases +- [x] **S17** Non-existent ID in `run_ids` → `No run found matching '' (tried run ID prefix and workflow name)`. — **PASS** + **finding**: `run_ids` also accepts ID prefixes and workflow names, which is broader than the field name suggests. +- [x] **S18** No matches → `{"runs": [], "next_cursor": null}`. — **PASS**. +- [x] **S19** Bogus `after=` → returns full first page (skip never applies). — **PASS** as documented. + +### Side observation +Search responses include the full `goal` text per run; a single `ImplementPlan` run can add ~30 KB to every search payload. Consider truncating `goal` (or excluding it from list responses) the way events have `max_content_length`. **Logged in Findings.** + +--- + +## 3. `fabro_run_gather` + +Source: `run_tools/gather.rs:56` + +### Happy path +- [x] **G1** Gather 1 already-terminal run → instant return, `timed_out=false`, `elapsed_seconds=0`. — **PASS**. +- [x] **G2** In-flight `gh-list` with `timeout=60, poll=5` → reaches `succeeded`, `timed_out=false`, `elapsed=30`. — **PASS**. +- [x] **G3** In-flight `gh-list` with `timeout=5, poll=5` → `timed_out=true`, `elapsed=5`, run still `starting`. — **PASS**. +- [x] **G4** Mix of 2 terminal + 1 in-flight, `timeout=90, poll=5` → all 3 succeeded, `timed_out=false`, `elapsed=40`. — **PASS**. + +### Validation +- [x] **G5** `run_ids=[]` → `run_ids must contain at least 1 item(s)`. — **PASS**. +- [x] **G6** 51 IDs → `run_ids must contain no more than 50 item(s)`. — **PASS**. +- [x] **G7** `timeout_seconds=601` → `timeout_seconds must be <= 600`. — **PASS**. +- [x] **G8** `poll_interval_seconds=4` → `poll_interval_seconds must be >= 5`. — **PASS**. +- [x] **G9** Omit both → call accepted; terminal run still returns instantly. Default values per source: `timeout=300, poll=15`. — **PASS**. + +### Edge cases +- [x] **G10** Non-existent run ID → `No run found matching '' (tried run ID prefix and workflow name)`. — **PASS** (same fuzzy match as search). +- [x] **G11** Poll cadence: G3 confirms last sleep clamps to deadline (`elapsed=5` exactly with `timeout=5, poll=5`). — **PASS** (inferred from G2/G3 timing). +- [ ] **G12** Run cancelled mid-gather → terminal `failed(status_reason=cancelled)` quickly. — **DEFERRED** to after I8 (cancel). +- [ ] **G13** Run becomes `blocked` — verify gather still waits. — **DEFERRED** to after Section 5 (interview workflow). + +--- + +## 4. `fabro_run_events` + +Source: `run_tools/events.rs:115` + +### Actions +- [x] **E1** `list` no filters → 45 events (`gh-list` has full lifecycle: run.*, sandbox.*, git.*, stage.*, etc.), `next_cursor=46`. — **PASS**. +- [x] **E2** `details` with 2 event_ids → returns exactly those 2 envelopes. — **PASS**. +- [x] **E3** `details` with no `event_ids` → `event_ids is required for details action`. — **PASS**. +- [x] **E4** `search query="list_prs"` → 14 events. Includes `run.created` because it embeds the full workflow definition (which contains the `list_prs` node ID). — **PASS** + **observation**: search ranges over the entire serialized envelope, so big embedded payloads (workflow defs, settings) can produce non-obvious hits. +- [x] **E5** `search` with missing `query` → `query is required for search action`. — **PASS**. + +### Filters +- [x] **E6** `event_types=["stage.started"]` → exactly 4 events (start, list_prs, list_issues, exit). — **PASS**. +- [x] **E7** `categories=["git","sandbox"]` → 12 events all with prefix `git.*` or `sandbox.*`. — **PASS**. +- [x] **E8** `created_after=17:03:10Z` + `created_before=17:03:13Z` → 5 events all timestamped 17:03:12.89x. — **PASS**. +- [x] **E9** Combined `event_types + offset + first` covered by E14. + +### Pagination & direction +- [x] **E10** Page 1 `first=10` → seqs 1–10, `next_cursor=11`. Page 2 `after=11, first=5` → seqs 11–15, `next_cursor=16`. No duplicates; contiguous. — **PASS**. +- [x] **E11** `direction=desc, first=5` → seqs 45, 44, 43, 42, 41; `next_cursor=41` (last seq, no +1 — per the desc branch). — **PASS**. +- [x] **E12** Default direction = asc (E10 confirms). — **PASS**. +- [x] **E13** `direction="weird"` → `direction must be 'asc' or 'desc'`. — **PASS**. +- [x] **E14** `event_types=["stage.started"], offset=2, first=5` → returned 2 events (seqs 29, 39) — correctly skipped the first 2 (15, 19) of the 4 matching. — **PASS**. +- [x] **E15** `limit=3` → 3 events. — **PASS** (alias works). + +### Truncation +- [x] **E16** `stage.completed, first=1, max_content_length=200` → 1 event, `truncated=true`, `event` is a JSON string. — **PASS**. +- [x] **E17** UTF-8 boundary — **VERIFIED via existing unit test** at `events.rs:269-312`. Can't easily reproduce through MCP surface (no multibyte event content in default fixtures). +- [x] **E18** Default `max_content_length=20000` → all 5 events `truncated=false` (including the ~5 KB `run.created`). — **PASS**. + +### Validation +- [x] **E19** `run_id=" "` (whitespace) → `run_id is required`. — **PASS**. +- [x] **E20** `first=201` → `first must be <= 200`. — **PASS**. +- [x] **E21** Non-existent run ID → fuzzy-match error (same as search/gather). — **PASS**. + +--- + +## 5. `fabro_run_interact` + +Source: `run_tools/interact.rs:201` + +### Actions + +#### `get` +- [x] **I1** Returns `{summary, projection}`; projection includes `spec`, `graph`, `status`, `checkpoints`, `pending_interviews`, `stages`, `sandbox`, `conclusion`, etc. — **PASS**. +- [x] **I2** Non-existent run → fuzzy match error. — **PASS**. + +#### `start` +- [x] **I3** Non-started run (from C2) → `start` transitions to `queued`. Second `start` → `an engine process is still running for this run — cannot start`. — **PASS**. + +#### `message` (steer) +- [ ] **I4** Steer a running LLM agent — **DEFERRED** (requires an active LLM agent stage; would burn LLM tokens; can be exercised manually once the answer bug below is resolved). +- [ ] **I5** `interrupt=true` — **DEFERRED** along with I4. +- [x] **I6** Missing `message` → `message is required for action message`. — **PASS**. +- [x] **I7** Message a terminal run → initially returned `Run not found.`. — **FIXED**: durable terminal runs without a live managed engine now return `409 run_not_steerable`; true missing runs remain `404`. + +#### `cancel` +- [x] **I8** Cancel a `gh-list` run during `starting`. Returns summary at request time (status=`starting`). Subsequent `gather` returned terminal `failed` within 5s; `get` projection shows `status: {kind: "failed", reason: "cancelled"}` and `conclusion.failure_reason: "Pipeline cancelled"`. — **PASS** + **observation**: `cancel`'s returned summary is a snapshot at request time, not the eventual terminal status. +- [x] **I9** Cancel an already-terminal run → initially returned `Run not found.`. — **FIXED**: durable terminal runs without a live managed engine now return `409` with `Run is already terminal and cannot be cancelled.`; true missing runs remain `404`. + +#### `archive` / `unarchive` +- [x] **I10** Archive terminal run → `archived=true` in summary; visible via `search archived=true`. — **FIXED**: default search now hides archived runs to match `/api/v1/runs`; `archived=true` still surfaces archived runs explicitly. +- [x] **I11** Unarchive → reverses (`archived=false`). — **PASS**. +- [x] **I12** Archive an active run → `run must be terminal (succeeded, failed, or dead) to archive; current status is starting`. — **PASS** (excellent error). + +#### `get_questions` +- [x] **I13** Terminal run → `questions: []`. — **PASS**. +- [x] **I14** Blocked interview run → returns full question record (id, text, options, question_type, stage, allow_freeform). — **PASS**. + +#### `answer` — `AnswerValue` shapes + +Re-check note: the earlier `yes_no` answer failure did not reproduce against `fabro server` `0.230.0-nightly.0` on `127.0.0.1:32276` (2026-05-11). Boolean answers are accepted for `yes_no` questions, and invalid question/type combinations are rejected by the API as expected. + +- [x] **I15** `answer=true` on the first `yes_no` question → submitted successfully (`submitted=true`) and advanced to the `confirmation` question. — **PASS**. Run `01KRCAQ9AS14KFCW4CXBZQ0CW9`. +- [x] **I16** `answer=false` on a fresh `yes_no` question → submitted successfully (`submitted=true`). — **PASS**. Run `01KRCATZ031CAPEPVB4CNFEE33`. +- [ ] **I17** `answer="some text"` — **NOT RE-TESTED**. Should be tested against a `freeform` question or a question with `allow_freeform=true`; text is not valid for the bundled `yes_no` question. +- [ ] **I18** `answer={"text":"hi"}` — **NOT RE-TESTED**. Same scope as I17. +- [x] **I19** `answer={"option":"Y"}` against the first `yes_no` question → `Answer does not match question type.` — **PASS / expectation corrected**. The MCP layer maps this shape to `selected`, but `server.rs:2670-2710` only accepts `yes`/`no` for `yes_no` and `confirmation`; `selected` belongs to `multiple_choice`. +- [ ] **I20** `answer={"options":[...]}` — **NOT RE-TESTED**. Should be tested against a `multi_select` question; `multi_selected` is not valid for `yes_no`. +- [x] **I21** `answer={"value":"yes"}` → `answer object must contain one of: option, options, text` (local validation). — **PASS**. +- [x] **I22** `answer=42` (number) → `unsupported answer value: 42; expected boolean, string, or object`. — **PASS** (local validation). +- [x] **I23** `answer={"option": 5}` → `answer option must be a string: invalid type: integer '5', expected a string`. — **PASS**. +- [x] **I24** `answer={"options": ["a", 2]}` → `answer options must be strings: invalid type: integer '2', expected a string`. — **PASS**. +- [x] **I25** `action=answer` without `question_id` → `question_id is required for action answer`. — **PASS**. +- [x] **I26** `action=answer` without `answer` → `answer is required for action answer`. — **PASS**. +- [x] **I27** Already-answered question — observed indirectly: the same question_id returned `Question no longer exists or was already answered.` on retry. — **PASS**. + +### Cross-cutting +- [x] **I28** `run_id=" "` → `run_id is required`. — **PASS**. +- [x] **I29** Action enum: `Get` and `get-questions` both rejected with `unknown variant 'X', expected one of: get, start, message, cancel, archive, unarchive, get_questions, answer`. — **PASS**. + +--- + +## 6. End-to-end scenarios (multi-tool) + +- [x] **X1 — Happy lifecycle** `gh-list` create → 35s gather → events filtered to `stage.started/completed` → 8 events for 4 stages (start, list_prs, list_issues, exit). Sequence matches workflow graph. — **PASS**. +- [x] **X2 — Cancel mid-run** Covered by I8: `gh-list` cancel during `starting` → gather returned terminal `failed` in 5s; projection shows `status_reason=cancelled`. — **PASS**. +- [ ] **X3 — Human-in-the-loop** — **PARTIAL**. The earlier yes/no answer blocker is no longer reproduced (I15/I16 now pass), and `gather` returning `timed_out=true` on a `blocked` run **was** verified (G13). Full interview completion remains unverified in this sweep. +- [ ] **X4 — Steering** — **DEFERRED** (requires active LLM agent). +- [x] **X5 — Archive flow** Covered by I10/I11: archive → search with `archived=true` returns it (also returned by default search — see I10 finding). Unarchive reverses. — **PASS** with caveat. +- [x] **X6 — Search/cursor under churn** Page 1 `first=3` → cursor saved. Created new run `01KRC625KG…` mid-flow. Page 2 with original cursor returned 3 older runs; a fresh page 1 placed the new run at position 1. — **ACCEPTED / SIMPLIFIED**. Pagination is not snapshot-isolated; clients that need newly inserted earlier results should restart the search. Code now applies filters before sorting/cursoring so unrelated runs outside the filtered result set do not trim filtered pages. +- [x] **X7 — Events while running** Started `gh-list` run, listed events `desc` immediately (max seq=8), gathered to completion, re-listed (max seq=46). Seq numbers grew monotonically; no early events lost. — **PASS**. +- [x] **X8 — Truncated event recovery** Fetched the `ImplementPlan` `run.created` event (embeds ~30 KB goal) at default `max_content_length=20000` → `truncated:true`, payload returned as a JSON string. — **PASS**. +- [ ] **X9 — Stranger inputs** — Skipped per scope decision. Trivially safe since inputs go through TOML conversion to be stored as values; the MCP layer never opens paths. + +--- + +## 7. Mechanics for the manual sweep + +- **Driver** — run these scenarios through an MCP client (e.g. Claude Code with the `fabro` MCP server configured) against a locally running `fabro server`. +- **Reusable run IDs** — keep a handful of already-terminal runs around (e.g. one `gh-list` succeeded, one failed `implement-plan`) as fixtures for `events`, `gather` (instant-return), `interact.get`, and `archive` scenarios. +- **Server unreachable cases** — stop the API server with the MCP client still connected to exercise error propagation paths. +- **Issue tracking** — file a GitHub issue per defect; link the scenario ID (e.g. `C13`) so this plan and the bugs cross-reference. diff --git a/docs/plans/2026-05-11-add-fabro-mcp-server-test-plan.md b/docs/plans/2026-05-11-add-fabro-mcp-server-test-plan.md new file mode 100644 index 000000000..d90a41459 --- /dev/null +++ b/docs/plans/2026-05-11-add-fabro-mcp-server-test-plan.md @@ -0,0 +1,292 @@ +# Fabro MCP Server Test Plan + +## Harness Requirements + +The agreed testing strategy still holds after reading the implementation plan. The plan narrows the tool contract to five Devin-shaped run tools and requires the implementation to live in a new `fabro-mcp-server` crate, but it does not add paid APIs, live LLM calls, external infrastructure, or browser/UI behavior. The highest-value evidence remains a real `fabro mcp start` subprocess driven over stdio and backed by Fabro's real local test server/auth harness. + +1. **Deterministic MCP stdio fixture** + - **Does:** constructs the exact command, environment, and cwd used to spawn `env!("CARGO_BIN_EXE_fabro") mcp start`. + - **Exposes:** `command: Vec`, `env: HashMap`, and `current_dir: PathBuf` usable by both `fabro_mcp::client::McpClient` and raw `std::process::Command` tests. + - **Complexity:** low. Add a narrow helper in `lib/crates/fabro-cli/tests/it/cmd/mcp.rs`; if needed, add `fabro_test::isolated_env(home_dir)` to mirror `apply_test_isolation`. + - **Tests depending on it:** 5, 6, 7, 8, 9, 10, 15, 16, 17. + +2. **MCP tool-call assertion helpers** + - **Does:** calls a named MCP tool, asserts tool success or tool error, extracts `structured_content`, and verifies fallback text is concise rather than a JSON dump. + - **Exposes:** `call_tool_json(...)`, `call_tool_error_text(...)`, and normalization helpers for run IDs, timestamps, paths, event IDs, cursors, durations, and elapsed times. + - **Complexity:** low to medium. Keep it local to `cmd/mcp.rs` unless more than one test file needs it. + - **Tests depending on it:** 8, 9, 10, 11, 12, 13, 15, 16, 17. + +3. **Real authenticated Fabro server fixture** + - **Does:** starts `RealAuthHarness::start_with_dev_token(...)`, seeds CLI dev-token auth into the test home, creates dry-run workflows through public CLI/MCP/API surfaces, and shuts down the server. + - **Exposes:** API target URL, persisted auth entry, HTTP client/server-visible state checks, and workflow fixture paths. + - **Complexity:** medium, mostly reuse existing `lib/crates/fabro-cli/tests/it/support/auth_harness.rs`. + - **Tests depending on it:** 8, 10, 11, 12, 13, 14, 17. + +## Test Plan + +1. **`fabro mcp` help exposes the MCP namespace** + - **Type:** integration + - **Disposition:** new + - **Harness:** output capture harness through existing `fabro_snapshot!` + - **Preconditions:** isolated `TestContext`; no auth or server required. + - **Actions:** run `fabro mcp --help`. + - **Expected outcome:** stdout snapshots a `Model Context Protocol server` namespace with `start`, `config`, and `init` subcommands; stderr is empty; exit status is 0. Source of truth: user request for `fabro mcp start`, `fabro mcp config`, `fabro mcp init `, and implementation plan CLI contract. + - **Interactions:** clap command tree, global CLI flags, snapshot filters. + +2. **`fabro mcp start --help` documents stdio startup options** + - **Type:** integration + - **Disposition:** new + - **Harness:** output capture harness through `fabro_snapshot!` + - **Preconditions:** isolated `TestContext`; no auth or server required. + - **Actions:** run `fabro mcp start --help`. + - **Expected outcome:** stdout snapshots usage `fabro mcp start [OPTIONS]` with `--server ` and `--storage-dir `; stderr is empty; exit status is 0. Source of truth: implementation plan CLI contract. + - **Interactions:** clap flattening for `ServerConnectionArgs`. + +3. **`fabro mcp config --help` documents config rendering options** + - **Type:** integration + - **Disposition:** new + - **Harness:** output capture harness through `fabro_snapshot!` + - **Preconditions:** isolated `TestContext`; no auth or server required. + - **Actions:** run `fabro mcp config --help`. + - **Expected outcome:** stdout snapshots usage and the same connection override flags as `start`; stderr is empty; exit status is 0. Source of truth: implementation plan CLI contract. + - **Interactions:** clap command help and global CLI flags. + +4. **`fabro mcp init --help` documents supported agent selection** + - **Type:** integration + - **Disposition:** new + - **Harness:** output capture harness through `fabro_snapshot!` + - **Preconditions:** isolated `TestContext`; no auth or server required. + - **Actions:** run `fabro mcp init --help`. + - **Expected outcome:** stdout snapshots required `` with supported values `claude`, `cursor`, and `windsurf`; exit status is 0. Source of truth: user request and implementation plan supported-agent contract. + - **Interactions:** clap value enum rendering. + +5. **`fabro mcp config` prints generic MCP client JSON** + - **Type:** integration + - **Disposition:** new + - **Harness:** output capture harness plus structured JSON parsing + - **Preconditions:** isolated `TestContext`; no auth or server required. + - **Actions:** run `fabro mcp config`; parse stdout as JSON. + - **Expected outcome:** stdout is valid JSON with `mcpServers.fabro.command == "fabro"` and `args == ["mcp", "start"]`; stderr is empty; exit status is 0. Source of truth: Daytona-shaped user request and implementation plan config JSON contract. + - **Interactions:** config rendering, stdout contract for a non-stdio command. + +6. **`fabro mcp config` preserves connection flags in generated startup args** + - **Type:** integration + - **Disposition:** new + - **Harness:** output capture harness plus structured JSON parsing + - **Preconditions:** isolated `TestContext`; no auth or server required. + - **Actions:** run `fabro mcp config --server https://example.test/api/v1 --storage-dir /tmp/fabro-mcp-storage`; parse stdout as JSON. + - **Expected outcome:** JSON contains `args == ["mcp", "start", "--server", "https://example.test/api/v1", "--storage-dir", "/tmp/fabro-mcp-storage"]`; stderr is empty; exit status is 0. Source of truth: implementation plan examples for flag preservation. + - **Interactions:** CLI argument forwarding into MCP client config. + +7. **`fabro mcp init ` writes idempotent config without clobbering unrelated keys** + - **Type:** integration + - **Disposition:** new + - **Harness:** direct filesystem artifact assertion in isolated home + - **Preconditions:** isolated `TestContext`; pre-existing Cursor config with `mcpServers.other` and unrelated top-level key. + - **Actions:** run `fabro mcp init cursor --server https://example.test/api/v1` twice; read `~/.cursor/mcp.json`. + - **Expected outcome:** parsed JSON preserves unrelated keys and existing `mcpServers.other`, contains exactly one `mcpServers.fabro` entry with command `fabro` and expected args, and the second run does not duplicate or reorder into an invalid shape. Source of truth: implementation plan idempotent config merge contract. + - **Interactions:** filesystem directory creation, JSON merge/write, test home isolation. + +8. **`fabro mcp init` writes each supported agent path** + - **Type:** integration + - **Disposition:** new + - **Harness:** direct filesystem artifact assertion in isolated home + - **Preconditions:** isolated `TestContext`; no existing Claude, Cursor, or Windsurf config. + - **Actions:** run `fabro mcp init claude`, `fabro mcp init cursor`, and `fabro mcp init windsurf` in separate contexts; read the platform-specific config file for each. + - **Expected outcome:** each config file exists at the path named by the implementation plan and contains `mcpServers.fabro` with `command: "fabro"` and `args: ["mcp", "start"]`. Source of truth: implementation plan agent path contract. + - **Interactions:** platform-specific path selection, filesystem writes. + +9. **`fabro mcp init` rejects invalid existing config without overwrite** + - **Type:** boundary + - **Disposition:** new + - **Harness:** output capture and filesystem artifact assertion + - **Preconditions:** isolated `TestContext`; Cursor config file contains invalid JSON bytes. + - **Actions:** run `fabro mcp init cursor`; read the same file after failure. + - **Expected outcome:** command exits non-zero with a clear error that includes the config path; the file content is byte-for-byte unchanged. Source of truth: implementation plan invalid JSON failure contract and error-handling strategy. + - **Interactions:** JSON parsing, write avoidance on error, CLI error rendering. + +10. **`fabro mcp start` initializes over stdio and lists the five run tools** + - **Type:** scenario + - **Disposition:** new + - **Harness:** interaction harness using deterministic MCP stdio fixture and `fabro_mcp::client::McpClient` + - **Preconditions:** isolated `TestContext`; no auth; no live Fabro server. + - **Actions:** spawn `fabro mcp start`; perform MCP `initialize`; call `tools/list`. + - **Expected outcome:** initialize succeeds without auth/server connectivity; `tools/list` returns exactly `fabro_run_create`, `fabro_run_search`, `fabro_run_interact`, `fabro_run_gather`, and `fabro_run_events`, each with an input schema. Source of truth: MCP lifecycle/tools spec as captured in the agreed strategy and implementation plan exact tool list. + - **Interactions:** `rmcp` stdio transport, existing `fabro-mcp` client crate, child process lifecycle. + +11. **`fabro mcp start` reserves stdout for JSON-RPC only** + - **Type:** regression + - **Disposition:** new + - **Harness:** raw subprocess stdio harness + - **Preconditions:** isolated `TestContext`; no auth; no live Fabro server. + - **Actions:** spawn `fabro mcp start`; write a JSON-RPC `initialize` request to stdin; read the first stdout line. + - **Expected outcome:** first stdout line parses as JSON and has `jsonrpc: "2.0"`; no leading human log/help text appears on stdout; stderr may contain logs. Source of truth: MCP stdio transport contract and implementation plan stdout invariant. + - **Interactions:** CLI logging initialization, raw process pipes, JSON-RPC framing. + +12. **MCP startup and tool discovery are fast without auth or server** + - **Type:** invariant + - **Disposition:** new + - **Harness:** interaction harness plus timing assertion + - **Preconditions:** isolated `TestContext`; no auth; no live Fabro server. + - **Actions:** measure elapsed time for spawning `fabro mcp start`, initializing, and calling `tools/list`. + - **Expected outcome:** operation completes under a generous smoke threshold, initially 2 seconds unless CI evidence requires a documented adjustment; all five tools are listed. Source of truth: agreed testing strategy performance smoke and implementation plan lazy API connection invariant. + - **Interactions:** process startup, `rmcp` initialization, tool schema generation. + +13. **`fabro_run_create` creates and starts a real dry-run using persisted CLI auth** + - **Type:** scenario + - **Disposition:** new + - **Harness:** interaction harness plus real authenticated Fabro server fixture + - **Preconditions:** `RealAuthHarness::start_with_dev_token(...)`; dev-token auth seeded into isolated home for the harness target; checked-in `simple.fabro` fixture installed. + - **Actions:** spawn `fabro mcp start --server `; call `fabro_run_create` with one run using `workflow`, `dry_run: true`, `auto_approve: true`, and label `source=mcp-test`. + - **Expected outcome:** tool result is not an MCP error; `structured_content.runs[0]` includes a run id, workflow, `started: true`, and status; fallback text exists and does not start with `{` or `[`; server-visible state contains the created run. Source of truth: user request for run-management MCP tools, implementation plan create semantics, OpenAPI `POST /api/v1/runs`, and `POST /api/v1/runs/{id}/start`. + - **Interactions:** persisted CLI auth store, Fabro API client, manifest builder/validation, run engine dry-run path. + +14. **`fabro_run_search` filters, paginates, and includes archived runs by default** + - **Type:** integration + - **Disposition:** new + - **Harness:** interaction harness plus real authenticated Fabro server fixture + - **Preconditions:** authenticated MCP server; at least two MCP-created dry-run runs with distinct labels; one terminal run archived through API or MCP. + - **Actions:** call `fabro_run_search` with `run_ids`, `workflow`, `labels`, `status`, `archived`, `first`, and `after` combinations. + - **Expected outcome:** results are normalized run summaries; filters include only matching runs; `first` limits page size and returns an opaque cursor when more results exist; archived runs appear unless `archived: false` is supplied. Source of truth: implementation plan search semantics and OpenAPI list-runs include-archived behavior adapted by the plan. + - **Interactions:** server run listing, status string normalization, timestamp/date parsing, cursor handling. + +15. **`fabro_run_interact get/start/message/cancel` uses selector resolution and server APIs** + - **Type:** integration + - **Disposition:** new + - **Harness:** interaction harness plus mocked HTTP server for precise API call assertions + - **Preconditions:** isolated `TestContext`; HTTP mock server with `/api/v1/runs/resolve`, `/runs/{id}`, `/state`, `/start`, `/steer`, and `/cancel` endpoints; CLI auth seeded if the mock requires auth. + - **Actions:** call `fabro_run_interact` with actions `get`, `start`, `message` with `interrupt: true`, and `cancel`, using a workflow-name selector rather than the exact run id. + - **Expected outcome:** each action first resolves the selector through `/runs/resolve`; calls the matching endpoint; returns a structured object with `run_id`, `action`, and action-specific `result`; tool errors are not produced for mocked successful API responses. Source of truth: implementation plan interact semantics and OpenAPI operation descriptions for retrieve, state, start, steer, and cancel. + - **Interactions:** run selector semantics, API error conversion, structured content projection. + +16. **`fabro_run_interact archive/unarchive` changes real server-visible archived state** + - **Type:** scenario + - **Disposition:** new + - **Harness:** interaction harness plus real authenticated Fabro server fixture + - **Preconditions:** authenticated MCP server; completed dry-run created through MCP or public CLI. + - **Actions:** call `fabro_run_interact` with `archive`; call `fabro_run_search` with `archived: true`; call `fabro_run_interact` with `unarchive`; call `fabro_run_search` with `archived: false`. + - **Expected outcome:** archive action succeeds for the terminal run; archived search shows the run; unarchive action succeeds; unarchived search shows the run as terminal and not archived. Source of truth: implementation plan interact actions and OpenAPI archive/unarchive contracts. + - **Interactions:** archive state transitions, list/search visibility, server-side idempotence. + +17. **`fabro_run_interact get_questions/answer` maps answer JSON to the API contract** + - **Type:** integration + - **Disposition:** new + - **Harness:** interaction harness plus mocked HTTP server for endpoint/body assertions + - **Preconditions:** isolated `TestContext`; HTTP mock server returns pending questions and accepts answer submissions. + - **Actions:** call `fabro_run_interact` with `get_questions`; call `answer` using representative payloads: `true`, `false`, string text, `{ "option": "a" }`, `{ "options": ["a", "b"] }`, and `{ "text": "hello" }`. + - **Expected outcome:** `get_questions` returns the API question list projection; `answer` sends `SubmitAnswerRequest` wire shapes with `kind: yes`, `no`, `text`, `selected`, and `multi_selected`, and returns a successful structured action result. Source of truth: implementation plan answer mapping and `lib/crates/fabro-api/tests/submit_answer_request_round_trip.rs`. + - **Interactions:** generated API type shape, JSON body serialization, human-in-the-loop endpoints. + +18. **`fabro_run_gather` waits for terminal runs and returns current state on timeout** + - **Type:** scenario + - **Disposition:** new + - **Harness:** interaction harness plus real authenticated Fabro server fixture + - **Preconditions:** authenticated MCP server; one completed dry-run and one submitted/non-terminal run available. + - **Actions:** call `fabro_run_gather` on the completed run; call it on the non-terminal run with `timeout_seconds: 1` and `poll_interval_seconds: 5`. + - **Expected outcome:** completed run result has `timed_out: false` and terminal status; timeout case returns a successful structured result with `timed_out: true`, current run summary, and bounded elapsed wall time rather than an MCP/process error. Source of truth: implementation plan gather semantics and agreed performance/timeout strategy. + - **Interactions:** selector resolution, polling loop, server retrieve endpoint, terminal status classification. + +19. **`fabro_run_events` lists, details, searches, filters, paginates, and truncates events** + - **Type:** integration + - **Disposition:** new + - **Harness:** interaction harness plus real authenticated Fabro server fixture + - **Preconditions:** authenticated MCP server; completed dry-run with stored events. + - **Actions:** call `fabro_run_events` with `action: "list"` and `first`; call `details` with returned event ids; call `search` with a known event-name substring; call filters for `event_types`, `categories`, `direction: "desc"`, `after`, `offset`, `limit`, and a small `max_content_length`. + - **Expected outcome:** returned events belong to the run; list ordering and pagination match requested parameters; details returns only requested event ids; search returns serialized events containing the query; category filtering uses event-name prefix; oversized serialized payloads are truncated with `truncated: true`; `next_cursor` is derived from the last returned sequence. Source of truth: implementation plan events semantics and OpenAPI `GET /api/v1/runs/{id}/events`. + - **Interactions:** event store pagination, event-name/category derivation, JSON serialization/truncation. + +20. **Local validation errors happen before auth or network lookup and do not stop the server** + - **Type:** boundary + - **Disposition:** new + - **Harness:** interaction harness with `--server http://127.0.0.1:9` and no auth + - **Preconditions:** isolated `TestContext`; no auth entry; unreachable server URL. + - **Actions:** call `fabro_run_gather` with 51 run ids; call `tools/list`; call `fabro_run_interact` action `message` without `message`; call `tools/list` again. + - **Expected outcome:** each invalid tool call returns an MCP tool error mentioning the invalid field (`run_ids` or `message`); no auth guidance or connection error masks the local validation failure; subsequent `tools/list` succeeds. Source of truth: implementation plan validate-before-client invariant and MCP tool-error contract. + - **Interactions:** parameter validation, lazy client initialization, MCP service liveness after errors. + +21. **Auth failures use existing Fabro login guidance and remain tool errors** + - **Type:** boundary + - **Disposition:** new + - **Harness:** interaction harness with protected real or mocked API target + - **Preconditions:** isolated `TestContext`; no saved auth for the target; server requires auth. + - **Actions:** spawn `fabro mcp start --server `; call a valid read tool such as `fabro_run_search`. + - **Expected outcome:** call returns an MCP tool error, not process exit; error text includes `Run \`fabro auth login\` to authenticate.`; subsequent `tools/list` still succeeds. Source of truth: user request for no separate MCP auth and implementation plan auth invariant. + - **Interactions:** auth store lookup, client connection, error classification/rendering. + +22. **Invalid create inputs are rejected with field-specific tool errors** + - **Type:** boundary + - **Disposition:** new + - **Harness:** interaction harness with no auth and unreachable server + - **Preconditions:** isolated `TestContext`; no auth entry. + - **Actions:** call `fabro_run_create` with empty `runs`, with 51 runs, and with `inputs` containing a null value. + - **Expected outcome:** each call returns an MCP tool error naming the invalid field/key before any auth/server error; server remains alive for a subsequent `tools/list`. Source of truth: implementation plan create validation and JSON-to-TOML null rejection. + - **Interactions:** schema/validation layer, JSON-to-TOML conversion. + +23. **Run tool successes always include structured content and concise text** + - **Type:** invariant + - **Disposition:** new + - **Harness:** MCP tool-call assertion helpers reused by scenario tests + - **Preconditions:** any successful calls from tests 13, 14, 16, 18, and 19. + - **Actions:** for each successful call, inspect `CallToolResult`. + - **Expected outcome:** `structured_content` is present; at least one text content item is present; text content is short and does not begin with `{` or `[`; `is_error` is absent or false. Source of truth: implementation plan successful tool-result invariant. + - **Interactions:** `rmcp::model::CallToolResult` construction and MCP client display fallback. + +24. **Pure conversion helpers cover JSON-to-TOML and answer-request mapping** + - **Type:** unit + - **Disposition:** new + - **Harness:** `cargo nextest run -p fabro-mcp-server run_tools` + - **Preconditions:** none beyond crate compilation. + - **Actions:** call conversion helpers directly for strings, bools, integers, floats, arrays, objects, null input, and every supported answer payload shape. + - **Expected outcome:** JSON-compatible input values map to equivalent `toml::Value`; null returns an error naming the key; answer payloads serialize to `SubmitAnswerRequest` wire JSON with documented `kind` values; unsupported answer objects return a tool error. Source of truth: implementation plan conversion requirements and `fabro-api` submit-answer round-trip tests. + - **Interactions:** serde, generated API types, conversion error text. + +25. **Existing MCP client crate behavior is not regressed** + - **Type:** regression + - **Disposition:** existing + - **Harness:** existing `fabro-mcp` crate tests + - **Preconditions:** repository builds with the new `fabro-mcp-server` crate added. + - **Actions:** run `cargo nextest run -p fabro-mcp`. + - **Expected outcome:** existing stdio client initialize/list/call tests pass. Source of truth: existing automated evidence and implementation plan decision to keep `fabro-mcp` as the external MCP client crate. + - **Interactions:** workspace dependency feature unification for `rmcp`, existing client transport behavior. + +26. **Relevant existing CLI run/auth regressions still pass** + - **Type:** regression + - **Disposition:** existing + - **Harness:** existing `fabro-cli` integration tests + - **Preconditions:** implementation complete. + - **Actions:** run the existing tests matching `scenario::auth::auth_login_refresh_logout_flow`, `scenario::lifecycle::dry_run_create_start_attach_works_with_default_run_lookup`, and `cmd::ps::ps_explicit_local_tcp_target_uses_auth_store`; if names drift, list tests and run the corresponding auth/lifecycle/local-target checks. + - **Expected outcome:** all selected tests pass unchanged. Source of truth: agreed strategy existing automated evidence and user requirement that MCP reuse CLI auth/config behavior. + - **Interactions:** auth refresh/logout, local server run lifecycle, server target resolution. + +27. **Final MCP command contract and workspace checks pass** + - **Type:** regression + - **Disposition:** extend + - **Harness:** repository command checks + - **Preconditions:** all feature implementation and snapshots complete. + - **Actions:** run `cargo nextest run -p fabro-cli --test it cmd::mcp`, `cargo +nightly-2026-04-14 fmt --check --all`, `cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings`, `ulimit -n 4096 && cargo nextest run --workspace`, and `cargo insta pending-snapshots`. + - **Expected outcome:** MCP command tests pass; formatting and clippy pass; workspace tests pass; no pending snapshots remain unless explicitly inspected and accepted for this feature. Source of truth: repository `AGENTS.md` build/test commands and snapshot policy. + - **Interactions:** entire workspace, rustfmt/clippy pinned nightly, nextest parallelism and file descriptor limit. + +## Coverage Summary + +Covered action space: + +- CLI executable commands: `fabro mcp --help`, `fabro mcp start --help`, `fabro mcp config --help`, `fabro mcp init --help`, `fabro mcp config`, `fabro mcp config --server --storage-dir`, and `fabro mcp init claude|cursor|windsurf`. +- MCP protocol actions: stdio process startup, `initialize`, `tools/list`, and `tools/call`. +- MCP tool actions: `fabro_run_create`; `fabro_run_search`; `fabro_run_interact` actions `get`, `start`, `message`, `cancel`, `archive`, `unarchive`, `get_questions`, `answer`; `fabro_run_gather`; `fabro_run_events` actions `list`, `details`, and `search`. +- Error and boundary behavior: invalid local parameters, too many run ids, null input conversion, missing action fields, unsupported answer shapes, invalid agent config JSON, missing auth, unreachable server after local validation, timeout expiry, and service liveness after tool errors. +- Integration boundaries: CLI auth store reuse, Fabro API client, real local Fabro server, run manifest construction/validation, event store, generated API answer types, and existing `fabro-mcp` client crate. +- Performance smoke: initialize plus `tools/list` without auth/server. + +Explicitly excluded per the agreed strategy: + +- Live LLM/provider tests. Dry-run workflows and local/mocked servers cover run-management behavior without external credentials or spend. +- Manual QA of agent apps. `init` tests assert Fabro's written config path and JSON merge contract, not whether Claude/Cursor/Windsurf accept the file in a live app. +- Browser/UI tests. This feature adds CLI and MCP stdio surfaces only. +- Differential tests against Daytona or Devin. Their docs inspired shape, but no runnable reference implementation is available or required. + +Residual risks: + +- Agent config formats may evolve externally; tests protect Fabro's chosen file/path contract only. +- MCP SDK behavior can change with `rmcp` upgrades; protocol tests and existing `fabro-mcp` tests should catch startup/list/call regressions. +- Full workspace tests may be slower and subject to local FD limits; use the documented `ulimit -n 4096` command before `cargo nextest run --workspace`. diff --git a/docs/plans/2026-05-11-add-fabro-mcp-server.md b/docs/plans/2026-05-11-add-fabro-mcp-server.md new file mode 100644 index 000000000..1e543a8a9 --- /dev/null +++ b/docs/plans/2026-05-11-add-fabro-mcp-server.md @@ -0,0 +1,1718 @@ +# Fabro MCP Server Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use trycycle-executing to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add a stdio MCP server to the `fabro` CLI, exposed as `fabro mcp start`, with `fabro mcp config` and `fabro mcp init ` support and first-class tools for managing Fabro runs. + +**Architecture:** Implement the Fabro MCP server in a new `fabro-mcp-server` crate, with `fabro-cli` owning only clap parsing and dispatch for `fabro mcp ...`. Use `rmcp` server macros and stdio transport for protocol correctness, connect lazily to the Fabro API through settings built from the same CLI auth/config inputs as existing Fabro commands, and return structured MCP tool results plus text fallbacks. Keep the existing `fabro-mcp` crate as the external MCP client/shared protocol support used by Fabro agents and tests, not as the server crate. + +**Tech Stack:** Rust, clap, tokio, new `fabro-mcp-server` crate, rmcp 1.3 stdio server transport, serde/schemars JSON schemas, fabro-client, fabro-api generated types, existing Fabro CLI integration test harness with insta snapshots. + +--- + +## File Structure + +- Create `lib/crates/fabro-mcp-server/Cargo.toml` + - New crate for the stdio MCP server implementation. Add direct `rmcp` dependency with server, macros, schemars, and stdio transport features, plus the Fabro crates needed for API access, run manifest construction, settings/auth-store resolution, and tests. The workspace already includes `lib/crates/*`, so no root workspace member edit is required. + +- Create `lib/crates/fabro-mcp-server/src/lib.rs` + - Export the server entry points and settings types consumed by `fabro-cli`: `McpServerSettings`, `McpConfigSettings`, `McpAgent`, `start(settings)`, `config_json(settings)`, and `init_agent(settings)`. + +- Create `lib/crates/fabro-mcp-server/src/server.rs` + - Own the stdio MCP service, tool registration, API client acquisition from explicit settings, and tool error shaping. + +- Create `lib/crates/fabro-mcp-server/src/run_tools.rs` + - Own run-management behavior behind the MCP tools: create/start, search, interact, gather, and events. + - This split keeps protocol boilerplate out of run semantics. + +- Create `lib/crates/fabro-mcp-server/src/config.rs` + - Own generic MCP config rendering and agent-specific config path/merge/write logic. + +- Modify `lib/crates/fabro-cli/Cargo.toml` + - Add a path dependency on the new `fabro-mcp-server` crate. `fabro-cli` should not depend directly on `rmcp` for the server implementation. + +- Modify `lib/crates/fabro-cli/src/args.rs` + - Add `McpNamespace`, `McpCommand`, `McpStartArgs`, `McpConfigArgs`, `McpInitArgs`, and `McpAgent`. + - Add `Commands::Mcp(McpNamespace)` and `Commands::name()` branch returning `mcp start`, `mcp config`, or `mcp init`. + +- Modify `lib/crates/fabro-cli/src/main.rs` + - Add `mod commands::mcp` dispatch. + - Keep `fabro mcp start` on the normal CLI logging path, which writes logs to stderr, and never write human output to stdout during stdio serving. + +- Modify `lib/crates/fabro-cli/src/commands/mod.rs` + - Export the new `mcp` command module. + +- Create `lib/crates/fabro-cli/src/commands/mcp/mod.rs` + - Own CLI dispatch for `start`, `config`, and `init`. + +- Modify `lib/crates/fabro-cli/src/commands/run/overrides.rs` + - If needed, move shared manifest override construction into a non-CLI crate or expose a small reusable helper without creating a dependency from `fabro-mcp-server` back to `fabro-cli`: + - label parsing + - goal layer construction + - execution/model/sandbox override construction + - Do not duplicate manifest override semantics in the MCP server crate. + +- Modify `lib/crates/fabro-cli/tests/it/cmd/mod.rs` + - Add `mod mcp;`. + +- Create `lib/crates/fabro-cli/tests/it/cmd/mcp.rs` + - Add CLI help/config/init snapshots and stdio MCP integration tests. + +- Optionally modify `lib/crates/fabro-cli/tests/it/support/mod.rs` + - Add only narrow helpers for spawning `fabro mcp start` or extracting MCP text/structured output if duplication appears in `cmd/mcp.rs`. + +Before editing Rust code, read: + +- `docs/internal/testing-strategy.md` because this plan adds CLI integration tests and unit tests. +- `docs/internal/error-handling-strategy.md` because MCP tool failures convert CLI/API/auth errors into user-visible tool errors. + +## User-Visible Contract + +The CLI contract is: + +```text +fabro mcp start [--server ] [--storage-dir ] +fabro mcp config [--server ] [--storage-dir ] +fabro mcp init [--server ] [--storage-dir ] +``` + +Supported agents for the first implementation: + +```text +claude +cursor +windsurf +``` + +`fabro mcp config` emits generic MCP client JSON to stdout: + +```json +{ + "mcpServers": { + "fabro": { + "command": "fabro", + "args": ["mcp", "start"] + } + } +} +``` + +When `--server` or `--storage-dir` is passed to `config` or `init`, preserve those choices in the emitted or written `args`, for example: + +```json +{ + "mcpServers": { + "fabro": { + "command": "fabro", + "args": ["mcp", "start", "--server", "https://example.test/api/v1"] + } + } +} +``` + +`fabro mcp init ` writes the same entry into the agent config file under `mcpServers.fabro`, preserving every unrelated existing key. Re-running it is idempotent. If the existing file is invalid JSON or its root is not an object, fail clearly and do not overwrite it. + +Agent config paths: + +- `claude` + - macOS: `~/Library/Application Support/Claude/claude_desktop_config.json` + - Linux: `~/.config/Claude/claude_desktop_config.json` + - Windows: `%APPDATA%\Claude\claude_desktop_config.json` +- `cursor` + - all platforms: `~/.cursor/mcp.json` +- `windsurf` + - all platforms: `~/.codeium/windsurf/mcp_config.json` + +The MCP server exposes exactly these tools in this first slice: + +```text +fabro_run_create +fabro_run_search +fabro_run_interact +fabro_run_gather +fabro_run_events +``` + +### Tool Semantics + +`fabro_run_create` + +- Input: + +```rust +#[derive(Debug, Deserialize, JsonSchema)] +struct FabroRunCreateParams { + runs: Vec, +} + +#[derive(Debug, Deserialize, JsonSchema)] +struct CreateRunSpec { + workflow: String, + cwd: Option, + run_id: Option, + goal: Option, + #[serde(default)] + inputs: HashMap, + #[serde(default)] + labels: HashMap, + dry_run: Option, + auto_approve: Option, + model: Option, + provider: Option, + sandbox: Option, + preserve_sandbox: Option, + start: Option, +} +``` + +- `runs` is required and must contain 1 to 50 entries. +- `workflow` is a workflow path or project workflow selector resolved from `cwd` when provided, otherwise from the MCP process cwd. +- `start` defaults to `true` because this is analogous to Devin session creation: creating a run for an agent should normally launch it. Passing `start: false` creates a submitted run without starting it. +- `inputs` object values are converted to `toml::Value` with JSON-compatible semantics: string, bool, integer, float, arrays, and objects are accepted; null is rejected with a tool error naming the key. +- Output is structured: + +```rust +#[derive(Debug, Serialize, JsonSchema)] +struct CreateRunsResult { + runs: Vec, +} + +#[derive(Debug, Serialize, JsonSchema)] +struct CreatedRunResult { + run_id: String, + workflow: String, + started: bool, + status: String, +} +``` + +`fabro_run_search` + +- Input: + +```rust +struct FabroRunSearchParams { + run_ids: Option>, + workflow: Option, + labels: Option>, + status: Option>, + archived: Option, + created_after: Option, + created_before: Option, + first: Option, + after: Option, +} +``` + +- Search starts from `Client::list_store_runs()`, which already includes archived runs. +- `status` uses existing `run_status_kind(...)` strings. +- `created_after` and `created_before` parse RFC3339 timestamps or `YYYY-MM-DD` dates. +- `first` defaults to 20 and has max 100. +- `after` is an opaque cursor containing the last run id from the previous page. For the first implementation, encode it as the run id string and document it as opaque in the tool description. +- Output contains normalized run summaries: + +```rust +struct RunSummaryResult { + run_id: String, + workflow_name: String, + workflow_slug: Option, + status: String, + archived: bool, + created_at: String, + started_at: Option, + completed_at: Option, + labels: HashMap, + source_directory: Option, + repo_origin_url: Option, + goal: String, +} +``` + +`fabro_run_interact` + +- Input: + +```rust +#[derive(Debug, Deserialize, JsonSchema)] +#[serde(rename_all = "snake_case")] +enum RunInteractAction { + Get, + Start, + Message, + Cancel, + Archive, + Unarchive, + GetQuestions, + Answer, +} + +struct FabroRunInteractParams { + action: RunInteractAction, + run_id: String, + message: Option, + interrupt: Option, + question_id: Option, + answer: Option, +} +``` + +- `run_id` accepts the same selector semantics as CLI commands by calling `Client::resolve_run(...)`. +- `get` returns summary plus projection from `retrieve_run` and `get_run_state`. +- `start` calls `start_run(resume = false)`. +- `message` calls `steer_run`; `message` is required and trimmed; `interrupt` defaults false. +- `cancel` calls `cancel_run`. +- `archive` and `unarchive` call existing API methods. +- `get_questions` calls `list_run_questions`. +- `answer` requires `question_id` and maps answer JSON into `SubmitAnswerRequest`: + - boolean true -> yes + - boolean false -> no + - string -> freeform + - `{ "option": "key" }` -> single choice + - `{ "options": ["a", "b"] }` -> multi choice + - `{ "text": "..." }` -> freeform +- Return a structured object with `run_id`, `action`, and action-specific `result`. + +`fabro_run_gather` + +- Input: + +```rust +struct FabroRunGatherParams { + run_ids: Vec, + timeout_seconds: Option, + poll_interval_seconds: Option, +} +``` + +- `run_ids` is required, max 50. +- `timeout_seconds` defaults to 300 and maxes at 600. +- `poll_interval_seconds` defaults to 15 and mins at 5. +- Resolve selectors once at the start. +- Poll `retrieve_run` until every run is terminal or timeout expires. +- Output contains each final or current run summary plus `timed_out: bool`. + +`fabro_run_events` + +- Input: + +```rust +#[derive(Debug, Deserialize, JsonSchema)] +#[serde(rename_all = "snake_case")] +enum RunEventsAction { + List, + Details, + Search, +} + +struct FabroRunEventsParams { + action: RunEventsAction, + run_id: String, + event_types: Option>, + categories: Option>, + direction: Option, + created_after: Option, + created_before: Option, + first: Option, + after: Option, + event_ids: Option>, + offset: Option, + limit: Option, + max_content_length: Option, + query: Option, +} +``` + +- Use `Client::list_run_events(...)` rather than SSE for deterministic request/response behavior. +- `list` returns paginated envelopes sorted ascending by default; `direction: "desc"` reverses after fetching. +- `details` filters by `event_ids`. +- `search` filters events whose serialized event JSON contains `query`. +- `event_types` match `event.event_name()`. +- `categories` are best-effort derived from the prefix before the first `.` in `event_name`, for example `run.completed` has category `run`. +- `first` defaults to 50 and maxes at 200. `limit` is accepted as an alias for compatibility with the Devin-shaped input. `after` maps to `since_seq`. +- `max_content_length` defaults to 20_000 and truncates only large serialized event payload strings, with a `truncated: true` marker in the returned event item. + +### Contracts And Invariants + +- `fabro mcp start` stdout is reserved for MCP JSON-RPC only. All logs, warnings, errors, tracing, and diagnostics must go to stderr. +- MCP initialize and tools/list must not require a live Fabro server. API connection is lazy and happens when a tool needs it. +- There is no separate MCP authentication. Tool calls use the same CLI auth store and `fabro-client` behavior as existing CLI commands. Auth failures returned from tools must include the existing user guidance: `Run \`fabro auth login\` to authenticate.` +- Tool failures are MCP tool errors, not process exits. The stdio server should stay alive after invalid arguments, not-found selectors, conflicts, auth failures, and API errors. +- Tool-level argument validation that does not need server state must run before acquiring the lazy Fabro API client. Every handler must convert raw MCP parameter structs into its tool-specific `Validated...` type before auth lookup, client creation, selector resolution, or API calls. Invalid local input such as empty run lists, too many run ids, malformed timestamps, missing required action fields, unsupported answer JSON, or timeout values must report that validation error even when the CLI is not authenticated or the server is unavailable. +- Every successful tool returns structured content and a concise text fallback. The text fallback is for clients that do not yet show MCP structured output. Do not return `rmcp::Json` directly from successful tools, because its text content is the full JSON payload. Instead, build a `CallToolResult` with `structured_content: Some(...)` and a short `Content::text(...)` summary. +- `rmcp 1.3` only accepts manually constructed `CallToolResult` values from tool handlers through `Result`. Do not use `Result` in `#[tool]` methods; it does not satisfy `IntoCallToolResult`. Expected Fabro failures must be returned as `Ok(CallToolResult::error(...))` so they are MCP tool errors and the server stays alive. Reserve `Err(ErrorData)` for unexpected serialization/framework failures. +- Run selectors must go through `Client::resolve_run(...)` to preserve existing Fabro prefix/workflow-name behavior. +- Run creation must reuse `build_run_manifest(...)` and server manifest validation. Do not fabricate run specs or bypass the same source-of-truth path as `fabro create`. +- Agent config writes must be idempotent and preserve unrelated user config. +- Do not add live LLM/provider tests for this first slice. Use dry-run workflows and local/test servers. + +## Strategy Decisions + +- **Implement the server in `fabro-mcp-server`:** The existing `fabro-mcp` crate remains the client/shared protocol support for agents consuming third-party MCP servers. The new `fabro-mcp-server` crate owns the Fabro server implementation and exposes explicit settings APIs so `fabro-cli` can wire `fabro mcp ...` commands without making the existing client crate a server crate. +- **Use `rmcp` instead of hand-rolled JSON-RPC:** The project already depends on `rmcp` and uses it for MCP client behavior. The server should use the same SDK to get initialize/tools/list/tools/call semantics, JSON schema generation, and stdio framing right. +- **Default create to start:** Devin's session creation starts usable sessions. For Fabro, a run that stays submitted unless the caller remembers a second tool call is a surprising first-use experience. `start: false` keeps the lower-level control available without making it the default. +- **Use five Devin-shaped tools instead of many tiny tools:** The user explicitly asked to adapt Devin sessions to Fabro runs. The five-tool shape is easier for MCP clients to discover and keeps later additions compatible. Internally, the Rust implementation should still split actions into small functions. +- **Lazy API connection:** MCP clients often list tools during startup. Requiring auth/server connectivity during initialize would make even configuration validation brittle. Lazy connection gives users useful tool discovery and clear per-tool auth errors. +- **Validate before connecting:** MCP clients often probe tools with incomplete or malformed payloads. Local validation must happen before API client acquisition so callers get actionable schema/argument errors instead of misleading auth or server availability failures. + +## Task 1: Add CLI Surface And Help Snapshots + +**Files:** +- Create: `lib/crates/fabro-mcp-server/Cargo.toml` +- Create: `lib/crates/fabro-mcp-server/src/lib.rs` +- Create: `lib/crates/fabro-mcp-server/src/config.rs` +- Create: `lib/crates/fabro-mcp-server/src/run_tools.rs` +- Create: `lib/crates/fabro-mcp-server/src/server.rs` +- Modify: `lib/crates/fabro-cli/Cargo.toml` +- Modify: `lib/crates/fabro-cli/src/args.rs` +- Modify: `lib/crates/fabro-cli/src/main.rs` +- Modify: `lib/crates/fabro-cli/src/commands/mod.rs` +- Create: `lib/crates/fabro-cli/src/commands/mcp/mod.rs` +- Create: `lib/crates/fabro-cli/tests/it/cmd/mcp.rs` +- Modify: `lib/crates/fabro-cli/tests/it/cmd/mod.rs` + +- [ ] **Step 1: Write failing CLI help tests** + +Add `mod mcp;` to `lib/crates/fabro-cli/tests/it/cmd/mod.rs`. + +Create `lib/crates/fabro-cli/tests/it/cmd/mcp.rs` with snapshots for: + +```rust +use fabro_test::{fabro_snapshot, test_context}; + +#[test] +fn help() { + let context = test_context!(); + let mut cmd = context.command(); + cmd.args(["mcp", "--help"]); + fabro_snapshot!(context.filters(), cmd, @""); +} + +#[test] +fn start_help() { + let context = test_context!(); + let mut cmd = context.command(); + cmd.args(["mcp", "start", "--help"]); + fabro_snapshot!(context.filters(), cmd, @""); +} + +#[test] +fn config_help() { + let context = test_context!(); + let mut cmd = context.command(); + cmd.args(["mcp", "config", "--help"]); + fabro_snapshot!(context.filters(), cmd, @""); +} + +#[test] +fn init_help() { + let context = test_context!(); + let mut cmd = context.command(); + cmd.args(["mcp", "init", "--help"]); + fabro_snapshot!(context.filters(), cmd, @""); +} +``` + +- [ ] **Step 2: Run the help tests and verify they fail** + +Run: + +```bash +cargo nextest run -p fabro-cli --test it cmd::mcp::help cmd::mcp::start_help cmd::mcp::config_help cmd::mcp::init_help +``` + +Expected: FAIL because `fabro mcp` does not exist. + +- [ ] **Step 3: Add new crate, clap arguments, and no-op dispatch** + +Create `lib/crates/fabro-mcp-server/Cargo.toml` with the package name +`fabro-mcp-server`. Add direct `rmcp` dependency there: + +```toml +rmcp = { workspace = true, features = ["server", "macros", "schemars", "transport-io"] } +``` + +Also add the Fabro crate dependencies needed for settings/auth, API calls, +manifest construction, and tests. In `lib/crates/fabro-cli/Cargo.toml`, add only +the path dependency: + +```toml +fabro-mcp-server = { path = "../fabro-mcp-server" } +``` + +In `lib/crates/fabro-cli/src/args.rs`, add: + +```rust +#[derive(Args)] +pub(crate) struct McpNamespace { + #[command(subcommand)] + pub(crate) command: McpCommand, +} + +#[derive(Subcommand)] +pub(crate) enum McpCommand { + /// Start the Fabro MCP server over stdio + Start(McpStartArgs), + /// Print MCP client configuration JSON + Config(McpConfigArgs), + /// Configure an MCP client to launch Fabro + Init(McpInitArgs), +} + +#[derive(Args, Debug, Clone, Default)] +pub(crate) struct McpStartArgs { + #[command(flatten)] + pub(crate) connection: ServerConnectionArgs, +} + +#[derive(Args, Debug, Clone, Default)] +pub(crate) struct McpConfigArgs { + #[command(flatten)] + pub(crate) connection: ServerConnectionArgs, +} + +#[derive(Args, Debug, Clone)] +pub(crate) struct McpInitArgs { + pub(crate) agent: McpAgent, + + #[command(flatten)] + pub(crate) connection: ServerConnectionArgs, +} + +#[derive(Debug, Clone, Copy, ValueEnum)] +pub(crate) enum McpAgent { + Claude, + Cursor, + Windsurf, +} +``` + +Add `Commands::Mcp(McpNamespace)` with help text `Model Context Protocol server`. + +In `Commands::name()`: + +```rust +Self::Mcp(ns) => match &ns.command { + McpCommand::Start(_) => "mcp start", + McpCommand::Config(_) => "mcp config", + McpCommand::Init(_) => "mcp init", +}, +``` + +In `commands/mod.rs`, add `pub(crate) mod mcp;`. + +Create `commands/mcp/mod.rs`: + +```rust +use anyhow::Result; + +use crate::args::{McpAgent, McpCommand, McpNamespace, ServerConnectionArgs}; +use crate::command_context::CommandContext; + +pub(crate) async fn dispatch(ns: McpNamespace, base_ctx: &CommandContext) -> Result<()> { + match ns.command { + McpCommand::Start(args) => { + fabro_mcp_server::start(server_settings(base_ctx, &args.connection)?).await + } + McpCommand::Config(args) => { + let json = fabro_mcp_server::config_json(config_settings(&args.connection)?)?; + print!("{json}"); + Ok(()) + } + McpCommand::Init(args) => { + fabro_mcp_server::init_agent(init_settings(args.agent, &args.connection)?)?; + Ok(()) + } + } +} +``` + +Add small conversion helpers in `commands/mcp/mod.rs` that turn CLI arguments +and `base_ctx.cwd()` into `fabro_mcp_server` settings. These helpers must pass +plain owned values such as server URL override, storage-dir override, home dir, +and cwd; the new crate must not depend on `fabro-cli::CommandContext`. + +Create `lib/crates/fabro-mcp-server/src/lib.rs`, `config.rs`, `run_tools.rs`, +and `server.rs` in this task. Use stub implementations that return `Ok(())` or +placeholder JSON for config/init for now, except `start(settings)` can +`anyhow::bail!("fabro mcp start is not implemented yet")` until Task 3. +`run_tools.rs` can contain only a placeholder module comment until Task 3 adds +the first types/helpers. + +In `main.rs`, dispatch: + +```rust +Commands::Mcp(ns) => { + commands::mcp::dispatch(ns, &base_ctx).await?; +} +``` + +- [ ] **Step 4: Run help tests and accept expected snapshots** + +Run: + +```bash +cargo nextest run -p fabro-cli --test it cmd::mcp::help cmd::mcp::start_help cmd::mcp::config_help cmd::mcp::init_help +cargo insta pending-snapshots +cargo insta accept +cargo nextest run -p fabro-cli --test it cmd::mcp::help cmd::mcp::start_help cmd::mcp::config_help cmd::mcp::init_help +``` + +Expected: first run produces snapshots to inspect, final run PASS. + +- [ ] **Step 5: Refactor and verify** + +Run: + +```bash +cargo +nightly-2026-04-14 fmt --all +cargo +nightly-2026-04-14 clippy -p fabro-cli --test it -- -D warnings +cargo +nightly-2026-04-14 clippy -p fabro-mcp-server --all-targets -- -D warnings +``` + +Expected: PASS. + +- [ ] **Step 6: Commit** + +```bash +git add lib/crates/fabro-mcp-server/Cargo.toml lib/crates/fabro-mcp-server/src/lib.rs lib/crates/fabro-mcp-server/src/config.rs lib/crates/fabro-mcp-server/src/run_tools.rs lib/crates/fabro-mcp-server/src/server.rs lib/crates/fabro-cli/Cargo.toml lib/crates/fabro-cli/src/args.rs lib/crates/fabro-cli/src/main.rs lib/crates/fabro-cli/src/commands/mod.rs lib/crates/fabro-cli/src/commands/mcp/mod.rs lib/crates/fabro-cli/tests/it/cmd/mod.rs lib/crates/fabro-cli/tests/it/cmd/mcp.rs +git commit -m "feat(cli): add mcp command surface" +``` + +## Task 2: Implement `fabro mcp config` And `fabro mcp init` + +**Files:** +- Modify: `lib/crates/fabro-mcp-server/src/config.rs` +- Modify: `lib/crates/fabro-mcp-server/src/lib.rs` +- Modify: `lib/crates/fabro-cli/src/commands/mcp/mod.rs` +- Modify: `lib/crates/fabro-cli/tests/it/cmd/mcp.rs` + +- [ ] **Step 1: Write failing config/init tests** + +Add tests: + +```rust +#[test] +fn config_prints_generic_mcp_json() { + let context = test_context!(); + let mut cmd = context.command(); + cmd.args(["mcp", "config"]); + fabro_snapshot!(context.filters(), cmd, @""); +} + +#[test] +fn config_preserves_connection_flags() { + let context = test_context!(); + let mut cmd = context.command(); + cmd.args([ + "mcp", + "config", + "--server", + "https://example.test/api/v1", + "--storage-dir", + "/tmp/fabro-mcp-storage", + ]); + fabro_snapshot!(context.filters(), cmd, @""); +} + +#[test] +fn init_cursor_writes_idempotent_config() { + let context = test_context!(); + context + .command() + .args(["mcp", "init", "cursor"]) + .assert() + .success(); + context + .command() + .args(["mcp", "init", "cursor"]) + .assert() + .success(); + + let config_path = context.home_dir.join(".cursor").join("mcp.json"); + let config: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(config_path).unwrap()).unwrap(); + fabro_json_snapshot!(context, config, @""); +} + +#[test] +fn init_claude_writes_platform_config() { + let context = test_context!(); + context + .command() + .args(["mcp", "init", "claude"]) + .assert() + .success(); + + let config_path = expected_claude_config_path(&context.home_dir); + let config: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(config_path).unwrap()).unwrap(); + fabro_json_snapshot!(context, config, @""); +} + +#[test] +fn init_windsurf_writes_config() { + let context = test_context!(); + context + .command() + .args(["mcp", "init", "windsurf"]) + .assert() + .success(); + + let config_path = context + .home_dir + .join(".codeium") + .join("windsurf") + .join("mcp_config.json"); + let config: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(config_path).unwrap()).unwrap(); + fabro_json_snapshot!(context, config, @""); +} + +#[test] +fn init_preserves_existing_servers() { + let context = test_context!(); + let config_path = context.home_dir.join(".cursor").join("mcp.json"); + std::fs::create_dir_all(config_path.parent().unwrap()).unwrap(); + std::fs::write( + &config_path, + r#"{"mcpServers":{"other":{"command":"other","args":["serve"]}},"theme":"dark"}"#, + ) + .unwrap(); + + context + .command() + .args(["mcp", "init", "cursor", "--server", "https://example.test/api/v1"]) + .assert() + .success(); + + let config: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(config_path).unwrap()).unwrap(); + fabro_json_snapshot!(context, config, @""); +} + +#[test] +fn init_invalid_json_fails_without_overwrite() { + let context = test_context!(); + let config_path = context.home_dir.join(".cursor").join("mcp.json"); + std::fs::create_dir_all(config_path.parent().unwrap()).unwrap(); + std::fs::write(&config_path, "{not json").unwrap(); + + let mut cmd = context.command(); + cmd.args(["mcp", "init", "cursor"]); + fabro_snapshot!(context.filters(), cmd, @""); + assert_eq!(std::fs::read_to_string(config_path).unwrap(), "{not json"); +} +``` + +Use `fabro_json_snapshot` where the parsed config is the contract. Add a small +`expected_claude_config_path(home_dir: &Path) -> PathBuf` helper in the test +module with the same platform branches as production so macOS, Linux, and +Windows path behavior is covered. Add the required import: + +```rust +use fabro_test::{fabro_json_snapshot, fabro_snapshot, test_context}; +``` + +- [ ] **Step 2: Run tests and verify they fail** + +Run: + +```bash +cargo nextest run -p fabro-cli --test it cmd::mcp::config_prints_generic_mcp_json cmd::mcp::config_preserves_connection_flags cmd::mcp::init_cursor_writes_idempotent_config cmd::mcp::init_claude_writes_platform_config cmd::mcp::init_windsurf_writes_config cmd::mcp::init_preserves_existing_servers cmd::mcp::init_invalid_json_fails_without_overwrite +``` + +Expected: FAIL because config/init are stubs. + +- [ ] **Step 3: Implement config rendering** + +In `lib/crates/fabro-mcp-server/src/config.rs`, implement: + +```rust +#![expect( + clippy::disallowed_methods, + reason = "MCP client config setup intentionally performs small synchronous JSON file reads/writes from a CLI command." +)] + +use std::path::PathBuf; + +use anyhow::{Context as _, Result, anyhow, bail}; +use serde_json::{Map, Value, json}; + +const SERVER_NAME: &str = "fabro"; + +pub fn config_json(settings: McpConfigSettings) -> Result { + serde_json::to_string_pretty(&generic_config(&settings)) + .map(|json| format!("{json}\n")) + .context("failed to render Fabro MCP client config") +} + +pub fn init_agent(settings: McpInitSettings) -> Result<()> { + let path = agent_config_path(settings.agent, &settings.home_dir)?; + let entry = server_entry(&settings.config); + merge_server_entry(&path, entry)?; + Ok(()) +} +``` + +`server_entry(...)` must emit command `fabro` and args built by: + +```rust +fn start_args(settings: &McpConfigSettings) -> Vec { + let mut args = vec!["mcp".to_string(), "start".to_string()]; + if let Some(server) = settings.server.as_ref() { + args.push("--server".to_string()); + args.push(server.clone()); + } + if let Some(storage_dir) = settings.storage_dir.as_deref() { + args.push("--storage-dir".to_string()); + args.push(storage_dir.display().to_string()); + } + args +} +``` + +Implement `merge_server_entry(path, entry)` so it: + +- creates the parent directory +- reads existing JSON if the file exists +- rejects invalid JSON with context including the path +- rejects non-object roots and non-object `mcpServers` +- inserts/replaces only `mcpServers.fabro` +- writes pretty JSON plus trailing newline + +Implement all three supported path mappings (`claude`, `cursor`, `windsurf`). +For `claude`, use `dirs::home_dir()` plus platform cfgs: + +- macOS: `Library/Application Support/Claude/claude_desktop_config.json` +- Linux: `.config/Claude/claude_desktop_config.json` +- Windows: `%APPDATA%\Claude\claude_desktop_config.json`, falling back to `~/AppData/Roaming/Claude/claude_desktop_config.json` when `APPDATA` is absent. The fallback is needed because integration tests run the compiled binary under `fabro_test::apply_test_isolation`, which clears ambient `APPDATA`. + +- [ ] **Step 4: Run config/init tests and accept snapshots** + +Run: + +```bash +cargo nextest run -p fabro-cli --test it cmd::mcp::config_prints_generic_mcp_json cmd::mcp::config_preserves_connection_flags cmd::mcp::init_cursor_writes_idempotent_config cmd::mcp::init_claude_writes_platform_config cmd::mcp::init_windsurf_writes_config cmd::mcp::init_preserves_existing_servers cmd::mcp::init_invalid_json_fails_without_overwrite +cargo insta pending-snapshots +cargo insta accept +cargo nextest run -p fabro-cli --test it cmd::mcp::config_prints_generic_mcp_json cmd::mcp::config_preserves_connection_flags cmd::mcp::init_cursor_writes_idempotent_config cmd::mcp::init_claude_writes_platform_config cmd::mcp::init_windsurf_writes_config cmd::mcp::init_preserves_existing_servers cmd::mcp::init_invalid_json_fails_without_overwrite +``` + +Expected: PASS. + +- [ ] **Step 5: Refactor and verify** + +Run: + +```bash +cargo +nightly-2026-04-14 fmt --all +cargo +nightly-2026-04-14 clippy -p fabro-cli --test it -- -D warnings +cargo +nightly-2026-04-14 clippy -p fabro-mcp-server --all-targets -- -D warnings +``` + +Expected: PASS. + +- [ ] **Step 6: Commit** + +```bash +git add lib/crates/fabro-mcp-server/src/config.rs lib/crates/fabro-mcp-server/src/lib.rs lib/crates/fabro-cli/src/commands/mcp/mod.rs lib/crates/fabro-cli/tests/it/cmd/mcp.rs +git commit -m "feat(cli): configure fabro mcp clients" +``` + +## Task 3: Add MCP Server Skeleton With Protocol Tests + +**Files:** +- Modify: `lib/crates/fabro-mcp-server/src/server.rs` +- Modify: `lib/crates/fabro-mcp-server/src/run_tools.rs` +- Modify: `lib/crates/fabro-mcp-server/src/lib.rs` +- Modify: `lib/crates/fabro-cli/tests/it/cmd/mcp.rs` + +- [ ] **Step 1: Write failing stdio protocol test** + +Add a test that uses the existing `fabro_mcp::client::McpClient` to spawn the compiled CLI: + +```rust +#[tokio::test(flavor = "multi_thread")] +async fn stdio_server_initializes_and_lists_run_tools() { + let context = test_context!(); + let config = fabro_mcp::config::McpServerSettings { + name: "fabro-under-test".to_string(), + transport: fabro_mcp::config::McpTransport::Stdio { + command: vec![ + env!("CARGO_BIN_EXE_fabro").to_string(), + "mcp".to_string(), + "start".to_string(), + ], + env: mcp_stdio_env(&context), + }, + startup_timeout_secs: 10, + tool_timeout_secs: 30, + }; + let client = fabro_mcp::client::McpClient::new(&config).unwrap(); + client.initialize(config.startup_timeout()).await.unwrap(); + + let tools = client.list_tools().await.unwrap(); + let names: Vec<_> = tools.iter().map(|(name, _, _)| name.as_str()).collect(); + assert_eq!( + names, + vec![ + "fabro_run_create", + "fabro_run_search", + "fabro_run_interact", + "fabro_run_gather", + "fabro_run_events", + ] + ); +} +``` + +`TestContext` does not currently expose a reusable command env map. Add a narrow +test helper that constructs a deterministic child-process environment instead +of reading from ambient user `HOME` or trying to reverse a built +`std::process::Command`: + +```rust +struct McpStdioFixture { + command: Vec, + env: HashMap, + current_dir: PathBuf, +} + +fn mcp_stdio_fixture(context: &fabro_test::TestContext, extra_args: &[&str]) -> McpStdioFixture { + let mut command = vec![ + env!("CARGO_BIN_EXE_fabro").to_string(), + "mcp".to_string(), + "start".to_string(), + ]; + command.extend(extra_args.iter().map(|arg| (*arg).to_string())); + + let mut env = fabro_test::isolated_env(&context.home_dir); + env.insert("HOME".to_string(), context.home_dir.display().to_string()); + env.insert("FABRO_HOME".to_string(), context.home_dir.join(".fabro").display().to_string()); + env.insert("NO_COLOR".to_string(), "1".to_string()); + + McpStdioFixture { + command, + env, + current_dir: context.temp_dir.clone(), + } +} +``` + +If `fabro_test::isolated_env` does not exist, add a similarly narrow helper to +`fabro_test` that returns the same env map used by +`fabro_test::apply_test_isolation`. Use `fixture.env.clone()` for +`fabro_mcp::config::McpServerSettings`, and use the same `fixture.env` plus +`fixture.current_dir` when spawning raw subprocess tests; raw subprocess helpers +must call `cmd.env_clear()` before applying this map. The production crate +should expose equivalent explicit settings (`McpServerSettings { server, +storage_dir, home_dir, cwd }`) so tests and CLI dispatch build settings from +owned values directly; do not depend on ambient process env in tests. + +- [ ] **Step 2: Run test and verify it fails** + +Run: + +```bash +cargo nextest run -p fabro-cli --test it cmd::mcp::stdio_server_initializes_and_lists_run_tools +``` + +Expected: FAIL because `fabro mcp start` is not implemented. + +- [ ] **Step 3: Implement rmcp server skeleton** + +In `lib/crates/fabro-mcp-server/src/server.rs`, implement: + +```rust +use std::path::PathBuf; +use std::sync::Arc; + +use anyhow::Result; +use rmcp::{ + ErrorData, ServerHandler, serve_server, + handler::server::{router::tool::ToolRouter, wrapper::Parameters}, + model::{CallToolResult, ServerCapabilities, ServerInfo}, + tool, tool_handler, tool_router, + transport::stdio, +}; +use tokio::sync::OnceCell; + +use fabro_client::Client; + +use crate::{McpServerSettings, run_tools}; + +#[derive(Clone)] +pub(crate) struct FabroMcpServer { + settings: Arc, + client: Arc>>, + cwd: PathBuf, + tool_router: ToolRouter, +} + +pub async fn start(settings: McpServerSettings) -> Result<()> { + let server = FabroMcpServer::new(Arc::new(settings)); + let service = serve_server(server, stdio()).await?; + service.waiting().await?; + Ok(()) +} +``` + +Implement `ServerHandler` through the `#[tool_handler]` impl, not a separate +plain impl. `rmcp::serve_server(...)` returns after initialization with a +running service handle; `fabro mcp start` must await `service.waiting()` so the +stdio process stays alive for later `tools/list` and `tools/call` requests. + +```rust +#[tool_handler(router = self.tool_router)] +impl ServerHandler for FabroMcpServer { + fn get_info(&self) -> ServerInfo { + ServerInfo::new(ServerCapabilities::builder().enable_tools().build()) + .with_instructions("Use these tools to create, inspect, control, wait for, and read events from Fabro workflow runs.") + } +} +``` + +Add tool functions with temporary placeholder results: + +```rust +#[tool_router] +impl FabroMcpServer { + pub(crate) fn new(settings: Arc) -> Self { ... } + + #[tool(name = "fabro_run_create", description = "...")] + async fn fabro_run_create( + &self, + params: Parameters, + ) -> Result { + let params = match run_tools::ValidatedCreateRuns::try_from(params.0) { + Ok(params) => params, + Err(err) => return Ok(run_tools::error_result(err)), + }; + let client = match self.client().await { + Ok(client) => client, + Err(err) => return Ok(run_tools::error_result(err)), + }; + match run_tools::create_runs(client, &self.cwd, params).await { + Ok(result) => run_tools::success_result(&result, run_tools::create_runs_text(&result)), + Err(err) => Ok(run_tools::error_result(err)), + } + } +} + +``` + +Use this same handler shape for all five tools: first normalize and validate +the parameter object into a tool-specific `Validated...` type, then acquire the +lazy client only after validation succeeds, call the corresponding `run_tools` +function, return successful values with `success_result(...)`, and convert +expected Fabro/API/validation failures with `error_result(...)`. + +`new(...)` should copy `settings.cwd.clone()` into the `cwd` field before +storing the settings, so tool calls resolve relative workflows against the MCP +process cwd captured at startup. + +Each placeholder in `run_tools.rs` should still define the input structs, +validated parameter structs, and `TryFrom` validation hooks for all five +tools in this task. The run functions can return +`Err(ToolError::message("not implemented"))` until later tasks, except the +module must compile and tools must be listed. Add a small crate-local tool error +type: + +```rust +#[derive(Debug)] +pub(crate) struct ToolError { + message: String, +} + +impl ToolError { + pub(crate) fn message(message: impl Into) -> Self { + Self { + message: message.into(), + } + } + + pub(crate) fn from_anyhow(err: anyhow::Error) -> Self { + Self::message(format_tool_error(err)) + } + + pub(crate) fn as_str(&self) -> &str { + &self.message + } +} + +pub(crate) type ToolResult = Result; +``` + +Then add result helpers: + +```rust +pub(crate) fn success_result( + value: &T, + text: impl Into, +) -> Result { + let structured_content = serde_json::to_value(value).map_err(|err| { + rmcp::ErrorData::internal_error( + format!("failed to serialize Fabro MCP tool result: {err}"), + None, + ) + })?; + let mut result = rmcp::model::CallToolResult::structured(structured_content); + result.content = vec![rmcp::model::Content::text(text.into())]; + Ok(result) +} + +pub(crate) fn error_result(err: ToolError) -> rmcp::model::CallToolResult { + rmcp::model::CallToolResult::error(vec![rmcp::model::Content::text( + err.as_str().to_string(), + )]) +} +``` + +Add one text helper per result type, for example `create_runs_text(...)`, so +fallback content is concise: `"created 1 Fabro run and started 1"`, not a full +JSON dump. + +Implement `client(&self)` with lazy connection: + +```rust +async fn client(&self) -> Result, run_tools::ToolError> { + self.client + .get_or_try_init(|| async { client_from_settings(&self.settings).await.map_err(run_tools::ToolError::from_anyhow) }) + .await + .map(Arc::clone) +} +``` + +Make `format_tool_error` append auth guidance when `fabro_util::exit::exit_class_for(&err) == Some(ExitClass::AuthRequired)`. + +- [ ] **Step 4: Run protocol test** + +Run: + +```bash +cargo nextest run -p fabro-cli --test it cmd::mcp::stdio_server_initializes_and_lists_run_tools +``` + +Expected: PASS listing all five tools. + +- [ ] **Step 5: Add stdout-purity regression** + +Add a raw subprocess test that: + +- spawns `fabro mcp start` +- sends a JSON-RPC initialize request on stdin +- reads the first stdout line +- asserts it parses as JSON and has `jsonrpc: "2.0"` +- asserts stderr may contain logs but stdout contains no leading human text + +Use `mcp_stdio_fixture(&context, &[])` for the raw subprocess helper so this +test and the `McpServerSettings` test use identical command, env, and cwd +values. + +Use a child timeout and kill-on-drop cleanup. The raw JSON should be: + +```json +{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-06-18","capabilities":{},"clientInfo":{"name":"fabro-test","version":"0.0.0"}}} +``` + +- [ ] **Step 6: Run protocol checks** + +Run: + +```bash +cargo nextest run -p fabro-cli --test it cmd::mcp::stdio_server_initializes_and_lists_run_tools cmd::mcp::stdio_start_writes_only_json_rpc_to_stdout +``` + +Expected: PASS. + +- [ ] **Step 7: Add startup/list-tools performance smoke** + +Add a lightweight smoke test that uses `mcp_stdio_fixture` to initialize the +server and call `tools/list` without a live Fabro server or auth. Assert the +combined initialize plus list-tools path completes within 2 seconds on the test +machine: + +```rust +#[tokio::test(flavor = "multi_thread")] +async fn stdio_startup_and_list_tools_is_fast() { + let context = test_context!(); + let start = std::time::Instant::now(); + let client = spawn_mcp_client(&context, &[]).await; + let tools = client.list_tools().await.unwrap(); + assert_eq!(tools.len(), 5); + assert!(start.elapsed() < std::time::Duration::from_secs(2)); +} +``` + +This is a smoke check, not a benchmark. If CI variance makes 2 seconds too +tight, keep the assertion but adjust the threshold in the implementation with a +comment explaining the observed bound. + +Run: + +```bash +cargo nextest run -p fabro-cli --test it cmd::mcp::stdio_startup_and_list_tools_is_fast +``` + +Expected: PASS. + +- [ ] **Step 8: Refactor and verify** + +Run: + +```bash +cargo +nightly-2026-04-14 fmt --all +cargo +nightly-2026-04-14 clippy -p fabro-cli --test it -- -D warnings +cargo +nightly-2026-04-14 clippy -p fabro-mcp-server --all-targets -- -D warnings +``` + +Expected: PASS. + +- [ ] **Step 9: Commit** + +```bash +git add lib/crates/fabro-mcp-server/src/server.rs lib/crates/fabro-mcp-server/src/run_tools.rs lib/crates/fabro-mcp-server/src/lib.rs lib/crates/fabro-cli/tests/it/cmd/mcp.rs +git commit -m "feat(cli): start fabro mcp stdio server" +``` + +## Task 4: Implement Run Create/Search Tools + +**Files:** +- Modify: `lib/crates/fabro-mcp-server/src/run_tools.rs` +- Modify: `lib/crates/fabro-cli/src/commands/run/overrides.rs` +- Modify: `lib/crates/fabro-cli/tests/it/cmd/mcp.rs` + +- [ ] **Step 1: Write failing create/search integration test** + +Add a test backed by an authenticated real Fabro server: + +```rust +#[tokio::test(flavor = "multi_thread")] +async fn mcp_create_and_search_manage_real_runs_with_cli_auth() { + let context = test_context!(); + let harness = RealAuthHarness::start_with_dev_token(fabro_test::GitHubAppState::default()).await; + let target_url = harness.api_target(); + let target: fabro_client::ServerTarget = target_url.parse().unwrap(); + seed_dev_token_auth(&context.home_dir, &target, TEST_DEV_TOKEN); + let workflow = context.install_fixture("simple.fabro"); + + let client = spawn_mcp_client(&context, &[ + "--server", + &target_url, + ]).await; + + let create = call_tool_json(&client, "fabro_run_create", serde_json::json!({ + "runs": [{ + "workflow": workflow, + "dry_run": true, + "auto_approve": true, + "labels": { "source": "mcp-test" } + }] + })).await; + let run_id = create["runs"][0]["run_id"].as_str().unwrap().to_string(); + assert_eq!(create["runs"][0]["started"], true); + + let search = call_tool_json(&client, "fabro_run_search", serde_json::json!({ + "run_ids": [run_id], + "labels": { "source": "mcp-test" }, + "first": 10 + })).await; + fabro_json_snapshot!(context, normalize_run_search(search), @""); + + harness.shutdown().await; +} +``` + +Implement `call_tool_json(...)` so it asserts `is_error != Some(true)`, extracts +`structured_content`, and verifies the first text content is present and does +not start with `{` or `[`; this makes the concise fallback contract automated +instead of a manual-only check. + +If `RealAuthHarness::start_with_dev_token` cannot create runs due missing server settings for local execution, use `TestContext` managed server plus explicit `seed_dev_token_auth` against its server target, but keep the test proving persisted CLI auth is used. + +- [ ] **Step 2: Run test and verify it fails** + +Run: + +```bash +cargo nextest run -p fabro-cli --test it cmd::mcp::mcp_create_and_search_manage_real_runs_with_cli_auth +``` + +Expected: FAIL because tools return not implemented. + +- [ ] **Step 3: Implement shared parameter and result types** + +In `run_tools.rs`, define public crate-visible structs for every tool input/output with: + +```rust +#[derive(Debug, serde::Deserialize, rmcp::schemars::JsonSchema)] +pub(crate) struct ... + +#[derive(Debug, serde::Serialize, rmcp::schemars::JsonSchema)] +pub(crate) struct ... +``` + +Use `#[serde(default)]` on optional map fields so omitted maps become empty maps where helpful. + +- [ ] **Step 4: Expose manifest override helpers** + +In `commands/run/overrides.rs`, make these helpers `pub(crate)` if needed: + +- `parse_labels` +- `model_from_args` +- `sandbox_layer` +- `execution_layer` +- `goal_layer_from_args` + +If changing visibility creates awkward API, instead add one new crate-visible function: + +```rust +pub(crate) fn manifest_overrides_from_parts(input: ManifestOverrideParts<'_>) -> Result +``` + +Prefer the single helper if more than three helpers would need visibility changes. + +- [ ] **Step 5: Implement `fabro_run_create`** + +Implementation outline: + +```rust +pub(crate) async fn create_runs( + client: Arc, + base_cwd: &Path, + params: ValidatedCreateRuns, +) -> ToolResult { + let mut created = Vec::with_capacity(params.runs.len()); + for spec in params.runs { + let cwd = spec.cwd.clone().unwrap_or_else(|| base_cwd.to_path_buf()); + let run_id = spec.run_id.as_deref().map(str::parse).transpose().map_err(tool_err)?; + let overrides = build_mcp_manifest_overrides(&spec, &cwd)?; + let manifest_args = mcp_manifest_args(&spec); + let built = build_run_manifest(ManifestBuildInput { + workflow: PathBuf::from(&spec.workflow), + cwd, + run_overrides: overrides.run, + cli_overrides: overrides.cli, + input_overrides: overrides.input_overrides, + args: manifest_args, + run_id, + user_settings_path: Some(active_settings_path(None)), + })?; + let validation = manifest_validation::validate_manifest(&RunLayer::default(), &built.manifest)?; + reject_validation_errors(validation)?; + let run_id = client.create_run_from_manifest(built.manifest).await?; + let started = spec.start.unwrap_or(true); + if started { + client.start_run(&run_id, false).await?; + } + let summary = client.retrieve_run(&run_id).await?; + created.push(CreatedRunResult::from_summary(summary, started)); + } + Ok(CreateRunsResult { runs: created }) +} +``` + +Important: the function signature in `server.rs` should pass both the lazy API client and the MCP process cwd from the captured `McpServerSettings`, not call `std::env::current_dir()` deep in the tool. The raw `FabroRunCreateParams` must only appear at the MCP handler boundary; `ValidatedCreateRuns::try_from(raw)` must run before acquiring the client, and `create_runs(...)` must accept `ValidatedCreateRuns`. +The outline above omits some `map_err(...)` calls for readability; the real +implementation must convert every `anyhow::Error` and API error into +`ToolError` with the shared formatting helper so `?` never tries to convert +`anyhow::Error` directly into `ToolError`. + +Implement `mcp_manifest_args(&CreateRunSpec) -> Option` +in `run_tools.rs`. It should mirror `manifest_builder::run_manifest_args` for +provenance: + +```rust +fn mcp_manifest_args(spec: &CreateRunSpec) -> Option { + let label = spec + .labels + .iter() + .map(|(key, value)| format!("{key}={value}")) + .collect::>(); + let input = spec + .inputs + .iter() + .map(|(key, value)| format!("{key}={value}")) + .collect::>(); + let payload = types::ManifestArgs { + auto_approve: spec.auto_approve.filter(|value| *value), + dry_run: spec.dry_run.filter(|value| *value), + label, + model: spec.model.clone(), + preserve_sandbox: spec.preserve_sandbox.filter(|value| *value), + provider: spec.provider.clone(), + sandbox: spec.sandbox.clone(), + docker_image: None, + input, + verbose: None, + }; + (!mcp_manifest_args_is_empty(&payload)).then_some(payload) +} +``` + +Keep the emptiness check local if `manifest_args_is_empty` is not accessible. +The `input` strings are only for provenance; authoritative input values come +from `input_overrides` after JSON-to-TOML conversion. + +- [ ] **Step 6: Implement JSON-to-TOML input conversion** + +Add unit tests in `run_tools.rs` for: + +- strings +- bools +- integers +- floats +- arrays +- objects +- null rejected with key name + +Run: + +```bash +cargo nextest run -p fabro-mcp-server run_tools +``` + +Expected: PASS after implementation. + +- [ ] **Step 7: Implement `fabro_run_search`** + +Use existing `server_runs::ServerRunSummaryInfo` where useful, but avoid adding public API only for tests. Search should: + +- fetch `client.list_store_runs().await` +- sort newest first using created/start timestamp and run id as tie-breaker +- apply filters +- page with `first` and `after` +- return `SearchRunsResult { runs, next_cursor }` + +Do not drop archived runs by default. `archived: Some(false)` should exclude them. + +- [ ] **Step 8: Run create/search tests** + +Run: + +```bash +cargo nextest run -p fabro-cli --test it cmd::mcp::mcp_create_and_search_manage_real_runs_with_cli_auth +``` + +Expected: PASS. + +- [ ] **Step 9: Refactor and verify** + +Run: + +```bash +cargo +nightly-2026-04-14 fmt --all +cargo +nightly-2026-04-14 clippy -p fabro-cli --test it -- -D warnings +cargo +nightly-2026-04-14 clippy -p fabro-mcp-server --all-targets -- -D warnings +``` + +Expected: PASS. + +- [ ] **Step 10: Commit** + +```bash +git add lib/crates/fabro-mcp-server/src/run_tools.rs lib/crates/fabro-cli/src/commands/run/overrides.rs lib/crates/fabro-cli/tests/it/cmd/mcp.rs +git commit -m "feat(cli): add mcp run create and search tools" +``` + +## Task 5: Implement Interact/Gather/Events Tools + +**Files:** +- Modify: `lib/crates/fabro-mcp-server/src/run_tools.rs` +- Modify: `lib/crates/fabro-cli/tests/it/cmd/mcp.rs` + +- [ ] **Step 1: Write failing lifecycle interaction test** + +Add a test that: + +- creates a dry-run auto-approved run with `fabro_run_create` +- calls `fabro_run_gather` with the run id +- calls `fabro_run_interact` action `get` +- calls `fabro_run_events` action `list` +- calls `fabro_run_interact` action `archive` +- calls `fabro_run_interact` action `unarchive` +- verifies server-visible state through API or a follow-up `fabro_run_search` + +Snapshot a normalized object: + +```rust +fabro_json_snapshot!( + context, + serde_json::json!({ + "gather": normalize_gather(gather), + "get_status": get["result"]["summary"]["status"], + "events_nonempty": events["events"].as_array().unwrap().is_empty() == false, + "archive_action": archive["action"], + "unarchive_action": unarchive["action"], + }), + @"" +); +``` + +- [ ] **Step 2: Write failing validation/error tests** + +Add tests for: + +- `fabro_run_gather` rejects more than 50 run ids without requiring auth or a reachable server. Start `fabro mcp start --server http://127.0.0.1:9` with no auth entry, call the tool, assert the error mentions `run_ids`, then call `tools/list` again to prove the server stayed alive. +- `fabro_run_gather` returns `timed_out: true` when the timeout expires before all requested runs are terminal. Use a real authenticated test server, create or select a non-terminal run, call gather with `timeout_seconds: 1` and `poll_interval_seconds: 5`, assert elapsed wall time is bounded, and verify the returned run summary is the current state rather than a process/tool failure. +- `fabro_run_interact` action `message` without `message` returns an MCP tool error and the server remains alive for a subsequent `fabro_run_search`. +- Missing auth against a protected remote target returns a tool error containing `Run \`fabro auth login\` to authenticate.` + +- [ ] **Step 3: Run tests and verify they fail** + +Run: + +```bash +cargo nextest run -p fabro-cli --test it cmd::mcp::mcp_lifecycle_tools_manage_real_run cmd::mcp::mcp_gather_rejects_too_many_runs cmd::mcp::mcp_gather_returns_timeout_result cmd::mcp::mcp_interact_error_does_not_stop_server cmd::mcp::mcp_tool_auth_error_mentions_login +``` + +Expected: FAIL because tools are incomplete. + +- [ ] **Step 4: Implement `fabro_run_interact`** + +Implement one small function per action: + +```rust +async fn interact_get(client: &Client, run_id: &RunId) -> Result +async fn interact_start(client: &Client, run_id: &RunId) -> Result +async fn interact_message(client: &Client, run_id: &RunId, message: Option, interrupt: bool) -> Result +async fn interact_cancel(client: &Client, run_id: &RunId) -> Result +async fn interact_archive(client: &Client, run_id: &RunId) -> Result +async fn interact_unarchive(client: &Client, run_id: &RunId) -> Result +async fn interact_get_questions(client: &Client, run_id: &RunId) -> Result +async fn interact_answer(client: &Client, run_id: &RunId, question_id: Option, answer: Option) -> Result +``` + +Resolve selectors once: + +```rust +let run_id = client.resolve_run(¶ms.run_id).await?.id; +``` + +Use `serde_json::to_value(...)` for API objects rather than manually copying complex projection/question structures. + +- [ ] **Step 5: Implement answer mapping tests and helper** + +Add unit tests for: + +```rust +assert_answer_json(json!(true), json!({"kind": "yes"})); +assert_answer_json(json!(false), json!({"kind": "no"})); +assert_answer_json(json!("hello"), json!({"kind": "text", "text": "hello"})); +assert_answer_json(json!({"option":"a"}), json!({"kind": "selected", "option_key": "a"})); +assert_answer_json( + json!({"options":["a","b"]}), + json!({"kind": "multi_selected", "option_keys": ["a", "b"]}), +); +assert_answer_json(json!({"text":"hello"}), json!({"kind": "text", "text": "hello"})); +``` + +Build the generated `fabro_api::types::SubmitAnswerRequest` through the +documented wire JSON shape and then serialize it back in tests: + +```rust +fn answer_to_submit_request(answer: serde_json::Value) -> ToolResult { + let payload = match answer { + serde_json::Value::Bool(true) => serde_json::json!({ "kind": "yes" }), + serde_json::Value::Bool(false) => serde_json::json!({ "kind": "no" }), + serde_json::Value::String(text) => serde_json::json!({ "kind": "text", "text": text }), + serde_json::Value::Object(mut object) => { + if let Some(option) = object.remove("option") { + serde_json::json!({ "kind": "selected", "option_key": option }) + } else if let Some(options) = object.remove("options") { + serde_json::json!({ "kind": "multi_selected", "option_keys": options }) + } else if let Some(text) = object.remove("text") { + serde_json::json!({ "kind": "text", "text": text }) + } else { + return Err(ToolError::message( + "answer object must contain one of: option, options, text", + )); + } + } + other => { + return Err(ToolError::message(format!( + "unsupported answer value: {other}; expected boolean, string, or object", + ))); + } + }; + serde_json::from_value(payload).map_err(|err| { + ToolError::message(format!("failed to build submit-answer request: {err}")) + }) +} +``` + +This matches the current API contract proven by +`lib/crates/fabro-api/tests/submit_answer_request_round_trip.rs`, which uses +`kind: yes`, `kind: no`, `kind: selected`, `kind: multi_selected`, and +`kind: text`. Do not introduce references to non-existent generated names such +as `SubmitAnswerRequestKind`. + +- [ ] **Step 6: Implement `fabro_run_gather`** + +Validation converts raw `FabroRunGatherParams` into `ValidatedGatherRuns` +before client acquisition: + +```rust +validate_len("run_ids", params.run_ids.len(), 1, 50)?; +let timeout = params.timeout_seconds.unwrap_or(300).min(600); +let poll = params.poll_interval_seconds.unwrap_or(15).max(5); +``` + +Implementation: + +- resolve all selectors at the start +- poll summaries until every `summary.lifecycle.status.is_terminal()` or deadline +- if the deadline expires, return `timed_out: true`, current run summaries, and `elapsed_seconds` as a successful structured tool result +- only return a tool error for selector/API failures, not for ordinary timeout expiry + +- [ ] **Step 7: Implement `fabro_run_events`** + +Fetch events using: + +```rust +let events = client + .list_run_events(&run_id, params.after, effective_limit_for_fetch(params)) + .await?; +``` + +Then apply filters in memory: + +- event ids +- event types using `event.event.event_name()` +- categories +- created_after/before +- query substring on serialized event JSON +- offset +- limit/first +- direction +- max_content_length truncation + +Output: + +```rust +struct RunEventsResult { + run_id: String, + action: RunEventsAction, + events: Vec, + next_cursor: Option, +} +``` + +`next_cursor` is last returned sequence plus one when at least one event was returned. + +- [ ] **Step 8: Run lifecycle and error tests** + +Run: + +```bash +cargo nextest run -p fabro-cli --test it cmd::mcp::mcp_lifecycle_tools_manage_real_run cmd::mcp::mcp_gather_rejects_too_many_runs cmd::mcp::mcp_gather_returns_timeout_result cmd::mcp::mcp_interact_error_does_not_stop_server cmd::mcp::mcp_tool_auth_error_mentions_login +``` + +Expected: PASS. + +- [ ] **Step 9: Refactor and verify** + +Run: + +```bash +cargo +nightly-2026-04-14 fmt --all +cargo +nightly-2026-04-14 clippy -p fabro-cli --test it -- -D warnings +cargo +nightly-2026-04-14 clippy -p fabro-mcp-server --all-targets -- -D warnings +``` + +Expected: PASS. + +- [ ] **Step 10: Commit** + +```bash +git add lib/crates/fabro-mcp-server/src/run_tools.rs lib/crates/fabro-cli/tests/it/cmd/mcp.rs +git commit -m "feat(cli): add mcp run control tools" +``` + +## Task 6: Final Contract Coverage And Workspace Verification + +**Files:** +- Modify as needed from prior tasks only. + +- [ ] **Step 1: Run all MCP command tests** + +Run: + +```bash +cargo nextest run -p fabro-cli --test it cmd::mcp +``` + +Expected: PASS. + +- [ ] **Step 2: Run existing relevant MCP client tests** + +Run: + +```bash +cargo nextest run -p fabro-mcp +``` + +Expected: PASS. This confirms the existing external MCP client crate was not regressed by dependency feature unification. + +- [ ] **Step 3: Run relevant existing CLI run/auth tests** + +Run: + +```bash +cargo nextest run -p fabro-cli --test it scenario::auth::auth_login_refresh_logout_flow scenario::lifecycle::dry_run_create_start_attach_works_with_default_run_lookup cmd::ps::ps_explicit_local_tcp_target_uses_auth_store +``` + +Expected: PASS. If exact test names drift, use `cargo nextest list -p fabro-cli --test it | rg 'auth_login_refresh_logout_flow|dry_run_create_start_attach|explicit_local_tcp'` and run the matching tests. + +- [ ] **Step 4: Run formatting and linting** + +Run: + +```bash +cargo +nightly-2026-04-14 fmt --check --all +cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings +``` + +Expected: PASS. + +- [ ] **Step 5: Run broader regression suite** + +Run: + +```bash +ulimit -n 4096 && cargo nextest run --workspace +``` + +Expected: PASS. + +- [ ] **Step 6: Inspect snapshots before accepting any remaining changes** + +Run: + +```bash +cargo insta pending-snapshots +``` + +Expected: no pending snapshots. If pending snapshots exist, inspect them before accepting. Only accept snapshots caused by this feature. + +- [ ] **Step 7: Final code review pass** + +Check manually: + +- `fabro mcp start` has no `printout!`, `println!`, `eprintln!` is only for stderr and not in server steady-state startup. +- all MCP tool argument validation returns tool errors, not process exits. +- successful MCP tools include both `structuredContent` and short text content; text content is not just serialized JSON. +- no tests write run internals directly. +- no live provider credentials are required. +- agent config merge preserves unrelated keys. +- auth failures include login guidance. + +- [ ] **Step 8: Commit final fixes if any** + +```bash +git status --short +git add +git commit -m "test(cli): cover fabro mcp server contract" +``` + +Only make this commit if Task 6 produced additional fixes or tests not already committed. diff --git a/docs/public/reference/cli.mdx b/docs/public/reference/cli.mdx index 8d9f1f939..7d5bfc2a7 100644 --- a/docs/public/reference/cli.mdx +++ b/docs/public/reference/cli.mdx @@ -79,6 +79,7 @@ fabro [OPTIONS] [COMMAND] | `fabro inspect` | Show detailed information about a workflow run | | `fabro install` | Set up the Fabro environment (LLMs, certs, GitHub) | | `fabro logs` | View the raw worker tracing log of a workflow run | +| `fabro mcp` | Model Context Protocol server | | `fabro model` | List and test LLM models | | `fabro pr` | Pull request operations | | `fabro preflight` | Validate run configuration without executing | @@ -510,6 +511,73 @@ fabro logs [OPTIONS] | `--server ` | Fabro server target: http(s) URL or absolute Unix socket path | | `-n, --tail ` | Lines from end (default: all) | +### `fabro mcp` + +Model Context Protocol server + +```bash +fabro mcp [OPTIONS] +``` + +#### Subcommands + +| Command | Description | +| --- | --- | +| `fabro mcp config` | Print MCP client configuration JSON | +| `fabro mcp init` | Configure an MCP client to launch Fabro | +| `fabro mcp start` | Start the Fabro MCP server over stdio | + +#### `fabro mcp config` + +Print MCP client configuration JSON + +```bash +fabro mcp config [OPTIONS] +``` + +#### Options + +| Option | Description | +| --- | --- | +| `--server ` | Fabro server target: http(s) URL or absolute Unix socket path | +| `--storage-dir ` | Local storage directory (default: ~/.fabro/storage) | + +#### `fabro mcp init` + +Configure an MCP client to launch Fabro + +```bash +fabro mcp init [OPTIONS] +``` + +#### Arguments + +| Name | Description | +| --- | --- | +| `AGENT` | Values: `claude`, `cursor`, `windsurf` | + +#### Options + +| Option | Description | +| --- | --- | +| `--server ` | Fabro server target: http(s) URL or absolute Unix socket path | +| `--storage-dir ` | Local storage directory (default: ~/.fabro/storage) | + +#### `fabro mcp start` + +Start the Fabro MCP server over stdio + +```bash +fabro mcp start [OPTIONS] +``` + +#### Options + +| Option | Description | +| --- | --- | +| `--server ` | Fabro server target: http(s) URL or absolute Unix socket path | +| `--storage-dir ` | Local storage directory (default: ~/.fabro/storage) | + ### `fabro model` List and test LLM models diff --git a/lib/crates/fabro-agent/src/mcp_integration.rs b/lib/crates/fabro-agent/src/mcp_integration.rs index ed4ef4574..4c1514d5c 100644 --- a/lib/crates/fabro-agent/src/mcp_integration.rs +++ b/lib/crates/fabro-agent/src/mcp_integration.rs @@ -62,6 +62,8 @@ mod tests { command: vec!["python3".into(), test_server], env: HashMap::new(), }, + current_dir: None, + clear_env: false, startup_timeout_secs: 10, tool_timeout_secs: 30, } diff --git a/lib/crates/fabro-agent/src/session.rs b/lib/crates/fabro-agent/src/session.rs index 045038a63..143898983 100644 --- a/lib/crates/fabro-agent/src/session.rs +++ b/lib/crates/fabro-agent/src/session.rs @@ -493,6 +493,8 @@ impl Session { resolved.push(McpServerSettings { name: config.name.clone(), transport: McpTransport::Http { url, headers }, + current_dir: config.current_dir.clone(), + clear_env: config.clear_env, startup_timeout_secs: config.startup_timeout_secs, tool_timeout_secs: config.tool_timeout_secs, }); @@ -3367,6 +3369,8 @@ mod tests { command: vec!["python3".into(), test_server], env: HashMap::new(), }, + current_dir: None, + clear_env: false, startup_timeout_secs: 10, tool_timeout_secs: 30, }], diff --git a/lib/crates/fabro-cli/Cargo.toml b/lib/crates/fabro-cli/Cargo.toml index d3f4c94d2..9da42ddaf 100644 --- a/lib/crates/fabro-cli/Cargo.toml +++ b/lib/crates/fabro-cli/Cargo.toml @@ -31,6 +31,8 @@ fabro-hooks = { path = "../fabro-hooks" } fabro-install = { path = "../fabro-install" } fabro-interview = { path = "../fabro-interview" } fabro-mcp = { path = "../fabro-mcp" } +fabro-mcp-server = { path = "../fabro-mcp-server" } +fabro-manifest = { path = "../fabro-manifest" } fabro-proc = { path = "../fabro-proc" } fabro-sandbox = { path = "../fabro-sandbox", features = ["daytona"] } fabro-checkpoint = { path = "../fabro-checkpoint" } diff --git a/lib/crates/fabro-cli/src/args.rs b/lib/crates/fabro-cli/src/args.rs index 8525fa0df..0ed502b1b 100644 --- a/lib/crates/fabro-cli/src/args.rs +++ b/lib/crates/fabro-cli/src/args.rs @@ -168,6 +168,49 @@ pub(crate) struct ServerConnectionArgs { pub(crate) target: ServerTargetArgs, } +#[derive(Args)] +pub(crate) struct McpNamespace { + #[command(subcommand)] + pub(crate) command: McpCommand, +} + +#[derive(Subcommand)] +pub(crate) enum McpCommand { + /// Start the Fabro MCP server over stdio + Start(McpStartArgs), + /// Print MCP client configuration JSON + Config(McpConfigArgs), + /// Configure an MCP client to launch Fabro + Init(McpInitArgs), +} + +#[derive(Args, Debug, Clone, Default)] +pub(crate) struct McpStartArgs { + #[command(flatten)] + pub(crate) connection: ServerConnectionArgs, +} + +#[derive(Args, Debug, Clone, Default)] +pub(crate) struct McpConfigArgs { + #[command(flatten)] + pub(crate) connection: ServerConnectionArgs, +} + +#[derive(Args, Debug, Clone)] +pub(crate) struct McpInitArgs { + pub(crate) agent: McpAgent, + + #[command(flatten)] + pub(crate) connection: ServerConnectionArgs, +} + +#[derive(Debug, Clone, Copy, ValueEnum)] +pub(crate) enum McpAgent { + Claude, + Cursor, + Windsurf, +} + #[derive(Args, Debug, Clone, Default)] pub(crate) struct InputOverrideArgs { /// Override a workflow input value (repeatable, format: KEY=VALUE) @@ -1118,6 +1161,8 @@ pub(crate) enum Commands { #[command(subcommand)] command: Option, }, + /// Model Context Protocol server + Mcp(McpNamespace), /// Server operations Server(ServerNamespace), /// Check environment and integration health @@ -1209,6 +1254,11 @@ impl Commands { Some(ModelsCommand::Test(_)) => "model test", None => "model", }, + Self::Mcp(ns) => match &ns.command { + McpCommand::Start(_) => "mcp start", + McpCommand::Config(_) => "mcp config", + McpCommand::Init(_) => "mcp init", + }, Self::Server(ns) => match &ns.command { ServerCommand::Start(_) => "server start", ServerCommand::Stop(_) => "server stop", diff --git a/lib/crates/fabro-cli/src/command_context.rs b/lib/crates/fabro-cli/src/command_context.rs index 34b7d3876..5403c57ce 100644 --- a/lib/crates/fabro-cli/src/command_context.rs +++ b/lib/crates/fabro-cli/src/command_context.rs @@ -102,6 +102,10 @@ impl CommandContext { &self.cwd } + pub(crate) fn storage_dir(&self) -> &Path { + &self.storage_dir + } + pub(crate) fn run_settings(&self) -> Result<&RunNamespace> { self.run_settings .as_ref() diff --git a/lib/crates/fabro-cli/src/commands/graph.rs b/lib/crates/fabro-cli/src/commands/graph.rs index dbfae41fc..1c046d311 100644 --- a/lib/crates/fabro-cli/src/commands/graph.rs +++ b/lib/crates/fabro-cli/src/commands/graph.rs @@ -12,13 +12,13 @@ use std::io::Write; use anyhow::{Context, bail}; use fabro_api::types; use fabro_config::user::active_settings_path; +use fabro_manifest::{ManifestBuildInput, build_run_manifest}; use fabro_util::terminal::Styles; use tracing::debug; use crate::args::{GraphArgs, GraphDirection, GraphOutputFormat}; use crate::command_context::CommandContext; use crate::commands::run::output::api_diagnostics_to_local; -use crate::manifest_builder::{ManifestBuildInput, build_run_manifest}; use crate::shared::{absolute_or_current, print_diagnostics, print_json_pretty, relative_path}; pub(crate) async fn run( diff --git a/lib/crates/fabro-cli/src/commands/mcp/mod.rs b/lib/crates/fabro-cli/src/commands/mcp/mod.rs new file mode 100644 index 000000000..f6a64a0ca --- /dev/null +++ b/lib/crates/fabro-cli/src/commands/mcp/mod.rs @@ -0,0 +1,91 @@ +use std::fmt::Write as _; + +use anyhow::{Context as _, Result}; + +use crate::args::{McpAgent, McpCommand, McpNamespace, ServerConnectionArgs}; +use crate::command_context::CommandContext; +use crate::server_client; + +pub(crate) async fn dispatch(ns: McpNamespace, base_ctx: &CommandContext) -> Result<()> { + match ns.command { + McpCommand::Start(args) => { + fabro_mcp_server::start(server_settings(base_ctx, &args.connection)?).await + } + McpCommand::Config(args) => { + let json = fabro_mcp_server::config_json(&config_settings(&args.connection))?; + let _ = write!(base_ctx.printer().stdout_important(), "{json}"); + Ok(()) + } + McpCommand::Init(args) => { + fabro_mcp_server::init_agent(&init_settings(args.agent, &args.connection)?)?; + Ok(()) + } + } +} + +fn server_settings( + base_ctx: &CommandContext, + connection: &ServerConnectionArgs, +) -> Result { + let connection_ctx = base_ctx.with_connection(connection)?; + let target = connection.target.clone(); + let user_settings = connection_ctx.user_settings().clone(); + let storage_dir = connection_ctx.storage_dir().to_path_buf(); + let base_config_path = connection_ctx.base_config_path().to_path_buf(); + let config_path = base_config_path.clone(); + let client_factory: fabro_mcp_server::FabroClientFactory = std::sync::Arc::new(move || { + let target = target.clone(); + let user_settings = user_settings.clone(); + let storage_dir = storage_dir.clone(); + let base_config_path = base_config_path.clone(); + let future: fabro_mcp_server::FabroClientFuture = Box::pin(async move { + server_client::connect_server_with_settings( + &target, + &user_settings, + &storage_dir, + &base_config_path, + ) + .await + }); + future + }); + Ok(fabro_mcp_server::FabroMcpServerSettings { + client_factory, + config_path, + cwd: base_ctx.cwd().to_path_buf(), + }) +} + +fn init_settings( + agent: McpAgent, + connection: &ServerConnectionArgs, +) -> Result { + Ok(fabro_mcp_server::McpInitSettings { + agent: McpAgentForServer(agent).into(), + config: config_settings(connection), + home_dir: home_dir()?, + }) +} + +fn config_settings(connection: &ServerConnectionArgs) -> fabro_mcp_server::McpConfigSettings { + fabro_mcp_server::McpConfigSettings { + server: connection.target.server.clone(), + storage_dir: connection.storage_dir.clone_path(), + } +} + +fn home_dir() -> Result { + dirs::home_dir().context("failed to resolve home directory for MCP config") +} + +struct McpAgentForServer(McpAgent); + +impl From for fabro_mcp_server::McpAgent { + fn from(value: McpAgentForServer) -> Self { + match value.0 { + McpAgent::Claude => Self::Claude, + McpAgent::Cursor => Self::Cursor, + McpAgent::Windsurf => Self::Windsurf, + } + } +} diff --git a/lib/crates/fabro-cli/src/commands/mod.rs b/lib/crates/fabro-cli/src/commands/mod.rs index 5f2b24b16..7e5c3ee69 100644 --- a/lib/crates/fabro-cli/src/commands/mod.rs +++ b/lib/crates/fabro-cli/src/commands/mod.rs @@ -7,6 +7,7 @@ pub(crate) mod dump; pub(crate) mod exec; pub(crate) mod graph; pub(crate) mod install; +pub(crate) mod mcp; pub(crate) mod model; pub(crate) mod parse; pub(crate) mod pr; diff --git a/lib/crates/fabro-cli/src/commands/preflight.rs b/lib/crates/fabro-cli/src/commands/preflight.rs index a3d1dbdf6..f51c508ad 100644 --- a/lib/crates/fabro-cli/src/commands/preflight.rs +++ b/lib/crates/fabro-cli/src/commands/preflight.rs @@ -1,5 +1,6 @@ use anyhow::bail; use fabro_config::user::active_settings_path; +use fabro_manifest::{ManifestBuildInput, build_run_manifest}; use fabro_util::terminal::Styles; use crate::args::PreflightArgs; @@ -8,7 +9,7 @@ use crate::commands::run::output::{ api_check_report_to_local, api_diagnostics_to_local, print_workflow_summary, }; use crate::commands::run::overrides::preflight_args_overrides; -use crate::manifest_builder::{ManifestBuildInput, build_run_manifest, preflight_manifest_args}; +use crate::manifest_args::preflight_manifest_args; use crate::shared::{cyan_spinner, print_json_pretty}; pub(crate) async fn execute( diff --git a/lib/crates/fabro-cli/src/commands/run/create.rs b/lib/crates/fabro-cli/src/commands/run/create.rs index c56a89024..bcbac4a8c 100644 --- a/lib/crates/fabro-cli/src/commands/run/create.rs +++ b/lib/crates/fabro-cli/src/commands/run/create.rs @@ -1,6 +1,7 @@ use anyhow::{Context as _, bail}; use fabro_config::RunLayer; use fabro_config::user::active_settings_path; +use fabro_manifest::{ManifestBuildInput, build_run_manifest}; use fabro_server::manifest_validation; use fabro_types::RunId; use fabro_util::terminal::Styles; @@ -9,7 +10,7 @@ use super::output::{api_diagnostics_to_local, print_workflow_summary}; use super::overrides::run_args_overrides; use crate::args::RunArgs; use crate::command_context::CommandContext; -use crate::manifest_builder::{ManifestBuildInput, build_run_manifest, run_manifest_args}; +use crate::manifest_args::run_manifest_args; pub(crate) struct CreatedRun { pub(crate) run_id: RunId, diff --git a/lib/crates/fabro-cli/src/commands/run/overrides.rs b/lib/crates/fabro-cli/src/commands/run/overrides.rs index 6e1174aae..d4de0183f 100644 --- a/lib/crates/fabro-cli/src/commands/run/overrides.rs +++ b/lib/crates/fabro-cli/src/commands/run/overrides.rs @@ -2,14 +2,11 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; use anyhow::{Result, anyhow}; -use fabro_config::{ - CliLayer, CliOutputLayer, ReplaceMap, RunExecutionLayer, RunGoalLayer, RunLayer, RunModelLayer, - RunSandboxLayer, parse_input_overrides, -}; +use fabro_config::{CliLayer, CliOutputLayer, RunGoalLayer, RunLayer, parse_input_overrides}; +use fabro_manifest::{RunOverrideInput, build_run_overrides}; use fabro_sandbox::SandboxProvider; use fabro_types::settings::cli::OutputVerbosity; use fabro_types::settings::interp::InterpString; -use fabro_types::settings::run::{ApprovalMode, RunMode}; use crate::args::{PreflightArgs, RunArgs}; @@ -32,47 +29,6 @@ pub(crate) fn parse_labels(labels: &[String]) -> HashMap { .collect() } -fn model_from_args(model: Option<&str>, provider: Option<&str>) -> Option { - if model.is_none() && provider.is_none() { - return None; - } - Some(RunModelLayer { - provider: provider.map(InterpString::parse), - name: model.map(InterpString::parse), - fallbacks: Vec::new(), - }) -} - -fn sandbox_layer( - sandbox: Option, - preserve: Option, -) -> Option { - if sandbox.is_none() && preserve.is_none() { - return None; - } - Some(RunSandboxLayer { - provider: sandbox.map(|p| p.to_string()), - preserve, - ..RunSandboxLayer::default() - }) -} - -fn execution_layer(dry_run: Option, auto_approve: Option) -> Option { - if dry_run.is_none() && auto_approve.is_none() { - return None; - } - Some(RunExecutionLayer { - mode: dry_run.map(|d| if d { RunMode::DryRun } else { RunMode::Normal }), - approval: auto_approve.map(|a| { - if a { - ApprovalMode::Auto - } else { - ApprovalMode::Prompt - } - }), - }) -} - fn cli_layer_for_verbose(verbose: bool) -> Option { verbose.then(|| CliLayer { output: Some(CliOutputLayer { @@ -119,24 +75,22 @@ fn current_dir_or_dot() -> PathBuf { } pub(crate) fn run_args_overrides(args: &RunArgs) -> Result { - let model = model_from_args(args.model.as_deref(), args.provider.as_deref()); - let sandbox = sandbox_layer( - args.sandbox.map(Into::into), - sparse_flag(args.preserve_sandbox), - ); - let execution = execution_layer(sparse_flag(args.dry_run), sparse_flag(args.auto_approve)); - let cwd = current_dir_or_dot(); let goal = goal_layer_from_args(args.goal.as_deref(), args.goal_file.as_deref(), &cwd)?; - - let run = RunLayer { - goal, - metadata: ReplaceMap::from(parse_labels(&args.label)), - model, - sandbox, - execution, - ..RunLayer::default() - }; + let sandbox = args.sandbox.map(SandboxProvider::from); + let sandbox_provider = sandbox.as_ref().map(ToString::to_string); + let mut run = build_run_overrides(RunOverrideInput { + goal: None, + model: args.model.as_deref(), + provider: args.provider.as_deref(), + sandbox: sandbox_provider.as_deref(), + docker_image: None, + preserve_sandbox: sparse_flag(args.preserve_sandbox), + dry_run: sparse_flag(args.dry_run), + auto_approve: sparse_flag(args.auto_approve), + labels: parse_labels(&args.label), + }); + run.goal = goal; Ok(ManifestSettingsOverrides { run: Some(run), @@ -146,21 +100,23 @@ pub(crate) fn run_args_overrides(args: &RunArgs) -> Result Result { - let model = model_from_args(args.model.as_deref(), args.provider.as_deref()); - let sandbox = args.sandbox.map(|s| RunSandboxLayer { - provider: Some(SandboxProvider::from(s).to_string()), - ..RunSandboxLayer::default() - }); - let cwd = current_dir_or_dot(); let goal = goal_layer_from_args(args.goal.as_deref(), args.goal_file.as_deref(), &cwd)?; - - let run = RunLayer { - goal, - model, - sandbox, - ..RunLayer::default() - }; + let sandbox_provider = args + .sandbox + .map(|sandbox| SandboxProvider::from(sandbox).to_string()); + let mut run = build_run_overrides(RunOverrideInput { + goal: None, + model: args.model.as_deref(), + provider: args.provider.as_deref(), + sandbox: sandbox_provider.as_deref(), + docker_image: None, + preserve_sandbox: None, + dry_run: None, + auto_approve: None, + labels: HashMap::new(), + }); + run.goal = goal; Ok(ManifestSettingsOverrides { run: Some(run), diff --git a/lib/crates/fabro-cli/src/commands/run/start.rs b/lib/crates/fabro-cli/src/commands/run/start.rs index 0af737d81..5ad57539b 100644 --- a/lib/crates/fabro-cli/src/commands/run/start.rs +++ b/lib/crates/fabro-cli/src/commands/run/start.rs @@ -8,5 +8,5 @@ pub(crate) async fn start_run_with_client( run_id: &RunId, resume: bool, ) -> Result<()> { - client.start_run(run_id, resume).await + client.start_run(run_id, resume).await.map(|_| ()) } diff --git a/lib/crates/fabro-cli/src/commands/runs/archive.rs b/lib/crates/fabro-cli/src/commands/runs/archive.rs index 9694bd829..38a5308fe 100644 --- a/lib/crates/fabro-cli/src/commands/runs/archive.rs +++ b/lib/crates/fabro-cli/src/commands/runs/archive.rs @@ -71,7 +71,7 @@ async fn run_bulk(action: Action, identifiers: &[String], ctx: &CommandContext) Action::Unarchive => client.unarchive_run(&run_id).await, }; match result { - Ok(()) => { + Ok(_) => { let run_id_string = run_id.to_string(); changed.push(run_id_string.clone()); if !json { diff --git a/lib/crates/fabro-cli/src/commands/validate.rs b/lib/crates/fabro-cli/src/commands/validate.rs index 74495fadf..d607c4b2c 100644 --- a/lib/crates/fabro-cli/src/commands/validate.rs +++ b/lib/crates/fabro-cli/src/commands/validate.rs @@ -1,13 +1,13 @@ use anyhow::bail; use fabro_config::RunLayer; use fabro_config::user::active_settings_path; +use fabro_manifest::{ManifestBuildInput, build_run_manifest}; use fabro_server::manifest_validation; use fabro_util::terminal::Styles; use crate::args::ValidateArgs; use crate::command_context::CommandContext; use crate::commands::run::output::api_diagnostics_to_local; -use crate::manifest_builder::{ManifestBuildInput, build_run_manifest}; use crate::shared::{print_diagnostics, print_json_pretty, relative_path}; pub(crate) fn run( diff --git a/lib/crates/fabro-cli/src/lib.rs b/lib/crates/fabro-cli/src/lib.rs deleted file mode 100644 index 34a4cfc3f..000000000 --- a/lib/crates/fabro-cli/src/lib.rs +++ /dev/null @@ -1,9 +0,0 @@ -#![expect( - dead_code, - reason = "the library exports manifest builder helpers while the binary owns most CLI dispatch" -)] - -mod args; -mod manifest_builder; - -pub use manifest_builder::{BuiltManifest, ManifestBuildInput, build_run_manifest}; diff --git a/lib/crates/fabro-cli/src/main.rs b/lib/crates/fabro-cli/src/main.rs index adf4562d6..43f39ad80 100644 --- a/lib/crates/fabro-cli/src/main.rs +++ b/lib/crates/fabro-cli/src/main.rs @@ -10,11 +10,7 @@ mod gh; mod landing; mod local_server; mod logging; -#[allow( - unreachable_pub, - reason = "The library exports manifest builder helpers for tests; the binary includes the same module privately." -)] -mod manifest_builder; +mod manifest_args; mod server_client; mod server_runs; mod shared; @@ -281,6 +277,9 @@ async fn main_inner(worker_token: Option) -> (String, Result<()>) { Commands::Model { command } => { commands::model::execute(command, &base_ctx).await?; } + Commands::Mcp(ns) => { + commands::mcp::dispatch(ns, &base_ctx).await?; + } Commands::Server(ns) => { Box::pin(commands::server::dispatch( ns.command, @@ -1196,7 +1195,7 @@ destination = "{destination}" .expect("should parse"); match *cli.command.unwrap() { Commands::RunCmd(RunCommands::Run(args)) => { - let manifest_args = manifest_builder::run_manifest_args(&args) + let manifest_args = manifest_args::run_manifest_args(&args) .expect("input-only args should be retained"); assert_eq!(manifest_args.input, vec!["foo=bar"]); } diff --git a/lib/crates/fabro-cli/src/manifest_args.rs b/lib/crates/fabro-cli/src/manifest_args.rs new file mode 100644 index 000000000..48fc1dbab --- /dev/null +++ b/lib/crates/fabro-cli/src/manifest_args.rs @@ -0,0 +1,39 @@ +use fabro_api::types; + +use crate::args::{PreflightArgs, RunArgs}; + +pub(crate) fn run_manifest_args(args: &RunArgs) -> Option { + let payload = types::ManifestArgs { + auto_approve: args.auto_approve.then_some(true), + dry_run: args.dry_run.then_some(true), + label: args.label.clone(), + model: args.model.clone(), + preserve_sandbox: args.preserve_sandbox.then_some(true), + provider: args.provider.clone(), + sandbox: args + .sandbox + .map(|provider| fabro_sandbox::SandboxProvider::from(provider).to_string()), + docker_image: None, + input: args.inputs.values.clone(), + verbose: args.verbose.then_some(true), + }; + (!fabro_manifest::manifest_args_is_empty(&payload)).then_some(payload) +} + +pub(crate) fn preflight_manifest_args(args: &PreflightArgs) -> Option { + let payload = types::ManifestArgs { + auto_approve: None, + dry_run: None, + label: Vec::new(), + model: args.model.clone(), + preserve_sandbox: None, + provider: args.provider.clone(), + sandbox: args + .sandbox + .map(|provider| fabro_sandbox::SandboxProvider::from(provider).to_string()), + docker_image: None, + input: args.inputs.values.clone(), + verbose: args.verbose.then_some(true), + }; + (!fabro_manifest::manifest_args_is_empty(&payload)).then_some(payload) +} diff --git a/lib/crates/fabro-cli/src/shared/utilities.rs b/lib/crates/fabro-cli/src/shared/utilities.rs index 433886932..7f3c9f20f 100644 --- a/lib/crates/fabro-cli/src/shared/utilities.rs +++ b/lib/crates/fabro-cli/src/shared/utilities.rs @@ -127,18 +127,7 @@ pub(crate) fn color_if(use_color: bool, color: Color) -> Option { } pub(crate) fn run_status_kind(status: RunStatus) -> &'static str { - match status { - RunStatus::Submitted => "submitted", - RunStatus::Queued => "queued", - RunStatus::Starting => "starting", - RunStatus::Running => "running", - RunStatus::Blocked { .. } => "blocked", - RunStatus::Paused { .. } => "paused", - RunStatus::Removing => "removing", - RunStatus::Succeeded { .. } => "succeeded", - RunStatus::Failed { .. } => "failed", - RunStatus::Dead => "dead", - } + status.kind().into() } pub(crate) fn split_run_path(s: &str) -> Option<(&str, &str)> { diff --git a/lib/crates/fabro-cli/tests/it/cmd/fabro.rs b/lib/crates/fabro-cli/tests/it/cmd/fabro.rs index ff151778f..6fc7aaff8 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/fabro.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/fabro.rs @@ -33,6 +33,7 @@ fn help() { archive Mark terminal runs as archived (reviewed, no further action needed). Archived runs are hidden from default listings unarchive Restore archived runs to their prior terminal status model List and test LLM models + mcp Model Context Protocol server server Server operations doctor Check environment and integration health version Show client and server version information diff --git a/lib/crates/fabro-cli/tests/it/cmd/mcp.rs b/lib/crates/fabro-cli/tests/it/cmd/mcp.rs new file mode 100644 index 000000000..358372879 --- /dev/null +++ b/lib/crates/fabro-cli/tests/it/cmd/mcp.rs @@ -0,0 +1,2305 @@ +#![expect( + clippy::disallowed_methods, + reason = "integration tests stage MCP config files with sync std::fs" +)] +#![expect( + clippy::disallowed_types, + reason = "raw stdio regression test intentionally uses blocking std pipes outside Tokio" +)] + +use std::collections::HashMap; +use std::io::{BufRead as _, Write as _}; +use std::path::{Path, PathBuf}; +use std::process::Stdio; + +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; + +use super::support::{mock_resolved_run, remote_run_summary_json}; +use crate::support::{ + RealAuthHarness, TEST_DEV_TOKEN, run_projection_json, seed_dev_token_auth, unique_run_id, +}; + +#[test] +fn help() { + let context = test_context!(); + let mut cmd = context.command(); + cmd.args(["mcp", "--help"]); + fabro_snapshot!(context.filters(), cmd, @" + success: true + exit_code: 0 + ----- stdout ----- + Model Context Protocol server + + Usage: fabro mcp [OPTIONS] + + Commands: + start Start the Fabro MCP server over stdio + config Print MCP client configuration JSON + init Configure an MCP client to launch Fabro + help Print this message or the help of the given subcommand(s) + + Options: + --json Output as JSON [env: FABRO_JSON=] + --debug Enable DEBUG-level logging (default is INFO) [env: FABRO_DEBUG=] + --no-upgrade-check Disable automatic upgrade check [env: FABRO_NO_UPGRADE_CHECK=true] + --quiet Suppress non-essential output [env: FABRO_QUIET=] + --verbose Enable verbose output [env: FABRO_VERBOSE=] + -h, --help Print help + ----- stderr ----- + "); +} + +#[test] +fn start_help() { + let context = test_context!(); + let mut cmd = context.command(); + cmd.args(["mcp", "start", "--help"]); + fabro_snapshot!(context.filters(), cmd, @" + success: true + exit_code: 0 + ----- stdout ----- + Start the Fabro MCP server over stdio + + Usage: fabro mcp start [OPTIONS] + + Options: + --json Output as JSON [env: FABRO_JSON=] + --storage-dir Local storage directory (default: ~/.fabro/storage) [env: FABRO_STORAGE_DIR=] + --debug Enable DEBUG-level logging (default is INFO) [env: FABRO_DEBUG=] + --server Fabro server target: http(s) URL or absolute Unix socket path [env: FABRO_SERVER=] + --no-upgrade-check Disable automatic upgrade check [env: FABRO_NO_UPGRADE_CHECK=true] + --quiet Suppress non-essential output [env: FABRO_QUIET=] + --verbose Enable verbose output [env: FABRO_VERBOSE=] + -h, --help Print help + ----- stderr ----- + "); +} + +#[test] +fn config_help() { + let context = test_context!(); + let mut cmd = context.command(); + cmd.args(["mcp", "config", "--help"]); + fabro_snapshot!(context.filters(), cmd, @" + success: true + exit_code: 0 + ----- stdout ----- + Print MCP client configuration JSON + + Usage: fabro mcp config [OPTIONS] + + Options: + --json Output as JSON [env: FABRO_JSON=] + --storage-dir Local storage directory (default: ~/.fabro/storage) [env: FABRO_STORAGE_DIR=] + --debug Enable DEBUG-level logging (default is INFO) [env: FABRO_DEBUG=] + --server Fabro server target: http(s) URL or absolute Unix socket path [env: FABRO_SERVER=] + --no-upgrade-check Disable automatic upgrade check [env: FABRO_NO_UPGRADE_CHECK=true] + --quiet Suppress non-essential output [env: FABRO_QUIET=] + --verbose Enable verbose output [env: FABRO_VERBOSE=] + -h, --help Print help + ----- stderr ----- + "); +} + +#[test] +fn init_help() { + let context = test_context!(); + let mut cmd = context.command(); + cmd.args(["mcp", "init", "--help"]); + fabro_snapshot!(context.filters(), cmd, @" + success: true + exit_code: 0 + ----- stdout ----- + Configure an MCP client to launch Fabro + + Usage: fabro mcp init [OPTIONS] + + Arguments: + [possible values: claude, cursor, windsurf] + + Options: + --json Output as JSON [env: FABRO_JSON=] + --storage-dir Local storage directory (default: ~/.fabro/storage) [env: FABRO_STORAGE_DIR=] + --debug Enable DEBUG-level logging (default is INFO) [env: FABRO_DEBUG=] + --server Fabro server target: http(s) URL or absolute Unix socket path [env: FABRO_SERVER=] + --no-upgrade-check Disable automatic upgrade check [env: FABRO_NO_UPGRADE_CHECK=true] + --quiet Suppress non-essential output [env: FABRO_QUIET=] + --verbose Enable verbose output [env: FABRO_VERBOSE=] + -h, --help Print help + ----- stderr ----- + "); +} + +#[test] +fn config_prints_generic_mcp_json() { + let context = test_context!(); + let mut cmd = context.command(); + cmd.args(["mcp", "config"]); + fabro_snapshot!(context.filters(), cmd, @r#" + success: true + exit_code: 0 + ----- stdout ----- + { + "mcpServers": { + "fabro": { + "command": "fabro", + "args": [ + "mcp", + "start" + ] + } + } + } + ----- stderr ----- + "#); +} + +#[test] +fn config_preserves_connection_flags() { + let context = test_context!(); + let mut cmd = context.command(); + cmd.args([ + "mcp", + "config", + "--server", + "https://example.test/api/v1", + "--storage-dir", + "/tmp/fabro-mcp-storage", + ]); + fabro_snapshot!(context.filters(), cmd, @r#" + success: true + exit_code: 0 + ----- stdout ----- + { + "mcpServers": { + "fabro": { + "command": "fabro", + "args": [ + "mcp", + "start", + "--server", + "https://example.test/api/v1", + "--storage-dir", + "/tmp/fabro-mcp-storage" + ] + } + } + } + ----- stderr ----- + "#); +} + +#[test] +fn init_cursor_writes_idempotent_config() { + let context = test_context!(); + context + .command() + .args(["mcp", "init", "cursor"]) + .assert() + .success(); + context + .command() + .args(["mcp", "init", "cursor"]) + .assert() + .success(); + + let config_path = context.home_dir.join(".cursor").join("mcp.json"); + let config: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(config_path).unwrap()).unwrap(); + fabro_json_snapshot!(context, config, @r#" + { + "mcpServers": { + "fabro": { + "command": "fabro", + "args": [ + "mcp", + "start" + ] + } + } + } + "#); +} + +#[test] +fn init_claude_writes_desktop_and_code_configs() { + let context = test_context!(); + context + .command() + .args(["mcp", "init", "claude"]) + .assert() + .success(); + + let desktop_config: serde_json::Value = serde_json::from_str( + &std::fs::read_to_string(expected_claude_desktop_config_path(&context.home_dir)).unwrap(), + ) + .unwrap(); + fabro_json_snapshot!(context, desktop_config, @r#" + { + "mcpServers": { + "fabro": { + "command": "fabro", + "args": [ + "mcp", + "start" + ] + } + } + } + "#); + + let code_config: serde_json::Value = serde_json::from_str( + &std::fs::read_to_string(context.home_dir.join(".claude.json")).unwrap(), + ) + .unwrap(); + fabro_json_snapshot!(context, code_config, @r#" + { + "mcpServers": { + "fabro": { + "command": "fabro", + "args": [ + "mcp", + "start" + ] + } + } + } + "#); +} + +#[test] +fn init_claude_preserves_existing_claude_code_config() { + let context = test_context!(); + let claude_code_path = context.home_dir.join(".claude.json"); + std::fs::write( + &claude_code_path, + r#"{"numStartups":42,"mcpServers":{"other":{"type":"http","url":"https://example.test/mcp"}}}"#, + ) + .unwrap(); + + context + .command() + .args(["mcp", "init", "claude"]) + .assert() + .success(); + + let config: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&claude_code_path).unwrap()).unwrap(); + fabro_json_snapshot!(context, config, @r#" + { + "numStartups": 42, + "mcpServers": { + "other": { + "type": "http", + "url": "https://example.test/mcp" + }, + "fabro": { + "command": "fabro", + "args": [ + "mcp", + "start" + ] + } + } + } + "#); +} + +#[test] +fn init_windsurf_writes_config() { + let context = test_context!(); + context + .command() + .args(["mcp", "init", "windsurf"]) + .assert() + .success(); + + let config_path = context + .home_dir + .join(".codeium") + .join("windsurf") + .join("mcp_config.json"); + let config: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(config_path).unwrap()).unwrap(); + fabro_json_snapshot!(context, config, @r#" + { + "mcpServers": { + "fabro": { + "command": "fabro", + "args": [ + "mcp", + "start" + ] + } + } + } + "#); +} + +#[test] +fn init_preserves_existing_servers() { + let context = test_context!(); + let config_path = context.home_dir.join(".cursor").join("mcp.json"); + std::fs::create_dir_all(config_path.parent().unwrap()).unwrap(); + std::fs::write( + &config_path, + r#"{"mcpServers":{"other":{"command":"other","args":["serve"]}},"theme":"dark"}"#, + ) + .unwrap(); + + context + .command() + .args([ + "mcp", + "init", + "cursor", + "--server", + "https://example.test/api/v1", + ]) + .assert() + .success(); + + let config: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(config_path).unwrap()).unwrap(); + fabro_json_snapshot!(context, config, @r#" + { + "mcpServers": { + "other": { + "command": "other", + "args": [ + "serve" + ] + }, + "fabro": { + "command": "fabro", + "args": [ + "mcp", + "start", + "--server", + "https://example.test/api/v1" + ] + } + }, + "theme": "dark" + } + "#); +} + +#[test] +fn init_invalid_json_fails_without_overwrite() { + let context = test_context!(); + let config_path = context.home_dir.join(".cursor").join("mcp.json"); + std::fs::create_dir_all(config_path.parent().unwrap()).unwrap(); + std::fs::write(&config_path, "{not json").unwrap(); + + let mut cmd = context.command(); + cmd.args(["mcp", "init", "cursor"]); + fabro_snapshot!(context.filters(), cmd, @" + success: false + exit_code: 1 + ----- stdout ----- + ----- stderr ----- + × failed to parse MCP config [HOME_DIR]/.cursor/mcp.json + ╰─▶ key must be a string at line 1 column 2 + "); + assert_eq!(std::fs::read_to_string(config_path).unwrap(), "{not json"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn stdio_server_initializes_and_lists_run_tools() { + let context = test_context!(); + let client = spawn_mcp_client(&context, &[]).await; + + let tools = client.list_tools().await.unwrap(); + let names: Vec<_> = tools.iter().map(|(name, _, _)| name.as_str()).collect(); + assert_eq!(names, vec![ + "fabro_run_create", + "fabro_run_events", + "fabro_run_gather", + "fabro_run_interact", + "fabro_run_search", + ]); + for (name, _, schema) in &tools { + assert!( + schema.is_object(), + "tool should have input schema: {schema}" + ); + let properties = schema + .get("properties") + .and_then(serde_json::Value::as_object) + .expect("tool input schema should have properties"); + for (property, property_schema) in properties { + assert!( + property_schema.is_object(), + "{name}.{property} should use an object JSON Schema, got {property_schema}" + ); + } + } + let interact_schema = tools + .iter() + .find(|(name, _, _)| name == "fabro_run_interact") + .map(|(_, _, schema)| schema) + .expect("fabro_run_interact tool should be listed"); + assert!( + interact_schema + .pointer("/properties/answer") + .is_some_and(serde_json::Value::is_object), + "fabro_run_interact.answer should have an object JSON Schema: {interact_schema}" + ); + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + +#[test] +fn stdio_start_writes_only_json_rpc_to_stdout() { + let context = test_context!(); + let fixture = mcp_stdio_fixture(&context, &[]); + let mut cmd = std::process::Command::new(&fixture.command[0]); + cmd.args(&fixture.command[1..]) + .env_clear() + .envs(&fixture.env) + .current_dir(&fixture.current_dir) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + + let mut child = cmd.spawn().unwrap(); + let mut stdin = child.stdin.take().unwrap(); + writeln!( + stdin, + r#"{{"jsonrpc":"2.0","id":1,"method":"initialize","params":{{"protocolVersion":"2025-06-18","capabilities":{{}},"clientInfo":{{"name":"fabro-test","version":"0.0.0"}}}}}}"# + ) + .unwrap(); + + let stdout = child.stdout.take().unwrap(); + let (tx, rx) = std::sync::mpsc::channel(); + std::thread::spawn(move || { + let mut line = String::new(); + let result = std::io::BufReader::new(stdout).read_line(&mut line); + let _ = tx.send(result.map(|_| line)); + }); + + let line = rx + .recv_timeout(std::time::Duration::from_secs(5)) + .expect("initialize response should arrive") + .expect("stdout should be readable"); + let value: serde_json::Value = serde_json::from_str(line.trim()).unwrap(); + assert_eq!(value["jsonrpc"], "2.0"); + + let _ = child.kill(); + let _ = child.wait(); +} + +#[tokio::test(flavor = "multi_thread")] +async fn stdio_startup_and_list_tools_is_fast() { + let context = test_context!(); + let start = std::time::Instant::now(); + let client = spawn_mcp_client(&context, &[]).await; + let tools = client.list_tools().await.unwrap(); + assert_eq!(tools.len(), 5); + assert!(start.elapsed() < std::time::Duration::from_secs(2)); + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_create_and_search_manage_real_runs_with_cli_auth() { + let context = test_context!(); + let harness = + RealAuthHarness::start_with_dev_token(fabro_test::GitHubAppState::default()).await; + let target_url = harness.api_target(); + let target: fabro_client::ServerTarget = target_url.parse().unwrap(); + seed_dev_token_auth(&context.home_dir, &target, TEST_DEV_TOKEN); + let workflow = context.install_fixture("simple.fabro"); + + let client = spawn_mcp_client(&context, &["--server", &target_url]).await; + + let create = call_tool_json( + &client, + "fabro_run_create", + serde_json::json!({ + "runs": [{ + "workflow": workflow, + "dry_run": true, + "auto_approve": true, + "labels": { "source": "mcp-test" } + }] + }), + ) + .await; + let run_id = create["runs"][0]["run_id"].as_str().unwrap().to_string(); + assert_eq!(create["runs"][0]["started"], true); + + let search = call_tool_json( + &client, + "fabro_run_search", + serde_json::json!({ + "run_ids": [run_id], + "labels": { "source": "mcp-test" }, + "first": 10 + }), + ) + .await; + fabro_json_snapshot!(context, normalize_run_search(search), @r#" + { + "runs": [ + { + "run_id": "[RUN_ID]", + "workflow_name": "Simple", + "workflow_slug": "simple", + "status": "queued", + "archived": false, + "created_at": "[TIMESTAMP]", + "started_at": null, + "completed_at": null, + "labels": { + "source": "mcp-test" + }, + "source_directory": "[SOURCE_DIRECTORY]", + "repo_origin_url": null, + "goal_preview": "Run tests and report results", + "goal_truncated": false + } + ], + "next_cursor": null + } + "#); + + client + .shutdown() + .await + .expect("MCP client should shut down"); + harness.shutdown().await; +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_run_tools_use_default_local_server_without_server_flag() { + let context = test_context!(); + let workflow = context.install_fixture("simple.fabro"); + let client = spawn_mcp_client(&context, &[]).await; + + let create = call_tool_json( + &client, + "fabro_run_create", + serde_json::json!({ + "runs": [{ + "workflow": workflow, + "dry_run": true, + "auto_approve": true, + "labels": { "source": "mcp-default-server-test" }, + "start": false + }] + }), + ) + .await; + let run_id = create["runs"][0]["run_id"].as_str().unwrap(); + let search = call_tool_json( + &client, + "fabro_run_search", + serde_json::json!({ "run_ids": [run_id], "first": 1 }), + ) + .await; + + assert_eq!(search["runs"][0]["run_id"], run_id); + assert_eq!( + search["runs"][0]["labels"]["source"], + "mcp-default-server-test" + ); + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_configured_unix_target_auto_spawns_like_cli() { + let mut context = test_context!(); + let isolated = fabro_test::isolated_storage_dir(); + let storage_dir = isolated.path().join("storage"); + let socket_path = isolated.path().join("configured.sock"); + write_mcp_server_settings(&mut context, &storage_dir, Some(&socket_path)); + let workflow = context.install_fixture("simple.fabro"); + + assert!(!socket_path.exists()); + let client = spawn_mcp_client(&context, &[]).await; + let run_id = create_mcp_run(&client, workflow, false).await; + let search = call_tool_json( + &client, + "fabro_run_search", + serde_json::json!({ "run_ids": [run_id], "first": 1 }), + ) + .await; + + assert_eq!(search["runs"][0]["run_id"], run_id); + assert!( + socket_path.exists(), + "configured Unix socket should be auto-spawned" + ); + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_explicit_unix_target_auto_spawns_like_cli() { + let mut context = test_context!(); + let isolated = fabro_test::isolated_storage_dir(); + let storage_dir = isolated.path().join("storage"); + let socket_path = isolated.path().join("explicit.sock"); + write_mcp_server_settings(&mut context, &storage_dir, None); + let workflow = context.install_fixture("simple.fabro"); + let storage_arg = storage_dir.display().to_string(); + let socket_arg = socket_path.display().to_string(); + + assert!(!socket_path.exists()); + let client = spawn_mcp_client(&context, &[ + "--storage-dir", + &storage_arg, + "--server", + &socket_arg, + ]) + .await; + let run_id = create_mcp_run(&client, workflow, false).await; + let search = call_tool_json( + &client, + "fabro_run_search", + serde_json::json!({ "run_ids": [run_id], "first": 1 }), + ) + .await; + + assert_eq!(search["runs"][0]["run_id"], run_id); + assert!( + socket_path.exists(), + "explicit Unix socket should be auto-spawned" + ); + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_missing_default_settings_reports_configure_first_error_and_stays_alive() { + let home = tempfile::tempdir().unwrap(); + let workspace = tempfile::tempdir().unwrap(); + let mut env = fabro_test::isolated_env(home.path()); + env.insert( + "FABRO_HOME".to_string(), + home.path().join(".fabro").display().to_string(), + ); + let client = spawn_mcp_client_from_fixture(McpStdioFixture { + command: vec![ + env!("CARGO_BIN_EXE_fabro").to_string(), + "mcp".to_string(), + "start".to_string(), + ], + env, + current_dir: workspace.path().to_path_buf(), + }) + .await; + + let error = call_tool_error_text( + &client, + "fabro_run_search", + serde_json::json!({ "run_ids": ["missing"], "first": 1 }), + ) + .await; + + assert!( + error.contains("Cannot reach Fabro server: no settings.toml configured."), + "{error}" + ); + assert_eq!(client.list_tools().await.unwrap().len(), 5); + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_search_filters_status_dates_and_paginates() { + let context = test_context!(); + let harness = + RealAuthHarness::start_with_dev_token(fabro_test::GitHubAppState::default()).await; + let target_url = harness.api_target(); + let target: fabro_client::ServerTarget = target_url.parse().unwrap(); + seed_dev_token_auth(&context.home_dir, &target, TEST_DEV_TOKEN); + let workflow = context.install_fixture("simple.fabro"); + let client = spawn_mcp_client(&context, &["--server", &target_url]).await; + let first = create_mcp_run(&client, workflow.clone(), false).await; + let second = create_mcp_run(&client, workflow, false).await; + + let page_one = call_tool_json( + &client, + "fabro_run_search", + serde_json::json!({ + "labels": { "source": "mcp-test" }, + "status": ["submitted"], + "archived": false, + "created_after": "2000-01-01", + "created_before": "2100-01-01T00:00:00Z", + "first": 1 + }), + ) + .await; + let cursor = page_one["next_cursor"] + .as_str() + .expect("first page should have cursor"); + let page_two = call_tool_json( + &client, + "fabro_run_search", + serde_json::json!({ + "labels": { "source": "mcp-test" }, + "status": ["submitted"], + "archived": false, + "after": cursor, + "first": 1 + }), + ) + .await; + + let page_one_id = page_one["runs"][0]["run_id"].as_str().unwrap(); + let page_two_id = page_two["runs"][0]["run_id"].as_str().unwrap(); + assert_ne!(page_one_id, page_two_id); + assert!([first.as_str(), second.as_str()].contains(&page_one_id)); + assert!([first.as_str(), second.as_str()].contains(&page_two_id)); + + client + .shutdown() + .await + .expect("MCP client should shut down"); + harness.shutdown().await; +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_search_hides_archived_runs_by_default() { + 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 active_id = unique_run_id(); + let archived_id = unique_run_id(); + let active = remote_run_summary_json( + &active_id, + "Simple", + "simple", + "Active run", + &serde_json::json!({ "kind": "succeeded", "reason": "completed" }), + "2026-04-05T12:00:00Z", + ); + let mut archived = remote_run_summary_json( + &archived_id, + "Simple", + "simple", + "Archived run", + &serde_json::json!({ "kind": "succeeded", "reason": "completed" }), + "2026-04-05T12:01:00Z", + ); + archived["lifecycle"]["archived"] = serde_json::json!(true); + archived["lifecycle"]["archived_at"] = serde_json::json!("2026-04-05T12:02:00Z"); + let active_resolve = mock_resolved_run_json(&server, &active_id, active, None); + let archived_resolve = mock_resolved_run_json(&server, &archived_id, archived, None); + + let client = spawn_mcp_client(&context, &["--server", &target_url]).await; + let result = call_tool_json( + &client, + "fabro_run_search", + serde_json::json!({ "run_ids": [active_id, archived_id], "first": 10 }), + ) + .await; + + assert_eq!(result["runs"].as_array().unwrap().len(), 1); + assert!( + result["runs"] + .as_array() + .unwrap() + .iter() + .all(|run| run["archived"] == false) + ); + active_resolve.assert(); + archived_resolve.assert(); + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_search_refreshes_expired_oauth_token() { + 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_oauth_auth( + &context.home_dir, + &target, + "expired-access", + "refresh-octocat", + ); + let run_id = unique_run_id(); + let expired_access = server.mock(|when, then| { + when.method(GET) + .path("/api/v1/runs/resolve") + .query_param("selector", run_id.clone()) + .header("authorization", "Bearer expired-access"); + then.status(401) + .header("Content-Type", "application/json") + .json_body(serde_json::json!({ + "errors": [{ + "detail": "access token expired", + "code": "access_token_expired" + }] + })); + }); + let refresh = server.mock(|when, then| { + when.method(POST) + .path("/auth/cli/refresh") + .header("authorization", "Bearer refresh-octocat"); + then.status(200) + .header("Content-Type", "application/json") + .json_body(serde_json::json!({ + "access_token": "fresh-access", + "access_token_expires_at": (Utc::now() + ChronoDuration::minutes(10)).to_rfc3339(), + "refresh_token": "fresh-refresh", + "refresh_token_expires_at": (Utc::now() + ChronoDuration::days(30)).to_rfc3339(), + "subject": { + "idp_issuer": "https://github.com", + "idp_subject": "12345", + "login": "octocat", + "name": "The Octocat", + "email": "octocat@example.com" + } + })); + }); + let fresh_access = server.mock(|when, then| { + when.method(GET) + .path("/api/v1/runs/resolve") + .header("authorization", "Bearer fresh-access") + .query_param("selector", run_id.clone()); + then.status(200) + .header("Content-Type", "application/json") + .json_body(remote_run_summary_json( + &run_id, + "Simple", + "simple", + "OAuth refreshed", + &serde_json::json!({ "kind": "submitted" }), + "2026-04-05T12:00:00Z", + )); + }); + let client = spawn_mcp_client(&context, &["--server", &target_url]).await; + + let result = call_tool_json( + &client, + "fabro_run_search", + serde_json::json!({ "run_ids": [run_id], "first": 1 }), + ) + .await; + + assert_eq!(result["runs"][0]["run_id"], run_id); + expired_access.assert(); + refresh.assert(); + fresh_access.assert(); + let stored = AuthStore::new(context.home_dir.join(".fabro/auth.json")) + .get(&target) + .unwrap() + .unwrap(); + let AuthEntry::OAuth(stored) = stored else { + panic!("expected refreshed OAuth entry"); + }; + assert_eq!(stored.access_token, "fresh-access"); + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_search_uses_fabro_auth_file_override() { + 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(); + let auth_file = context.temp_dir.join("custom-auth.json"); + AuthStore::new(auth_file.clone()) + .put( + &target, + AuthEntry::DevToken(DevTokenEntry { + token: TEST_DEV_TOKEN.to_string(), + logged_in_at: Utc::now(), + }), + ) + .expect("custom auth store should be seeded"); + let run_id = unique_run_id(); + let authorization = format!("Bearer {TEST_DEV_TOKEN}"); + let resolve = mock_resolved_run_json( + &server, + &run_id, + remote_run_summary_json( + &run_id, + "Simple", + "simple", + "Custom auth file", + &serde_json::json!({ "kind": "submitted" }), + "2026-04-05T12:00:00Z", + ), + Some(&authorization), + ); + let mut fixture = mcp_stdio_fixture(&context, &["--server", &target_url]); + fixture.env.insert( + "FABRO_AUTH_FILE".to_string(), + auth_file.display().to_string(), + ); + let client = spawn_mcp_client_from_fixture(fixture).await; + + let result = call_tool_json( + &client, + "fabro_run_search", + serde_json::json!({ "run_ids": [run_id], "first": 1 }), + ) + .await; + + assert_eq!(result["runs"][0]["run_id"], run_id); + resolve.assert(); + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_search_orders_by_started_timestamp_before_created_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 submitted_id = unique_run_id(); + let running_id = unique_run_id(); + let submitted = remote_run_summary_json( + &submitted_id, + "Simple", + "simple", + "Submitted later", + &serde_json::json!({ "kind": "submitted" }), + "2026-04-05T12:10:00Z", + ); + let mut running = remote_run_summary_json( + &running_id, + "Simple", + "simple", + "Started later", + &serde_json::json!({ "kind": "running" }), + "2026-04-05T12:00:00Z", + ); + running["timestamps"]["started_at"] = serde_json::json!("2026-04-05T12:20:00Z"); + let submitted_resolve = mock_resolved_run_json(&server, &submitted_id, submitted, None); + let running_resolve = mock_resolved_run_json(&server, &running_id, running, None); + + let client = spawn_mcp_client(&context, &["--server", &target_url]).await; + let result = call_tool_json( + &client, + "fabro_run_search", + serde_json::json!({ "run_ids": [submitted_id, running_id], "first": 2 }), + ) + .await; + + assert_eq!(result["runs"][0]["run_id"], running_id); + assert_eq!(result["runs"][1]["run_id"], submitted_id); + submitted_resolve.assert(); + running_resolve.assert(); + client + .shutdown() + .await + .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 newer_resolve = mock_resolved_run_json(&server, &newer_created_id, newer_created, None); + let older_resolve = mock_resolved_run_json(&server, &older_created_id, older_created, None); + + 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); + newer_resolve.assert(); + older_resolve.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!(); + let harness = + RealAuthHarness::start_with_dev_token(fabro_test::GitHubAppState::default()).await; + let target_url = harness.api_target(); + let target: fabro_client::ServerTarget = target_url.parse().unwrap(); + seed_dev_token_auth(&context.home_dir, &target, TEST_DEV_TOKEN); + let workflow = context.install_fixture("simple.fabro"); + let client = spawn_mcp_client(&context, &["--server", &target_url]).await; + let run_id = create_mcp_run(&client, workflow, true).await; + let cancel = call_tool_json( + &client, + "fabro_run_interact", + serde_json::json!({ "run_id": run_id, "action": "cancel" }), + ) + .await; + + let gather = call_tool_json( + &client, + "fabro_run_gather", + serde_json::json!({ + "run_ids": [run_id], + "timeout_seconds": 20, + "poll_interval_seconds": 5 + }), + ) + .await; + let run_id = gather["runs"][0]["run_id"].as_str().unwrap().to_string(); + let get = call_tool_json( + &client, + "fabro_run_interact", + serde_json::json!({ "run_id": run_id, "action": "get" }), + ) + .await; + let events = call_tool_json( + &client, + "fabro_run_events", + serde_json::json!({ "run_id": run_id, "action": "list", "first": 5 }), + ) + .await; + let archive = call_tool_json( + &client, + "fabro_run_interact", + serde_json::json!({ "run_id": run_id, "action": "archive" }), + ) + .await; + let archived_search = call_tool_json( + &client, + "fabro_run_search", + serde_json::json!({ "run_ids": [run_id], "archived": true }), + ) + .await; + let unarchive = call_tool_json( + &client, + "fabro_run_interact", + serde_json::json!({ "run_id": run_id, "action": "unarchive" }), + ) + .await; + let search = call_tool_json( + &client, + "fabro_run_search", + serde_json::json!({ "run_ids": [run_id], "archived": false }), + ) + .await; + + fabro_json_snapshot!( + context, + serde_json::json!({ + "gather": normalize_gather(gather), + "cancel_action": cancel["action"], + "get_status": get["result"]["summary"]["status"], + "events_nonempty": events["events"].as_array().is_some_and(|events| !events.is_empty()), + "archive_action": archive["action"], + "archived_search_count": archived_search["runs"].as_array().unwrap().len(), + "archived_search_archived": archived_search["runs"][0]["archived"], + "unarchive_action": unarchive["action"], + "unarchived_search_count": search["runs"].as_array().unwrap().len(), + }), + @r#" + { + "gather": { + "runs": [ + { + "run_id": "[RUN_ID]", + "workflow_name": "Simple", + "workflow_slug": "simple", + "status": "failed", + "archived": false, + "created_at": "[TIMESTAMP]", + "started_at": null, + "completed_at": "[TIMESTAMP]", + "labels": { + "source": "mcp-test" + }, + "source_directory": "[SOURCE_DIRECTORY]", + "repo_origin_url": null, + "goal": "Run tests and report results" + } + ], + "timed_out": false, + "elapsed_seconds": "[ELAPSED]" + }, + "cancel_action": "cancel", + "get_status": "failed", + "events_nonempty": true, + "archive_action": "archive", + "archived_search_count": 1, + "archived_search_archived": true, + "unarchive_action": "unarchive", + "unarchived_search_count": 1 + } + "# + ); + + client + .shutdown() + .await + .expect("MCP client should shut down"); + harness.shutdown().await; +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_gather_rejects_too_many_runs() { + let context = test_context!(); + let client = spawn_mcp_client(&context, &["--server", "http://127.0.0.1:9"]).await; + let run_ids = (0..51) + .map(|index| format!("run_{index}")) + .collect::>(); + + let error = call_tool_error_text( + &client, + "fabro_run_gather", + serde_json::json!({ "run_ids": run_ids }), + ) + .await; + + assert!(error.contains("run_ids"), "{error}"); + assert_eq!(client.list_tools().await.unwrap().len(), 5); + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_gather_rejects_invalid_timeout_values_before_auth() { + let context = test_context!(); + let client = spawn_mcp_client(&context, &["--server", "http://127.0.0.1:9"]).await; + + let timeout_error = call_tool_error_text( + &client, + "fabro_run_gather", + serde_json::json!({ + "run_ids": ["run_123"], + "timeout_seconds": 601, + "poll_interval_seconds": 5 + }), + ) + .await; + let poll_error = call_tool_error_text( + &client, + "fabro_run_gather", + serde_json::json!({ + "run_ids": ["run_123"], + "timeout_seconds": 300, + "poll_interval_seconds": 4 + }), + ) + .await; + + assert!(timeout_error.contains("timeout_seconds"), "{timeout_error}"); + assert!( + !timeout_error.contains("fabro auth login"), + "{timeout_error}" + ); + assert!(poll_error.contains("poll_interval_seconds"), "{poll_error}"); + assert!(!poll_error.contains("fabro auth login"), "{poll_error}"); + assert_eq!(client.list_tools().await.unwrap().len(), 5); + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_gather_returns_timeout_result() { + let context = test_context!(); + let harness = + RealAuthHarness::start_with_dev_token(fabro_test::GitHubAppState::default()).await; + let target_url = harness.api_target(); + let target: fabro_client::ServerTarget = target_url.parse().unwrap(); + seed_dev_token_auth(&context.home_dir, &target, TEST_DEV_TOKEN); + let workflow = context.install_fixture("simple.fabro"); + let client = spawn_mcp_client(&context, &["--server", &target_url]).await; + let run_id = create_mcp_run(&client, workflow, false).await; + + let start = std::time::Instant::now(); + let gather = call_tool_json( + &client, + "fabro_run_gather", + serde_json::json!({ + "run_ids": [run_id], + "timeout_seconds": 1, + "poll_interval_seconds": 5 + }), + ) + .await; + + assert_eq!(gather["timed_out"], true); + assert!(start.elapsed() < std::time::Duration::from_secs(4)); + assert_eq!(gather["runs"][0]["status"], "submitted"); + + client + .shutdown() + .await + .expect("MCP client should shut down"); + harness.shutdown().await; +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_interact_error_does_not_stop_server() { + let context = test_context!(); + let client = spawn_mcp_client(&context, &["--server", "http://127.0.0.1:9"]).await; + + let error = call_tool_error_text( + &client, + "fabro_run_interact", + serde_json::json!({ "run_id": "run_123", "action": "message" }), + ) + .await; + + assert!(error.contains("message"), "{error}"); + assert_eq!(client.list_tools().await.unwrap().len(), 5); + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_interact_actions_resolve_selector_and_call_expected_endpoints() { + 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 run_id = unique_run_id(); + let selector = "nightly"; + let resolve = mock_resolved_run(&server, selector, &run_id); + let retrieve = server.mock(|when, then| { + when.method(GET).path(format!("/api/v1/runs/{run_id}")); + then.status(200) + .header("Content-Type", "application/json") + .json_body(remote_run_summary_json( + &run_id, + "Simple", + "simple", + "Run tests", + &serde_json::json!({ "kind": "running" }), + "2026-04-05T12:00:00Z", + )); + }); + let projection = server.mock(|when, then| { + when.method(GET) + .path(format!("/api/v1/runs/{run_id}/state")); + then.status(200) + .header("Content-Type", "application/json") + .json_body(run_projection_json( + &run_id, + &serde_json::json!({ "kind": "running" }), + )); + }); + let start = server.mock(|when, then| { + when.method(POST) + .path(format!("/api/v1/runs/{run_id}/start")) + .json_body(serde_json::json!({ "resume": false })); + then.status(200) + .header("Content-Type", "application/json") + .json_body(remote_run_summary_json( + &run_id, + "Simple", + "simple", + "Run tests", + &serde_json::json!({ "kind": "running" }), + "2026-04-05T12:00:00Z", + )); + }); + let message = server.mock(|when, then| { + when.method(POST) + .path(format!("/api/v1/runs/{run_id}/steer")) + .json_body(serde_json::json!({ "text": "continue", "interrupt": true })); + then.status(202); + }); + let cancel = server.mock(|when, then| { + when.method(POST) + .path(format!("/api/v1/runs/{run_id}/cancel")); + then.status(200) + .header("Content-Type", "application/json") + .json_body(remote_run_summary_json( + &run_id, + "Simple", + "simple", + "Run tests", + &serde_json::json!({ "kind": "running" }), + "2026-04-05T12:00:00Z", + )); + }); + + let client = spawn_mcp_client(&context, &["--server", &target_url]).await; + let get = call_tool_json( + &client, + "fabro_run_interact", + serde_json::json!({ "run_id": selector, "action": "get" }), + ) + .await; + let start_result = call_tool_json( + &client, + "fabro_run_interact", + serde_json::json!({ "run_id": selector, "action": "start" }), + ) + .await; + let message_result = call_tool_json( + &client, + "fabro_run_interact", + serde_json::json!({ + "run_id": selector, + "action": "message", + "message": "continue", + "interrupt": true + }), + ) + .await; + let cancel_result = call_tool_json( + &client, + "fabro_run_interact", + serde_json::json!({ "run_id": selector, "action": "cancel" }), + ) + .await; + + assert_eq!(get["result"]["summary"]["run_id"], run_id); + assert_eq!(start_result["result"]["summary"]["run_id"], run_id); + assert_eq!(message_result["result"]["message"], "continue"); + assert_eq!(message_result["result"]["interrupt"], true); + assert_eq!(cancel_result["result"]["summary"]["run_id"], run_id); + resolve.assert_calls(4); + retrieve.assert_calls(1); + projection.assert(); + start.assert(); + message.assert(); + cancel.assert(); + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_create_validation_errors_happen_before_auth_or_network() { + let context = test_context!(); + let client = spawn_mcp_client(&context, &["--server", "http://127.0.0.1:9"]).await; + let too_many = (0..51) + .map(|index| serde_json::json!({ "workflow": format!("wf-{index}.fabro") })) + .collect::>(); + + let empty = call_tool_error_text( + &client, + "fabro_run_create", + serde_json::json!({ "runs": [] }), + ) + .await; + let many = call_tool_error_text( + &client, + "fabro_run_create", + serde_json::json!({ "runs": too_many }), + ) + .await; + let null = call_tool_error_text( + &client, + "fabro_run_create", + serde_json::json!({ + "runs": [{ + "workflow": "simple.fabro", + "inputs": { "decision": null } + }] + }), + ) + .await; + + assert!(empty.contains("runs"), "{empty}"); + assert!(many.contains("runs"), "{many}"); + assert!(null.contains("decision"), "{null}"); + assert_eq!(client.list_tools().await.unwrap().len(), 5); + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_interact_answer_validation_happens_before_auth_or_network() { + let context = test_context!(); + let client = spawn_mcp_client(&context, &["--server", "http://127.0.0.1:9"]).await; + + let error = call_tool_error_text( + &client, + "fabro_run_interact", + serde_json::json!({ + "run_id": "nightly", + "action": "answer", + "question_id": "q-1", + "answer": { "value": "yes" } + }), + ) + .await; + + assert!(error.contains("option, options, text"), "{error}"); + assert_eq!(client.list_tools().await.unwrap().len(), 5); + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_interact_questions_and_answers_use_api_wire_contract() { + 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 run_id = unique_run_id(); + let selector = "nightly"; + let resolve = mock_resolved_run(&server, selector, &run_id); + let questions = server.mock(|when, then| { + when.method(GET) + .path(format!("/api/v1/runs/{run_id}/questions")) + .query_param("page[limit]", "100") + .query_param("page[offset]", "0"); + then.status(200) + .header("Content-Type", "application/json") + .json_body(serde_json::json!({ + "data": [{ + "id": "q-1", + "text": "Proceed?", + "stage": "gate", + "question_type": "yes_no", + "options": [], + "allow_freeform": false, + "timeout_seconds": null, + "context_display": null + }], + "meta": { "has_more": false } + })); + }); + let expected_answers = [ + ( + serde_json::json!(true), + serde_json::json!({ "kind": "yes" }), + ), + ( + serde_json::json!(false), + serde_json::json!({ "kind": "no" }), + ), + ( + serde_json::json!("Looks good"), + serde_json::json!({ "kind": "text", "text": "Looks good" }), + ), + ( + serde_json::json!({ "option": "approve" }), + serde_json::json!({ "kind": "selected", "option_key": "approve" }), + ), + ( + serde_json::json!({ "options": ["approve", "notify"] }), + serde_json::json!({ "kind": "multi_selected", "option_keys": ["approve", "notify"] }), + ), + ( + serde_json::json!({ "text": "Freeform" }), + serde_json::json!({ "kind": "text", "text": "Freeform" }), + ), + ]; + let answer_mocks = expected_answers + .iter() + .map(|(_, expected_body)| { + server.mock(|when, then| { + when.method(POST) + .path(format!("/api/v1/runs/{run_id}/questions/q-1/answer")) + .json_body(expected_body.clone()); + then.status(204); + }) + }) + .collect::>(); + + let client = spawn_mcp_client(&context, &["--server", &target_url]).await; + let question_result = call_tool_json( + &client, + "fabro_run_interact", + serde_json::json!({ "run_id": selector, "action": "get_questions" }), + ) + .await; + assert_eq!(question_result["result"]["questions"][0]["id"], "q-1"); + + for (answer, _) in expected_answers { + let result = call_tool_json( + &client, + "fabro_run_interact", + serde_json::json!({ + "run_id": selector, + "action": "answer", + "question_id": "q-1", + "answer": answer + }), + ) + .await; + assert_eq!(result["result"]["submitted"], true); + } + + resolve.assert_calls(7); + questions.assert(); + for answer in answer_mocks { + answer.assert(); + } + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_events_filters_find_matches_beyond_first_page() { + 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 run_id = unique_run_id(); + let resolve = mock_resolved_run(&server, "nightly", &run_id); + let events = (1..=60) + .map(|sequence| { + let event_name = if sequence == 60 { + "stage.started" + } else { + "run.started" + }; + let properties = if sequence == 60 { + serde_json::json!({ + "index": 1, + "handler_type": "prompt", + "attempt": 1, + "max_attempts": 1 + }) + } else { + serde_json::json!({ + "name": "Simple", + "goal": format!("ordinary event {sequence}") + }) + }; + serde_json::json!({ + "seq": sequence, + "id": format!("evt-{sequence}"), + "ts": "2026-04-05T12:00:00Z", + "run_id": run_id, + "event": event_name, + "properties": properties, + "actor": null + }) + }) + .collect::>(); + let first_event = events[0].clone(); + let _limited_events = server.mock(|when, then| { + when.method(GET) + .path(format!("/api/v1/runs/{run_id}/events")) + .query_param("limit", "1"); + then.status(200) + .header("Content-Type", "application/json") + .json_body(serde_json::json!({ + "data": [first_event], + "meta": { "has_more": true } + })); + }); + let list_events = server.mock(|when, then| { + when.method(GET) + .path(format!("/api/v1/runs/{run_id}/events")) + .query_param_missing("limit"); + then.status(200) + .header("Content-Type", "application/json") + .json_body(serde_json::json!({ + "data": events, + "meta": { "has_more": false } + })); + }); + let client = spawn_mcp_client(&context, &["--server", &target_url]).await; + + let details = call_tool_json( + &client, + "fabro_run_events", + serde_json::json!({ + "run_id": "nightly", + "action": "details", + "event_ids": ["evt-60"], + "first": 1 + }), + ) + .await; + let filtered = call_tool_json( + &client, + "fabro_run_events", + serde_json::json!({ + "run_id": "nightly", + "action": "search", + "categories": ["stage"], + "query": "prompt", + "first": 1, + "max_content_length": 32 + }), + ) + .await; + + assert_eq!(details["events"][0]["event_id"], "evt-60"); + assert_eq!(filtered["events"][0]["event_id"], "evt-60"); + assert_eq!(filtered["events"][0]["truncated"], true); + resolve.assert_calls(2); + list_events.assert_calls(2); + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_events_requires_action_specific_inputs_before_auth() { + let context = test_context!(); + let client = spawn_mcp_client(&context, &["--server", "http://127.0.0.1:9"]).await; + + let details_error = call_tool_error_text( + &client, + "fabro_run_events", + serde_json::json!({ + "run_id": "run_123", + "action": "details" + }), + ) + .await; + let search_error = call_tool_error_text( + &client, + "fabro_run_events", + serde_json::json!({ + "run_id": "run_123", + "action": "search" + }), + ) + .await; + + assert!(details_error.contains("event_ids"), "{details_error}"); + assert!( + !details_error.contains("fabro auth login"), + "{details_error}" + ); + assert!(search_error.contains("query"), "{search_error}"); + assert!(!search_error.contains("fabro auth login"), "{search_error}"); + assert_eq!(client.list_tools().await.unwrap().len(), 5); + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_events_desc_after_offset_and_limit_page_over_requested_order() { + 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 run_id = unique_run_id(); + let resolve = mock_resolved_run(&server, "nightly", &run_id); + let events = (1..=5) + .map(|sequence| { + serde_json::json!({ + "seq": sequence, + "id": format!("evt-{sequence}"), + "ts": format!("2026-04-05T12:00:0{sequence}Z"), + "run_id": run_id, + "event": "run.started", + "properties": { "name": format!("event {sequence}") }, + "actor": null + }) + }) + .collect::>(); + let first_event = events[0].clone(); + let limited_events = server.mock(|when, then| { + when.method(GET) + .path(format!("/api/v1/runs/{run_id}/events")) + .query_param("limit", "1"); + then.status(200) + .header("Content-Type", "application/json") + .json_body(serde_json::json!({ + "data": [first_event], + "meta": { "has_more": true } + })); + }); + let full_events = server.mock(|when, then| { + when.method(GET) + .path(format!("/api/v1/runs/{run_id}/events")) + .query_param_missing("limit") + .query_param_missing("since_seq"); + then.status(200) + .header("Content-Type", "application/json") + .json_body(serde_json::json!({ + "data": events, + "meta": { "has_more": false } + })); + }); + let after_events = server.mock(|when, then| { + when.method(GET) + .path(format!("/api/v1/runs/{run_id}/events")) + .query_param("since_seq", "2") + .query_param("limit", "3"); + then.status(200) + .header("Content-Type", "application/json") + .json_body(serde_json::json!({ + "data": [ + { + "seq": 2, + "id": "evt-2", + "ts": "2026-04-05T12:00:02Z", + "run_id": run_id, + "event": "run.started", + "properties": { "name": "event 2" }, + "actor": null + }, + { + "seq": 3, + "id": "evt-3", + "ts": "2026-04-05T12:00:03Z", + "run_id": run_id, + "event": "run.started", + "properties": { "name": "event 3" }, + "actor": null + }, + { + "seq": 4, + "id": "evt-4", + "ts": "2026-04-05T12:00:04Z", + "run_id": run_id, + "event": "run.started", + "properties": { "name": "event 4" }, + "actor": null + } + ], + "meta": { "has_more": true } + })); + }); + let client = spawn_mcp_client(&context, &["--server", &target_url]).await; + + let desc = call_tool_json( + &client, + "fabro_run_events", + serde_json::json!({ + "run_id": "nightly", + "action": "list", + "direction": "desc", + "first": 1 + }), + ) + .await; + let paged = call_tool_json( + &client, + "fabro_run_events", + serde_json::json!({ + "run_id": "nightly", + "action": "list", + "after": 2, + "offset": 1, + "limit": 2 + }), + ) + .await; + + assert_eq!(desc["events"][0]["event_id"], "evt-5"); + assert_eq!(desc["next_cursor"], 5); + assert_eq!(paged["events"][0]["event_id"], "evt-3"); + assert_eq!(paged["events"][1]["event_id"], "evt-4"); + assert_eq!(paged["next_cursor"], 5); + resolve.assert_calls(2); + limited_events.assert_calls(0); + full_events.assert(); + after_events.assert(); + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_events_desc_cursor_continues_to_older_events() { + 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 run_id = unique_run_id(); + let resolve = mock_resolved_run(&server, "nightly", &run_id); + let events = (1..=5) + .map(|sequence| { + serde_json::json!({ + "seq": sequence, + "id": format!("evt-{sequence}"), + "ts": format!("2026-04-05T12:00:0{sequence}Z"), + "run_id": run_id, + "event": "run.started", + "properties": { "name": format!("event {sequence}") }, + "actor": null + }) + }) + .collect::>(); + let full_events = server.mock(|when, then| { + when.method(GET) + .path(format!("/api/v1/runs/{run_id}/events")) + .query_param_missing("limit") + .query_param_missing("since_seq"); + then.status(200) + .header("Content-Type", "application/json") + .json_body(serde_json::json!({ + "data": events, + "meta": { "has_more": false } + })); + }); + let client = spawn_mcp_client(&context, &["--server", &target_url]).await; + + let first_page = call_tool_json( + &client, + "fabro_run_events", + serde_json::json!({ + "run_id": "nightly", + "action": "list", + "direction": "desc", + "first": 2 + }), + ) + .await; + let second_page = call_tool_json( + &client, + "fabro_run_events", + serde_json::json!({ + "run_id": "nightly", + "action": "list", + "direction": "desc", + "after": first_page["next_cursor"], + "first": 2 + }), + ) + .await; + + assert_eq!(first_page["events"][0]["event_id"], "evt-5"); + assert_eq!(first_page["events"][1]["event_id"], "evt-4"); + assert_eq!(first_page["next_cursor"], 4); + assert_eq!(second_page["events"][0]["event_id"], "evt-3"); + assert_eq!(second_page["events"][1]["event_id"], "evt-2"); + assert_eq!(second_page["next_cursor"], 2); + resolve.assert_calls(2); + full_events.assert_calls(2); + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_events_offset_beyond_fetch_cap_reaches_later_pages() { + 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 run_id = unique_run_id(); + let resolve = mock_resolved_run(&server, "nightly", &run_id); + let events = (1..=300) + .map(|sequence| { + serde_json::json!({ + "seq": sequence, + "id": format!("evt-{sequence}"), + "ts": "2026-04-05T12:00:00Z", + "run_id": run_id, + "event": "run.started", + "properties": { "name": format!("event {sequence}") }, + "actor": null + }) + }) + .collect::>(); + let first_251_events = events.iter().take(251).cloned().collect::>(); + let bounded_events = server.mock(|when, then| { + when.method(GET) + .path(format!("/api/v1/runs/{run_id}/events")) + .query_param("limit", "251"); + then.status(200) + .header("Content-Type", "application/json") + .json_body(serde_json::json!({ + "data": first_251_events, + "meta": { "has_more": true } + })); + }); + let client = spawn_mcp_client(&context, &["--server", &target_url]).await; + + let paged = call_tool_json( + &client, + "fabro_run_events", + serde_json::json!({ + "run_id": "nightly", + "action": "list", + "offset": 250, + "first": 1 + }), + ) + .await; + + assert_eq!(paged["events"][0]["event_id"], "evt-251"); + assert_eq!(paged["next_cursor"], 252); + resolve.assert(); + bounded_events.assert(); + client + .shutdown() + .await + .expect("MCP client should shut down"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn mcp_tool_auth_error_mentions_login() { + let context = test_context!(); + let harness = + RealAuthHarness::start_with_dev_token(fabro_test::GitHubAppState::default()).await; + let target_url = harness.api_target(); + let client = spawn_mcp_client(&context, &["--server", &target_url]).await; + + let error = call_tool_error_text( + &client, + "fabro_run_search", + serde_json::json!({ "first": 1 }), + ) + .await; + + assert!( + error.contains("Run `fabro auth login` to authenticate."), + "{error}" + ); + assert_eq!(client.list_tools().await.unwrap().len(), 5); + + client + .shutdown() + .await + .expect("MCP client should shut down"); + harness.shutdown().await; +} + +fn expected_claude_desktop_config_path(home_dir: &Path) -> PathBuf { + #[cfg(target_os = "macos")] + { + home_dir + .join("Library") + .join("Application Support") + .join("Claude") + .join("claude_desktop_config.json") + } + #[cfg(target_os = "linux")] + { + home_dir + .join(".config") + .join("Claude") + .join("claude_desktop_config.json") + } + #[cfg(target_os = "windows")] + { + home_dir + .join("AppData") + .join("Roaming") + .join("Claude") + .join("claude_desktop_config.json") + } +} + +struct McpStdioFixture { + command: Vec, + env: HashMap, + current_dir: PathBuf, +} + +fn mcp_stdio_fixture(context: &fabro_test::TestContext, extra_args: &[&str]) -> McpStdioFixture { + let mut command = vec![ + env!("CARGO_BIN_EXE_fabro").to_string(), + "mcp".to_string(), + "start".to_string(), + ]; + command.extend(extra_args.iter().map(|arg| (*arg).to_string())); + + let mut env = fabro_test::isolated_env(&context.home_dir); + env.insert( + "FABRO_HOME".to_string(), + context.home_dir.join(".fabro").display().to_string(), + ); + + McpStdioFixture { + command, + env, + current_dir: context.temp_dir.clone(), + } +} + +fn write_mcp_server_settings( + context: &mut fabro_test::TestContext, + storage_dir: &Path, + socket_path: Option<&Path>, +) { + context.manage_storage_dir(storage_dir); + let cli_target = socket_path.map_or_else(String::new, |path| { + format!( + r#" +[cli.target] +type = "unix" +path = "{}" +"#, + path.display() + ) + }); + context.write_home( + ".fabro/settings.toml", + format!( + r#"_version = 1 + +[server.storage] +root = "{}" + +[server.auth] +methods = ["dev-token"] +{cli_target}"#, + storage_dir.display() + ), + ); +} + +async fn spawn_mcp_client(context: &fabro_test::TestContext, extra_args: &[&str]) -> McpClient { + let fixture = mcp_stdio_fixture(context, extra_args); + spawn_mcp_client_from_fixture(fixture).await +} + +async fn spawn_mcp_client_from_fixture(fixture: McpStdioFixture) -> McpClient { + let config = McpServerSettings { + name: "fabro-under-test".to_string(), + transport: McpTransport::Stdio { + command: fixture.command, + env: fixture.env, + }, + current_dir: Some(fixture.current_dir), + clear_env: true, + startup_timeout_secs: 10, + tool_timeout_secs: 30, + }; + let client = McpClient::new(&config).expect("MCP client should build"); + client + .initialize(config.startup_timeout()) + .await + .expect("MCP server should initialize"); + client +} + +async fn call_tool_json( + client: &McpClient, + name: &str, + arguments: serde_json::Value, +) -> serde_json::Value { + let result = client + .call_tool(name, arguments, std::time::Duration::from_secs(30)) + .await + .expect("tool call should complete"); + assert_ne!( + result.is_error, + Some(true), + "tool returned error: {result:?}" + ); + let text = result + .content + .first() + .and_then(|content| serde_json::to_value(content).ok()) + .and_then(|content| content["text"].as_str().map(ToOwned::to_owned)) + .expect("tool result should include text fallback"); + assert!(!text.starts_with('{') && !text.starts_with('[')); + result + .structured_content + .expect("tool result should include structured content") +} + +async fn call_tool_error_text( + client: &McpClient, + name: &str, + arguments: serde_json::Value, +) -> String { + let result = client + .call_tool(name, arguments, std::time::Duration::from_secs(30)) + .await + .expect("tool call should complete"); + assert_eq!(result.is_error, Some(true), "tool should return error"); + result + .content + .first() + .and_then(|content| serde_json::to_value(content).ok()) + .and_then(|content| content["text"].as_str().map(ToOwned::to_owned)) + .expect("tool error should include text") +} + +async fn create_mcp_run(client: &McpClient, workflow: PathBuf, start: bool) -> String { + let create = call_tool_json( + client, + "fabro_run_create", + serde_json::json!({ + "runs": [{ + "workflow": workflow, + "dry_run": true, + "auto_approve": true, + "labels": { "source": "mcp-test" }, + "start": start + }] + }), + ) + .await; + create["runs"][0]["run_id"] + .as_str() + .expect("create result should include run id") + .to_string() +} + +fn seed_oauth_auth( + home_dir: &Path, + target: &fabro_client::ServerTarget, + access_token: &str, + refresh_token: &str, +) { + let now = Utc::now(); + AuthStore::new(home_dir.join(".fabro/auth.json")) + .put( + target, + AuthEntry::OAuth(OAuthEntry { + access_token: access_token.to_string(), + access_token_expires_at: now - ChronoDuration::minutes(1), + refresh_token: refresh_token.to_string(), + refresh_token_expires_at: now + ChronoDuration::days(30), + subject: StoredSubject { + idp_issuer: "https://github.com".to_string(), + idp_subject: "12345".to_string(), + login: "octocat".to_string(), + name: "The Octocat".to_string(), + email: "octocat@example.com".to_string(), + }, + logged_in_at: now, + }), + ) + .unwrap_or_else(|err| panic!("failed to seed OAuth auth: {err}")); +} + +fn normalize_run_search(mut value: serde_json::Value) -> serde_json::Value { + if let Some(runs) = value["runs"].as_array_mut() { + for run in runs { + run["run_id"] = serde_json::json!("[RUN_ID]"); + run["created_at"] = serde_json::json!("[TIMESTAMP]"); + if run["started_at"].is_string() { + run["started_at"] = serde_json::json!("[TIMESTAMP]"); + } + if run["completed_at"].is_string() { + run["completed_at"] = serde_json::json!("[TIMESTAMP]"); + } + if run["source_directory"].is_string() { + run["source_directory"] = serde_json::json!("[SOURCE_DIRECTORY]"); + } + if run["repo_origin_url"].is_string() { + run["repo_origin_url"] = serde_json::json!("[REPO_ORIGIN_URL]"); + } + } + } + value +} + +fn normalize_gather(mut value: serde_json::Value) -> serde_json::Value { + value["elapsed_seconds"] = serde_json::json!("[ELAPSED]"); + if let Some(runs) = value["runs"].as_array_mut() { + for run in runs { + run["run_id"] = serde_json::json!("[RUN_ID]"); + run["created_at"] = serde_json::json!("[TIMESTAMP]"); + if run["started_at"].is_string() { + run["started_at"] = serde_json::json!("[TIMESTAMP]"); + } + if run["completed_at"].is_string() { + run["completed_at"] = serde_json::json!("[TIMESTAMP]"); + } + if run["source_directory"].is_string() { + run["source_directory"] = serde_json::json!("[SOURCE_DIRECTORY]"); + } + if run["repo_origin_url"].is_string() { + run["repo_origin_url"] = serde_json::json!("[REPO_ORIGIN_URL]"); + } + } + } + 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() +} + +fn mock_resolved_run_json<'a>( + server: &'a MockServer, + selector: &str, + body: serde_json::Value, + authorization: Option<&str>, +) -> httpmock::Mock<'a> { + server.mock(|when, then| { + let when = when + .method(GET) + .path("/api/v1/runs/resolve") + .query_param("selector", selector); + if let Some(authorization) = authorization { + when.header("authorization", authorization); + } + then.status(200) + .header("Content-Type", "application/json") + .json_body(body); + }) +} diff --git a/lib/crates/fabro-cli/tests/it/cmd/mod.rs b/lib/crates/fabro-cli/tests/it/cmd/mod.rs index 58a98b5e3..0dd5da69b 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/mod.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/mod.rs @@ -20,6 +20,7 @@ mod inspect; mod install; mod json_global; mod logs; +mod mcp; mod model; mod model_list; mod model_test; diff --git a/lib/crates/fabro-cli/tests/manifest_path_round_trip.rs b/lib/crates/fabro-cli/tests/manifest_path_round_trip.rs index 9cf4c50f8..d94f95221 100644 --- a/lib/crates/fabro-cli/tests/manifest_path_round_trip.rs +++ b/lib/crates/fabro-cli/tests/manifest_path_round_trip.rs @@ -5,7 +5,7 @@ use std::path::PathBuf; -use fabro_cli::{ManifestBuildInput, build_run_manifest}; +use fabro_manifest::{ManifestBuildInput, build_run_manifest}; use fabro_workflow::ManifestPath; #[test] diff --git a/lib/crates/fabro-client/src/client.rs b/lib/crates/fabro-client/src/client.rs index 97627466b..3654fd740 100644 --- a/lib/crates/fabro-client/src/client.rs +++ b/lib/crates/fabro-client/src/client.rs @@ -794,25 +794,27 @@ impl Client { Ok(bytes) } - pub async fn start_run(&self, run_id: &RunId, resume: bool) -> Result<()> { - self.send_api(|client| async move { - client - .start_run() - .id(run_id.to_string()) - .body(types::StartRunRequest { resume }) - .send() - .await - }) - .await?; - Ok(()) + pub async fn start_run(&self, run_id: &RunId, resume: bool) -> Result { + let response = self + .send_api(|client| async move { + client + .start_run() + .id(run_id.to_string()) + .body(types::StartRunRequest { resume }) + .send() + .await + }) + .await?; + convert_type(response.into_inner()) } - pub async fn cancel_run(&self, run_id: &RunId) -> Result<()> { - self.send_api( - |client| async move { client.cancel_run().id(run_id.to_string()).send().await }, - ) - .await?; - Ok(()) + pub async fn cancel_run(&self, run_id: &RunId) -> Result { + let response = self + .send_api( + |client| async move { client.cancel_run().id(run_id.to_string()).send().await }, + ) + .await?; + convert_type(response.into_inner()) } pub async fn interrupt_run(&self, run_id: &RunId) -> Result<()> { @@ -844,20 +846,22 @@ impl Client { Ok(()) } - pub async fn archive_run(&self, run_id: &RunId) -> Result<()> { - self.send_api( - |client| async move { client.archive_run().id(run_id.to_string()).send().await }, - ) - .await?; - Ok(()) + pub async fn archive_run(&self, run_id: &RunId) -> Result { + let response = self + .send_api( + |client| async move { client.archive_run().id(run_id.to_string()).send().await }, + ) + .await?; + convert_type(response.into_inner()) } - pub async fn unarchive_run(&self, run_id: &RunId) -> Result<()> { - self.send_api(|client| async move { - client.unarchive_run().id(run_id.to_string()).send().await - }) - .await?; - Ok(()) + pub async fn unarchive_run(&self, run_id: &RunId) -> Result { + let response = self + .send_api( + |client| async move { client.unarchive_run().id(run_id.to_string()).send().await }, + ) + .await?; + convert_type(response.into_inner()) } pub async fn rewind_run( @@ -1123,6 +1127,50 @@ impl Client { Ok(all_events) } + pub async fn list_run_events_until( + &self, + run_id: &RunId, + since_seq: Option, + max_events: usize, + ) -> Result> { + if max_events == 0 { + return Ok(Vec::new()); + } + + let mut next_since_seq = since_seq; + let mut all_events = Vec::new(); + while all_events.len() < max_events { + let remaining = max_events - all_events.len(); + let response = self + .send_api(|client| async move { + let mut request = client + .list_run_events() + .id(run_id.to_string()) + .limit(remaining.min(1000) as u64); + if let Some(seq) = next_since_seq.and_then(non_zero_u64_from_u32) { + request = request.since_seq(seq); + } + request.send().await + }) + .await?; + let parsed = response.into_inner(); + let page_events = parsed + .data + .into_iter() + .map(convert_type::<_, EventEnvelope>) + .collect::>>()?; + let next_page_since_seq = page_events.last().map(|event| event.seq.saturating_add(1)); + all_events.extend(page_events); + + if !parsed.meta.has_more || next_page_since_seq.is_none() { + break; + } + next_since_seq = next_page_since_seq; + } + + Ok(all_events) + } + pub async fn attach_run_events( &self, run_id: &RunId, diff --git a/lib/crates/fabro-config/src/resolve/run.rs b/lib/crates/fabro-config/src/resolve/run.rs index 1a85289f2..fe4c94240 100644 --- a/lib/crates/fabro-config/src/resolve/run.rs +++ b/lib/crates/fabro-config/src/resolve/run.rs @@ -346,6 +346,8 @@ pub(crate) fn resolve_mcp_entry(name: &str, entry: &McpEntryLayer) -> McpServerS McpServerSettings { name: name.to_string(), transport, + current_dir: None, + clear_env: false, startup_timeout_secs, tool_timeout_secs, } diff --git a/lib/crates/fabro-manifest/Cargo.toml b/lib/crates/fabro-manifest/Cargo.toml new file mode 100644 index 000000000..417dc9af0 --- /dev/null +++ b/lib/crates/fabro-manifest/Cargo.toml @@ -0,0 +1,29 @@ +[package] +name = "fabro-manifest" +edition.workspace = true +version.workspace = true +publish = false +license.workspace = true +description = "Fabro run manifest construction" + +[lib] +doctest = false + +[lints] +workspace = true + +[dependencies] +anyhow.workspace = true +fabro-api = { path = "../fabro-api" } +fabro-config = { path = "../fabro-config" } +fabro-github = { path = "../fabro-github" } +fabro-graphviz = { path = "../fabro-graphviz" } +fabro-template = { path = "../fabro-template" } +fabro-types = { path = "../fabro-types" } +fabro-workflow = { path = "../fabro-workflow" } +git2.workspace = true +toml.workspace = true + +[dev-dependencies] +tempfile = "3" +temp-env = "0.3" diff --git a/lib/crates/fabro-cli/src/manifest_builder.rs b/lib/crates/fabro-manifest/src/lib.rs similarity index 89% rename from lib/crates/fabro-cli/src/manifest_builder.rs rename to lib/crates/fabro-manifest/src/lib.rs index b2436db77..6d6944ffc 100644 --- a/lib/crates/fabro-cli/src/manifest_builder.rs +++ b/lib/crates/fabro-manifest/src/lib.rs @@ -10,19 +10,21 @@ use anyhow::{Context, Result, anyhow}; use fabro_api::types; use fabro_config::project::{self, discover_project_config, resolve_workflow_path}; use fabro_config::run::{resolve_run_goal_from_layer, resolve_run_goal_from_namespace}; -use fabro_config::{CliLayer, DaytonaDockerfileLayer, RunLayer, WorkflowSettingsBuilder}; +use fabro_config::{ + CliLayer, DaytonaDockerfileLayer, DockerSandboxLayer, ReplaceMap, RunExecutionLayer, + RunGoalLayer, RunLayer, RunModelLayer, RunSandboxLayer, WorkflowSettingsBuilder, +}; use fabro_graphviz::graph::AttrValue; use fabro_graphviz::parser; use fabro_template::{TemplateContext, render as render_template}; -use fabro_types::settings::run::{ResolvedGoalSource, ResolvedRunGoal}; +use fabro_types::settings::interp::InterpString; +use fabro_types::settings::run::{ApprovalMode, ResolvedGoalSource, ResolvedRunGoal, RunMode}; use fabro_types::{DirtyStatus, GitContext, PreRunPushOutcome, RunId, WorkflowSettings}; use fabro_workflow::ManifestPath; use fabro_workflow::git::{ GitSyncStatus, branch_needs_push, head_sha, push_branch_noninteractive, sync_status, }; -use crate::args::{PreflightArgs, RunArgs}; - #[derive(Debug, Default)] pub struct ManifestBuildInput { pub workflow: PathBuf, @@ -43,6 +45,80 @@ pub struct BuiltManifest { pub target_path: PathBuf, } +#[derive(Debug, Default)] +pub struct RunOverrideInput<'a> { + pub goal: Option<&'a str>, + pub model: Option<&'a str>, + pub provider: Option<&'a str>, + pub sandbox: Option<&'a str>, + pub docker_image: Option<&'a str>, + pub preserve_sandbox: Option, + pub dry_run: Option, + pub auto_approve: Option, + pub labels: HashMap, +} + +#[must_use] +pub fn build_run_overrides(input: RunOverrideInput<'_>) -> RunLayer { + let goal = input + .goal + .map(|goal| RunGoalLayer::Inline(InterpString::parse(goal))); + let model = (input.model.is_some() || input.provider.is_some()).then(|| RunModelLayer { + provider: input.provider.map(InterpString::parse), + name: input.model.map(InterpString::parse), + fallbacks: Vec::new(), + }); + let sandbox = (input.sandbox.is_some() + || input.docker_image.is_some() + || input.preserve_sandbox.is_some()) + .then(|| RunSandboxLayer { + provider: input.sandbox.map(ToOwned::to_owned), + docker: input.docker_image.map(|image| DockerSandboxLayer { + image: Some(image.to_string()), + ..DockerSandboxLayer::default() + }), + preserve: input.preserve_sandbox, + ..RunSandboxLayer::default() + }); + let execution = + (input.dry_run.is_some() || input.auto_approve.is_some()).then(|| RunExecutionLayer { + mode: input.dry_run.map(|dry_run| { + if dry_run { + RunMode::DryRun + } else { + RunMode::Normal + } + }), + approval: input.auto_approve.map(|auto_approve| { + if auto_approve { + ApprovalMode::Auto + } else { + ApprovalMode::Prompt + } + }), + }); + + RunLayer { + goal, + metadata: ReplaceMap::from(input.labels), + model, + sandbox, + execution, + ..RunLayer::default() + } +} + +#[must_use] +pub fn build_sparse_run_overrides(input: RunOverrideInput<'_>) -> Option { + let run = build_run_overrides(input); + (run.goal.is_some() + || !run.metadata.is_empty() + || run.model.is_some() + || run.sandbox.is_some() + || run.execution.is_some()) + .then_some(run) +} + struct CollectContext<'a> { cwd: &'a Path, inputs: &'a HashMap, @@ -172,42 +248,6 @@ pub fn build_run_manifest(input: ManifestBuildInput) -> Result { }) } -pub(crate) fn run_manifest_args(args: &RunArgs) -> Option { - let payload = types::ManifestArgs { - auto_approve: args.auto_approve.then_some(true), - dry_run: args.dry_run.then_some(true), - label: args.label.clone(), - model: args.model.clone(), - preserve_sandbox: args.preserve_sandbox.then_some(true), - provider: args.provider.clone(), - sandbox: args - .sandbox - .map(|provider| fabro_sandbox::SandboxProvider::from(provider).to_string()), - docker_image: None, - input: args.inputs.values.clone(), - verbose: args.verbose.then_some(true), - }; - (!manifest_args_is_empty(&payload)).then_some(payload) -} - -pub(crate) fn preflight_manifest_args(args: &PreflightArgs) -> Option { - let payload = types::ManifestArgs { - auto_approve: None, - dry_run: None, - label: Vec::new(), - model: args.model.clone(), - preserve_sandbox: None, - provider: args.provider.clone(), - sandbox: args - .sandbox - .map(|provider| fabro_sandbox::SandboxProvider::from(provider).to_string()), - docker_image: None, - input: args.inputs.values.clone(), - verbose: args.verbose.then_some(true), - }; - (!manifest_args_is_empty(&payload)).then_some(payload) -} - fn collect_workflow_entry( context: &mut CollectContext<'_>, workflow: &Path, @@ -650,7 +690,7 @@ fn manifest_path_from_absolute(path: &Path, cwd: &Path) -> Result .ok_or_else(|| anyhow!("Failed to compute manifest path for {}", path.display())) } -fn manifest_args_is_empty(args: &types::ManifestArgs) -> bool { +pub fn manifest_args_is_empty(args: &types::ManifestArgs) -> bool { args.auto_approve.is_none() && args.dry_run.is_none() && args.label.is_empty() @@ -667,6 +707,65 @@ fn manifest_args_is_empty(args: &types::ManifestArgs) -> bool { mod tests { use super::*; + #[test] + fn build_run_overrides_sets_common_cli_and_mcp_layers() { + let overrides = build_run_overrides(RunOverrideInput { + goal: Some("ship it"), + model: Some("gpt-5.4-mini"), + provider: Some("openai"), + sandbox: Some("local"), + docker_image: None, + preserve_sandbox: Some(true), + dry_run: Some(true), + auto_approve: Some(false), + labels: [("source".to_string(), "mcp".to_string())] + .into_iter() + .collect(), + }); + + let goal = overrides.goal.expect("goal override"); + assert!(matches!(goal, fabro_config::RunGoalLayer::Inline(_))); + assert_eq!( + overrides + .model + .as_ref() + .unwrap() + .name + .as_ref() + .unwrap() + .as_source(), + "gpt-5.4-mini" + ); + assert_eq!( + overrides + .model + .as_ref() + .unwrap() + .provider + .as_ref() + .unwrap() + .as_source(), + "openai" + ); + assert_eq!( + overrides.sandbox.as_ref().unwrap().provider.as_deref(), + Some("local") + ); + assert_eq!(overrides.sandbox.as_ref().unwrap().preserve, Some(true)); + assert_eq!( + overrides.execution.as_ref().unwrap().mode, + Some(RunMode::DryRun) + ); + assert_eq!( + overrides.execution.as_ref().unwrap().approval, + Some(ApprovalMode::Prompt) + ); + assert_eq!( + overrides.metadata.0.get("source").map(String::as_str), + Some("mcp") + ); + } + #[test] fn build_manifest_bundles_imports_prompts_and_children() { let temp = tempfile::tempdir().unwrap(); diff --git a/lib/crates/fabro-mcp-server/Cargo.toml b/lib/crates/fabro-mcp-server/Cargo.toml new file mode 100644 index 000000000..15f481431 --- /dev/null +++ b/lib/crates/fabro-mcp-server/Cargo.toml @@ -0,0 +1,31 @@ +[package] +name = "fabro-mcp-server" +edition.workspace = true +version.workspace = true +publish = false +license.workspace = true +description = "Fabro MCP stdio server" + +[lib] +doctest = false + +[lints] +workspace = true + +[dependencies] +anyhow.workspace = true +chrono = { workspace = true, features = ["serde"] } +fabro-api = { path = "../fabro-api" } +fabro-client = { path = "../fabro-client" } +fabro-manifest = { path = "../fabro-manifest" } +fabro-config = { path = "../fabro-config" } +fabro-server = { path = "../fabro-server" } +fabro-types = { path = "../fabro-types" } +fabro-util = { path = "../fabro-util" } +futures.workspace = true +rmcp = { workspace = true, features = ["server", "macros", "schemars", "transport-io"] } +schemars = "1.2.1" +serde.workspace = true +serde_json.workspace = true +tokio.workspace = true +toml.workspace = true diff --git a/lib/crates/fabro-mcp-server/src/config.rs b/lib/crates/fabro-mcp-server/src/config.rs new file mode 100644 index 000000000..9eb8a67c2 --- /dev/null +++ b/lib/crates/fabro-mcp-server/src/config.rs @@ -0,0 +1,146 @@ +#![expect( + clippy::disallowed_methods, + reason = "MCP client config setup intentionally performs small synchronous JSON file reads/writes from a CLI command." +)] + +use std::path::{Path, PathBuf}; + +use anyhow::{Context as _, Result, anyhow}; +use serde_json::map::Entry; +use serde_json::{Map, Value, json}; + +use crate::{McpAgent, McpConfigSettings, McpInitSettings}; + +const SERVER_NAME: &str = "fabro"; + +pub fn config_json(settings: &McpConfigSettings) -> Result { + serde_json::to_string_pretty(&generic_config(settings)) + .map(|json| format!("{json}\n")) + .context("failed to render Fabro MCP client config") +} + +pub fn init_agent(settings: &McpInitSettings) -> Result<()> { + let entry = server_entry(&settings.config); + for path in agent_config_paths(settings.agent, &settings.home_dir) { + merge_server_entry(&path, entry.clone())?; + } + Ok(()) +} + +fn generic_config(settings: &McpConfigSettings) -> Value { + json!({ + "mcpServers": { + SERVER_NAME: server_entry(settings) + } + }) +} + +fn server_entry(settings: &McpConfigSettings) -> Value { + json!({ + "command": "fabro", + "args": start_args(settings), + }) +} + +fn start_args(settings: &McpConfigSettings) -> Vec { + let mut args = vec!["mcp".to_string(), "start".to_string()]; + if let Some(server) = settings.server.as_ref() { + args.push("--server".to_string()); + args.push(server.clone()); + } + if let Some(storage_dir) = settings.storage_dir.as_deref() { + args.push("--storage-dir".to_string()); + args.push(storage_dir.display().to_string()); + } + args +} + +fn merge_server_entry(path: &Path, entry: Value) -> Result<()> { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent) + .with_context(|| format!("failed to create {}", parent.display()))?; + } + + let mut root = match std::fs::read_to_string(path) { + Ok(contents) => serde_json::from_str::(&contents) + .with_context(|| format!("failed to parse MCP config {}", path.display()))?, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => Value::Object(Map::new()), + Err(err) => return Err(err).with_context(|| format!("failed to read {}", path.display())), + }; + + let root_object = root + .as_object_mut() + .ok_or_else(|| anyhow!("MCP config {} must contain a JSON object", path.display()))?; + + let servers = match root_object.entry("mcpServers") { + Entry::Vacant(entry) => entry.insert(Value::Object(Map::new())), + Entry::Occupied(entry) => entry.into_mut(), + }; + let servers_object = servers.as_object_mut().ok_or_else(|| { + anyhow!( + "MCP config {} field mcpServers must contain a JSON object", + path.display() + ) + })?; + servers_object.insert(SERVER_NAME.to_string(), entry); + + let rendered = serde_json::to_string_pretty(&root) + .map(|json| format!("{json}\n")) + .with_context(|| format!("failed to render MCP config {}", path.display()))?; + std::fs::write(path, rendered).with_context(|| format!("failed to write {}", path.display())) +} + +fn agent_config_paths(agent: McpAgent, home_dir: &Path) -> Vec { + match agent { + McpAgent::Claude => vec![ + claude_desktop_config_path(home_dir), + claude_code_config_path(home_dir), + ], + McpAgent::Cursor => vec![home_dir.join(".cursor").join("mcp.json")], + McpAgent::Windsurf => vec![ + home_dir + .join(".codeium") + .join("windsurf") + .join("mcp_config.json"), + ], + } +} + +fn claude_code_config_path(home_dir: &Path) -> PathBuf { + home_dir.join(".claude.json") +} + +fn claude_desktop_config_path(home_dir: &Path) -> PathBuf { + #[cfg(target_os = "macos")] + { + home_dir + .join("Library") + .join("Application Support") + .join("Claude") + .join("claude_desktop_config.json") + } + + #[cfg(target_os = "linux")] + { + home_dir + .join(".config") + .join("Claude") + .join("claude_desktop_config.json") + } + + #[cfg(target_os = "windows")] + { + let app_data = std::env::var_os("APPDATA") + .map(PathBuf::from) + .unwrap_or_else(|| home_dir.join("AppData").join("Roaming")); + app_data.join("Claude").join("claude_desktop_config.json") + } + + #[cfg(not(any(target_os = "macos", target_os = "linux", target_os = "windows")))] + { + home_dir + .join(".config") + .join("Claude") + .join("claude_desktop_config.json") + } +} diff --git a/lib/crates/fabro-mcp-server/src/lib.rs b/lib/crates/fabro-mcp-server/src/lib.rs new file mode 100644 index 000000000..b8351bab5 --- /dev/null +++ b/lib/crates/fabro-mcp-server/src/lib.rs @@ -0,0 +1,55 @@ +mod config; +mod run_tools; +mod server; + +use std::future::Future; +use std::path::PathBuf; +use std::pin::Pin; +use std::sync::Arc; + +use anyhow::Result; +pub use config::{config_json, init_agent}; +use fabro_client::Client; +pub use server::start; + +pub type FabroClientFuture = Pin> + Send>>; + +pub type FabroClientFactory = Arc FabroClientFuture + Send + Sync>; + +#[derive(Clone)] +pub struct FabroMcpServerSettings { + pub client_factory: FabroClientFactory, + pub config_path: PathBuf, + pub cwd: PathBuf, +} + +impl std::fmt::Debug for FabroMcpServerSettings { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + formatter + .debug_struct("FabroMcpServerSettings") + .field("client_factory", &"") + .field("config_path", &self.config_path) + .field("cwd", &self.cwd) + .finish() + } +} + +#[derive(Debug, Clone, Default)] +pub struct McpConfigSettings { + pub server: Option, + pub storage_dir: Option, +} + +#[derive(Debug, Clone)] +pub struct McpInitSettings { + pub agent: McpAgent, + pub config: McpConfigSettings, + pub home_dir: PathBuf, +} + +#[derive(Debug, Clone, Copy)] +pub enum McpAgent { + Claude, + Cursor, + Windsurf, +} diff --git a/lib/crates/fabro-mcp-server/src/run_tools.rs b/lib/crates/fabro-mcp-server/src/run_tools.rs new file mode 100644 index 000000000..78c5ed14f --- /dev/null +++ b/lib/crates/fabro-mcp-server/src/run_tools.rs @@ -0,0 +1,21 @@ +#![allow( + dead_code, + reason = "MCP DTO fields are consumed by serde and schema generation even when not read directly." +)] + +mod common; +mod create; +mod events; +mod gather; +mod interact; +mod manifest; +mod search; + +pub(crate) use common::{ToolError, error_result, success_result}; +pub(crate) use create::{FabroRunCreateParams, ValidatedCreateRuns, create_runs, create_runs_text}; +pub(crate) use events::{FabroRunEventsParams, ValidatedRunEvents, run_events, run_events_text}; +pub(crate) use gather::{FabroRunGatherParams, ValidatedGatherRuns, gather_runs, gather_runs_text}; +pub(crate) use interact::{ + FabroRunInteractParams, ValidatedInteractRun, interact_run, interact_run_text, +}; +pub(crate) use search::{FabroRunSearchParams, ValidatedSearchRuns, search_runs, search_runs_text}; diff --git a/lib/crates/fabro-mcp-server/src/run_tools/common.rs b/lib/crates/fabro-mcp-server/src/run_tools/common.rs new file mode 100644 index 000000000..d708ed3ad --- /dev/null +++ b/lib/crates/fabro-mcp-server/src/run_tools/common.rs @@ -0,0 +1,141 @@ +use std::collections::HashMap; + +use chrono::{DateTime, NaiveDate, Utc}; +use fabro_client::Client; +use fabro_types::{Run, RunId, RunStatus}; +use fabro_util::exit::{self, ExitClass}; +use rmcp::model::{CallToolResult, Content}; +use schemars::JsonSchema; +use serde::Serialize; + +#[derive(Debug)] +pub(crate) struct ToolError { + message: String, +} + +impl ToolError { + pub(crate) fn message(message: impl Into) -> Self { + Self { + message: message.into(), + } + } + + pub(crate) fn from_anyhow(err: &anyhow::Error) -> Self { + Self::message(format_tool_error(err)) + } + + pub(crate) fn as_str(&self) -> &str { + &self.message + } +} + +pub(super) type ToolResult = Result; + +#[derive(Debug, Serialize, JsonSchema)] +pub(crate) struct RunSummaryResult { + pub(crate) run_id: String, + pub(crate) workflow_name: String, + pub(crate) workflow_slug: Option, + pub(crate) status: String, + pub(crate) archived: bool, + pub(crate) created_at: String, + pub(crate) started_at: Option, + pub(crate) completed_at: Option, + pub(crate) labels: HashMap, + pub(crate) source_directory: Option, + pub(crate) repo_origin_url: Option, + pub(crate) goal: String, +} + +pub(crate) fn success_result( + value: &T, + text: impl Into, +) -> Result { + let structured_content = serde_json::to_value(value).map_err(|err| { + rmcp::ErrorData::internal_error( + format!("failed to serialize Fabro MCP tool result: {err}"), + None, + ) + })?; + let mut result = CallToolResult::structured(structured_content); + result.content = vec![Content::text(text.into())]; + Ok(result) +} + +pub(crate) fn error_result(err: ToolError) -> CallToolResult { + CallToolResult::error(vec![Content::text(err.message)]) +} + +pub(super) fn validate_len(name: &str, len: usize, min: usize, max: usize) -> ToolResult<()> { + if len < min { + return Err(ToolError::message(format!( + "{name} must contain at least {min} item(s)" + ))); + } + if len > max { + return Err(ToolError::message(format!( + "{name} must contain no more than {max} item(s)" + ))); + } + Ok(()) +} + +pub(super) async fn retrieve_run(client: &Client, run_id: &RunId) -> ToolResult { + client + .retrieve_run(run_id) + .await + .map_err(|err| ToolError::from_anyhow(&err)) +} + +pub(super) fn run_summary_result(run: &Run) -> RunSummaryResult { + RunSummaryResult { + run_id: run.id.to_string(), + workflow_name: run.workflow.name.clone(), + workflow_slug: run.workflow.slug.clone(), + status: run_status_kind(run.lifecycle.status).to_string(), + archived: run.lifecycle.archived, + created_at: run.timestamps.created_at.to_rfc3339(), + started_at: run + .timestamps + .started_at + .map(|timestamp| timestamp.to_rfc3339()), + completed_at: run + .timestamps + .completed_at + .map(|timestamp| timestamp.to_rfc3339()), + labels: run.labels.clone(), + source_directory: run.source_directory.clone(), + repo_origin_url: run + .repository + .as_ref() + .and_then(|repository| repository.origin_url.clone()), + goal: run.goal.clone(), + } +} + +pub(super) fn parse_datetime_filter(name: &str, raw: &str) -> ToolResult> { + if let Ok(timestamp) = DateTime::parse_from_rfc3339(raw) { + return Ok(timestamp.with_timezone(&Utc)); + } + let date = NaiveDate::parse_from_str(raw, "%Y-%m-%d").map_err(|err| { + ToolError::message(format!("{name} must be RFC3339 or YYYY-MM-DD: {err}")) + })?; + let datetime = date + .and_hms_opt(0, 0, 0) + .ok_or_else(|| ToolError::message(format!("{name} contains an invalid date")))?; + Ok(DateTime::from_naive_utc_and_offset(datetime, Utc)) +} + +pub(super) fn run_status_kind(status: RunStatus) -> &'static str { + status.kind().into() +} + +fn format_tool_error(err: &anyhow::Error) -> String { + let mut rendered = format!("{err:#}"); + if exit::exit_class_for(err) == Some(ExitClass::AuthRequired) + && !rendered.contains("fabro auth login") + { + rendered.push_str("\nRun `fabro auth login` to authenticate."); + } + rendered +} diff --git a/lib/crates/fabro-mcp-server/src/run_tools/create.rs b/lib/crates/fabro-mcp-server/src/run_tools/create.rs new file mode 100644 index 000000000..56745ff43 --- /dev/null +++ b/lib/crates/fabro-mcp-server/src/run_tools/create.rs @@ -0,0 +1,231 @@ +use std::borrow::Cow; +use std::collections::HashMap; +use std::path::{Path, PathBuf}; +use std::sync::Arc; + +use fabro_client::Client; +use fabro_types::RunId; +use schemars::{JsonSchema, Schema, SchemaGenerator, json_schema}; +use serde::{Deserialize, Serialize}; +use serde_json::Value; + +use super::common::{ToolError, ToolResult}; +use super::{common, manifest}; + +#[derive(Debug, Deserialize, JsonSchema)] +pub(crate) struct FabroRunCreateParams { + pub(crate) runs: Vec, +} + +#[derive(Debug, Deserialize, JsonSchema)] +pub(crate) struct CreateRunSpec { + pub(crate) workflow: String, + pub(crate) cwd: Option, + pub(crate) run_id: Option, + pub(crate) goal: Option, + #[serde(default)] + pub(crate) inputs: HashMap, + #[serde(default)] + pub(crate) labels: HashMap, + pub(crate) dry_run: Option, + pub(crate) auto_approve: Option, + pub(crate) model: Option, + pub(crate) provider: Option, + pub(crate) sandbox: Option, + pub(crate) preserve_sandbox: Option, + pub(crate) start: Option, +} + +#[derive(Debug, Deserialize)] +#[serde(transparent)] +pub(crate) struct RunInputValue(Value); + +impl From for RunInputValue { + fn from(value: Value) -> Self { + Self(value) + } +} + +impl RunInputValue { + fn into_inner(self) -> Value { + self.0 + } +} + +impl JsonSchema for RunInputValue { + fn inline_schema() -> bool { + true + } + + fn schema_name() -> Cow<'static, str> { + "RunInputValue".into() + } + + fn json_schema(_: &mut SchemaGenerator) -> Schema { + json_schema!({ + "description": "Run input override value. Inputs are TOML-compatible scalar values: string, boolean, integer, or float.", + "anyOf": [ + { "type": "string" }, + { "type": "boolean" }, + { "type": "integer" }, + { "type": "number" } + ] + }) + } +} + +#[derive(Debug)] +pub(crate) struct ValidatedCreateRuns { + pub(crate) runs: Vec, +} + +#[derive(Debug)] +pub(crate) struct ValidatedCreateRunSpec { + pub(crate) workflow: String, + pub(crate) cwd: Option, + pub(crate) run_id: Option, + pub(crate) goal: Option, + pub(crate) inputs: HashMap, + pub(crate) labels: HashMap, + pub(crate) dry_run: Option, + pub(crate) auto_approve: Option, + pub(crate) model: Option, + pub(crate) provider: Option, + pub(crate) sandbox: Option, + pub(crate) preserve_sandbox: Option, + pub(crate) start: Option, +} + +impl TryFrom for ValidatedCreateRuns { + type Error = ToolError; + + fn try_from(params: FabroRunCreateParams) -> Result { + common::validate_len("runs", params.runs.len(), 1, 50)?; + let runs = params + .runs + .into_iter() + .map(ValidatedCreateRunSpec::try_from) + .collect::, _>>()?; + Ok(Self { runs }) + } +} + +impl TryFrom for ValidatedCreateRunSpec { + type Error = ToolError; + + fn try_from(spec: CreateRunSpec) -> Result { + let run_id = spec + .run_id + .as_deref() + .map(str::parse::) + .transpose() + .map_err(|err| { + ToolError::message(format!("run_id must be a valid Fabro run id: {err}")) + })?; + let inputs = spec + .inputs + .into_iter() + .map(|(key, value)| { + let value = value.into_inner(); + manifest::json_to_toml_value(&key, &value).map(|value| (key, value)) + }) + .collect::>>()?; + Ok(Self { + workflow: spec.workflow, + cwd: spec.cwd, + run_id, + goal: spec.goal, + inputs, + labels: spec.labels, + dry_run: spec.dry_run, + auto_approve: spec.auto_approve, + model: spec.model, + provider: spec.provider, + sandbox: spec.sandbox, + preserve_sandbox: spec.preserve_sandbox, + start: spec.start, + }) + } +} + +#[derive(Debug, Serialize, JsonSchema)] +pub(crate) struct CreateRunsResult { + pub(crate) runs: Vec, +} + +#[derive(Debug, Serialize, JsonSchema)] +pub(crate) struct CreatedRunResult { + pub(crate) run_id: String, + pub(crate) workflow: String, + pub(crate) started: bool, + pub(crate) status: String, +} + +pub(crate) async fn create_runs( + client: Arc, + base_cwd: &Path, + user_settings_path: &Path, + params: ValidatedCreateRuns, +) -> ToolResult { + let mut created = Vec::with_capacity(params.runs.len()); + for spec in params.runs { + let cwd = spec.cwd.clone().unwrap_or_else(|| base_cwd.to_path_buf()); + let manifest = manifest::build_mcp_run_manifest(&spec, &cwd, user_settings_path)?; + let run_id = client + .create_run_from_manifest(manifest) + .await + .map_err(|err| ToolError::from_anyhow(&err))?; + let started = spec.start.unwrap_or(true); + let summary = if started { + client + .start_run(&run_id, false) + .await + .map_err(|err| ToolError::from_anyhow(&err))? + } else { + client + .retrieve_run(&run_id) + .await + .map_err(|err| ToolError::from_anyhow(&err))? + }; + created.push(CreatedRunResult { + run_id: summary.id.to_string(), + workflow: spec.workflow, + started, + status: common::run_status_kind(summary.lifecycle.status).to_string(), + }); + } + Ok(CreateRunsResult { runs: created }) +} + +pub(crate) fn create_runs_text(result: &CreateRunsResult) -> String { + let started = result.runs.iter().filter(|run| run.started).count(); + format!( + "created {} Fabro run(s), started {started}", + result.runs.len() + ) +} + +#[cfg(test)] +mod tests { + use schemars::SchemaGenerator; + use serde_json::json; + + use super::*; + + #[test] + fn run_input_value_schema_allows_only_json_scalars() { + let mut generator = SchemaGenerator::default(); + let schema = RunInputValue::json_schema(&mut generator); + let schema = serde_json::to_value(schema).expect("schema should serialize"); + + assert_eq!( + schema["anyOf"], + json!([ + { "type": "string" }, + { "type": "boolean" }, + { "type": "integer" }, + { "type": "number" }, + ]) + ); + } +} diff --git a/lib/crates/fabro-mcp-server/src/run_tools/events.rs b/lib/crates/fabro-mcp-server/src/run_tools/events.rs new file mode 100644 index 000000000..42b7c44cc --- /dev/null +++ b/lib/crates/fabro-mcp-server/src/run_tools/events.rs @@ -0,0 +1,313 @@ +use std::sync::Arc; + +use chrono::{DateTime, Utc}; +use fabro_client::Client; +use fabro_types::EventEnvelope; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; +use serde_json::Value; + +use super::common; +use super::common::{ToolError, ToolResult}; + +#[derive(Debug, Clone, Copy, Deserialize, Serialize, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub(crate) enum RunEventsAction { + List, + Details, + Search, +} + +#[derive(Debug, Deserialize, JsonSchema)] +pub(crate) struct FabroRunEventsParams { + pub(crate) action: RunEventsAction, + pub(crate) run_id: String, + pub(crate) event_types: Option>, + pub(crate) categories: Option>, + pub(crate) direction: Option, + pub(crate) created_after: Option, + pub(crate) created_before: Option, + pub(crate) first: Option, + pub(crate) after: Option, + pub(crate) event_ids: Option>, + pub(crate) offset: Option, + pub(crate) limit: Option, + pub(crate) max_content_length: Option, + pub(crate) query: Option, +} + +#[derive(Debug)] +pub(crate) struct ValidatedRunEvents { + pub(crate) raw: FabroRunEventsParams, + pub(crate) descending: bool, + pub(crate) first: usize, + pub(crate) created_after: Option>, + pub(crate) created_before: Option>, +} + +impl TryFrom for ValidatedRunEvents { + type Error = ToolError; + + fn try_from(params: FabroRunEventsParams) -> Result { + if params.run_id.trim().is_empty() { + return Err(ToolError::message("run_id is required")); + } + let first = params.first.or(params.limit).unwrap_or(50); + if first > 200 { + return Err(ToolError::message("first must be <= 200")); + } + let descending = match params.direction.as_deref() { + None | Some("asc") => false, + Some("desc") => true, + Some(_) => return Err(ToolError::message("direction must be `asc` or `desc`")), + }; + let created_after = params + .created_after + .as_deref() + .map(|created_after| common::parse_datetime_filter("created_after", created_after)) + .transpose()?; + let created_before = params + .created_before + .as_deref() + .map(|created_before| common::parse_datetime_filter("created_before", created_before)) + .transpose()?; + if matches!(params.action, RunEventsAction::Details) + && params.event_ids.as_ref().is_none_or(Vec::is_empty) + { + return Err(ToolError::message( + "event_ids is required for details action", + )); + } + if matches!(params.action, RunEventsAction::Search) + && params + .query + .as_deref() + .is_none_or(|query| query.trim().is_empty()) + { + return Err(ToolError::message("query is required for search action")); + } + Ok(Self { + raw: params, + descending, + first, + created_after, + created_before, + }) + } +} + +#[derive(Debug, Serialize, JsonSchema)] +pub(crate) struct RunEventsResult { + pub(crate) run_id: String, + pub(crate) action: RunEventsAction, + pub(crate) events: Vec, + pub(crate) next_cursor: Option, +} + +#[derive(Debug, Serialize, JsonSchema)] +pub(crate) struct RunEventResult { + pub(crate) event_id: String, + pub(crate) sequence: u32, + pub(crate) event: Value, + pub(crate) truncated: bool, +} + +pub(crate) async fn run_events( + client: Arc, + params: ValidatedRunEvents, +) -> ToolResult { + let descending = params.descending; + let first = params.first; + let created_after = params.created_after; + let created_before = params.created_before; + let raw = params.raw; + let run_id = client + .resolve_run(&raw.run_id) + .await + .map_err(|err| ToolError::from_anyhow(&err))? + .id; + let fetch_after = if descending { None } else { raw.after }; + let mut events = if let Some(limit) = event_fetch_limit(&raw, first) { + client + .list_run_events_until(&run_id, fetch_after, limit) + .await + } else { + client.list_run_events(&run_id, fetch_after, None).await + } + .map_err(|err| ToolError::from_anyhow(&err))?; + if descending { + if let Some(after) = raw.after { + events.retain(|event| event.seq < after); + } + } + filter_events(&mut events, &raw, created_after, created_before); + if descending { + events.reverse(); + } + let offset = raw.offset.unwrap_or(0); + let page = events + .into_iter() + .skip(offset) + .take(first) + .collect::>(); + let max_content_length = raw.max_content_length.unwrap_or(20_000); + let results = page + .iter() + .map(|event| run_event_result(event, max_content_length)) + .collect::>>()?; + let next_cursor = page.last().map(|event| { + if descending { + event.seq + } else { + event.seq.saturating_add(1) + } + }); + + Ok(RunEventsResult { + run_id: run_id.to_string(), + action: raw.action, + events: results, + next_cursor, + }) +} + +pub(crate) fn run_events_text(result: &RunEventsResult) -> String { + format!("returned {} Fabro event(s)", result.events.len()) +} + +fn event_fetch_limit(params: &FabroRunEventsParams, first: usize) -> Option { + let needs_full_scan = params.event_ids.is_some() + || params.event_types.is_some() + || params.categories.is_some() + || params.created_after.is_some() + || params.created_before.is_some() + || params.direction.as_deref() == Some("desc") + || matches!( + params.action, + RunEventsAction::Details | RunEventsAction::Search + ); + if needs_full_scan { + return None; + } + + let requested = first.saturating_add(params.offset.unwrap_or(0)); + Some(requested.max(1)) +} + +fn filter_events( + events: &mut Vec, + params: &FabroRunEventsParams, + created_after: Option>, + created_before: Option>, +) { + if let Some(event_ids) = params.event_ids.as_ref() { + events.retain(|event| event_ids.contains(&event.event.id)); + } + if let Some(event_types) = params.event_types.as_ref() { + events.retain(|event| { + event_types + .iter() + .any(|event_type| event_type == event.event.event_name()) + }); + } + if let Some(categories) = params.categories.as_ref() { + events.retain(|event| { + let category = event + .event + .event_name() + .split('.') + .next() + .unwrap_or_default(); + categories.iter().any(|candidate| candidate == category) + }); + } + if let Some(cutoff) = created_after { + events.retain(|event| event.event.ts >= cutoff); + } + if let Some(cutoff) = created_before { + events.retain(|event| event.event.ts <= cutoff); + } + if matches!(params.action, RunEventsAction::Search) { + if let Some(query) = params.query.as_deref() { + events.retain(|event| { + serde_json::to_string(event).is_ok_and(|serialized| serialized.contains(query)) + }); + } + } +} + +fn run_event_result( + event: &EventEnvelope, + max_content_length: usize, +) -> ToolResult { + let mut serialized = serde_json::to_string(event) + .map_err(|err| ToolError::message(format!("failed to serialize event: {err}")))?; + let truncated = serialized.len() > max_content_length; + let event_value = if truncated { + serialized.truncate(floor_char_boundary(&serialized, max_content_length)); + Value::String(serialized) + } else { + serde_json::to_value(event) + .map_err(|err| ToolError::message(format!("failed to serialize event: {err}")))? + }; + Ok(RunEventResult { + event_id: event.event.id.clone(), + sequence: event.seq, + event: event_value, + truncated, + }) +} + +fn floor_char_boundary(value: &str, max_len: usize) -> usize { + let mut boundary = max_len.min(value.len()); + while !value.is_char_boundary(boundary) { + boundary -= 1; + } + boundary +} + +#[cfg(test)] +mod tests { + use chrono::Utc; + use fabro_types::{EventBody, EventEnvelope, RunEvent, fixtures}; + use serde_json::{Value, json}; + + use super::*; + + #[test] + fn run_event_result_truncates_at_utf8_boundary() { + let event = EventEnvelope { + seq: 1, + event: RunEvent { + id: "evt_utf8".to_string(), + ts: Utc::now(), + run_id: fixtures::RUN_1, + node_id: None, + node_label: None, + stage_id: None, + parallel_group_id: None, + parallel_branch_id: None, + session_id: None, + parent_session_id: None, + tool_call_id: None, + actor: None, + body: EventBody::Unknown { + name: "test.utf8".to_string(), + properties: json!({ "message": "éééé" }), + }, + }, + }; + let serialized = serde_json::to_string(&event).unwrap(); + let first_multibyte = serialized + .find('é') + .expect("serialized event should contain é"); + + let result = run_event_result(&event, first_multibyte + 1).unwrap(); + + assert!(result.truncated); + let Value::String(event_json) = result.event else { + panic!("truncated events should return string payloads"); + }; + assert!(event_json.is_char_boundary(event_json.len())); + } +} diff --git a/lib/crates/fabro-mcp-server/src/run_tools/gather.rs b/lib/crates/fabro-mcp-server/src/run_tools/gather.rs new file mode 100644 index 000000000..48e59b33f --- /dev/null +++ b/lib/crates/fabro-mcp-server/src/run_tools/gather.rs @@ -0,0 +1,109 @@ +use std::sync::Arc; +use std::time::{Duration, Instant}; + +use fabro_client::Client; +use futures::future::try_join_all; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; +use tokio::time; + +use super::common; +use super::common::{RunSummaryResult, ToolError, ToolResult}; + +#[derive(Debug, Deserialize, JsonSchema)] +pub(crate) struct FabroRunGatherParams { + pub(crate) run_ids: Vec, + pub(crate) timeout_seconds: Option, + pub(crate) poll_interval_seconds: Option, +} + +#[derive(Debug)] +pub(crate) struct ValidatedGatherRuns { + pub(crate) run_ids: Vec, + pub(crate) timeout_seconds: u64, + pub(crate) poll_interval_seconds: u64, +} + +impl TryFrom for ValidatedGatherRuns { + type Error = ToolError; + + fn try_from(params: FabroRunGatherParams) -> Result { + common::validate_len("run_ids", params.run_ids.len(), 1, 50)?; + if params.timeout_seconds.is_some_and(|timeout| timeout > 600) { + return Err(ToolError::message("timeout_seconds must be <= 600")); + } + if params + .poll_interval_seconds + .is_some_and(|interval| interval < 5) + { + return Err(ToolError::message("poll_interval_seconds must be >= 5")); + } + Ok(Self { + run_ids: params.run_ids, + timeout_seconds: params.timeout_seconds.unwrap_or(300), + poll_interval_seconds: params.poll_interval_seconds.unwrap_or(15), + }) + } +} + +#[derive(Debug, Serialize, JsonSchema)] +pub(crate) struct GatherRunsResult { + pub(crate) runs: Vec, + pub(crate) timed_out: bool, + pub(crate) elapsed_seconds: u64, +} + +pub(crate) async fn gather_runs( + client: Arc, + params: ValidatedGatherRuns, +) -> ToolResult { + let start = Instant::now(); + let deadline = start + Duration::from_secs(params.timeout_seconds); + let run_ids = try_join_all(params.run_ids.into_iter().map(|selector| { + let client = Arc::clone(&client); + async move { + client + .resolve_run(&selector) + .await + .map(|run| run.id) + .map_err(|err| ToolError::from_anyhow(&err)) + } + })) + .await?; + + loop { + let summaries = try_join_all(run_ids.iter().map(|run_id| { + let client = Arc::clone(&client); + async move { common::retrieve_run(&client, run_id).await } + })) + .await?; + if summaries + .iter() + .all(|run| run.lifecycle.status.is_terminal()) + { + return Ok(GatherRunsResult { + runs: summaries.iter().map(common::run_summary_result).collect(), + timed_out: false, + elapsed_seconds: start.elapsed().as_secs(), + }); + } + let now = Instant::now(); + if now >= deadline { + return Ok(GatherRunsResult { + runs: summaries.iter().map(common::run_summary_result).collect(), + timed_out: true, + elapsed_seconds: start.elapsed().as_secs(), + }); + } + let sleep_for = Duration::from_secs(params.poll_interval_seconds).min(deadline - now); + time::sleep(sleep_for).await; + } +} + +pub(crate) fn gather_runs_text(result: &GatherRunsResult) -> String { + format!( + "gathered {} Fabro run(s), timed_out={}", + result.runs.len(), + result.timed_out + ) +} diff --git a/lib/crates/fabro-mcp-server/src/run_tools/interact.rs b/lib/crates/fabro-mcp-server/src/run_tools/interact.rs new file mode 100644 index 000000000..abd6b6cf7 --- /dev/null +++ b/lib/crates/fabro-mcp-server/src/run_tools/interact.rs @@ -0,0 +1,399 @@ +use std::borrow::Cow; +use std::sync::Arc; + +use fabro_api::types; +use fabro_client::Client; +use fabro_types::RunId; +use schemars::{JsonSchema, Schema, SchemaGenerator, json_schema}; +use serde::{Deserialize, Serialize}; +use serde_json::{Value, json}; + +use super::common; +use super::common::{ToolError, ToolResult}; + +#[derive(Debug, Clone, Copy, Deserialize, Serialize, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub(crate) enum RunInteractAction { + Get, + Start, + Message, + Cancel, + Archive, + Unarchive, + GetQuestions, + Answer, +} + +#[derive(Debug, Deserialize, JsonSchema)] +pub(crate) struct FabroRunInteractParams { + pub(crate) action: RunInteractAction, + pub(crate) run_id: String, + pub(crate) message: Option, + pub(crate) interrupt: Option, + pub(crate) question_id: Option, + pub(crate) answer: Option, +} + +#[derive(Debug, Deserialize)] +#[serde(transparent)] +pub(crate) struct AnswerValue(Value); + +impl From for AnswerValue { + fn from(value: Value) -> Self { + Self(value) + } +} + +impl AnswerValue { + fn into_inner(self) -> Value { + self.0 + } +} + +impl JsonSchema for AnswerValue { + fn inline_schema() -> bool { + true + } + + fn schema_name() -> Cow<'static, str> { + "AnswerValue".into() + } + + fn json_schema(_: &mut SchemaGenerator) -> Schema { + json_schema!({ + "description": "Answer payload for a pending Fabro question. Use a boolean for yes/no, a string or {\"text\": \"...\"} for freeform text, {\"option\": \"key\"} for a single choice, or {\"options\": [\"key\"]} for multi-select.", + "anyOf": [ + { "type": "boolean" }, + { "type": "string" }, + { + "type": "object", + "properties": { + "option": { "type": "string" } + }, + "required": ["option"], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "options": { + "type": "array", + "items": { "type": "string" } + } + }, + "required": ["options"], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "text": { "type": "string" } + }, + "required": ["text"], + "additionalProperties": false + } + ] + }) + } +} + +#[derive(Debug)] +pub(crate) struct ValidatedInteractRun { + pub(crate) run_id: String, + pub(crate) action: ValidatedInteractAction, +} + +#[derive(Debug)] +pub(crate) enum ValidatedInteractAction { + Get, + Start, + Message { + message: String, + interrupt: bool, + }, + Cancel, + Archive, + Unarchive, + GetQuestions, + Answer { + question_id: String, + body: types::SubmitAnswerRequest, + }, +} + +impl ValidatedInteractAction { + fn action(&self) -> RunInteractAction { + match self { + Self::Get => RunInteractAction::Get, + Self::Start => RunInteractAction::Start, + Self::Message { .. } => RunInteractAction::Message, + Self::Cancel => RunInteractAction::Cancel, + Self::Archive => RunInteractAction::Archive, + Self::Unarchive => RunInteractAction::Unarchive, + Self::GetQuestions => RunInteractAction::GetQuestions, + Self::Answer { .. } => RunInteractAction::Answer, + } + } +} + +impl TryFrom for ValidatedInteractRun { + type Error = ToolError; + + fn try_from(params: FabroRunInteractParams) -> Result { + if params.run_id.trim().is_empty() { + return Err(ToolError::message("run_id is required")); + } + let action = match params.action { + RunInteractAction::Get => ValidatedInteractAction::Get, + RunInteractAction::Start => ValidatedInteractAction::Start, + RunInteractAction::Message => { + let Some(message) = params + .message + .as_deref() + .map(str::trim) + .filter(|message| !message.is_empty()) + else { + return Err(ToolError::message("message is required for action message")); + }; + ValidatedInteractAction::Message { + message: message.to_string(), + interrupt: params.interrupt.unwrap_or(false), + } + } + RunInteractAction::Cancel => ValidatedInteractAction::Cancel, + RunInteractAction::Archive => ValidatedInteractAction::Archive, + RunInteractAction::Unarchive => ValidatedInteractAction::Unarchive, + RunInteractAction::GetQuestions => ValidatedInteractAction::GetQuestions, + RunInteractAction::Answer => { + let Some(question_id) = params + .question_id + .as_deref() + .map(str::trim) + .filter(|question_id| !question_id.is_empty()) + else { + return Err(ToolError::message( + "question_id is required for action answer", + )); + }; + let Some(answer) = params.answer else { + return Err(ToolError::message("answer is required for action answer")); + }; + ValidatedInteractAction::Answer { + question_id: question_id.to_string(), + body: answer_to_submit_request(answer.into_inner())?, + } + } + }; + Ok(Self { + run_id: params.run_id.trim().to_string(), + action, + }) + } +} + +#[derive(Debug, Serialize, JsonSchema)] +pub(crate) struct InteractRunResult { + pub(crate) run_id: String, + pub(crate) action: RunInteractAction, + pub(crate) result: Value, +} + +pub(crate) async fn interact_run( + client: Arc, + params: ValidatedInteractRun, +) -> ToolResult { + let run_id = client + .resolve_run(¶ms.run_id) + .await + .map_err(|err| ToolError::from_anyhow(&err))? + .id; + let action = params.action.action(); + let result = match params.action { + ValidatedInteractAction::Get => interact_get(&client, &run_id).await?, + ValidatedInteractAction::Start => { + let summary = client + .start_run(&run_id, false) + .await + .map_err(|err| ToolError::from_anyhow(&err))?; + json!({ "summary": common::run_summary_result(&summary) }) + } + ValidatedInteractAction::Message { message, interrupt } => { + client + .steer_run(&run_id, message.clone(), interrupt) + .await + .map_err(|err| ToolError::from_anyhow(&err))?; + json!({ "message": message, "interrupt": interrupt }) + } + ValidatedInteractAction::Cancel => { + let summary = client + .cancel_run(&run_id) + .await + .map_err(|err| ToolError::from_anyhow(&err))?; + json!({ "summary": common::run_summary_result(&summary) }) + } + ValidatedInteractAction::Archive => { + let summary = client + .archive_run(&run_id) + .await + .map_err(|err| ToolError::from_anyhow(&err))?; + json!({ "summary": common::run_summary_result(&summary) }) + } + ValidatedInteractAction::Unarchive => { + let summary = client + .unarchive_run(&run_id) + .await + .map_err(|err| ToolError::from_anyhow(&err))?; + json!({ "summary": common::run_summary_result(&summary) }) + } + ValidatedInteractAction::GetQuestions => { + let questions = client + .list_run_questions(&run_id) + .await + .map_err(|err| ToolError::from_anyhow(&err))?; + json!({ "questions": questions }) + } + ValidatedInteractAction::Answer { question_id, body } => { + client + .submit_run_answer(&run_id, &question_id, body) + .await + .map_err(|err| ToolError::from_anyhow(&err))?; + json!({ "question_id": question_id, "submitted": true }) + } + }; + + Ok(InteractRunResult { + run_id: run_id.to_string(), + action, + result, + }) +} + +pub(crate) fn interact_run_text(result: &InteractRunResult) -> String { + format!( + "completed {:?} for Fabro run {}", + result.action, result.run_id + ) +} + +async fn interact_get(client: &Client, run_id: &RunId) -> ToolResult { + let summary = common::retrieve_run(client, run_id).await?; + let projection = client + .get_run_state(run_id) + .await + .map_err(|err| ToolError::from_anyhow(&err))?; + Ok(json!({ + "summary": common::run_summary_result(&summary), + "projection": projection, + })) +} + +fn answer_to_submit_request(answer: Value) -> ToolResult { + match answer { + Value::Bool(true) => Ok(types::SubmitAnswerYesRequest { + kind: types::SubmitAnswerYesRequestKind::Yes, + } + .into()), + Value::Bool(false) => Ok(types::SubmitAnswerNoRequest { + kind: types::SubmitAnswerNoRequestKind::No, + } + .into()), + Value::String(text) => Ok(text_answer_request(text)), + Value::Object(mut object) => { + if let Some(option) = object.remove("option") { + let option_key = serde_json::from_value::(option).map_err(|err| { + ToolError::message(format!("answer option must be a string: {err}")) + })?; + Ok(types::SubmitAnswerSelectedRequest { + kind: types::SubmitAnswerSelectedRequestKind::Selected, + option_key, + } + .into()) + } else if let Some(options) = object.remove("options") { + let option_keys = + serde_json::from_value::>(options).map_err(|err| { + ToolError::message(format!("answer options must be strings: {err}")) + })?; + Ok(types::SubmitAnswerMultiSelectedRequest { + kind: types::SubmitAnswerMultiSelectedRequestKind::MultiSelected, + option_keys, + } + .into()) + } else if let Some(text) = object.remove("text") { + let text = serde_json::from_value::(text).map_err(|err| { + ToolError::message(format!("answer text must be a string: {err}")) + })?; + Ok(text_answer_request(text)) + } else { + Err(ToolError::message( + "answer object must contain one of: option, options, text", + )) + } + } + other => Err(ToolError::message(format!( + "unsupported answer value: {other}; expected boolean, string, or object", + ))), + } +} + +fn text_answer_request(text: String) -> types::SubmitAnswerRequest { + types::SubmitAnswerTextRequest { + kind: types::SubmitAnswerTextRequestKind::Text, + text, + } + .into() +} + +#[cfg(test)] +mod tests { + use serde_json::json; + + use super::*; + + #[test] + fn answer_payloads_map_to_submit_answer_wire_json() { + let cases = [ + (json!(true), json!({ "kind": "yes" })), + (json!(false), json!({ "kind": "no" })), + (json!("hello"), json!({ "kind": "text", "text": "hello" })), + ( + json!({ "option": "a" }), + json!({ "kind": "selected", "option_key": "a" }), + ), + ( + json!({ "options": ["a", "b"] }), + json!({ "kind": "multi_selected", "option_keys": ["a", "b"] }), + ), + ( + json!({ "text": "hello" }), + json!({ "kind": "text", "text": "hello" }), + ), + ]; + + for (answer, expected) in cases { + let request = answer_to_submit_request(answer).unwrap(); + assert_eq!(serde_json::to_value(request).unwrap(), expected); + } + } + + #[test] + fn unsupported_answer_object_is_rejected() { + let err = answer_to_submit_request(json!({ "value": "yes" })).unwrap_err(); + + assert!(err.as_str().contains("option, options, text")); + } + + #[test] + fn interact_answer_validation_rejects_unsupported_json_before_api_calls() { + let err = ValidatedInteractRun::try_from(FabroRunInteractParams { + action: RunInteractAction::Answer, + run_id: "run_123".to_string(), + message: None, + interrupt: None, + question_id: Some("question-1".to_string()), + answer: Some(json!({ "value": "yes" }).into()), + }) + .unwrap_err(); + + assert!(err.as_str().contains("option, options, text")); + } +} diff --git a/lib/crates/fabro-mcp-server/src/run_tools/manifest.rs b/lib/crates/fabro-mcp-server/src/run_tools/manifest.rs new file mode 100644 index 000000000..581bf21d2 --- /dev/null +++ b/lib/crates/fabro-mcp-server/src/run_tools/manifest.rs @@ -0,0 +1,178 @@ +use std::path::{Path, PathBuf}; + +use fabro_api::types; +use fabro_config::{CliLayer, RunLayer}; +use fabro_manifest::{self, ManifestBuildInput, RunOverrideInput}; +use fabro_server::manifest_validation; +use serde_json::Value; + +use super::common::{ToolError, ToolResult}; +use super::create::ValidatedCreateRunSpec; + +pub(super) fn build_mcp_run_manifest( + spec: &ValidatedCreateRunSpec, + cwd: &Path, + user_settings_path: &Path, +) -> ToolResult { + let built = fabro_manifest::build_run_manifest(ManifestBuildInput { + workflow: PathBuf::from(&spec.workflow), + cwd: cwd.to_path_buf(), + run_overrides: mcp_run_overrides(spec), + cli_overrides: Some(CliLayer::default()), + input_overrides: spec.inputs.clone(), + args: mcp_manifest_args(spec), + run_id: spec.run_id, + user_settings_path: Some(user_settings_path.to_path_buf()), + }) + .map_err(|err| ToolError::from_anyhow(&err))?; + let validation = manifest_validation::validate_manifest(&RunLayer::default(), &built.manifest) + .map_err(|err| ToolError::from_anyhow(&err))?; + if !validation.ok { + return Err(ToolError::message("workflow manifest validation failed")); + } + Ok(built.manifest) +} + +pub(super) fn json_to_toml_value(key: &str, value: &Value) -> ToolResult { + match value { + Value::Null => Err(ToolError::message(format!( + "input `{key}` cannot be null; use a string, boolean, or number" + ))), + Value::Bool(value) => Ok(toml::Value::Boolean(*value)), + Value::Number(value) => { + if let Some(integer) = value.as_i64() { + Ok(toml::Value::Integer(integer)) + } else if let Some(float) = value.as_f64() { + Ok(toml::Value::Float(float)) + } else { + Err(ToolError::message(format!( + "input `{key}` contains a number outside TOML's supported range" + ))) + } + } + Value::String(value) => Ok(toml::Value::String(value.clone())), + Value::Array(_) => Err(ToolError::message(format!( + "input `{key}` does not support array values; use a string, boolean, or number", + ))), + Value::Object(_) => Err(ToolError::message(format!( + "input `{key}` does not support object values; use a string, boolean, or number", + ))), + } +} + +fn mcp_manifest_args(spec: &ValidatedCreateRunSpec) -> Option { + let mut input = spec + .inputs + .iter() + .map(|(key, value)| format!("{key}={value}")) + .collect::>(); + input.sort(); + let mut label = spec + .labels + .iter() + .map(|(key, value)| format!("{key}={value}")) + .collect::>(); + label.sort(); + let payload = types::ManifestArgs { + auto_approve: spec.auto_approve.filter(|value| *value), + docker_image: None, + dry_run: spec.dry_run.filter(|value| *value), + input, + label, + model: spec.model.clone(), + preserve_sandbox: spec.preserve_sandbox.filter(|value| *value), + provider: spec.provider.clone(), + sandbox: spec.sandbox.clone(), + verbose: None, + }; + (!fabro_manifest::manifest_args_is_empty(&payload)).then_some(payload) +} + +fn mcp_run_overrides(spec: &ValidatedCreateRunSpec) -> Option { + fabro_manifest::build_sparse_run_overrides(RunOverrideInput { + goal: spec.goal.as_deref(), + model: spec.model.as_deref(), + provider: spec.provider.as_deref(), + sandbox: spec.sandbox.as_deref(), + docker_image: None, + preserve_sandbox: spec.preserve_sandbox, + dry_run: spec.dry_run, + auto_approve: spec.auto_approve, + labels: spec.labels.clone(), + }) +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use serde_json::{Value, json}; + + use super::super::create::CreateRunSpec; + use super::*; + + #[test] + fn json_inputs_convert_scalar_values_to_toml_values() { + let cases = [ + (json!("hello"), toml::Value::String("hello".to_string())), + (json!(true), toml::Value::Boolean(true)), + (json!(42), toml::Value::Integer(42)), + (json!(0.5), toml::Value::Float(0.5)), + ]; + + for (json, expected) in cases { + assert_eq!(json_to_toml_value("input", &json).unwrap(), expected); + } + } + + #[test] + fn json_input_arrays_and_objects_are_rejected() { + let array_err = json_to_toml_value("matrix", &json!(["a", 1])).unwrap_err(); + assert_eq!( + array_err.as_str(), + "input `matrix` does not support array values; use a string, boolean, or number", + ); + + let object_err = json_to_toml_value("settings", &json!({ "enabled": true })).unwrap_err(); + assert_eq!( + object_err.as_str(), + "input `settings` does not support object values; use a string, boolean, or number", + ); + } + + #[test] + fn json_input_null_is_rejected_with_key_name() { + let err = json_to_toml_value("goal", &Value::Null).unwrap_err(); + + assert_eq!( + err.as_str(), + "input `goal` cannot be null; use a string, boolean, or number", + ); + } + + #[test] + fn mcp_manifest_args_preserve_input_provenance() { + let spec = ValidatedCreateRunSpec::try_from(CreateRunSpec { + workflow: "simple".to_string(), + run_id: None, + cwd: None, + goal: None, + inputs: HashMap::from([ + ("count".to_string(), json!(3).into()), + ("decision".to_string(), json!("approve").into()), + ]), + labels: HashMap::new(), + model: None, + provider: None, + sandbox: None, + dry_run: None, + auto_approve: None, + preserve_sandbox: None, + start: None, + }) + .expect("create spec should validate"); + let args = mcp_manifest_args(&spec).expect("input args should be present"); + + assert_eq!(args.input, vec![r"count=3", r#"decision="approve""#]); + } +} diff --git a/lib/crates/fabro-mcp-server/src/run_tools/search.rs b/lib/crates/fabro-mcp-server/src/run_tools/search.rs new file mode 100644 index 000000000..0a5b2bb15 --- /dev/null +++ b/lib/crates/fabro-mcp-server/src/run_tools/search.rs @@ -0,0 +1,378 @@ +use std::collections::HashMap; +use std::sync::Arc; + +use fabro_client::Client; +use fabro_types::{Run, RunStatusKind}; +use futures::future::try_join_all; +use schemars::JsonSchema; +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>, + pub(crate) workflow: Option, + pub(crate) labels: Option>, + pub(crate) status: Option>, + pub(crate) archived: Option, + pub(crate) created_after: Option, + pub(crate) created_before: Option, + pub(crate) first: Option, + pub(crate) after: Option, +} + +#[derive(Debug)] +pub(crate) struct ValidatedSearchRuns { + pub(crate) raw: FabroRunSearchParams, + pub(crate) status: Option>, +} + +impl TryFrom for ValidatedSearchRuns { + type Error = ToolError; + + fn try_from(params: FabroRunSearchParams) -> Result { + if params.first.is_some_and(|first| first > 100) { + return Err(ToolError::message("first must be <= 100")); + } + if let Some(run_ids) = params.run_ids.as_ref() { + common::validate_len("run_ids", run_ids.len(), 1, 100)?; + } + let status = params + .status + .as_ref() + .map(|statuses| { + statuses + .iter() + .map(|status| { + status.parse::().map_err(|_| { + ToolError::message(format!("unknown run status `{status}`")) + }) + }) + .collect::>>() + }) + .transpose()?; + if let Some(created_after) = params.created_after.as_deref() { + common::parse_datetime_filter("created_after", created_after)?; + } + if let Some(created_before) = params.created_before.as_deref() { + common::parse_datetime_filter("created_before", created_before)?; + } + Ok(Self { + raw: params, + status, + }) + } +} + +#[derive(Debug, Serialize, JsonSchema)] +pub(crate) struct SearchRunsResult { + pub(crate) runs: Vec, + pub(crate) next_cursor: Option, +} + +#[derive(Debug, Serialize, JsonSchema)] +pub(crate) struct SearchRunSummaryResult { + pub(crate) run_id: String, + pub(crate) workflow_name: String, + pub(crate) workflow_slug: Option, + pub(crate) status: String, + pub(crate) archived: bool, + pub(crate) created_at: String, + pub(crate) started_at: Option, + pub(crate) completed_at: Option, + pub(crate) labels: HashMap, + pub(crate) source_directory: Option, + pub(crate) repo_origin_url: Option, + pub(crate) goal_preview: String, + pub(crate) goal_truncated: bool, +} + +pub(crate) async fn search_runs( + client: Arc, + params: ValidatedSearchRuns, +) -> ToolResult { + let status = params.status; + let raw = params.raw; + let runs = if let Some(run_ids) = raw.run_ids.as_ref() { + resolve_requested_runs(&client, run_ids).await? + } else { + client + .list_store_runs() + .await + .map_err(|err| ToolError::from_anyhow(&err))? + }; + let page = filter_sort_and_page_runs(runs, &raw, status.as_deref())?; + + Ok(SearchRunsResult { + 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::(); + let truncated = chars.next().is_some(); + if truncated { + preview.push_str("..."); + } + (preview, truncated) +} + +struct RunSearchPage { + runs: Vec, + next_cursor: Option, +} + +fn filter_sort_and_page_runs( + mut runs: Vec, + raw: &FabroRunSearchParams, + status: Option<&[RunStatusKind]>, +) -> ToolResult { + if let Some(workflow) = raw.workflow.as_deref() { + runs.retain(|run| { + run.workflow.name == workflow || run.workflow.slug.as_deref() == Some(workflow) + }); + } + if let Some(labels) = raw.labels.as_ref() { + runs.retain(|run| { + labels + .iter() + .all(|(key, value)| run.labels.get(key) == Some(value)) + }); + } + if let Some(status) = status { + runs.retain(|run| { + status + .iter() + .any(|status| *status == run.lifecycle.status.kind()) + }); + } + let archived = raw.archived.unwrap_or(false); + runs.retain(|run| run.lifecycle.archived == archived); + if let Some(created_after) = raw.created_after.as_deref() { + let cutoff = common::parse_datetime_filter("created_after", created_after)?; + runs.retain(|run| run.timestamps.created_at >= cutoff); + } + if let Some(created_before) = raw.created_before.as_deref() { + let cutoff = common::parse_datetime_filter("created_before", created_before)?; + runs.retain(|run| run.timestamps.created_at <= cutoff); + } + + runs.sort_by(|a, b| { + 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)) + }); + + if let Some(after) = raw.after.as_deref() { + if let Some(position) = runs.iter().position(|run| run.id.to_string() == after) { + runs = runs.into_iter().skip(position + 1).collect(); + } + } + + let first = raw.first.unwrap_or(20).min(100); + let has_more = runs.len() > first; + let page = runs.into_iter().take(first).collect::>(); + let next_cursor = has_more + .then(|| page.last().map(|run| run.id.to_string())) + .flatten(); + Ok(RunSearchPage { + runs: page, + next_cursor, + }) +} + +pub(crate) fn search_runs_text(result: &SearchRunsResult) -> String { + format!("found {} Fabro run(s)", result.runs.len()) +} + +async fn resolve_requested_runs(client: &Arc, run_ids: &[String]) -> ToolResult> { + let runs = try_join_all(run_ids.iter().map(|run_id| { + let client = Arc::clone(client); + async move { + client + .resolve_run(run_id) + .await + .map_err(|err| ToolError::from_anyhow(&err)) + } + })) + .await?; + + let mut unique = HashMap::new(); + for run in runs { + unique.entry(run.id).or_insert(run); + } + Ok(unique.into_values().collect()) +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use chrono::{TimeZone, Utc}; + use fabro_types::{RunLifecycle, RunLinks, RunOrigin, RunStatus, RunTimestamps, WorkflowRef}; + + use super::*; + + #[test] + fn cursor_is_applied_after_filters() { + let matching_newer = run("01KRBZW5C00000000000000001", "keep", 30); + let unrelated_cursor = run("01KRBZW4DW0000000000000002", "skip", 20); + let matching_older = run("01KRBZW3EF0000000000000003", "keep", 10); + + let result = filter_sort_and_page_runs( + vec![ + matching_older.clone(), + unrelated_cursor.clone(), + matching_newer.clone(), + ], + &FabroRunSearchParams { + run_ids: None, + workflow: None, + labels: Some(HashMap::from([("group".to_string(), "keep".to_string())])), + status: None, + archived: None, + created_after: None, + created_before: None, + first: Some(10), + after: Some(unrelated_cursor.id.to_string()), + }, + None, + ) + .expect("filtering should succeed"); + + let ids = result.runs.iter().map(|run| run.id).collect::>(); + assert_eq!(ids, vec![matching_newer.id, matching_older.id]); + } + + #[test] + fn omitted_archived_filter_hides_archived_runs_by_default() { + let active = run("01KRBZW5C00000000000000001", "keep", 30); + let archived = archived_run("01KRBZW4DW0000000000000002", "keep", 20); + + let result = filter_sort_and_page_runs( + vec![archived.clone(), active.clone()], + &FabroRunSearchParams { + run_ids: None, + workflow: None, + labels: None, + status: None, + archived: None, + created_after: None, + created_before: None, + first: Some(10), + after: None, + }, + None, + ) + .expect("filtering should succeed"); + + let ids = result.runs.iter().map(|run| run.id).collect::>(); + 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) + } + + fn archived_run(id: &str, group: &str, seconds: u32) -> Run { + run_with_archived(id, group, seconds, true) + } + + fn run_with_archived(id: &str, group: &str, seconds: u32, archived: bool) -> Run { + let created_at = Utc.with_ymd_and_hms(2026, 5, 11, 12, 0, seconds).unwrap(); + Run { + id: id.parse().expect("test run id should parse"), + title: "test".to_string(), + goal: "test".to_string(), + workflow: WorkflowRef { + slug: Some("simple".to_string()), + name: "Simple".to_string(), + }, + automation: None, + repository: None, + created_by: None, + origin: RunOrigin::default(), + labels: HashMap::from([("group".to_string(), group.to_string())]), + lifecycle: RunLifecycle { + status: RunStatus::Submitted, + pending_control: None, + queue_position: None, + error: None, + archived, + archived_at: None, + }, + sandbox: None, + models: Vec::new(), + source_directory: None, + timestamps: RunTimestamps { + created_at, + started_at: None, + last_event_at: None, + completed_at: None, + duration_ms: None, + elapsed_secs: None, + }, + billing: None, + diff: None, + pull_request: None, + current_question: None, + superseded_by: None, + links: RunLinks { web: None }, + } + } +} diff --git a/lib/crates/fabro-mcp-server/src/server.rs b/lib/crates/fabro-mcp-server/src/server.rs new file mode 100644 index 000000000..e82b2e404 --- /dev/null +++ b/lib/crates/fabro-mcp-server/src/server.rs @@ -0,0 +1,171 @@ +use std::path::PathBuf; +use std::sync::Arc; + +use anyhow::Result; +use fabro_client::Client; +use rmcp::handler::server::router::tool::ToolRouter; +use rmcp::handler::server::wrapper::Parameters; +use rmcp::model::{CallToolResult, ServerCapabilities, ServerInfo}; +use rmcp::transport::stdio; +use rmcp::{ErrorData, ServerHandler, serve_server, tool, tool_handler, tool_router}; +use tokio::sync::OnceCell; + +use crate::{FabroMcpServerSettings, run_tools}; + +#[derive(Clone)] +pub(crate) struct FabroMcpServer { + settings: Arc, + client: Arc>>, + cwd: PathBuf, + tool_router: ToolRouter, +} + +pub async fn start(settings: FabroMcpServerSettings) -> Result<()> { + let server = FabroMcpServer::new(Arc::new(settings)); + let service = serve_server(server, stdio()).await?; + service.waiting().await?; + Ok(()) +} + +#[tool_handler(router = self.tool_router)] +impl ServerHandler for FabroMcpServer { + fn get_info(&self) -> ServerInfo { + ServerInfo::new(ServerCapabilities::builder().enable_tools().build()) + .with_instructions("Use these tools to create, inspect, control, wait for, and read events from Fabro workflow runs.") + } +} + +#[tool_router(router = tool_router)] +impl FabroMcpServer { + pub(crate) fn new(settings: Arc) -> Self { + let cwd = settings.cwd.clone(); + Self { + settings, + client: Arc::new(OnceCell::new()), + cwd, + tool_router: Self::tool_router(), + } + } + + #[tool( + name = "fabro_run_create", + description = "Create one or more Fabro workflow runs, starting them by default." + )] + async fn fabro_run_create( + &self, + params: Parameters, + ) -> Result { + let params = match run_tools::ValidatedCreateRuns::try_from(params.0) { + Ok(params) => params, + Err(err) => return Ok(run_tools::error_result(err)), + }; + let client = match self.client().await { + Ok(client) => client, + Err(err) => return Ok(run_tools::error_result(err)), + }; + match run_tools::create_runs(client, &self.cwd, &self.settings.config_path, params).await { + Ok(result) => run_tools::success_result(&result, run_tools::create_runs_text(&result)), + Err(err) => Ok(run_tools::error_result(err)), + } + } + + #[tool( + name = "fabro_run_search", + description = "Search Fabro workflow runs by id, workflow, labels, status, archival state, and creation time." + )] + async fn fabro_run_search( + &self, + params: Parameters, + ) -> Result { + let params = match run_tools::ValidatedSearchRuns::try_from(params.0) { + Ok(params) => params, + Err(err) => return Ok(run_tools::error_result(err)), + }; + let client = match self.client().await { + Ok(client) => client, + Err(err) => return Ok(run_tools::error_result(err)), + }; + match run_tools::search_runs(client, params).await { + Ok(result) => run_tools::success_result(&result, run_tools::search_runs_text(&result)), + Err(err) => Ok(run_tools::error_result(err)), + } + } + + #[tool( + name = "fabro_run_interact", + description = "Get, start, message, cancel, archive, unarchive, inspect questions, or answer a Fabro run." + )] + async fn fabro_run_interact( + &self, + params: Parameters, + ) -> Result { + let params = match run_tools::ValidatedInteractRun::try_from(params.0) { + Ok(params) => params, + Err(err) => return Ok(run_tools::error_result(err)), + }; + let client = match self.client().await { + Ok(client) => client, + Err(err) => return Ok(run_tools::error_result(err)), + }; + match run_tools::interact_run(client, params).await { + Ok(result) => run_tools::success_result(&result, run_tools::interact_run_text(&result)), + Err(err) => Ok(run_tools::error_result(err)), + } + } + + #[tool( + name = "fabro_run_gather", + description = "Wait for Fabro runs to reach terminal states, returning current state on timeout." + )] + async fn fabro_run_gather( + &self, + params: Parameters, + ) -> Result { + let params = match run_tools::ValidatedGatherRuns::try_from(params.0) { + Ok(params) => params, + Err(err) => return Ok(run_tools::error_result(err)), + }; + let client = match self.client().await { + Ok(client) => client, + Err(err) => return Ok(run_tools::error_result(err)), + }; + match run_tools::gather_runs(client, params).await { + Ok(result) => run_tools::success_result(&result, run_tools::gather_runs_text(&result)), + Err(err) => Ok(run_tools::error_result(err)), + } + } + + #[tool( + name = "fabro_run_events", + description = "List, inspect, or search stored events for a Fabro workflow run." + )] + async fn fabro_run_events( + &self, + params: Parameters, + ) -> Result { + let params = match run_tools::ValidatedRunEvents::try_from(params.0) { + Ok(params) => params, + Err(err) => return Ok(run_tools::error_result(err)), + }; + let client = match self.client().await { + Ok(client) => client, + Err(err) => return Ok(run_tools::error_result(err)), + }; + match run_tools::run_events(client, params).await { + Ok(result) => run_tools::success_result(&result, run_tools::run_events_text(&result)), + Err(err) => Ok(run_tools::error_result(err)), + } + } + + async fn client(&self) -> Result, run_tools::ToolError> { + self.client + .get_or_try_init(|| async { + (self.settings.client_factory)() + .await + .map(Arc::new) + .map_err(|err| run_tools::ToolError::from_anyhow(&err)) + }) + .await + .map(Arc::clone) + } +} diff --git a/lib/crates/fabro-mcp/src/client.rs b/lib/crates/fabro-mcp/src/client.rs index 1c4e537a6..1775affff 100644 --- a/lib/crates/fabro-mcp/src/client.rs +++ b/lib/crates/fabro-mcp/src/client.rs @@ -22,6 +22,8 @@ enum ClientState { Connecting(Option), /// Handshake complete, ready for tool calls. Ready(Arc>), + /// Connection was explicitly closed. + Closed, } enum PendingTransport { @@ -51,9 +53,15 @@ impl McpClient { .stderr(Stdio::piped()) .kill_on_drop(true); + if config.clear_env { + cmd.env_clear(); + } if !env.is_empty() { cmd.envs(env); } + if let Some(current_dir) = config.current_dir.as_ref() { + cmd.current_dir(current_dir); + } #[cfg(unix)] cmd.process_group(0); @@ -116,6 +124,7 @@ impl McpClient { .take() .ok_or_else(|| anyhow!("client already initializing"))?, ClientState::Ready(_) => return Err(anyhow!("client already initialized")), + ClientState::Closed => return Err(anyhow!("MCP client is shut down")), }; // Drop the lock before the blocking handshake @@ -243,11 +252,38 @@ impl McpClient { Ok(result) } + pub async fn shutdown(self) -> Result<()> { + let service = { + let mut guard = self.state.lock().await; + match std::mem::replace(&mut *guard, ClientState::Closed) { + ClientState::Connecting(_) | ClientState::Closed => None, + ClientState::Ready(service) => Some(service), + } + }; + + if let Some(service) = service { + match Arc::try_unwrap(service) { + Ok(mut service) => { + service + .close_with_timeout(Duration::from_secs(2)) + .await + .context("failed to shut down MCP client")?; + } + Err(service) => { + service.cancellation_token().cancel(); + } + } + } + + Ok(()) + } + async fn service(&self) -> Result>> { let guard = self.state.lock().await; match &*guard { ClientState::Ready(service) => Ok(Arc::clone(service)), ClientState::Connecting(_) => Err(anyhow!("MCP client not initialized")), + ClientState::Closed => Err(anyhow!("MCP client is shut down")), } } } diff --git a/lib/crates/fabro-mcp/tests/stdio_integration.rs b/lib/crates/fabro-mcp/tests/stdio_integration.rs index 8da5c24d6..bd0b2ed73 100644 --- a/lib/crates/fabro-mcp/tests/stdio_integration.rs +++ b/lib/crates/fabro-mcp/tests/stdio_integration.rs @@ -13,6 +13,8 @@ fn test_server_config() -> McpServerSettings { command: vec!["python3".into(), test_server], env: HashMap::new(), }, + current_dir: None, + clear_env: false, startup_timeout_secs: 10, tool_timeout_secs: 30, } @@ -30,6 +32,78 @@ async fn stdio_client_initialize_and_list_tools() { assert_eq!(tools[0].1, "Echo back the message"); } +#[tokio::test] +#[expect( + clippy::disallowed_methods, + reason = "stdio integration test stages a local process cwd and inherits PATH for python3 lookup" +)] +async fn stdio_client_uses_configured_cwd_and_exact_env() { + let test_server = format!("{}/tests/test_mcp_server.py", env!("CARGO_MANIFEST_DIR")); + let temp_dir = std::env::temp_dir().join(format!( + "fabro-mcp-stdio-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + std::fs::create_dir(&temp_dir).unwrap(); + let canonical_temp_dir = std::fs::canonicalize(&temp_dir).unwrap(); + let mut env = HashMap::new(); + env.insert( + "PATH".to_string(), + std::env::var("PATH").expect("PATH should be set for python3 lookup"), + ); + env.insert("FABRO_MCP_TEST_SENTINEL".to_string(), "fixture".to_string()); + let config = McpServerSettings { + name: "test-echo".into(), + transport: McpTransport::Stdio { + command: vec!["python3".into(), test_server], + env, + }, + current_dir: Some(canonical_temp_dir.clone()), + clear_env: true, + startup_timeout_secs: 10, + tool_timeout_secs: 30, + }; + let client = McpClient::new(&config).unwrap(); + client.initialize(config.startup_timeout()).await.unwrap(); + + let cwd = client + .call_tool( + "echo", + serde_json::json!({"message": "__cwd__"}), + Duration::from_secs(5), + ) + .await + .unwrap(); + assert_eq!( + call_result_to_string(&cwd).unwrap(), + canonical_temp_dir.display().to_string() + ); + let sentinel = client + .call_tool( + "echo", + serde_json::json!({"message": "__env:FABRO_MCP_TEST_SENTINEL__"}), + Duration::from_secs(5), + ) + .await + .unwrap(); + assert_eq!(call_result_to_string(&sentinel).unwrap(), "fixture"); + let home = client + .call_tool( + "echo", + serde_json::json!({"message": "__env:HOME__"}), + Duration::from_secs(5), + ) + .await + .unwrap(); + assert_eq!(call_result_to_string(&home).unwrap(), ""); + + client.shutdown().await.unwrap(); + std::fs::remove_dir(&temp_dir).unwrap(); +} + #[tokio::test] async fn stdio_client_call_tool_echo() { let config = test_server_config(); diff --git a/lib/crates/fabro-mcp/tests/test_mcp_server.py b/lib/crates/fabro-mcp/tests/test_mcp_server.py index 619d0f365..63f95b689 100644 --- a/lib/crates/fabro-mcp/tests/test_mcp_server.py +++ b/lib/crates/fabro-mcp/tests/test_mcp_server.py @@ -5,6 +5,7 @@ Speaks JSON-RPC 2.0 over stdin/stdout per the MCP specification. Exposes a single tool: echo(message) -> message. """ import json +import os import sys SERVER_INFO = { @@ -53,6 +54,11 @@ def handle_request(req): arguments = params.get("arguments", {}) if tool_name == "echo": msg = arguments.get("message", "") + if msg == "__cwd__": + msg = os.getcwd() + elif msg.startswith("__env:") and msg.endswith("__"): + key = msg[len("__env:") : -len("__")] + msg = os.environ.get(key, "") return { "jsonrpc": "2.0", "id": req_id, diff --git a/lib/crates/fabro-server/Cargo.toml b/lib/crates/fabro-server/Cargo.toml index e78933660..304ae45ec 100644 --- a/lib/crates/fabro-server/Cargo.toml +++ b/lib/crates/fabro-server/Cargo.toml @@ -35,6 +35,7 @@ fabro-sandbox = { path = "../fabro-sandbox", features = ["daytona", "docker"] } fabro-github = { path = "../fabro-github" } fabro-agent = { path = "../fabro-agent" } fabro-llm = { path = "../fabro-llm" } +fabro-manifest = { path = "../fabro-manifest" } fabro-model = { path = "../fabro-model" } fabro-proc = { path = "../fabro-proc" } fabro-types = { path = "../fabro-types" } diff --git a/lib/crates/fabro-server/src/run_manifest.rs b/lib/crates/fabro-server/src/run_manifest.rs index 0bced72f6..2178196ee 100644 --- a/lib/crates/fabro-server/src/run_manifest.rs +++ b/lib/crates/fabro-server/src/run_manifest.rs @@ -9,8 +9,7 @@ use fabro_api::types; use fabro_auth::auth_issue_message; use fabro_config::run::parse_run_layer_from_settings_toml; use fabro_config::{ - CliLayer, CliOutputLayer, DaytonaDockerfileLayer, DockerSandboxLayer, ReplaceMap, - RunExecutionLayer, RunLayer, RunModelLayer, RunSandboxLayer, WorkflowSettingsBuilder, + CliLayer, CliOutputLayer, DaytonaDockerfileLayer, RunLayer, WorkflowSettingsBuilder, parse_input_overrides, }; use fabro_graphviz::graph::{Graph, is_llm_handler_type}; @@ -28,8 +27,8 @@ use fabro_static::EnvVars; use fabro_types::settings::cli::OutputVerbosity; use fabro_types::settings::interp::InterpString; use fabro_types::settings::run::{ - ApprovalMode, DaytonaNetworkLayer, DaytonaSettings, DockerSettings, DockerfileSource, RunGoal, - RunMode, RunNamespace, + DaytonaNetworkLayer, DaytonaSettings, DockerSettings, DockerfileSource, RunGoal, RunMode, + RunNamespace, }; use fabro_types::{RunId, WorkflowSettings}; use fabro_util::check_report::{CheckDetail, CheckReport, CheckResult, CheckSection, CheckStatus}; @@ -316,46 +315,16 @@ fn manifest_args_overrides( return Ok(ManifestSettingsOverrides::default()); }; - let model = (args.model.is_some() || args.provider.is_some()).then(|| RunModelLayer { - provider: args.provider.as_deref().map(InterpString::parse), - name: args.model.as_deref().map(InterpString::parse), - fallbacks: Vec::new(), - }); - let sandbox = - (args.sandbox.is_some() || args.preserve_sandbox.is_some() || args.docker_image.is_some()) - .then(|| RunSandboxLayer { - provider: args.sandbox.clone(), - preserve: args.preserve_sandbox, - docker: args.docker_image.as_ref().map(|image| DockerSandboxLayer { - image: Some(image.clone()), - ..DockerSandboxLayer::default() - }), - ..RunSandboxLayer::default() - }); - - let execution_has_any = args.dry_run.is_some() || args.auto_approve.is_some(); - let execution = execution_has_any.then(|| RunExecutionLayer { - mode: args - .dry_run - .map(|d| if d { RunMode::DryRun } else { RunMode::Normal }), - approval: args.auto_approve.map(|a| { - if a { - ApprovalMode::Auto - } else { - ApprovalMode::Prompt - } - }), - }); - - let run_has_any = - model.is_some() || sandbox.is_some() || execution.is_some() || !args.label.is_empty(); - - let run = run_has_any.then(|| RunLayer { - model, - sandbox, - execution, - metadata: ReplaceMap::from(parse_labels(&args.label)), - ..RunLayer::default() + let run = fabro_manifest::build_sparse_run_overrides(fabro_manifest::RunOverrideInput { + goal: None, + model: args.model.as_deref(), + provider: args.provider.as_deref(), + sandbox: args.sandbox.as_deref(), + docker_image: args.docker_image.as_deref(), + preserve_sandbox: args.preserve_sandbox, + dry_run: args.dry_run, + auto_approve: args.auto_approve, + labels: parse_labels(&args.label), }); // Verbose is a CLI output concern in v2; route it through cli.output.verbosity. diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 386f5021c..1cefd3507 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -2322,6 +2322,15 @@ async fn load_pending_control( .and_then(|summary| summary.lifecycle.pending_control)) } +async fn durable_run_status(state: &AppState, run_id: RunId) -> anyhow::Result> { + Ok(state + .store + .runs() + .find(&run_id) + .await? + .map(|summary| summary.lifecycle.status)) +} + fn fail_managed_run(state: &Arc, run_id: RunId, reason: FailureReason, message: String) { let mut runs = state.runs.lock().expect("runs lock poisoned"); if let Some(managed_run) = runs.get_mut(&run_id) { diff --git a/lib/crates/fabro-server/src/server/handler/lifecycle.rs b/lib/crates/fabro-server/src/server/handler/lifecycle.rs index 469f74bc1..80e5d2918 100644 --- a/lib/crates/fabro-server/src/server/handler/lifecycle.rs +++ b/lib/crates/fabro-server/src/server/handler/lifecycle.rs @@ -5,7 +5,7 @@ use super::super::{ Principal, RequiredUser, Response, RewindRequest, RewindResponse, Router, RunAnswerTransport, RunControlAction, RunExecutionMode, RunId, RunStatus, StartRunRequest, State, StatusCode, Storage, TimelineEntryResponse, WORKER_CANCEL_GRACE, WorkflowError, append_control_request, - get, load_pending_control, managed_run, operations, parse_run_id_path, + durable_run_status, get, load_pending_control, managed_run, operations, parse_run_id_path, persist_cancelled_run_status, post, reject_if_archived, sleep, update_live_run_from_event, workflow_event, }; @@ -171,7 +171,7 @@ async fn cancel_run( .into_response(); } }; - let (persist_cancelled_status, answer_transport, cancel_token, cancel_tx, worker_pid) = { + let cancel_target = { let mut runs = state.runs.lock().expect("runs lock poisoned"); match runs.get_mut(&id) { Some(managed_run) => match managed_run.status { @@ -192,7 +192,7 @@ async fn cancel_run( reason: FailureReason::Cancelled, }; } - ( + Some(( persist_cancelled_status, managed_run.answer_transport.clone(), managed_run.cancel_token.clone(), @@ -200,16 +200,21 @@ async fn cancel_run( .then(|| managed_run.cancel_tx.take()) .flatten(), managed_run.worker_pid, - ) + )) } _ => { return ApiError::new(StatusCode::CONFLICT, "Run is not cancellable.") .into_response(); } }, - None => return ApiError::not_found("Run not found.").into_response(), + None => None, } }; + let Some((persist_cancelled_status, answer_transport, cancel_token, cancel_tx, worker_pid)) = + cancel_target + else { + return unmanaged_cancel_response(state.as_ref(), id).await; + }; if pending_control != Some(RunControlAction::Cancel) { if let Err(err) = append_control_request( @@ -256,6 +261,23 @@ async fn cancel_run( run_response(state.as_ref(), id, StatusCode::OK).await } +async fn unmanaged_cancel_response(state: &AppState, id: RunId) -> Response { + match durable_run_status(state, id).await { + Ok(Some(status)) if status.is_terminal() => ApiError::new( + StatusCode::CONFLICT, + "Run is already terminal and cannot be cancelled.", + ) + .into_response(), + Ok(Some(_)) => { + ApiError::new(StatusCode::CONFLICT, "Run is not cancellable.").into_response() + } + Ok(None) => ApiError::not_found("Run not found.").into_response(), + Err(err) => { + ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()).into_response() + } + } +} + /// How `pause_run` should enact the transition, chosen from the current run /// status. enum PauseMode { diff --git a/lib/crates/fabro-server/src/server/handler/steer.rs b/lib/crates/fabro-server/src/server/handler/steer.rs index 98a4b319a..069f37a4c 100644 --- a/lib/crates/fabro-server/src/server/handler/steer.rs +++ b/lib/crates/fabro-server/src/server/handler/steer.rs @@ -9,7 +9,9 @@ use fabro_api::types::SteerRunRequest; use fabro_types::Principal; use fabro_workflow::run_status::RunStatus; -use super::super::{AnswerTransportError, AppState, parse_run_id_path, reject_if_archived}; +use super::super::{ + AnswerTransportError, AppState, durable_run_status, parse_run_id_path, reject_if_archived, +}; use crate::error::ApiError; use crate::principal_middleware::RequiredUser; @@ -77,75 +79,72 @@ async fn control_run( // Status + steerability gate. Take the answer_transport snapshot under // the same lock so we can hand it off without further state races. - let answer_transport = { + let managed_answer_transport = { let runs = state.runs.lock().expect("runs lock poisoned"); - let Some(managed_run) = runs.get(&id) else { - return ApiError::not_found("Run not found.").into_response(); - }; - match managed_run.status { - RunStatus::Blocked { .. } => { - return ApiError::with_code( - StatusCode::CONFLICT, - "Run is blocked on a question; use the interview-answer endpoint instead.", - "use_answer_endpoint", - ) - .into_response(); + match runs.get(&id) { + Some(managed_run) => { + match managed_run.status { + RunStatus::Blocked { .. } => { + return ApiError::with_code( + StatusCode::CONFLICT, + "Run is blocked on a question; use the interview-answer endpoint \ + instead.", + "use_answer_endpoint", + ) + .into_response(); + } + RunStatus::Submitted + | RunStatus::Queued + | RunStatus::Starting + | RunStatus::Paused { .. } => { + return ApiError::with_code( + StatusCode::CONFLICT, + "Run is not currently running.", + "run_not_steerable", + ) + .into_response(); + } + RunStatus::Failed { .. } + | RunStatus::Succeeded { .. } + | RunStatus::Removing + | RunStatus::Dead => { + return terminal_control_response(&control); + } + RunStatus::Running => {} + } + // Steerability predicate. Best-effort, target-oriented: + // - If at least one API-mode session is active → forward. + // - Else if no agent stages are active at all → forward (worker hub buffers + // for the next session). + // - Else (active agents exist but all are non-steerable) → 409. + if managed_run.active_api_stages.is_empty() + && !managed_run.active_non_steerable_agent_stages.is_empty() + { + return ApiError::with_code( + StatusCode::CONFLICT, + "All currently running agent stages use a non-steerable backend.", + "agent_not_steerable", + ) + .into_response(); + } + if managed_run.active_api_stages.is_empty() && control.requires_active_api_session() + { + return ApiError::with_code( + StatusCode::CONFLICT, + "Run has no active API-mode agent session.", + "no_active_api_session", + ) + .into_response(); + } + Some(managed_run.answer_transport.clone()) } - RunStatus::Submitted - | RunStatus::Queued - | RunStatus::Starting - | RunStatus::Paused { .. } => { - return ApiError::with_code( - StatusCode::CONFLICT, - "Run is not currently running.", - "run_not_steerable", - ) - .into_response(); - } - RunStatus::Failed { .. } - | RunStatus::Succeeded { .. } - | RunStatus::Removing - | RunStatus::Dead => { - let code = if matches!(&control, RunControlRequest::Interrupt) { - "run_not_interruptible" - } else { - "run_not_steerable" - }; - return ApiError::with_code( - StatusCode::CONFLICT, - "Run is no longer steerable.", - code, - ) - .into_response(); - } - RunStatus::Running => {} + None => None, } - // Steerability predicate. Best-effort, target-oriented: - // - If at least one API-mode session is active → forward. - // - Else if no agent stages are active at all → forward (worker hub buffers - // for the next session). - // - Else (active agents exist but all are non-steerable) → 409. - if managed_run.active_api_stages.is_empty() - && !managed_run.active_non_steerable_agent_stages.is_empty() - { - return ApiError::with_code( - StatusCode::CONFLICT, - "All currently running agent stages use a non-steerable backend.", - "agent_not_steerable", - ) - .into_response(); - } - if managed_run.active_api_stages.is_empty() && control.requires_active_api_session() { - return ApiError::with_code( - StatusCode::CONFLICT, - "Run has no active API-mode agent session.", - "no_active_api_session", - ) - .into_response(); - } - managed_run.answer_transport.clone() }; + let Some(answer_transport) = managed_answer_transport else { + return unmanaged_control_response(state.as_ref(), id, &control).await; + }; let Some(answer_transport) = answer_transport else { return ApiError::with_code( StatusCode::SERVICE_UNAVAILABLE, @@ -180,3 +179,32 @@ async fn control_run( .into_response(), } } + +fn terminal_control_response(control: &RunControlRequest) -> Response { + let code = if matches!(control, RunControlRequest::Interrupt) { + "run_not_interruptible" + } else { + "run_not_steerable" + }; + ApiError::with_code(StatusCode::CONFLICT, "Run is no longer steerable.", code).into_response() +} + +async fn unmanaged_control_response( + state: &AppState, + id: fabro_types::RunId, + control: &RunControlRequest, +) -> Response { + match durable_run_status(state, id).await { + Ok(Some(status)) if status.is_terminal() => terminal_control_response(control), + Ok(Some(_)) => ApiError::with_code( + StatusCode::SERVICE_UNAVAILABLE, + "Run has no live worker control channel.", + "worker_control_unavailable", + ) + .into_response(), + Ok(None) => ApiError::not_found("Run not found.").into_response(), + Err(err) => { + ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()).into_response() + } + } +} diff --git a/lib/crates/fabro-server/src/server/tests.rs b/lib/crates/fabro-server/src/server/tests.rs index e67707ee9..d4dc471bd 100644 --- a/lib/crates/fabro-server/src/server/tests.rs +++ b/lib/crates/fabro-server/src/server/tests.rs @@ -6877,6 +6877,40 @@ async fn cancel_nonexistent_run_returns_not_found() { assert_status!(response, StatusCode::NOT_FOUND).await; } +#[tokio::test] +async fn cancel_terminal_durable_run_returns_conflict() { + let state = test_app_state(); + let app = crate::test_support::build_test_router(Arc::clone(&state)); + let run_id = fixtures::RUN_1; + create_durable_run_with_events(&state, run_id, &[ + workflow_event::Event::WorkflowRunCompleted { + duration_ms: 1000, + artifact_count: 0, + status: "succeeded".to_string(), + reason: SuccessReason::Completed, + total_usd_micros: None, + final_git_commit_sha: None, + final_patch: None, + diff_summary: None, + billing: None, + }, + ]) + .await; + + let req = Request::builder() + .method("POST") + .uri(api(&format!("/runs/{run_id}/cancel"))) + .body(Body::empty()) + .unwrap(); + + let response = app.oneshot(req).await.unwrap(); + let body = response_json!(response, StatusCode::CONFLICT).await; + assert_eq!( + body["errors"][0]["detail"], + "Run is already terminal and cannot be cancelled." + ); +} + #[tokio::test] async fn steer_nonexistent_run_returns_not_found() { let app = test_app_with(); @@ -6893,6 +6927,39 @@ async fn steer_nonexistent_run_returns_not_found() { assert_status!(response, StatusCode::NOT_FOUND).await; } +#[tokio::test] +async fn steer_terminal_durable_run_returns_run_not_steerable() { + let state = test_app_state(); + let app = crate::test_support::build_test_router(Arc::clone(&state)); + let run_id = fixtures::RUN_1; + create_durable_run_with_events(&state, run_id, &[ + workflow_event::Event::WorkflowRunCompleted { + duration_ms: 1000, + artifact_count: 0, + status: "succeeded".to_string(), + reason: SuccessReason::Completed, + total_usd_micros: None, + final_git_commit_sha: None, + final_patch: None, + diff_summary: None, + billing: None, + }, + ]) + .await; + + let req = Request::builder() + .method("POST") + .uri(api(&format!("/runs/{run_id}/steer"))) + .header("content-type", "application/json") + .body(Body::from(r#"{"text":"try again"}"#)) + .unwrap(); + + let response = app.oneshot(req).await.unwrap(); + let body = response_json!(response, StatusCode::CONFLICT).await; + assert_eq!(body["errors"][0]["code"], "run_not_steerable"); + assert_eq!(body["errors"][0]["detail"], "Run is no longer steerable."); +} + #[tokio::test] async fn steer_empty_text_returns_bad_request() { let state = test_app_state(); diff --git a/lib/crates/fabro-test/src/lib.rs b/lib/crates/fabro-test/src/lib.rs index 77841bce7..8d90c1a3f 100644 --- a/lib/crates/fabro-test/src/lib.rs +++ b/lib/crates/fabro-test/src/lib.rs @@ -152,6 +152,43 @@ pub fn apply_test_isolation(cmd: &mut std::process::Command, home_dir: &Path) { apply_test_isolation_with_lookup(cmd, home_dir, |name| std::env::var_os(name)); } +#[must_use] +pub fn isolated_env(home_dir: &Path) -> HashMap { + let mut env = HashMap::new(); + if let Some(coverage) = + std::env::var_os(EnvVars::LLVM_PROFILE_FILE).and_then(|value| value.into_string().ok()) + { + env.insert(EnvVars::LLVM_PROFILE_FILE.to_string(), coverage); + } + if let Some(path) = std::env::var_os(EnvVars::PATH).and_then(|value| value.into_string().ok()) { + env.insert(EnvVars::PATH.to_string(), path); + } + env.insert(EnvVars::NO_COLOR.to_string(), "1".to_string()); + env.insert(EnvVars::HOME.to_string(), home_dir.display().to_string()); + env.insert( + EnvVars::FABRO_NO_UPGRADE_CHECK.to_string(), + "true".to_string(), + ); + env.insert( + EnvVars::FABRO_HTTP_PROXY_POLICY.to_string(), + "disabled".to_string(), + ); + env.insert(EnvVars::FABRO_TELEMETRY.to_string(), "off".to_string()); + env.insert( + EnvVars::FABRO_SUPPRESS_OPEN_BROWSER.to_string(), + "1".to_string(), + ); + env.insert( + EnvVars::FABRO_SERVER_MAX_CONCURRENT_RUNS.to_string(), + "64".to_string(), + ); + env.insert( + EnvVars::FABRO_TEST_IN_MEMORY_STORE.to_string(), + "1".to_string(), + ); + env +} + fn apply_test_isolation_with_lookup( cmd: &mut std::process::Command, home_dir: &Path, diff --git a/lib/crates/fabro-types/src/lib.rs b/lib/crates/fabro-types/src/lib.rs index 8931662dd..e4da87ec6 100644 --- a/lib/crates/fabro-types/src/lib.rs +++ b/lib/crates/fabro-types/src/lib.rs @@ -104,5 +104,6 @@ pub use stage_id::{InvalidStageVisit, ParallelBranchId, StageId}; pub use start::StartRecord; pub use status::{ BlockedReason, FailureReason, InvalidTransition, ParseFailureReasonError, - ParseSuccessReasonError, RunControlAction, RunStatus, SuccessReason, TerminalStatus, + ParseSuccessReasonError, RunControlAction, RunStatus, RunStatusKind, SuccessReason, + TerminalStatus, }; diff --git a/lib/crates/fabro-types/src/settings/run.rs b/lib/crates/fabro-types/src/settings/run.rs index 9148d986c..18ae07b4c 100644 --- a/lib/crates/fabro-types/src/settings/run.rs +++ b/lib/crates/fabro-types/src/settings/run.rs @@ -7,6 +7,7 @@ //! behavior, and artifact collection. use std::collections::HashMap; +use std::path::PathBuf; use std::time::Duration as StdDuration; use serde::ser::SerializeStruct; @@ -330,6 +331,10 @@ pub struct RunAgentSettings { pub struct McpServerSettings { pub name: String, pub transport: McpTransport, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub current_dir: Option, + #[serde(default, skip_serializing_if = "is_false")] + pub clear_env: bool, pub startup_timeout_secs: u64, pub tool_timeout_secs: u64, } @@ -342,6 +347,8 @@ impl Default for McpServerSettings { command: Vec::new(), env: HashMap::new(), }, + current_dir: None, + clear_env: false, startup_timeout_secs: 10, tool_timeout_secs: 60, } @@ -378,6 +385,14 @@ pub enum McpTransport { }, } +#[expect( + clippy::trivially_copy_pass_by_ref, + reason = "serde skip_serializing_if helpers receive borrowed field values" +)] +fn is_false(value: &bool) -> bool { + !*value +} + #[derive(Debug, Clone, Copy, Deserialize, PartialEq, Eq, Default, Serialize)] #[serde(rename_all = "snake_case")] pub enum TlsMode { diff --git a/lib/crates/fabro-types/src/status.rs b/lib/crates/fabro-types/src/status.rs index d59f89b65..cf2d7d1bb 100644 --- a/lib/crates/fabro-types/src/status.rs +++ b/lib/crates/fabro-types/src/status.rs @@ -2,6 +2,35 @@ use std::fmt; use std::str::FromStr; use serde::{Deserialize, Serialize}; +use strum::{Display, EnumString, IntoStaticStr}; + +#[derive( + Debug, + Clone, + Copy, + PartialEq, + Eq, + Hash, + Serialize, + Deserialize, + Display, + EnumString, + IntoStaticStr, +)] +#[serde(rename_all = "snake_case")] +#[strum(serialize_all = "snake_case")] +pub enum RunStatusKind { + Submitted, + Queued, + Starting, + Running, + Blocked, + Paused, + Removing, + Succeeded, + Failed, + Dead, +} #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(tag = "kind", rename_all = "snake_case")] @@ -19,6 +48,10 @@ pub enum RunStatus { } impl RunStatus { + pub fn kind(self) -> RunStatusKind { + self.into() + } + /// Whether the run has reached a terminal outcome and stops poll loops, /// finalization, and similar "done" handling. pub fn is_terminal(self) -> bool { @@ -138,6 +171,23 @@ impl RunStatus { } } +impl From for RunStatusKind { + fn from(status: RunStatus) -> Self { + match status { + RunStatus::Submitted => Self::Submitted, + RunStatus::Queued => Self::Queued, + RunStatus::Starting => Self::Starting, + RunStatus::Running => Self::Running, + RunStatus::Blocked { .. } => Self::Blocked, + RunStatus::Paused { .. } => Self::Paused, + RunStatus::Removing => Self::Removing, + RunStatus::Succeeded { .. } => Self::Succeeded, + RunStatus::Failed { .. } => Self::Failed, + RunStatus::Dead => Self::Dead, + } + } +} + impl fmt::Display for RunStatus { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { diff --git a/lib/crates/fabro-workflow/src/operations/start.rs b/lib/crates/fabro-workflow/src/operations/start.rs index a0d65ecf3..0cee89dbc 100644 --- a/lib/crates/fabro-workflow/src/operations/start.rs +++ b/lib/crates/fabro-workflow/src/operations/start.rs @@ -547,6 +547,8 @@ fn runtime_mcp_server(settings: &ResolvedMcpServerSettings) -> McpServerSettings env: env.clone(), }, }, + current_dir: None, + clear_env: false, startup_timeout_secs: settings.startup_timeout_secs, tool_timeout_secs: settings.tool_timeout_secs, } diff --git a/lib/crates/fabro-workflow/tests/it/daytona_integration.rs b/lib/crates/fabro-workflow/tests/it/daytona_integration.rs index 289613a27..c74725414 100644 --- a/lib/crates/fabro-workflow/tests/it/daytona_integration.rs +++ b/lib/crates/fabro-workflow/tests/it/daytona_integration.rs @@ -2149,6 +2149,8 @@ async fn daytona_playwright_mcp_sandbox_transport() { port: mcp_port, env: std::collections::HashMap::new(), }, + current_dir: None, + clear_env: false, startup_timeout_secs: 30, tool_timeout_secs: 120, }; @@ -2205,6 +2207,8 @@ async fn daytona_playwright_mcp_sandbox_transport() { fabro_mcp::config::McpServerSettings { name: mcp_config.name.clone(), transport: fabro_mcp::config::McpTransport::Http { url, headers }, + current_dir: mcp_config.current_dir.clone(), + clear_env: mcp_config.clear_env, startup_timeout_secs: mcp_config.startup_timeout_secs, tool_timeout_secs: mcp_config.tool_timeout_secs, }