diff --git a/.env.example b/.env.example index 1093d69ee..bdb08589e 100644 --- a/.env.example +++ b/.env.example @@ -1,6 +1,7 @@ ANTHROPIC_API_KEY= BRAVE_SEARCH_API_KEY= DAYTONA_API_KEY= +DEEPSEEK_API_KEY= FIREWORKS_API_KEY= GEMINI_API_KEY= INCEPTION_API_KEY= diff --git a/Cargo.lock b/Cargo.lock index a485c5cc4..57596ad82 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1854,7 +1854,7 @@ checksum = "d7a1e2f27636f116493b8b860f5546edb47c8d8f8ea73e1d2a20be88e28d1fea" [[package]] name = "daytona-api-client" version = "0.1.0" -source = "git+https://github.com/brynary/daytona-sdk-rust?rev=fc58e22f7f25183df6264276ee186bbc32635738#fc58e22f7f25183df6264276ee186bbc32635738" +source = "git+https://github.com/brynary/daytona-sdk-rust?rev=73c9c458dd1a1d096afd3521175637af82afd8d8#73c9c458dd1a1d096afd3521175637af82afd8d8" dependencies = [ "reqwest 0.13.2", "reqwest-middleware", @@ -1868,7 +1868,7 @@ dependencies = [ [[package]] name = "daytona-sdk" version = "0.1.0" -source = "git+https://github.com/brynary/daytona-sdk-rust?rev=fc58e22f7f25183df6264276ee186bbc32635738#fc58e22f7f25183df6264276ee186bbc32635738" +source = "git+https://github.com/brynary/daytona-sdk-rust?rev=73c9c458dd1a1d096afd3521175637af82afd8d8#73c9c458dd1a1d096afd3521175637af82afd8d8" dependencies = [ "daytona-api-client", "daytona-toolbox-client", @@ -1888,7 +1888,7 @@ dependencies = [ [[package]] name = "daytona-toolbox-client" version = "0.1.0" -source = "git+https://github.com/brynary/daytona-sdk-rust?rev=fc58e22f7f25183df6264276ee186bbc32635738#fc58e22f7f25183df6264276ee186bbc32635738" +source = "git+https://github.com/brynary/daytona-sdk-rust?rev=73c9c458dd1a1d096afd3521175637af82afd8d8#73c9c458dd1a1d096afd3521175637af82afd8d8" dependencies = [ "reqwest 0.13.2", "reqwest-middleware", @@ -2239,7 +2239,7 @@ dependencies = [ [[package]] name = "fabro-acp" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "agent-client-protocol", "agent-client-protocol-tokio", @@ -2258,7 +2258,7 @@ dependencies = [ [[package]] name = "fabro-agent" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -2304,7 +2304,7 @@ dependencies = [ [[package]] name = "fabro-api" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "chrono", "fabro-automation", @@ -2327,7 +2327,7 @@ dependencies = [ [[package]] name = "fabro-auth" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -2352,7 +2352,7 @@ dependencies = [ [[package]] name = "fabro-automation" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "chrono", @@ -2371,11 +2371,11 @@ dependencies = [ [[package]] name = "fabro-build-support" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" [[package]] name = "fabro-checkpoint" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "chrono", "fabro-config", @@ -2391,7 +2391,7 @@ dependencies = [ [[package]] name = "fabro-cli" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "assert_cmd", @@ -2493,7 +2493,7 @@ dependencies = [ [[package]] name = "fabro-client" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "bytes", @@ -2522,7 +2522,7 @@ dependencies = [ [[package]] name = "fabro-config" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "chrono", @@ -2552,7 +2552,7 @@ dependencies = [ [[package]] name = "fabro-core" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "async-trait", "fabro-types", @@ -2567,7 +2567,7 @@ dependencies = [ [[package]] name = "fabro-db" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "chrono", @@ -2579,7 +2579,7 @@ dependencies = [ [[package]] name = "fabro-dev" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "assert_cmd", @@ -2598,7 +2598,7 @@ dependencies = [ [[package]] name = "fabro-dump" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "bytes", @@ -2612,7 +2612,7 @@ dependencies = [ [[package]] name = "fabro-environment" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "chrono", @@ -2634,7 +2634,7 @@ dependencies = [ [[package]] name = "fabro-github" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "base64", @@ -2656,7 +2656,7 @@ dependencies = [ [[package]] name = "fabro-graphviz" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "fabro-types", @@ -2671,7 +2671,7 @@ dependencies = [ [[package]] name = "fabro-hooks" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "async-trait", "fabro-agent", @@ -2694,7 +2694,7 @@ dependencies = [ [[package]] name = "fabro-http" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "fabro-static", "http 1.4.0", @@ -2704,7 +2704,7 @@ dependencies = [ [[package]] name = "fabro-install" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "base64", @@ -2723,7 +2723,7 @@ dependencies = [ [[package]] name = "fabro-interview" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "async-trait", "dialoguer", @@ -2738,7 +2738,7 @@ dependencies = [ [[package]] name = "fabro-llm" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -2766,6 +2766,7 @@ dependencies = [ "rand 0.9.4", "serde", "serde_json", + "sha2 0.10.9", "strum 0.28.0", "thiserror 2.0.18", "tokio", @@ -2779,7 +2780,7 @@ dependencies = [ [[package]] name = "fabro-macros" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "clap", "fabro-options-metadata", @@ -2790,7 +2791,7 @@ dependencies = [ [[package]] name = "fabro-manifest" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "fabro-api", @@ -2808,7 +2809,7 @@ dependencies = [ [[package]] name = "fabro-mcp" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "axum", @@ -2828,7 +2829,7 @@ dependencies = [ [[package]] name = "fabro-mcp-server" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "chrono", @@ -2851,11 +2852,12 @@ dependencies = [ "tempfile", "tokio", "toml 0.8.23", + "tracing", ] [[package]] name = "fabro-mcp-store" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "chrono", "fabro-db", @@ -2873,7 +2875,7 @@ dependencies = [ [[package]] name = "fabro-model" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "fabro-static", "http 1.4.0", @@ -2889,7 +2891,7 @@ dependencies = [ [[package]] name = "fabro-oauth" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "axum", @@ -2911,7 +2913,7 @@ dependencies = [ [[package]] name = "fabro-options-metadata" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "serde", "serde_json", @@ -2919,7 +2921,7 @@ dependencies = [ [[package]] name = "fabro-proc" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "cc", "libc", @@ -2928,7 +2930,7 @@ dependencies = [ [[package]] name = "fabro-redact" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "aho-corasick", "ref-cast", @@ -2944,7 +2946,7 @@ dependencies = [ [[package]] name = "fabro-sandbox" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -2988,7 +2990,7 @@ dependencies = [ [[package]] name = "fabro-server" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -3081,7 +3083,7 @@ dependencies = [ [[package]] name = "fabro-slack" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "fabro-http", "fabro-interview", @@ -3103,18 +3105,18 @@ dependencies = [ [[package]] name = "fabro-spa" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "rust-embed", ] [[package]] name = "fabro-static" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" [[package]] name = "fabro-store" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "async-trait", "bytes", @@ -3144,7 +3146,7 @@ dependencies = [ [[package]] name = "fabro-telemetry" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "base64", @@ -3170,7 +3172,7 @@ dependencies = [ [[package]] name = "fabro-template" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "fabro-types", @@ -3184,7 +3186,7 @@ dependencies = [ [[package]] name = "fabro-test" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "assert_cmd", @@ -3209,7 +3211,7 @@ dependencies = [ [[package]] name = "fabro-tool" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -3230,7 +3232,7 @@ dependencies = [ [[package]] name = "fabro-tracker" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "async-trait", @@ -3244,7 +3246,7 @@ dependencies = [ [[package]] name = "fabro-types" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "chrono", "clap", @@ -3267,7 +3269,7 @@ dependencies = [ [[package]] name = "fabro-util" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "console 0.15.11", @@ -3290,7 +3292,7 @@ dependencies = [ [[package]] name = "fabro-validate" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "fabro-acp", "fabro-graphviz", @@ -3303,7 +3305,7 @@ dependencies = [ [[package]] name = "fabro-variable" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "chrono", @@ -3320,7 +3322,7 @@ dependencies = [ [[package]] name = "fabro-vault" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "chrono", @@ -3339,7 +3341,7 @@ dependencies = [ [[package]] name = "fabro-workflow" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "assert_cmd", @@ -8504,7 +8506,7 @@ dependencies = [ [[package]] name = "twin-github" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "axum", "base64", @@ -8523,7 +8525,7 @@ dependencies = [ [[package]] name = "twin-openai" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" dependencies = [ "anyhow", "async-stream", diff --git a/Cargo.toml b/Cargo.toml index 78ab08a99..d96400032 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,7 +11,7 @@ resolver = "2" [workspace.package] edition = "2021" -version = "0.311.0-nightly.0" +version = "0.312.0-nightly.0" license = "MIT" [workspace.dependencies] @@ -96,8 +96,8 @@ twin-openai = { path = "test/twin/openai" } twin-github = { path = "test/twin/github" } tokio-tungstenite = { version = "0.26", features = ["rustls-tls-webpki-roots"] } futures-util = "0.3" -daytona-sdk = { git = "https://github.com/brynary/daytona-sdk-rust", rev = "fc58e22f7f25183df6264276ee186bbc32635738", package = "daytona-sdk" } -daytona-api-client = { git = "https://github.com/brynary/daytona-sdk-rust", rev = "fc58e22f7f25183df6264276ee186bbc32635738", package = "daytona-api-client" } +daytona-sdk = { git = "https://github.com/brynary/daytona-sdk-rust", rev = "73c9c458dd1a1d096afd3521175637af82afd8d8", package = "daytona-sdk" } +daytona-api-client = { git = "https://github.com/brynary/daytona-sdk-rust", rev = "73c9c458dd1a1d096afd3521175637af82afd8d8", package = "daytona-api-client" } sentry = { version = "0.35", default-features = false, features = ["backtrace", "contexts", "ureq", "rustls"] } fork = "0.2" exec = "0.3" diff --git a/apps/fabro-web/app/components/stage-renderers/helpers.test.ts b/apps/fabro-web/app/components/stage-renderers/helpers.test.ts index 33c661081..a7be45a24 100644 --- a/apps/fabro-web/app/components/stage-renderers/helpers.test.ts +++ b/apps/fabro-web/app/components/stage-renderers/helpers.test.ts @@ -195,15 +195,11 @@ describe("parseParallelOverview", () => { const overview = parseParallelOverview(events); expect(overview).toEqual({ branchCount: 3, - successCount: 2, - failureCount: 1, - durationMs: 12000, results: [ { id: "branch-a", index: null, itemLabel: null, status: "succeeded" }, { id: "branch-b", index: null, itemLabel: null, status: "succeeded" }, { id: "branch-c", index: null, itemLabel: null, status: "failed" }, ], - isComplete: true, }); }); @@ -249,7 +245,6 @@ describe("parseParallelOverview", () => { }), ]; const overview = parseParallelOverview(events); - expect(overview.isComplete).toBe(false); expect(overview.branchCount).toBe(4); expect(overview.results).toEqual([]); }); diff --git a/apps/fabro-web/app/components/stage-renderers/helpers.ts b/apps/fabro-web/app/components/stage-renderers/helpers.ts index 868dcfc4d..afbed444f 100644 --- a/apps/fabro-web/app/components/stage-renderers/helpers.ts +++ b/apps/fabro-web/app/components/stage-renderers/helpers.ts @@ -172,35 +172,28 @@ export interface ParallelBranchSummary { export interface ParallelOverview { branchCount: number | null; - successCount: number | null; - failureCount: number | null; - durationMs: number | null; results: ParallelBranchSummary[]; - isComplete: boolean; } /** * Roll up the `parallel.started` (announces branch count) and - * `parallel.completed` (carries the rolled-up results) events for a parallel + * `parallel.completed` (carries the per-branch results) events for a parallel * stage. Pre-completion, only the announce data is available. + * + * Only branch identity is parsed. The event's own `success_count`, + * `failure_count` and `duration_ms` rollups are deliberately ignored: the + * renderer counts the branch rows it actually draws, and duration comes from + * the stage record via `StageMetaBar`. */ export function parseParallelOverview(events: EventEnvelope[]): ParallelOverview { let branchCount: number | null = null; - let successCount: number | null = null; - let failureCount: number | null = null; - let durationMs: number | null = null; let results: ParallelBranchSummary[] = []; - let isComplete = false; for (const event of events) { const props: UnknownRecord = event.properties ?? {}; if (event.event === "parallel.started") { branchCount = getNumber(props, "branch_count") ?? branchCount; } else if (event.event === "parallel.completed") { - isComplete = true; - successCount = getNumber(props, "success_count") ?? successCount; - failureCount = getNumber(props, "failure_count") ?? failureCount; - durationMs = getNumber(props, "duration_ms") ?? durationMs; const rawResults = getArray(props, "results") ?? []; results = rawResults .map((entry) => { @@ -218,14 +211,7 @@ export function parseParallelOverview(events: EventEnvelope[]): ParallelOverview } } - return { - branchCount, - successCount, - failureCount, - durationMs, - results, - isComplete, - }; + return { branchCount, results }; } export interface ReducerTranscript { diff --git a/apps/fabro-web/app/components/stage-renderers/parallel-children.test.tsx b/apps/fabro-web/app/components/stage-renderers/parallel-children.test.tsx index 0800fddb0..2ebc5c822 100644 --- a/apps/fabro-web/app/components/stage-renderers/parallel-children.test.tsx +++ b/apps/fabro-web/app/components/stage-renderers/parallel-children.test.tsx @@ -144,6 +144,25 @@ describe("ParallelChildren", () => { expect(statValue(renderer, "Failed")).toBe("0"); }); + test("shows the recorded stage duration when cancellation interrupts the fan-out", () => { + const renderer = renderParallel( + [startedEvent(2)], + [], + makeStage({ + id: "fork@1", + name: "fork", + nodeId: "fork", + handler: "parallel", + status: StageState.CANCELLED, + duration: "53m 29s", + }), + ); + + // No `parallel.completed` event is emitted for an interrupted fan-out, so + // the stage record is the only duration there is. + expect(textContent(renderer.root)).toContain("53m 29s"); + }); + test("keeps looped fork links scoped to the selected fork visit", () => { const renderer = renderParallel( [startedEvent(1)], diff --git a/apps/fabro-web/app/components/stage-renderers/parallel-children.tsx b/apps/fabro-web/app/components/stage-renderers/parallel-children.tsx index 21c7c9271..fe9d9b04d 100644 --- a/apps/fabro-web/app/components/stage-renderers/parallel-children.tsx +++ b/apps/fabro-web/app/components/stage-renderers/parallel-children.tsx @@ -6,7 +6,6 @@ import type { EventEnvelope } from "@qltysh/fabro-api-client"; import type { Stage } from "../stage-sidebar"; import { formatStageLabel, stageStatusLabel, stageStatusTone } from "../../lib/stage-sidebar"; -import { formatDurationMs } from "../../lib/format"; import { StageMetaBar } from "./meta-bar"; import { parseParallelOverview } from "./helpers"; import type { ParallelBranchSummary } from "./helpers"; @@ -184,9 +183,11 @@ export function ParallelChildren({ return (
+ {/* The meta bar owns duration for every stage renderer, including the + live clock while running, so the tiles below stay outcome-only. */} -
+
0 ? "danger" : "default"} /> -
diff --git a/apps/fabro-web/public/images/providers/deepseek.svg b/apps/fabro-web/public/images/providers/deepseek.svg new file mode 100644 index 000000000..5d6efa991 --- /dev/null +++ b/apps/fabro-web/public/images/providers/deepseek.svg @@ -0,0 +1,3 @@ + + + diff --git a/docs/public/administration/server-configuration.mdx b/docs/public/administration/server-configuration.mdx index 2b526ada9..b6fa8d2b7 100644 --- a/docs/public/administration/server-configuration.mdx +++ b/docs/public/administration/server-configuration.mdx @@ -388,6 +388,7 @@ fabro secret set GEMINI_API_KEY AI... | `MINIMAX_API_KEY` | Minimax | | `INCEPTION_API_KEY` | Inception (Mercury) | | `POOLSIDE_API_KEY` | Poolside (Laguna) | +| `DEEPSEEK_API_KEY` | DeepSeek | | `OPENROUTER_API_KEY` | OpenRouter (when enabled) | | `MODAL_TOKEN_ID` and `MODAL_TOKEN_SECRET` | Modal (when enabled) | | `FIREWORKS_API_KEY` | Fireworks AI (when enabled) | diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index 80681f93a..9a118fdd9 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -9184,9 +9184,6 @@ components: environment: type: string description: Named environment slug to select for the run. - docker_image: - type: string - description: Per-run environment image override. verbose: type: boolean dry_run: diff --git a/docs/public/core-concepts/models.mdx b/docs/public/core-concepts/models.mdx index 47be54ee4..df78987d7 100644 --- a/docs/public/core-concepts/models.mdx +++ b/docs/public/core-concepts/models.mdx @@ -59,13 +59,15 @@ Fabro performs this selection once when creating a run and persists the chosen p | `gemini-3.1-flash-lite` | gemini | `gemini-flash-lite`, `gemini-3.1-flash-lite-preview` | 1M | $0.25 / $1.50 | 200 tok/s | | `kimi-k2.5` | moonshot | | 262K | $0.60 / $3.00 | 50 tok/s | | `kimi-k3` | moonshot | `kimi` | 1M | $3.00 / $15.00 | n/a | +| `deepseek-v4-flash` | deepseek | `deepseek`, `deepseek-v4`, `deepseek-flash` | 1,048,576 | $0.14 / $0.28 | n/a | +| `deepseek-v4-pro` | deepseek | | 1,048,576 | $0.435 / $0.87 | n/a | | `laguna-s-2.1` | poolside | `laguna`, `laguna-s` | 1M | $0.10 / $0.20 | n/a | | `laguna-xs-2.1` | poolside | `laguna-xs` | 262K | $0.10 / $0.20 | n/a | | `glm-5.2` | zai | `glm`, `glm5`, `glm52`, `glm5.2` | 1M | $1.40 / $4.40 | n/a | | `minimax-m2.5` | minimax | `minimax` | 197K | $0.30 / $1.20 | 45 tok/s | | `mercury-2` | inception | `mercury` | 131K | $0.25 / $0.75 | 1000 tok/s | -Each provider requires its own API key. Server-backed workflows read provider credentials from the server vault (for example `ANTHROPIC_API_KEY`, `OPENAI_API_KEY`, `GEMINI_API_KEY`, or `POOLSIDE_API_KEY` set with `fabro secret set` or `fabro provider login`). Standalone SDK/CLI flows can opt into env-backed credential sources explicitly. See the [Quick Start](/getting-started/quick-start) for setup. +Each provider requires its own API key. Server-backed workflows read provider credentials from the server vault (for example `ANTHROPIC_API_KEY`, `OPENAI_API_KEY`, `GEMINI_API_KEY`, `DEEPSEEK_API_KEY`, or `POOLSIDE_API_KEY` set with `fabro secret set` or `fabro provider login`). Standalone SDK/CLI flows can opt into env-backed credential sources explicitly. See the [Quick Start](/getting-started/quick-start) for setup. Claude Fable 5 is available as an explicit model but is not the default Anthropic model. If Fable refuses a request, Fabro reports the refusal as a content-filter LLM error and applies the configured `run.model.fallbacks` chain when one is present. diff --git a/docs/public/docs.json b/docs/public/docs.json index e00f0d711..faf0d0f84 100644 --- a/docs/public/docs.json +++ b/docs/public/docs.json @@ -96,6 +96,7 @@ "integrations/daytona", "integrations/litellm", "integrations/bedrock", + "integrations/deepseek", "integrations/poolside", "integrations/openrouter", "integrations/modal", diff --git a/docs/public/integrations/deepseek.mdx b/docs/public/integrations/deepseek.mdx new file mode 100644 index 000000000..23fcdb361 --- /dev/null +++ b/docs/public/integrations/deepseek.mdx @@ -0,0 +1,146 @@ +--- +title: "DeepSeek" +description: "Run DeepSeek V4 Flash and Pro through DeepSeek's direct API or supported gateways" +--- + +[DeepSeek](https://www.deepseek.com/) provides an OpenAI-compatible API for DeepSeek V4. Fabro includes direct access to V4 Flash and V4 Pro. The same Fabro model IDs also work through the optional Fireworks AI and OpenRouter providers. + +## Prerequisites + +- A [DeepSeek Platform](https://platform.deepseek.com/) account +- A DeepSeek API key from [platform.deepseek.com/api_keys](https://platform.deepseek.com/api_keys) +- A running Fabro server + +## Configure direct access + +The direct `deepseek` provider is enabled in the built-in catalog. Store its key in the target Fabro server vault: + +```bash +fabro provider login --provider deepseek + +# For a non-default remote server: +fabro provider login --server https://your-fabro.example --provider deepseek + +# Or set the vault token directly: +fabro secret set DEEPSEEK_API_KEY +fabro secret --server https://your-fabro.example set DEEPSEEK_API_KEY +``` + +Standalone SDK usage outside a Fabro server can use an env-backed credential source explicitly: + +```bash +export DEEPSEEK_API_KEY= +``` + +Fabro sends bearer-authenticated Chat Completions requests to `https://api.deepseek.com`. + +## Included models + +| Fabro model ID | DeepSeek API model ID | Context | Max output | Role | +|---|---|---:|---:|---| +| `deepseek-v4-flash` | `deepseek-v4-flash` | 1,048,576 | 384,000 | Provider default, small default, and connectivity probe; aliases `deepseek`, `deepseek-v4`, `deepseek-flash` | +| `deepseek-v4-pro` | `deepseek-v4-pro` | 1,048,576 | 384,000 | Higher-capability V4 model | + +Both models support text input, tool calling, native reasoning, streaming, JSON output, and automatic prompt caching. They do not support image input. Thinking mode is enabled by default. + +## Agent profile + +DeepSeek V4 uses Fabro's `openai` agent profile on every route. This profile is the closest match for DeepSeek's general coding behavior: it supplies project `AGENTS.md` instructions, standard JSON function tools, and a JSON-compatible file editor on Chat Completions routes. The setting is model-specific, so it remains the same through direct DeepSeek, Fireworks AI, and OpenRouter. + +Fabro does not use the `gpt56` profile for DeepSeek. That profile has a smaller Codex-specific tool set for GPT-5.6 models. + +## Use DeepSeek models + +```bash +fabro model list --provider deepseek +fabro model test --provider deepseek --model deepseek-v4-flash --deep +fabro run workflow.fabro --provider deepseek --model deepseek +``` + +In workflow stylesheets: + +```dot title="workflow.fabro" +digraph Example { + graph [ + model_stylesheet=" + * { model: deepseek-v4-flash; } + .difficult { model: deepseek-v4-pro; } + " + ] + + start [shape=Mdiamond, label="Start"] + work [label="Implement", prompt="Implement and verify the requested change."] + exit [shape=Msquare, label="Exit"] + + start -> work -> exit +} +``` + +## Thinking and reasoning effort + +DeepSeek enables thinking by default at `high` effort. Fabro advertises only effort values that produce a distinct model behavior on each route: + +| Route | V4 Flash | V4 Pro | +|---|---|---| +| Direct DeepSeek | `low`, `high`, `max` | `high`, `max` | +| Fireworks AI | `high`, `max` | `high`, `max` | +| OpenRouter | `low`, `high`, `max` | `high`, `xhigh` | + +DeepSeek currently maps a V4 Pro request for `low` to `high`; its documentation says this mapping will change in early August 2026. Fireworks maps `low` and `medium` to `high`, and maps `xhigh` to `max`. OpenRouter names the Pro maximum tier `xhigh`. + +Fabro omits `temperature` and `top_p` for these models because DeepSeek ignores sampling parameters while thinking is enabled. + +To disable thinking on the direct provider, omit typed `reasoning_effort` and pass DeepSeek's native toggle through provider options: + +```json +{ + "provider_options": { + "deepseek": { + "thinking": { "type": "disabled" } + } + } +} +``` + +DeepSeek requires `reasoning_content` from an assistant tool call to appear in every later request in that tool-use turn. Fabro captures this content and replays it with the assistant message. This keeps multi-step tool calls valid and prevents DeepSeek's HTTP 400 response for missing reasoning history. + +## Prompt caching and pricing + +DeepSeek applies prefix caching automatically. Fabro reads DeepSeek's `prompt_cache_hit_tokens` usage field and reports cache-read tokens separately from uncached input tokens. + +The built-in catalog uses DeepSeek's published prices per million tokens: + +| Model | Uncached input | Cache hit | Output | +|---|---:|---:|---:| +| `deepseek-v4-flash` | $0.14 | $0.0028 | $0.28 | +| `deepseek-v4-pro` | $0.435 | $0.003625 | $0.87 | + +DeepSeek does not return an in-band dollar cost. Fabro calculates an estimated cost from these catalog rates and the reported token buckets. + +## Use a gateway + +The `deepseek-v4-flash`, `deepseek-v4-pro`, `deepseek`, `deepseek-v4`, and `deepseek-flash` selectors are portable across direct DeepSeek, Fireworks AI, and OpenRouter routes. Use `--provider` or a provider-qualified model selector when you need a specific route. + +See the [Fireworks AI integration](/integrations/fireworks) and [OpenRouter integration](/integrations/openrouter) for gateway setup and provider-specific pricing. + +## Troubleshooting + +**"No API key configured"** — Store `DEEPSEEK_API_KEY` on the target server with `fabro provider login --provider deepseek`. The server runtime resolves the key from its vault, not from process env. + +**Unknown model** — Use `deepseek-v4-flash` or `deepseek-v4-pro`. The retired `deepseek-chat` and `deepseek-reasoner` API IDs are not in the Fabro catalog. + +**A tool continuation returns HTTP 400** — Keep the assistant thinking content in conversation history. Fabro does this automatically when it replays tool-call turns. + +## Further reading + + + + Official authentication, endpoints, and API reference. + + + Official limits, features, and token prices. + + + Official thinking toggles, effort mappings, and tool-call replay rules. + + diff --git a/docs/public/integrations/fireworks.mdx b/docs/public/integrations/fireworks.mdx index 7c3331de6..26e873705 100644 --- a/docs/public/integrations/fireworks.mdx +++ b/docs/public/integrations/fireworks.mdx @@ -50,7 +50,7 @@ The built-in catalog gives Fireworks offerings the same human-facing model slugs | --- | --- | | `kimi-k2.7-code` | `accounts/fireworks/models/kimi-k2p7-code`; provider default | | `kimi-k2.6` | `accounts/fireworks/models/kimi-k2p6` | -| `deepseek-v4-pro`, `deepseek-v4-flash` | `accounts/fireworks/models/deepseek-v4-...` | +| `deepseek-v4-pro`, `deepseek-v4-flash` (`deepseek`, `deepseek-v4`, `deepseek-flash`) | `accounts/fireworks/models/deepseek-v4-...` | | `glm-5.2` | `accounts/fireworks/models/glm-5p2` | | `minimax-m2.7` | `accounts/fireworks/models/minimax-m2p7` | | `qwen3.7-plus` | `accounts/fireworks/models/qwen3p7-plus` | diff --git a/docs/public/integrations/openrouter.mdx b/docs/public/integrations/openrouter.mdx index c3bf7ff2c..d82dbc4e7 100644 --- a/docs/public/integrations/openrouter.mdx +++ b/docs/public/integrations/openrouter.mdx @@ -53,7 +53,7 @@ The built-in catalog gives OpenRouter offerings the same human-facing model slug | `claude-haiku-4-5` | `anthropic/claude-haiku-4.5`; provider small default | | `gpt-5.6-sol`, `gpt-5.6-terra`, `gpt-5.6-luna`, `gpt-5.4`, `gpt-5.5` | Matching `openai/...` API IDs | | `gemini-3.1-pro-preview`, `gemini-3.5-flash` | `google/...` API IDs | -| `deepseek-v4-pro` (`deepseek`, `deepseek-v4`), `deepseek-v4-flash` (`deepseek-flash`) | `deepseek/...` API IDs | +| `deepseek-v4-pro`, `deepseek-v4-flash` (`deepseek`, `deepseek-v4`, `deepseek-flash`) | `deepseek/...` API IDs; Flash uses the `deepseek-v4-flash-0731` release | | `kimi-k3`, `kimi-k2.6`, `qwen3-coder`, `qwen3.6-flash` | Vendor-prefixed API IDs | | `laguna-s-2.1`, `laguna-xs-2.1` | `poolside/...`; native reasoning and tool use | | `glm-5.2` (`glm`, `glm5`, `glm52`, `glm5.2`), `glm-4.6` | `z-ai/...` API IDs | diff --git a/docs/public/reference/sdk.mdx b/docs/public/reference/sdk.mdx index 402953e5c..12dee0001 100644 --- a/docs/public/reference/sdk.mdx +++ b/docs/public/reference/sdk.mdx @@ -371,6 +371,7 @@ For env-backed usage, `EnvCredentialSource` checks for API key environment varia | `MINIMAX_API_KEY` | Minimax | | `INCEPTION_API_KEY` | Inception | | `POOLSIDE_API_KEY` | Poolside | +| `DEEPSEEK_API_KEY` | DeepSeek | | `OPENROUTER_API_KEY` | OpenRouter, when enabled in settings | The first provider registered becomes the default. Provider base URLs come from the model catalog. For vault-backed usage inside Fabro, use `fabro_auth::VaultCredentialSource` instead. diff --git a/lib/apps/fabro-cli/src/commands/install.rs b/lib/apps/fabro-cli/src/commands/install.rs index 2db9d4470..973524899 100644 --- a/lib/apps/fabro-cli/src/commands/install.rs +++ b/lib/apps/fabro-cli/src/commands/install.rs @@ -3521,6 +3521,7 @@ root = "{}" assert!(ids.contains(&ProviderId::new("inception"))); assert!(ids.contains(&ProviderId::new("venice"))); assert!(ids.contains(&ProviderId::new("poolside"))); + assert!(ids.contains(&ProviderId::new("deepseek"))); assert!(!ids.contains(&ProviderId::new("fireworks"))); assert!(!ids.contains(&ProviderId::new("ollama"))); assert!(!ids.contains(&ProviderId::new("litellm"))); diff --git a/lib/apps/fabro-cli/src/commands/run/overrides.rs b/lib/apps/fabro-cli/src/commands/run/overrides.rs index 40fa46f6a..bd154ad87 100644 --- a/lib/apps/fabro-cli/src/commands/run/overrides.rs +++ b/lib/apps/fabro-cli/src/commands/run/overrides.rs @@ -75,7 +75,6 @@ pub(crate) fn run_args_overrides(args: &RunArgs) -> Result Result Option { preserve_sandbox: args.preserve_sandbox.then_some(true), provider: args.provider.clone(), environment: args.environment.clone(), - docker_image: None, input: args.inputs.values.clone(), verbose: args.verbose.then_some(true), }; @@ -27,7 +26,6 @@ pub(crate) fn preflight_manifest_args(args: &PreflightArgs) -> Option= deadline { + let _ = child.kill(); + let _ = child.wait(); + panic!("MCP server did not exit after its executable was replaced"); + } + thread::sleep(Duration::from_millis(50)); + }; + + assert!(status.success(), "MCP server should exit successfully"); +} + #[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 start = Instant::now(); let client = spawn_mcp_client(&context, &[]).await; let tools = client.list_tools().await.unwrap(); assert_eq!(tools.len(), MCP_RUN_TOOL_NAMES.len()); - assert!(start.elapsed() < std::time::Duration::from_secs(2)); + assert!(start.elapsed() < Duration::from_secs(2)); client .shutdown() .await @@ -1602,12 +1625,21 @@ async fn mcp_get_resolves_selector_and_returns_summary_projection_and_questions( let projection = server.mock(|when, then| { when.method(GET) .path(format!("/api/v1/runs/{run_id}/state")); + let mut body = run_projection_json(&run_id, &serde_json::json!({ "kind": "running" })); + body["spec"]["settings"]["run"]["model"] = serde_json::json!({ + "provider": "openai", + "name": "gpt-5.6-sol", + "fallbacks": { + "gpt-5.6-sol": ["gpt-5.6-terra"] + }, + "controls": { + "reasoning_effort": null, + "speed": null + } + }); then.status(200) .header("Content-Type", "application/json") - .json_body(run_projection_json( - &run_id, - &serde_json::json!({ "kind": "running" }), - )); + .json_body(body); }); let questions = server.mock(|when, then| { when.method(GET) @@ -1644,6 +1676,10 @@ async fn mcp_get_resolves_selector_and_returns_summary_projection_and_questions( assert_eq!(get["summary"]["workflow_name"], "Simple"); assert_eq!(get["summary"]["workflow_slug"], "simple"); assert_eq!(get["projection"]["status"]["kind"], "running"); + assert_eq!( + get["projection"]["spec"]["settings"]["run"]["model"]["fallbacks"]["gpt-5.6-sol"][0], + "gpt-5.6-terra" + ); assert_eq!(get["questions"][0]["id"], "q-1"); resolve.assert(); retrieve.assert(); @@ -2001,6 +2037,82 @@ async fn mcp_events_filters_find_matches_beyond_first_page() { .expect("MCP client should shut down"); } +#[tokio::test(flavor = "multi_thread")] +async fn mcp_events_decodes_run_created_with_model_keyed_fallbacks() { + 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 mut settings = serde_json::to_value(WorkflowSettings::default()) + .expect("workflow settings should serialize"); + settings["run"]["model"] = serde_json::json!({ + "provider": "openai", + "name": "gpt-5.6-sol", + "fallbacks": { + "gpt-5.6-sol": ["gpt-5.6-terra"] + }, + "controls": { + "reasoning_effort": null, + "speed": null + } + }); + let event = serde_json::json!({ + "seq": 1, + "id": "evt-created", + "ts": "2026-04-05T12:00:00Z", + "run_id": run_id, + "event": "run.created", + "properties": { + "settings": settings, + "graph": Graph::new("Remote Workflow"), + "labels": {}, + "run_dir": "/tmp/run", + "source_directory": "/srv/repo", + "provenance": test_support::test_run_provenance() + }, + "actor": null + }); + let 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": [event], + "meta": { "has_more": false } + })); + }); + let client = spawn_mcp_client(&context, &["--server", &target_url]).await; + + let result = call_tool_json( + &client, + "fabro_run_events", + serde_json::json!({ + "run_id": "nightly", + "action": "search", + "query": "gpt-5.6-terra", + "first": 1 + }), + ) + .await; + + assert_eq!( + result["events"][0]["event"]["properties"]["settings"]["run"]["model"]["fallbacks"]["gpt-5.6-sol"] + [0], + "gpt-5.6-terra" + ); + resolve.assert(); + events.assert(); + 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!(); @@ -2379,6 +2491,45 @@ fn mcp_stdio_fixture(context: &fabro_test::TestContext, extra_args: &[&str]) -> } } +const MCP_INITIALIZE_REQUEST: &str = r#"{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-06-18","capabilities":{},"clientInfo":{"name":"fabro-test","version":"0.0.0"}}}"#; + +/// Starts `fabro mcp start` as a raw child process, sends `initialize`, and +/// returns the decoded response. Unlike `spawn_mcp_client`, the caller keeps +/// the `Child` and its stdin, so it can observe how and when the server exits. +fn spawn_stdio_server( + fixture: &McpStdioFixture, + program: &Path, +) -> (Child, ChildStdin, serde_json::Value) { + let mut child = Command::new(program) + .args(&fixture.command[1..]) + .env_clear() + .envs(&fixture.env) + .current_dir(&fixture.current_dir) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("MCP server should start"); + + let mut stdin = child.stdin.take().expect("MCP stdin should be available"); + writeln!(stdin, "{MCP_INITIALIZE_REQUEST}").expect("initialize request should be written"); + + let stdout = child.stdout.take().expect("MCP stdout should be available"); + let (tx, rx) = std::sync::mpsc::channel(); + 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(Duration::from_secs(5)) + .expect("initialize response should arrive") + .expect("MCP stdout should be readable"); + + let response = serde_json::from_str(line.trim()).expect("response should be JSON"); + (child, stdin, response) +} + fn write_mcp_server_settings( context: &mut fabro_test::TestContext, storage_dir: &Path, diff --git a/lib/apps/fabro-mcp-server/Cargo.toml b/lib/apps/fabro-mcp-server/Cargo.toml index cdbf6b65f..758a11fd3 100644 --- a/lib/apps/fabro-mcp-server/Cargo.toml +++ b/lib/apps/fabro-mcp-server/Cargo.toml @@ -32,7 +32,8 @@ serde_json.workspace = true strum.workspace = true tokio.workspace = true toml.workspace = true +tracing.workspace = true [dev-dependencies] httpmock = "0.8" -tempfile = "3" \ No newline at end of file +tempfile = "3" diff --git a/lib/apps/fabro-mcp-server/src/config.rs b/lib/apps/fabro-mcp-server/src/config.rs index 9eb8a67c2..ec7154348 100644 --- a/lib/apps/fabro-mcp-server/src/config.rs +++ b/lib/apps/fabro-mcp-server/src/config.rs @@ -9,9 +9,7 @@ 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"; +use crate::{McpAgent, McpConfigSettings, McpInitSettings, SERVER_NAME}; pub fn config_json(settings: &McpConfigSettings) -> Result { serde_json::to_string_pretty(&generic_config(settings)) diff --git a/lib/apps/fabro-mcp-server/src/executable_monitor.rs b/lib/apps/fabro-mcp-server/src/executable_monitor.rs new file mode 100644 index 000000000..ae0eb6b30 --- /dev/null +++ b/lib/apps/fabro-mcp-server/src/executable_monitor.rs @@ -0,0 +1,139 @@ +//! Detects when an upgrade replaces the executable that launched this MCP +//! server. +//! +//! MCP hosts can keep stdio servers alive for days. Without this check, an old +//! process keeps its old API response decoder after the `fabro` file on disk is +//! upgraded. + +use std::fs::Metadata; +use std::path::{self, PathBuf}; +use std::time::Duration; +use std::{env, fs, io}; + +use tokio::time::{self, Instant, MissedTickBehavior}; + +const CHECK_INTERVAL: Duration = Duration::from_secs(1); + +pub(crate) struct ExecutableMonitor { + path: PathBuf, + identity: Identity, +} + +impl ExecutableMonitor { + pub(crate) fn current() -> io::Result { + Self::new(invoked_executable_path()?) + } + + fn new(path: PathBuf) -> io::Result { + let identity = identity(&fs::metadata(&path)?); + Ok(Self { path, identity }) + } + + pub(crate) async fn wait_until_replaced(self) { + let mut interval = time::interval_at(Instant::now() + CHECK_INTERVAL, CHECK_INTERVAL); + interval.set_missed_tick_behavior(MissedTickBehavior::Skip); + loop { + interval.tick().await; + if self.was_replaced() { + return; + } + } + } + + /// Reads the identity synchronously. This is a `stat` of a page-cached + /// inode once per second, so handing it to Tokio's blocking pool would + /// cost more than the call itself and would keep a pool thread resident + /// for the life of the server. + fn was_replaced(&self) -> bool { + !fs::metadata(&self.path).is_ok_and(|metadata| identity(&metadata) == self.identity) + } +} + +/// Identifies the file behind an executable path. Upgrades always swap a new +/// file into place — `fabro upgrade` renames over the old one and Homebrew +/// repoints a symlink — so the identity changes even though the path does not. +#[cfg(unix)] +type Identity = (u64, u64); + +#[cfg(unix)] +fn identity(metadata: &Metadata) -> Identity { + use std::os::unix::fs::MetadataExt as _; + + (metadata.dev(), metadata.ino()) +} + +#[cfg(not(unix))] +type Identity = (u64, Option); + +#[cfg(not(unix))] +fn identity(metadata: &Metadata) -> Identity { + (metadata.len(), metadata.modified().ok()) +} + +/// Resolves the executable path to watch. +/// +/// `argv[0]` wins when it carries a directory, because it names the path the +/// host actually launched, symlink included. MCP hosts normally launch a bare +/// `fabro` found on `PATH`, which leaves `current_exe`: it reports the symlink +/// on macOS, and the Homebrew symlink's own target on Linux. +fn invoked_executable_path() -> io::Result { + match env::args_os().next().map(PathBuf::from) { + Some(invoked) if invoked.components().count() > 1 => path::absolute(invoked), + _ => env::current_exe(), + } +} + +#[cfg(test)] +mod tests { + use tokio::fs as async_fs; + + use super::*; + + #[tokio::test] + async fn unchanged_executable_is_current() { + let directory = tempfile::tempdir().expect("temp directory should exist"); + let executable = directory.path().join("fabro"); + async_fs::write(&executable, b"current") + .await + .expect("fixture executable should be written"); + let monitor = ExecutableMonitor::new(executable).unwrap(); + + assert!(!monitor.was_replaced()); + } + + #[tokio::test] + async fn atomic_executable_replacement_is_detected() { + let directory = tempfile::tempdir().expect("temp directory should exist"); + let executable = directory.path().join("fabro"); + let replacement = directory.path().join("fabro-new"); + async_fs::write(&executable, b"old") + .await + .expect("old fixture executable should be written"); + async_fs::write(&replacement, b"new executable") + .await + .expect("new fixture executable should be written"); + let monitor = ExecutableMonitor::new(executable.clone()).unwrap(); + + async_fs::rename(&replacement, &executable) + .await + .expect("fixture executable should be replaced"); + + assert!(monitor.was_replaced()); + } + + #[tokio::test] + async fn removed_executable_is_detected() { + let directory = tempfile::tempdir().expect("temp directory should exist"); + let executable = directory.path().join("fabro"); + async_fs::write(&executable, b"current") + .await + .expect("fixture executable should be written"); + let monitor = ExecutableMonitor::new(executable.clone()).unwrap(); + + async_fs::remove_file(executable) + .await + .expect("fixture executable should be removed"); + + assert!(monitor.was_replaced()); + } +} diff --git a/lib/apps/fabro-mcp-server/src/lib.rs b/lib/apps/fabro-mcp-server/src/lib.rs index ad6264dcb..068f99c14 100644 --- a/lib/apps/fabro-mcp-server/src/lib.rs +++ b/lib/apps/fabro-mcp-server/src/lib.rs @@ -1,4 +1,5 @@ mod config; +mod executable_monitor; mod manifest_builder; mod server; @@ -12,6 +13,10 @@ pub use config::{config_json, init_agent}; use fabro_client::Client; pub use server::start; +/// The name this MCP server reports over the wire and registers under in agent +/// config files. +pub(crate) const SERVER_NAME: &str = "fabro"; + pub type FabroClientFuture = Pin> + Send>>; pub type FabroClientFactory = Arc FabroClientFuture + Send + Sync>; diff --git a/lib/apps/fabro-mcp-server/src/server.rs b/lib/apps/fabro-mcp-server/src/server.rs index c1b932308..6a9c9941d 100644 --- a/lib/apps/fabro-mcp-server/src/server.rs +++ b/lib/apps/fabro-mcp-server/src/server.rs @@ -1,19 +1,24 @@ use std::path::PathBuf; use std::sync::Arc; +use std::time::Duration; use anyhow::Result; use fabro_tool::fabro_client::ClientBackend; use fabro_tool::{self as run_tools, FabroToolBackend}; +use fabro_util::version::FABRO_VERSION; use rmcp::handler::server::router::tool::ToolRouter; use rmcp::handler::server::wrapper::Parameters; -use rmcp::model::{CallToolResult, Content, ServerCapabilities, ServerInfo}; +use rmcp::model::{CallToolResult, Content, Implementation, ServerCapabilities, ServerInfo}; use rmcp::transport::stdio; use rmcp::{ErrorData, ServerHandler, serve_server, tool, tool_handler, tool_router}; use serde::Serialize; use tokio::sync::OnceCell; +use tokio::time; +use tracing::warn; -use crate::FabroMcpServerSettings; +use crate::executable_monitor::ExecutableMonitor; use crate::manifest_builder::McpRunManifestBuilder; +use crate::{FabroMcpServerSettings, SERVER_NAME}; #[derive(Clone)] pub(crate) struct FabroMcpServer { @@ -23,10 +28,44 @@ pub(crate) struct FabroMcpServer { tool_router: ToolRouter, } +/// How long to wait for the MCP service to stop after an upgrade is detected. +/// The wait is bounded because the transport closes by writing to stdout, which +/// blocks if the host has stopped reading. +const SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(5); + pub async fn start(settings: FabroMcpServerSettings) -> Result<()> { - let server = FabroMcpServer::new(Arc::new(settings)); - let service = serve_server(server, stdio()).await?; - service.waiting().await?; + let monitor = match ExecutableMonitor::current() { + Ok(monitor) => Some(monitor), + Err(error) => { + warn!( + %error, + "Upgrade detection is unavailable; this MCP server will keep running after an \ + upgrade replaces it" + ); + None + } + }; + let service = serve_server(FabroMcpServer::new(Arc::new(settings)), stdio()).await?; + let Some(monitor) = monitor else { + service.waiting().await?; + return Ok(()); + }; + + let cancellation = service.cancellation_token(); + let mut service_wait = Box::pin(service.waiting()); + tokio::select! { + result = &mut service_wait => { + result?; + } + () = monitor.wait_until_replaced() => { + // An upgrade replaced the executable, so stop serving and let the + // host reconnect to the new one. The CLI exits the process rather + // than returning, because Tokio's stdin worker stays blocked on a + // read that only the host can end. + cancellation.cancel(); + let _ = time::timeout(SHUTDOWN_TIMEOUT, service_wait).await; + } + } Ok(()) } @@ -34,6 +73,7 @@ pub async fn start(settings: FabroMcpServerSettings) -> Result<()> { impl ServerHandler for FabroMcpServer { fn get_info(&self) -> ServerInfo { ServerInfo::new(ServerCapabilities::builder().enable_tools().build()) + .with_server_info(Implementation::new(SERVER_NAME, FABRO_VERSION).with_title("Fabro")) .with_instructions("Use these tools to create, inspect, control, wait for, and read events from Fabro workflow runs.") } } @@ -251,6 +291,22 @@ mod tests { use super::*; use crate::FabroMcpServerSettings; + #[test] + fn server_info_reports_fabro_version() { + let settings = FabroMcpServerSettings { + cwd: PathBuf::from("."), + config_path: PathBuf::from("fabro.toml"), + client_factory: Arc::new(|| { + Box::pin(async { panic!("client should not be constructed while reading info") }) + }), + }; + let info = FabroMcpServer::new(Arc::new(settings)).get_info(); + + assert_eq!(info.server_info.name, "fabro"); + assert_eq!(info.server_info.title.as_deref(), Some("Fabro")); + assert_eq!(info.server_info.version, FABRO_VERSION); + } + #[test] fn fabro_run_pair_tool_is_registered_with_stage_based_schema() { let settings = FabroMcpServerSettings { diff --git a/lib/apps/fabro-server/Cargo.toml b/lib/apps/fabro-server/Cargo.toml index e24dd114b..f0acde539 100644 --- a/lib/apps/fabro-server/Cargo.toml +++ b/lib/apps/fabro-server/Cargo.toml @@ -121,5 +121,6 @@ tokio-util.workspace = true tokio-tungstenite.workspace = true fabro-macros = { path = "../../foundation/fabro-macros" } fabro-sandbox = { path = "../../components/fabro-sandbox", features = ["test-support"] } +fabro-store = { path = "../../components/fabro-store", features = ["test-support"] } fabro-test = { workspace = true } fabro-types = { path = "../../foundation/fabro-types", features = ["test-support"] } diff --git a/lib/apps/fabro-server/src/lib.rs b/lib/apps/fabro-server/src/lib.rs index 5fc735ba0..9a7e12978 100644 --- a/lib/apps/fabro-server/src/lib.rs +++ b/lib/apps/fabro-server/src/lib.rs @@ -34,6 +34,7 @@ pub mod manifest_validation; mod migrations; mod principal_middleware; mod request_id; +mod run_compiler; mod run_files; mod run_files_security; mod run_manifest; diff --git a/lib/apps/fabro-server/src/run_compiler.rs b/lib/apps/fabro-server/src/run_compiler.rs new file mode 100644 index 000000000..437df572b --- /dev/null +++ b/lib/apps/fabro-server/src/run_compiler.rs @@ -0,0 +1,1034 @@ +//! The create-time run compiler: the single pipeline that turns an acquired +//! workflow bundle into a complete, persistable run. +//! +//! The pipeline has four stages, each its own function with typed input and +//! output: +//! +//! 1. [`normalize_source`] — resolve the bundle entrypoint and parse the +//! bundle-relative settings sources (workflow and project layers, with +//! dockerfile references inlined from bundled files). +//! 2. [`layer_settings`] + [`apply_run_variables`] + graph compilation — layer +//! settings from every configured source, substitute the run-scoped variable +//! snapshot, then parse/transform/validate the graph through the +//! fabro-workflow pipeline. +//! 3. Model pinning — materialize run-level model settings against the catalog +//! and the configured provider set. Stages 2's graph compilation and stage 3 +//! share one blocking dispatch via [`compile_and_pin`]. +//! 4. [`assemble_run`] — purely assemble the complete persistence input; no +//! field is mutated after assembly. +//! +//! The input is deliberately source-neutral: it speaks in terms of an +//! acquired [`WorkflowBundle`], not any wire request type, so non-HTTP +//! callers and alternative workflow sources can drive the same pipeline. +//! Callers own source acquisition, run-id resolution, variable snapshotting, +//! and (for HTTP callers) all wire mapping — including turning +//! [`RunCompilerError`] into HTTP responses. + +use std::collections::HashMap; +use std::path::PathBuf; +use std::sync::Arc; + +use fabro_config::parse::{self, ParseError, SettingsSource}; +use fabro_config::{ + CliLayer, EnvironmentDockerfileLayer, EnvironmentImageLayer, EnvironmentLayer, MergeMap, + RunLayer, SettingsLayer, WorkflowSettingsBuilder, +}; +use fabro_model::{Catalog, ProviderId}; +use fabro_types::settings::interp::{InterpString, ResolveError}; +use fabro_types::settings::run::{McpServerSettings, RunGoal}; +use fabro_types::{ + AutomationRef, GitContext, ManifestPath, RunId, RunProvenance, WorkflowSettings, +}; +use fabro_util::workspace_glob::{WorkspaceGlob, WorkspaceGlobError}; +use fabro_workflow::Error as WorkflowError; +use fabro_workflow::operations::{ + self, CompiledRun, CreateRunCompileInput, CreateRunPersistenceInput, + CreateRunPersistenceMetadata, MaterializedRun, WorkflowInput, +}; +use fabro_workflow::workflow_bundle::{BundledWorkflow, WorkflowBundle}; +use tokio::task; + +/// One project settings source in the acquired source's path namespace. +#[derive(Debug)] +pub(crate) struct ProjectSettingsSource { + pub(crate) path: std::result::Result, + pub(crate) toml: String, +} + +/// A project settings path that the source adapter could not normalize. +#[derive(Debug, thiserror::Error)] +pub(crate) enum ProjectSettingsPathError { + #[error("project settings path is missing")] + Missing, + + #[error("invalid project settings path: {path}")] + Invalid { path: String }, +} + +/// Transport-neutral inputs for compiling one submitted run. +/// +/// Identity (`run_id`), lineage, title, git metadata, and provenance are +/// resolved by the caller. This boundary owns only source normalization, +/// settings resolution, workflow compilation, model pinning, and +/// persistence-input assembly. +#[derive(Debug)] +pub(crate) struct RawRunCompilerInput { + pub(crate) workflow_bundle: WorkflowBundle, + pub(crate) entrypoint: ManifestPath, + pub(crate) cwd: PathBuf, + pub(crate) server_run_defaults: RunLayer, + pub(crate) server_environment_defaults: MergeMap, + pub(crate) server_mcp_catalog: HashMap, + pub(crate) project_settings: Vec, + pub(crate) user_toml: Vec, + pub(crate) run_overrides: Option, + pub(crate) cli_overrides: Option, + pub(crate) input_overrides: HashMap, + pub(crate) inline_goal_override: Option, + pub(crate) run_id: Option, + pub(crate) title: Option, + pub(crate) parent_id: Option, + pub(crate) git: Option, + pub(crate) storage_root: PathBuf, + pub(crate) workflow_slug: Option, + pub(crate) provenance: RunProvenance, + pub(crate) web_url: Option, + pub(crate) submitted_manifest_bytes: Option>, + pub(crate) automation: Option, +} + +/// Stage-one output: the selected bundled workflow and all client settings +/// sources have been parsed and normalized, but no settings have been layered. +pub(crate) struct NormalizedRun { + workflow_bundle: WorkflowBundle, + entrypoint: ManifestPath, + workflow: BundledWorkflow, + workflow_layer: Option, + project_layers: Vec, + user_toml: Vec, + cwd: PathBuf, + server_run_defaults: RunLayer, + server_environment_defaults: MergeMap, + server_mcp_catalog: HashMap, + run_overrides: Option, + cli_overrides: Option, + input_overrides: HashMap, + inline_goal_override: Option, + metadata: RunMetadata, +} + +struct RunMetadata { + run_id: Option, + storage_root: PathBuf, + workflow_slug: Option, + submitted_manifest_bytes: Option>, + title: Option, + automation: Option, + git: Option, + parent_id: Option, + provenance: RunProvenance, + web_url: Option, +} + +/// Settings-layered output. Variable substitution is a separate stage so +/// callers can snapshot run variables after settings resolution and apply +/// the snapshot through [`apply_run_variables`]. +pub(crate) struct LayeredRun { + workflow_bundle: WorkflowBundle, + entrypoint: ManifestPath, + workflow: BundledWorkflow, + settings: WorkflowSettings, + cwd: PathBuf, + metadata: RunMetadata, +} + +/// Variable-substituted stage output. Callers may inspect the resolved +/// settings before policy checks, then move it into [`compile_and_pin`]. +pub(crate) struct PreparedRun { + layered: LayeredRun, + vars: HashMap, +} + +impl PreparedRun { + pub(crate) fn settings(&self) -> &WorkflowSettings { + &self.layered.settings + } + + pub(crate) fn with_identity( + mut self, + run_id: Option, + parent_id: Option, + title: Option, + ) -> Self { + self.layered.metadata.run_id = run_id; + self.layered.metadata.parent_id = parent_id; + self.layered.metadata.title = title; + self + } + + pub(crate) fn parent_id(&self) -> Option { + self.layered.metadata.parent_id + } + + pub(crate) fn resolve_run_id(mut self) -> (Self, RunId) { + let run_id = self.layered.metadata.run_id.unwrap_or_default(); + self.layered.metadata.run_id = Some(run_id); + (self, run_id) + } + + pub(crate) fn with_web_url(mut self, web_url: Option) -> Self { + self.layered.metadata.web_url = web_url; + self + } +} + +/// Graph-compiled stage output, retaining the metadata needed by later pure +/// assembly. +struct GraphCompiledRun { + compiled: CompiledRun, + metadata: RunMetadata, +} + +/// Model-pinned stage output ready for pure persistence-input assembly. +pub(crate) struct PinnedRun { + materialized: MaterializedRun, + metadata: RunMetadata, +} + +#[derive(Debug, thiserror::Error)] +pub(crate) enum RunCompilerError { + /// The acquired source bundle is invalid: missing entrypoint or broken + /// bundled-file references. + #[error(transparent)] + InvalidSource(#[from] InvalidSourceError), + + /// A settings source or path is invalid, or the layered settings failed to + /// resolve. + #[error(transparent)] + InvalidSettings(Box), + + /// The run-variable snapshot could not be substituted into the resolved + /// run settings. + #[error("Run config variable interpolation failed: {0}")] + VariableInterpolation(#[from] VariableInterpolationError), + + /// Graph compilation or model pinning failed in the workflow engine. The + /// full [`WorkflowError`] is preserved so callers can distinguish + /// validation, parse, and model-selection failures. + #[error(transparent)] + Workflow(#[from] WorkflowError), +} + +// The `Display` strings below are pinned to the pre-extraction wire +// contract: both the create handler and the manifest preparation path render +// them directly into HTTP 400 details. +#[derive(Debug, thiserror::Error)] +pub(crate) enum InvalidSourceError { + #[error("manifest target path is missing from workflows map")] + MissingEntrypoint { entrypoint: ManifestPath }, + + #[error("unsupported dockerfile reference: {reference}")] + UnsupportedDockerfileReference { + config_path: ManifestPath, + reference: String, + }, + + #[error("missing bundled dockerfile: {dockerfile_path}")] + MissingDockerfile { + config_path: ManifestPath, + dockerfile_path: ManifestPath, + }, +} + +#[derive(Debug, thiserror::Error)] +pub(crate) enum InvalidSettingsError { + #[error("Failed to parse run config TOML")] + Parse { + path: ManifestPath, + #[source] + source: ParseError, + }, + + #[error(transparent)] + User(fabro_config::Error), + + #[error("failed to resolve manifest settings")] + Resolve { + #[source] + source: fabro_config::ResolveErrors, + }, + + #[error("{}", project_path_error(.source))] + ProjectPath { source: ProjectSettingsPathError }, +} + +#[derive(Debug, thiserror::Error)] +pub(crate) enum VariableInterpolationError { + #[error(transparent)] + Interpolation(#[from] ResolveError), + + #[error("run.artifacts.include[{index}]: {source}")] + ArtifactGlob { + index: usize, + #[source] + source: WorkspaceGlobError, + }, +} + +pub(crate) type Result = std::result::Result; + +fn project_path_error(source: &ProjectSettingsPathError) -> String { + match source { + ProjectSettingsPathError::Missing => { + "invalid manifest project config path: missing path".to_string() + } + ProjectSettingsPathError::Invalid { path } => { + format!("invalid manifest project config path: {path}") + } + } +} + +fn invalid_settings(source: InvalidSettingsError) -> RunCompilerError { + RunCompilerError::InvalidSettings(Box::new(source)) +} + +/// Normalize the bundle entrypoint and parse workflow/project settings while +/// resolving dockerfile references against the selected workflow's files. +pub(crate) fn normalize_source(input: RawRunCompilerInput) -> Result { + let RawRunCompilerInput { + workflow_bundle, + entrypoint, + cwd, + server_run_defaults, + server_environment_defaults, + server_mcp_catalog, + project_settings, + user_toml, + run_overrides, + cli_overrides, + input_overrides, + inline_goal_override, + run_id, + title, + parent_id, + git, + storage_root, + workflow_slug, + provenance, + web_url, + submitted_manifest_bytes, + automation, + } = input; + let mut workflow = workflow_bundle + .workflow(&entrypoint) + .cloned() + .ok_or_else(|| InvalidSourceError::MissingEntrypoint { + entrypoint: entrypoint.clone(), + })?; + workflow.path = entrypoint.clone(); + + let workflow_layer = workflow + .config + .as_ref() + .map(|config| { + settings_layer_with_resolved_dockerfiles( + &config.source, + &config.path, + &workflow.files, + SettingsSource::Workflow, + ) + }) + .transpose()?; + let project_layers = project_settings + .into_iter() + .map(|project| { + let path = project + .path + .map_err(|source| invalid_settings(InvalidSettingsError::ProjectPath { source }))?; + settings_layer_with_resolved_dockerfiles( + &project.toml, + &path, + &workflow.files, + SettingsSource::Project, + ) + }) + .collect::>>()?; + + Ok(NormalizedRun { + workflow_bundle, + entrypoint, + workflow, + workflow_layer, + project_layers, + user_toml, + cwd, + server_run_defaults, + server_environment_defaults, + server_mcp_catalog, + run_overrides, + cli_overrides, + input_overrides, + inline_goal_override, + metadata: RunMetadata { + run_id, + storage_root, + workflow_slug, + submitted_manifest_bytes, + title, + automation, + git, + parent_id, + provenance, + web_url, + }, + }) +} + +/// Layer settings from every configured source and apply the submitted input +/// and goal overrides. +pub(crate) fn layer_settings(normalized: NormalizedRun) -> Result { + let NormalizedRun { + workflow_bundle, + entrypoint, + workflow, + workflow_layer, + project_layers, + user_toml, + cwd, + server_run_defaults, + server_environment_defaults, + server_mcp_catalog, + run_overrides, + cli_overrides, + input_overrides, + inline_goal_override, + metadata, + } = normalized; + let mut builder = WorkflowSettingsBuilder::new() + .server_manifest_defaults(server_run_defaults, server_environment_defaults) + .server_mcp_catalog(server_mcp_catalog); + if let Some(run) = run_overrides { + builder = builder.run_overrides(run); + } + if let Some(cli) = cli_overrides { + builder = builder.cli_overrides(cli); + } + if let Some(layer) = workflow_layer { + builder = builder.workflow_layer(layer); + } + for layer in project_layers { + builder = builder.project_layer(layer); + } + for source in user_toml { + builder = builder + .user_toml(&source) + .map_err(|source| invalid_settings(InvalidSettingsError::User(source)))?; + } + let mut settings = builder + .build() + .map_err(|source| invalid_settings(InvalidSettingsError::Resolve { source }))?; + settings.run.inputs.extend(input_overrides); + if let Some(goal) = inline_goal_override { + settings.run.goal = Some(RunGoal::Inline(InterpString::parse(&goal))); + } + + Ok(LayeredRun { + workflow_bundle, + entrypoint, + workflow, + settings, + cwd, + metadata, + }) +} + +/// Apply a run-variable snapshot to the layered settings and validate the +/// resulting artifact globs. The snapshot is also retained for graph template +/// rendering during compilation. +pub(crate) fn apply_run_variables( + mut layered: LayeredRun, + vars: HashMap, +) -> Result { + substitute_run_variables(&vars, &mut layered.settings)?; + Ok(PreparedRun { layered, vars }) +} + +/// Compile and validate the graph, then pin run-level model settings, in one +/// dispatch on Tokio's blocking pool: graph compilation is CPU-heavy and may +/// read a goal file, and pinning is pure CPU that belongs alongside it. +pub(crate) async fn compile_and_pin( + prepared: PreparedRun, + configured_providers: Vec, + catalog: Arc, +) -> Result { + task::spawn_blocking(move || { + let compiled = compile_graph(prepared, configured_providers, Arc::clone(&catalog))?; + pin_models(compiled, &catalog) + }) + .await + .map_err(|source| { + RunCompilerError::Workflow(WorkflowError::engine_with_source( + "workflow create task failed", + source, + )) + })? +} + +/// Stage two's graph compilation: parse, transform, and validate through the +/// fabro-workflow pipeline, with undefined template variables promoted to +/// hard errors. +fn compile_graph( + prepared: PreparedRun, + configured_providers: Vec, + catalog: Arc, +) -> Result { + let PreparedRun { + layered: + LayeredRun { + workflow_bundle, + entrypoint, + workflow, + settings, + cwd, + metadata, + }, + vars, + } = prepared; + let compiled = operations::compile_create_run( + CreateRunCompileInput { + workflow: WorkflowInput::Bundled(workflow), + settings, + vars, + cwd, + workflow_path: Some(entrypoint), + workflow_bundle: Some(workflow_bundle), + configured_providers, + }, + catalog, + )?; + + Ok(GraphCompiledRun { compiled, metadata }) +} + +/// Stage three: pin concrete model and provider selections against the +/// catalog and the configured provider set. +fn pin_models(compiled: GraphCompiledRun, catalog: &Catalog) -> Result { + let GraphCompiledRun { compiled, metadata } = compiled; + let materialized = operations::materialize_create_run(compiled, catalog)?; + Ok(PinnedRun { + materialized, + metadata, + }) +} + +/// Stage four: purely assemble the complete persistence input. Every durable +/// field — run id, submitted source bytes, automation reference — is set here +/// once; nothing mutates the result afterwards. +pub(crate) fn assemble_run(pinned: PinnedRun) -> CreateRunPersistenceInput { + let PinnedRun { + materialized, + metadata, + } = pinned; + let RunMetadata { + run_id, + storage_root, + workflow_slug, + submitted_manifest_bytes, + title, + automation, + git, + parent_id, + provenance, + web_url, + } = metadata; + operations::assemble_create_run_persistence_input(materialized, CreateRunPersistenceMetadata { + run_id: run_id.expect("run ID should be resolved before compilation"), + storage_root, + workflow_slug, + submitted_manifest_bytes, + title, + automation, + git, + fork_source_ref: None, + parent_id, + provenance, + web_url, + }) +} + +/// Parse one bundle-relative settings source, rejecting keys that are not +/// allowed for `settings_source` and inlining dockerfile references from the +/// bundled files. +/// +/// Parses via [`SettingsLayer`] so unknown nested keys (like a stale +/// `[server.integrations.github.permissions]` after the move to +/// `[run.integrations.github.permissions]`) trip `deny_unknown_fields`. +pub(crate) fn settings_layer_with_resolved_dockerfiles( + source: &str, + config_path: &ManifestPath, + files: &HashMap, + settings_source: SettingsSource, +) -> Result { + let parse_error = |source| { + invalid_settings(InvalidSettingsError::Parse { + path: config_path.clone(), + source, + }) + }; + let mut layer = source.parse::().map_err(parse_error)?; + parse::validate_settings_source(&layer, settings_source).map_err(parse_error)?; + resolve_dockerfiles(&mut layer, config_path, files)?; + Ok(layer) +} + +fn resolve_dockerfiles( + layer: &mut SettingsLayer, + config_path: &ManifestPath, + files: &HashMap, +) -> Result<()> { + for environment in layer.environments.values_mut() { + if let Some(image) = environment.image.as_mut() { + resolve_dockerfile(image, config_path, files)?; + } + } + if let Some(image) = layer + .run + .as_mut() + .and_then(|run| run.environment.as_mut()) + .and_then(|environment| environment.image.as_mut()) + { + resolve_dockerfile(image, config_path, files)?; + } + Ok(()) +} + +fn resolve_dockerfile( + image: &mut EnvironmentImageLayer, + config_path: &ManifestPath, + files: &HashMap, +) -> Result<()> { + let Some(EnvironmentDockerfileLayer::Path { path }) = image.dockerfile.as_ref() else { + return Ok(()); + }; + let reference = path.clone(); + let dockerfile_path = ManifestPath::from_reference(config_path.parent_or_dot(), &reference) + .ok_or_else(|| InvalidSourceError::UnsupportedDockerfileReference { + config_path: config_path.clone(), + reference: reference.clone(), + })?; + let content = files.get(&dockerfile_path).cloned().ok_or_else(|| { + InvalidSourceError::MissingDockerfile { + config_path: config_path.clone(), + dockerfile_path: dockerfile_path.clone(), + } + })?; + image.dockerfile = Some(EnvironmentDockerfileLayer::Inline(content)); + Ok(()) +} + +/// Substitute run-scoped variables into the resolved run settings, then +/// re-validate the artifact-include globs: a substituted variable can make a +/// previously-safe glob unsafe. +pub(crate) fn substitute_run_variables( + variables: &HashMap, + settings: &mut WorkflowSettings, +) -> std::result::Result<(), VariableInterpolationError> { + settings + .run + .substitute_variables(|name| variables.get(name).cloned())?; + for (index, pattern) in settings.run.artifacts.include.iter().enumerate() { + WorkspaceGlob::try_new(pattern) + .map_err(|source| VariableInterpolationError::ArtifactGlob { index, source })?; + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + use std::error::Error as _; + + use fabro_config::EnvironmentDockerfileLayer; + use fabro_graphviz::graph::AttrValue; + use fabro_model::Catalog; + use fabro_types::settings::interp::ResolveCtx; + use fabro_types::settings::run::RunGoal; + use fabro_types::{AutomationRef, Principal, RunProvenance, SystemActorKind}; + use fabro_workflow::workflow_bundle::ParsedWorkflowConfig; + + use super::*; + + const DOT: &str = r#"digraph Test { + graph [goal="Graph goal"] + start [shape=Mdiamond] + work [prompt="Ship {{ inputs.target }} for {{ vars.owner }}", model="gpt-5.4"] + exit [shape=Msquare] + start -> work -> exit + }"#; + + fn manifest_path(value: &str) -> ManifestPath { + ManifestPath::from_wire(value).expect("fixture manifest path should be valid") + } + + fn provenance() -> RunProvenance { + RunProvenance { + server: None, + client: None, + subject: Principal::System { + system_kind: SystemActorKind::Engine, + }, + } + } + + fn workflow( + entrypoint: &ManifestPath, + workflow_toml: Option<&str>, + files: HashMap, + ) -> BundledWorkflow { + BundledWorkflow { + path: entrypoint.clone(), + source: DOT.to_string(), + config: workflow_toml.map(|source| ParsedWorkflowConfig { + path: manifest_path("flows/workflow.toml"), + source: source.to_string(), + }), + files, + } + } + + fn raw_input( + workflow_toml: Option<&str>, + files: HashMap, + ) -> RawRunCompilerInput { + let entrypoint = manifest_path("flows/workflow.fabro"); + let workflow = workflow(&entrypoint, workflow_toml, files); + RawRunCompilerInput { + workflow_bundle: WorkflowBundle::new(HashMap::from([(entrypoint.clone(), workflow)])), + entrypoint, + cwd: PathBuf::from("/workspace"), + server_run_defaults: RunLayer::default(), + server_environment_defaults: fabro_environment::seeded_catalog_layer(), + server_mcp_catalog: HashMap::new(), + project_settings: Vec::new(), + user_toml: Vec::new(), + run_overrides: None, + cli_overrides: None, + input_overrides: HashMap::new(), + inline_goal_override: None, + run_id: Some(RunId::new()), + title: None, + parent_id: None, + git: None, + storage_root: PathBuf::from("/tmp/fabro-storage"), + workflow_slug: None, + provenance: provenance(), + web_url: None, + submitted_manifest_bytes: None, + automation: None, + } + } + + fn test_provider_ids() -> Vec { + Catalog::builtin().all_provider_ids().into_iter().collect() + } + + fn prepare_run( + input: RawRunCompilerInput, + vars: HashMap, + ) -> Result { + apply_run_variables(layer_settings(normalize_source(input)?)?, vars) + } + + #[test] + fn normalize_source_rejects_missing_entrypoint() { + let mut input = raw_input(None, HashMap::new()); + input.entrypoint = manifest_path("flows/missing.fabro"); + + let Err(error) = normalize_source(input) else { + panic!("missing entrypoint should fail"); + }; + + assert!(matches!( + error, + RunCompilerError::InvalidSource(InvalidSourceError::MissingEntrypoint { .. }) + )); + } + + #[test] + fn normalize_source_rejects_missing_dockerfile_with_pinned_message() { + let workflow_toml = r#" +_version = 1 + +[run.environment.image] +dockerfile = { path = "Dockerfile" } +"#; + + let Err(error) = normalize_source(raw_input(Some(workflow_toml), HashMap::new())) else { + panic!("missing dockerfile should fail"); + }; + + assert!(matches!( + error, + RunCompilerError::InvalidSource(InvalidSourceError::MissingDockerfile { .. }) + )); + assert_eq!( + error.to_string(), + "missing bundled dockerfile: flows/Dockerfile" + ); + } + + #[test] + fn normalize_source_preserves_settings_parse_source_chain() { + let workflow_toml = r#" +_version = 1 + +[run.unknown-table] +key = "value" +"#; + + let Err(error) = normalize_source(raw_input(Some(workflow_toml), HashMap::new())) else { + panic!("unknown settings key should fail"); + }; + + assert_eq!(error.to_string(), "Failed to parse run config TOML"); + let source = error + .source() + .expect("parse error should retain the TOML source"); + assert!(source.to_string().contains("unknown")); + } + + #[test] + fn normalize_source_resolves_bundled_dockerfile() { + let workflow_toml = r#" +_version = 1 + +[run.environment.image] +dockerfile = { path = "Dockerfile" } +"#; + let normalized = normalize_source(raw_input( + Some(workflow_toml), + HashMap::from([( + manifest_path("flows/Dockerfile"), + "FROM ubuntu:24.04\n".to_string(), + )]), + )) + .expect("bundled dockerfile should resolve"); + let dockerfile = normalized + .workflow_layer + .as_ref() + .and_then(|layer| layer.run.as_ref()) + .and_then(|run| run.environment.as_ref()) + .and_then(|environment| environment.image.as_ref()) + .and_then(|image| image.dockerfile.as_ref()); + + assert_eq!( + dockerfile, + Some(&EnvironmentDockerfileLayer::Inline( + "FROM ubuntu:24.04\n".to_string() + )) + ); + } + + #[test] + fn settings_apply_precedence_vars_inputs_and_safe_artifact_globs() { + let workflow_toml = r#" +_version = 1 + +[run.metadata] +layer = "workflow" +owner = "{{ vars.owner }}" + +[run.inputs] +target = "workflow" + +[run.artifacts] +include = ["reports/{{ vars.owner }}/*.json"] +"#; + let mut input = raw_input(Some(workflow_toml), HashMap::new()); + input.project_settings.push(ProjectSettingsSource { + path: Ok(manifest_path(".fabro/project.toml")), + toml: r#" +_version = 1 + +[run.metadata] +layer = "project" +"# + .to_string(), + }); + input.user_toml = vec![ + r#" +_version = 1 + +[run.metadata] +layer = "user" +"# + .to_string(), + ]; + input.run_overrides = Some( + toml::from_str::( + r#" +_version = 1 + +[run.metadata] +layer = "args" +owner = "{{ vars.owner }}" +"#, + ) + .expect("args settings should parse") + .run + .expect("args run layer should exist"), + ); + input.input_overrides.insert( + "target".to_string(), + toml::Value::String("override".to_string()), + ); + input.inline_goal_override = Some("Ship {{ vars.owner }}".to_string()); + + let prepared = prepare_run( + input, + HashMap::from([("owner".to_string(), "payments".to_string())]), + ) + .expect("settings should prepare"); + let settings = prepared.settings(); + + assert_eq!( + settings.run.metadata.get("layer").map(String::as_str), + Some("args") + ); + assert_eq!( + settings.run.metadata.get("owner").map(String::as_str), + Some("payments") + ); + assert_eq!( + settings.run.inputs.get("target"), + Some(&toml::Value::String("override".to_string())) + ); + assert_eq!(settings.run.artifacts.include, vec![ + "reports/payments/*.json" + ]); + let Some(RunGoal::Inline(goal)) = settings.run.goal.as_ref() else { + panic!("inline goal override should win"); + }; + assert_eq!( + goal.resolve_with(&mut ResolveCtx::default()).unwrap(), + "Ship payments" + ); + } + + #[test] + fn settings_reject_artifact_glob_made_unsafe_by_variable() { + let workflow_toml = r#" +_version = 1 + +[run.artifacts] +include = ["reports/{{ vars.path }}/*.json"] +"#; + let input = raw_input(Some(workflow_toml), HashMap::new()); + + let Err(error) = prepare_run( + input, + HashMap::from([("path".to_string(), "../secrets".to_string())]), + ) else { + panic!("unsafe artifact glob should fail"); + }; + + assert!(matches!( + error, + RunCompilerError::VariableInterpolation(VariableInterpolationError::ArtifactGlob { + index: 0, + source: WorkspaceGlobError::ParentTraversal { .. }, + }) + )); + } + + #[test] + fn graph_vars_are_hard_errors_and_successfully_render_when_present() { + let catalog = Arc::new(Catalog::from_builtin().unwrap()); + let missing = prepare_run(raw_input(None, HashMap::new()), HashMap::new()) + .expect("settings preparation should not compile graph vars"); + let Err(error) = compile_graph(missing, test_provider_ids(), Arc::clone(&catalog)) else { + panic!("missing graph variable should be a hard error"); + }; + assert!(matches!( + error, + RunCompilerError::Workflow(WorkflowError::ValidationFailed { .. }) + )); + + let mut input = raw_input(None, HashMap::new()); + input.input_overrides.insert( + "target".to_string(), + toml::Value::String("checkout".to_string()), + ); + let prepared = prepare_run( + input, + HashMap::from([("owner".to_string(), "payments".to_string())]), + ) + .expect("settings should prepare"); + let compiled = compile_graph(prepared, test_provider_ids(), catalog) + .expect("graph variables should render"); + let work = &compiled.compiled.validated().graph().nodes["work"]; + + assert_eq!( + work.attrs.get("prompt").and_then(AttrValue::as_str), + Some("Ship checkout for payments") + ); + assert_eq!( + work.attrs.get("provider").and_then(AttrValue::as_str), + Some("openai") + ); + } + + #[test] + fn assembly_retains_entrypoint_and_run_metadata() { + let run_id = RunId::new(); + let parent_id = RunId::new(); + let automation = AutomationRef { + id: "nightly".to_string(), + name: Some("Nightly".to_string()), + trigger_id: Some("schedule".to_string()), + }; + let submitted = b"submitted manifest".to_vec(); + let mut input = raw_input(None, HashMap::new()); + input.run_id = Some(run_id); + input.parent_id = Some(parent_id); + input.title = Some("Compiler boundary".to_string()); + input.workflow_slug = Some("compiler-boundary".to_string()); + input.web_url = Some(format!("https://fabro.test/runs/{run_id}")); + input.submitted_manifest_bytes = Some(submitted.clone()); + input.automation = Some(automation.clone()); + input.input_overrides.insert( + "target".to_string(), + toml::Value::String("checkout".to_string()), + ); + let expected_entrypoint = input.entrypoint.clone(); + let catalog = Arc::new(Catalog::from_builtin().unwrap()); + + let prepared = prepare_run( + input, + HashMap::from([("owner".to_string(), "payments".to_string())]), + ) + .expect("settings should prepare"); + let compiled = compile_graph(prepared, test_provider_ids(), Arc::clone(&catalog)) + .expect("graph should compile"); + let pinned = pin_models(compiled, &catalog).expect("models should pin"); + let persistence = assemble_run(pinned); + + assert_eq!(persistence.run_id(), run_id); + assert_eq!(persistence.workflow_slug(), Some("compiler-boundary")); + assert_eq!( + persistence.submitted_manifest_bytes(), + Some(submitted.as_slice()) + ); + assert_eq!(persistence.automation(), Some(&automation)); + assert_eq!( + persistence + .definition() + .map(|definition| &definition.workflow_path), + Some(&expected_entrypoint) + ); + assert_eq!( + persistence.materialized().settings().run.goal.as_ref(), + Some(&RunGoal::Inline(InterpString::parse("Graph goal"))) + ); + } +} diff --git a/lib/apps/fabro-server/src/run_manifest.rs b/lib/apps/fabro-server/src/run_manifest.rs index ac3b94071..642199d54 100644 --- a/lib/apps/fabro-server/src/run_manifest.rs +++ b/lib/apps/fabro-server/src/run_manifest.rs @@ -7,11 +7,10 @@ use std::time::Duration; use anyhow::{Context as _, Result, anyhow, bail}; use fabro_api::types; use fabro_auth::auth_issue_message; -use fabro_config::parse::{self, SettingsSource}; +use fabro_config::parse::SettingsSource; use fabro_config::{ - CliLayer, CliOutputLayer, EnvironmentDockerfileLayer, EnvironmentImageLayer, EnvironmentLayer, - MergeMap, RunLayer, SettingsLayer, WorkflowSettingsBuilder, parse_input_overrides, - parse_labels, project, + CliLayer, CliOutputLayer, EnvironmentLayer, MergeMap, RunLayer, SettingsLayer, + WorkflowSettingsBuilder, parse_input_overrides, parse_labels, project, }; use fabro_graphviz::graph::{Graph, is_llm_handler_type}; use fabro_graphviz::render::apply_direction; @@ -30,16 +29,14 @@ use fabro_types::settings::cli::OutputVerbosity; use fabro_types::settings::interp::InterpString; use fabro_types::settings::run::{EnvironmentProvider, McpServerSettings, RunGoal, RunNamespace}; use fabro_types::{ - ManifestPath, RunId, RunNoticeLevel, RunProvenance, SandboxProviderKind, ServerSettings, - WorkflowSettings, + ManifestPath, RunId, RunNoticeLevel, SandboxProviderKind, ServerSettings, WorkflowSettings, }; use fabro_util::check_report::{CheckDetail, CheckReport, CheckResult, CheckSection, CheckStatus}; use fabro_validate::Severity; use fabro_workflow::Error as WorkflowError; use fabro_workflow::model_fallback::resolve_model_fallbacks; use fabro_workflow::operations::{ - CreateRunInput, ValidateInput, WorkflowInput, validate, validate_with_catalog, - validate_with_ready_providers, + ValidateInput, WorkflowInput, validate, validate_with_catalog, validate_with_ready_providers, }; use fabro_workflow::pipeline::Validated; use fabro_workflow::run_materialization::materialize_run_with_ready_providers; @@ -48,6 +45,7 @@ use futures_util::stream::{self, StreamExt}; use tokio::process::Command; use tokio::time; +use crate::run_compiler; use crate::server::AppState; use crate::server_secrets::LlmClientResult; @@ -56,21 +54,17 @@ pub(crate) struct PreparedManifest { pub cwd: PathBuf, pub git: Option, pub root_source: String, - pub run_id: Option, - pub parent_id: Option, - pub title: Option, pub settings: WorkflowSettings, pub target_path: ManifestPath, - pub workflow_bundle: WorkflowBundle, pub workflow_input: BundledWorkflow, pub source_directory: PathBuf, } #[derive(Clone, Debug, Default)] -struct ManifestSettingsOverrides { - run: Option, - cli: Option, - input_overrides: HashMap, +pub(crate) struct ManifestSettingsOverrides { + pub(crate) run: Option, + pub(crate) cli: Option, + pub(crate) input_overrides: HashMap, } #[cfg(test)] @@ -157,34 +151,33 @@ pub(crate) fn prepare_manifest_with_environment_defaults( { settings.run.goal = Some(RunGoal::Inline(InterpString::parse(&goal.text))); } - let title = manifest + manifest .title .as_ref() .map(|title| fabro_types::normalize_explicit_run_title(title.as_str())) .transpose()?; + manifest + .run_id + .as_deref() + .map(str::parse::) + .transpose() + .context("invalid run ID")?; + manifest + .parent_id + .as_deref() + .map(str::parse::) + .transpose() + .context("invalid parent run ID")?; + let source_directory = project::resolve_working_directory_from_run(&settings.run, &cwd); Ok(PreparedManifest { - cwd: cwd.clone(), + cwd, git: manifest.git.clone(), root_source, - run_id: manifest - .run_id - .as_deref() - .map(str::parse::) - .transpose() - .context("invalid run ID")?, - parent_id: manifest - .parent_id - .as_deref() - .map(str::parse::) - .transpose() - .context("invalid parent run ID")?, - title, - settings: settings.clone(), + settings, target_path, - workflow_bundle, workflow_input, - source_directory: project::resolve_working_directory_from_run(&settings.run, &cwd), + source_directory, }) } @@ -235,34 +228,6 @@ fn manifest_validate_input( } } -pub(crate) fn create_run_input( - prepared: PreparedManifest, - configured_providers: Vec, - provenance: RunProvenance, - web_url: Option, - vars: HashMap, -) -> CreateRunInput { - CreateRunInput { - workflow: WorkflowInput::Bundled(prepared.workflow_input), - settings: prepared.settings, - vars, - cwd: prepared.cwd, - workflow_slug: None, - workflow_path: Some(prepared.target_path), - workflow_bundle: Some(prepared.workflow_bundle), - submitted_manifest_bytes: None, - run_id: prepared.run_id, - title: prepared.title, - automation: None, - git: prepared.git, - fork_source_ref: None, - parent_id: prepared.parent_id, - provenance, - configured_providers, - web_url, - } -} - pub(crate) async fn run_preflight( state: &AppState, prepared: &PreparedManifest, @@ -375,19 +340,16 @@ fn settings_layer_with_resolved_dockerfiles( files: &HashMap, settings_source: SettingsSource, ) -> Result { - // Parse via `SettingsLayer` so unknown nested keys (like a stale - // `[server.integrations.github.permissions]` after the move to - // `[run.integrations.github.permissions]`) trip `deny_unknown_fields`. - let mut layer = source - .parse::() - .context("Failed to parse run config TOML")?; - parse::validate_settings_source(&layer, settings_source) - .context("Failed to parse run config TOML")?; - resolve_manifest_dockerfiles(&mut layer, config_path, files)?; - Ok(layer) + run_compiler::settings_layer_with_resolved_dockerfiles( + source, + config_path, + files, + settings_source, + ) + .map_err(anyhow::Error::new) } -fn manifest_args_overrides( +pub(crate) fn manifest_args_overrides( args: Option<&types::ManifestArgs>, ) -> Result { let Some(args) = args else { @@ -399,7 +361,6 @@ fn manifest_args_overrides( model: args.model.as_deref(), provider: args.provider.as_deref(), environment: args.environment.as_deref(), - docker_image: args.docker_image.as_deref(), preserve_sandbox: args.preserve_sandbox, dry_run: args.dry_run, auto_approve: args.auto_approve, @@ -424,50 +385,6 @@ fn manifest_args_overrides( }) } -fn resolve_manifest_dockerfiles( - layer: &mut SettingsLayer, - config_path: &ManifestPath, - files: &HashMap, -) -> Result<()> { - for environment in layer.environments.values_mut() { - if let Some(image) = environment.image.as_mut() { - resolve_manifest_dockerfile(image, config_path, files)?; - } - } - if let Some(image) = layer - .run - .as_mut() - .and_then(|run| run.environment.as_mut()) - .and_then(|environment| environment.image.as_mut()) - { - resolve_manifest_dockerfile(image, config_path, files)?; - } - Ok(()) -} - -fn resolve_manifest_dockerfile( - image: &mut EnvironmentImageLayer, - config_path: &ManifestPath, - files: &HashMap, -) -> Result<()> { - let source = image.dockerfile.as_mut(); - let Some(source) = source else { - return Ok(()); - }; - let EnvironmentDockerfileLayer::Path { path } = &*source else { - return Ok(()); - }; - let path_owned = path.clone(); - let manifest_path = ManifestPath::from_reference(config_path.parent_or_dot(), &path_owned) - .ok_or_else(|| anyhow!("unsupported dockerfile reference: {path_owned}"))?; - let content = files - .get(&manifest_path) - .cloned() - .ok_or_else(|| anyhow!("missing bundled dockerfile: {manifest_path}"))?; - *source = EnvironmentDockerfileLayer::Inline(content); - Ok(()) -} - fn manifest_project_config_path( config: &types::ManifestConfig, cwd: &Path, @@ -2149,7 +2066,6 @@ root = "/srv/fabro" preserve_sandbox: None, provider: None, environment: None, - docker_image: None, input: Vec::new(), verbose: None, }); @@ -2182,7 +2098,6 @@ override = "server" preserve_sandbox: None, provider: None, environment: None, - docker_image: None, input: vec!["override=cli".to_string()], verbose: None, }); diff --git a/lib/apps/fabro-server/src/run_tool_manifest.rs b/lib/apps/fabro-server/src/run_tool_manifest.rs index 584b6586f..32cb14bd3 100644 --- a/lib/apps/fabro-server/src/run_tool_manifest.rs +++ b/lib/apps/fabro-server/src/run_tool_manifest.rs @@ -58,7 +58,6 @@ pub fn run_tool_manifest_args(spec: &ValidatedCreateRunSpec) -> Option Option model: spec.model.as_deref(), provider: spec.provider.as_deref(), environment: spec.environment.as_deref(), - docker_image: None, preserve_sandbox: spec.preserve_sandbox, dry_run: spec.dry_run, auto_approve: spec.auto_approve, diff --git a/lib/apps/fabro-server/src/server/handler/runs.rs b/lib/apps/fabro-server/src/server/handler/runs.rs index 0327d0cda..6048ed046 100644 --- a/lib/apps/fabro-server/src/server/handler/runs.rs +++ b/lib/apps/fabro-server/src/server/handler/runs.rs @@ -1,7 +1,9 @@ use std::collections::{HashMap, HashSet}; use std::io::ErrorKind; +use std::path::PathBuf; use std::sync::Arc; +use anyhow::Context as _; use axum::extract::{Path, Query, State}; use axum::http::{HeaderMap, StatusCode, header}; use axum::response::{IntoResponse, Response}; @@ -13,25 +15,25 @@ use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; use bytes::Bytes; use chrono::{DateTime, Utc}; use fabro_api::types::{ - BoardColumn, RunManifest, SubmitAnswerRequest, UpdateRunParentRequest, UpdateRunRequest, + BoardColumn, ManifestConfigType, ManifestGoalType, RunManifest, SubmitAnswerRequest, + UpdateRunParentRequest, UpdateRunRequest, }; -use fabro_config::Storage; +use fabro_config::{CliLayer, RunLayer, Storage}; use fabro_interview::AnswerSubmission; use fabro_llm::client::Client as LlmClient; use fabro_store::{ RunSummaryListQuery, RunSummarySort, RunSummarySortDirection, RunSummaryVisibility, }; -use fabro_types::settings::ResolveError; use fabro_types::{ - AutomationRef, Principal, Run, RunClientProvenance, RunId, RunProvenance, RunServerProvenance, - RunStatusKind, StageContextWindow, StageContextWindowStaleness, + AutomationRef, ManifestPath, Principal, Run, RunClientProvenance, RunId, RunProvenance, + RunServerProvenance, RunStatusKind, StageContextWindow, StageContextWindowStaleness, StageContextWindowUnavailableReason, StageHandler, StageModelUsage, StageProjection, - SystemActorKind, WorkflowSettings, parse_blob_ref, + SystemActorKind, parse_blob_ref, }; use fabro_util::version::FABRO_VERSION; -use fabro_util::workspace_glob::{WorkspaceGlob, WorkspaceGlobError}; use fabro_workflow::command_log::{command_log_path, read_json_string_blob, read_log_slice}; use fabro_workflow::run_status::RunStatus; +use fabro_workflow::workflow_bundle::WorkflowBundle; use fabro_workflow::{Error as WorkflowError, operations}; use strum::VariantArray as _; use tokio::fs; @@ -48,6 +50,9 @@ use crate::principal_middleware::{ RequireCommandLog, RequireRunManagementTarget, RequireRunScoped, RequireRunStageScoped, RequiredRunManagementActor, RequiredUser, }; +use crate::run_compiler::{ + self, ProjectSettingsPathError, ProjectSettingsSource, RawRunCompilerInput, RunCompilerError, +}; use crate::run_files::{list_run_commits, list_run_files}; use crate::run_manifest; use crate::run_selector::{ResolveRunError, resolve_run_by_selector}; @@ -552,6 +557,143 @@ pub(crate) struct CreateRunFromManifestRequest { pub(crate) automation: Option, } +struct ManifestRunCompilerAdapter { + workflow_bundle: WorkflowBundle, + entrypoint: ManifestPath, + cwd: PathBuf, + project_settings: Vec, + user_toml: Vec, + run_overrides: Option, + cli_overrides: Option, + input_overrides: HashMap, + inline_goal_override: Option, +} + +fn adapt_manifest_source_for_run_compiler( + manifest: &RunManifest, +) -> anyhow::Result { + if manifest.version != 1 { + anyhow::bail!("unsupported manifest version {}", manifest.version); + } + let cwd = PathBuf::from(&manifest.cwd); + let entrypoint = ManifestPath::from_wire(&manifest.target.path) + .ok_or_else(|| anyhow::anyhow!("invalid manifest target path: {}", manifest.target.path))?; + let workflow_bundle = run_manifest::workflow_bundle_from_manifest(&manifest.workflows)?; + if workflow_bundle.workflow(&entrypoint).is_none() { + anyhow::bail!("manifest target path is missing from workflows map"); + } + let overrides = run_manifest::manifest_args_overrides(manifest.args.as_ref()) + .context("failed to parse manifest args")?; + let project_settings = manifest + .configs + .iter() + .filter(|config| config.type_ == ManifestConfigType::Project) + .filter_map(|config| config.source.as_ref().map(|source| (config, source))) + .map(|(config, source)| ProjectSettingsSource { + path: normalize_project_settings_path(config.path.as_deref(), &cwd), + toml: source.clone(), + }) + .collect(); + let user_toml = manifest + .configs + .iter() + .filter(|config| config.type_ == ManifestConfigType::User) + .filter_map(|config| config.source.clone()) + .collect(); + let inline_goal_override = manifest + .goal + .as_ref() + .filter(|goal| goal.type_ != ManifestGoalType::Graph) + .map(|goal| goal.text.clone()); + + Ok(ManifestRunCompilerAdapter { + workflow_bundle, + entrypoint, + cwd, + project_settings, + user_toml, + run_overrides: overrides.run, + cli_overrides: overrides.cli, + input_overrides: overrides.input_overrides, + inline_goal_override, + }) +} + +fn normalize_project_settings_path( + path: Option<&str>, + cwd: &std::path::Path, +) -> Result { + let path = path.ok_or(ProjectSettingsPathError::Missing)?; + let path_ref = std::path::Path::new(path); + let manifest_path = if path_ref.is_absolute() { + ManifestPath::from_absolute(path_ref, cwd) + } else { + ManifestPath::from_wire(path) + }; + manifest_path.ok_or_else(|| ProjectSettingsPathError::Invalid { + path: path.to_string(), + }) +} + +struct ManifestRunIdentity { + run_id: Option, + parent_id: Option, + title: Option, +} + +fn manifest_run_identity( + manifest: &RunManifest, + explicit_run_id: Option, +) -> anyhow::Result { + let title = manifest + .title + .as_ref() + .map(|title| fabro_types::normalize_explicit_run_title(title.as_str())) + .transpose()?; + let manifest_run_id = manifest + .run_id + .as_deref() + .map(str::parse::) + .transpose() + .context("invalid run ID")?; + let parent_id = manifest + .parent_id + .as_deref() + .map(str::parse::) + .transpose() + .context("invalid parent run ID")?; + Ok(ManifestRunIdentity { + run_id: explicit_run_id.or(manifest_run_id), + parent_id, + title, + }) +} + +/// Map a [`RunCompilerError`] onto the create endpoint's pre-extraction wire +/// contract. The 400 details for source, settings, and interpolation errors +/// are the error types' own `Display` strings, which are pinned to the +/// legacy messages. +fn run_compiler_error_response(error: RunCompilerError) -> Response { + match error { + RunCompilerError::InvalidSource(_) + | RunCompilerError::InvalidSettings(_) + | RunCompilerError::VariableInterpolation(_) => { + ApiError::bad_request(error.to_string()).into_response() + } + RunCompilerError::Workflow( + WorkflowError::ValidationFailed { .. } | WorkflowError::Parse(_), + ) => ApiError::bad_request("Validation failed").into_response(), + RunCompilerError::Workflow( + err @ (WorkflowError::ModelSelection(_) | WorkflowError::ModelReference(_)), + ) => ApiError::bad_request(err.to_string()).into_response(), + RunCompilerError::Workflow(err) => ApiError::new( + StatusCode::INTERNAL_SERVER_ERROR, + format!("Failed to persist run state: {err}"), + ) + .into_response(), + } +} + pub(crate) async fn create_run_from_manifest( state: Arc, request: CreateRunFromManifestRequest, @@ -568,15 +710,43 @@ pub(crate) async fn create_run_from_manifest( let manifest_run_defaults = state.manifest_run_defaults(); let manifest_environment_defaults = state.environment_store().catalog_layer(); let manifest_mcp_server_catalog = state.mcp_server_store().catalog_settings(); - let mut prepared = match run_manifest::prepare_manifest_with_environment_defaults( - manifest_run_defaults.as_ref(), - manifest_environment_defaults.as_ref(), - &manifest_mcp_server_catalog, - &manifest, - ) { - Ok(prepared) => prepared, + let manifest_adapter = match adapt_manifest_source_for_run_compiler(&manifest) { + Ok(adapter) => adapter, Err(err) => return ApiError::bad_request(err.to_string()).into_response(), }; + let title_generation_target = manifest_adapter.entrypoint.clone(); + let raw_compiler_input = RawRunCompilerInput { + workflow_bundle: manifest_adapter.workflow_bundle, + entrypoint: manifest_adapter.entrypoint, + cwd: manifest_adapter.cwd, + server_run_defaults: manifest_run_defaults.as_ref().clone(), + server_environment_defaults: manifest_environment_defaults.as_ref().clone(), + server_mcp_catalog: manifest_mcp_server_catalog, + project_settings: manifest_adapter.project_settings, + user_toml: manifest_adapter.user_toml, + run_overrides: manifest_adapter.run_overrides, + cli_overrides: manifest_adapter.cli_overrides, + input_overrides: manifest_adapter.input_overrides, + inline_goal_override: manifest_adapter.inline_goal_override, + run_id: None, + title: None, + parent_id: None, + git: manifest.git.clone(), + storage_root: state.server_storage_dir(), + workflow_slug: None, + provenance: run_provenance(&headers, &actor), + web_url: None, + submitted_manifest_bytes: Some(submitted_manifest_bytes), + automation, + }; + let normalized = match run_compiler::normalize_source(raw_compiler_input) { + Ok(normalized) => normalized, + Err(err) => return run_compiler_error_response(err), + }; + let layered = match run_compiler::layer_settings(normalized) { + Ok(layered) => layered, + Err(err) => return run_compiler_error_response(err), + }; let vars = match snapshot_run_variables(&state).await { Ok(vars) => vars, Err(err) => { @@ -584,20 +754,24 @@ pub(crate) async fn create_run_from_manifest( .into_response(); } }; - if let Err(err) = substitute_run_variables(&vars, &mut prepared.settings) { - return ApiError::bad_request(format!("Run config variable interpolation failed: {err}")) - .into_response(); - } - let run_id = explicit_run_id - .or(prepared.run_id) - .unwrap_or_else(RunId::new); - let provider = run_manifest::effective_sandbox_provider(&prepared.settings.run); + let prepared = match run_compiler::apply_run_variables(layered, vars) { + Ok(prepared) => prepared, + Err(err) => return run_compiler_error_response(err), + }; + let identity = match manifest_run_identity(&manifest, explicit_run_id) { + Ok(identity) => identity, + Err(err) => return ApiError::bad_request(err.to_string()).into_response(), + }; + let prepared = prepared.with_identity(identity.run_id, identity.parent_id, identity.title); + let (prepared, run_id) = prepared.resolve_run_id(); + let prepared = prepared.with_web_url(state.run_web_url(&run_id)); + let provider = run_manifest::effective_sandbox_provider(&prepared.settings().run); if let Some(error) = run_manifest::sandbox_provider_policy_error(&state.server_settings(), provider) { return ApiError::bad_request(error).into_response(); } - if let Some(parent_id) = prepared.parent_id { + if let Some(parent_id) = prepared.parent_id() { if parent_id == run_id { return ApiError::bad_request("A run cannot be its own parent.").into_response(); } @@ -607,7 +781,6 @@ pub(crate) async fn create_run_from_manifest( } info!(run_id = %run_id, "Run created"); - let web_url = state.run_web_url(&run_id); let catalog = state.catalog(); // Resolve once: we need both the provider IDs (for the run create input // and ask-fabro-readiness) and the LLM client itself (for the spawned @@ -628,34 +801,21 @@ pub(crate) async fn create_run_from_manifest( ready_provider_ids.clone() } }; - let provenance = run_provenance(&headers, &actor); - let mut create_input = run_manifest::create_run_input( - prepared.clone(), - run_materialization_provider_ids, - provenance, - web_url.clone(), - vars, - ); - create_input.run_id = Some(run_id); - create_input.submitted_manifest_bytes = Some(submitted_manifest_bytes); - create_input.automation = automation; - - let storage_root = state.server_storage_dir(); - let created = match Box::pin(operations::create( + let pinned = + match run_compiler::compile_and_pin(prepared, run_materialization_provider_ids, catalog) + .await + { + Ok(pinned) => pinned, + Err(err) => return run_compiler_error_response(err), + }; + let persistence_input = run_compiler::assemble_run(pinned); + let created = match Box::pin(operations::persist_create_run( state.stores.runs.as_ref(), - create_input, - storage_root, - catalog, + persistence_input, )) .await { Ok(created) => created, - Err(WorkflowError::ValidationFailed { .. } | WorkflowError::Parse(_)) => { - return ApiError::bad_request("Validation failed").into_response(); - } - Err(err @ (WorkflowError::ModelSelection(_) | WorkflowError::ModelReference(_))) => { - return ApiError::bad_request(err.to_string()).into_response(); - } Err(err) => { return ApiError::new( StatusCode::INTERNAL_SERVER_ERROR, @@ -699,7 +859,7 @@ pub(crate) async fn create_run_from_manifest( let run_spec = created.persisted.run_spec(); let workflow = run_title_generation::workflow_summary(&run_spec.graph); let run_inputs = run_spec.settings.run.inputs.clone(); - let workflow_target = prepared.target_path.to_string(); + let workflow_target = title_generation_target.to_string(); let title_catalog = state.catalog(); let title_model = title_catalog.small_default_for_configured_ids(&ready_provider_ids); let title_model_id = title_model.id.clone(); @@ -844,7 +1004,7 @@ async fn run_preflight( .into_response(); } }; - if let Err(err) = substitute_run_variables(&vars, &mut prepared.settings) { + if let Err(err) = run_compiler::substitute_run_variables(&vars, &mut prepared.settings) { return ApiError::bad_request(format!("Run config variable interpolation failed: {err}")) .into_response(); } @@ -897,7 +1057,7 @@ async fn validate_run_manifest( .into_response(); } }; - if let Err(err) = substitute_run_variables(&vars, &mut prepared.settings) { + if let Err(err) = run_compiler::substitute_run_variables(&vars, &mut prepared.settings) { return ApiError::bad_request(format!("Run config variable interpolation failed: {err}")) .into_response(); } @@ -925,33 +1085,6 @@ async fn snapshot_run_variables( state.stores.variables.value_map().await } -#[derive(Debug, thiserror::Error)] -enum RunVariableSubstitutionError { - #[error(transparent)] - Interpolation(#[from] ResolveError), - - #[error("run.artifacts.include[{index}]: {source}")] - ArtifactGlob { - index: usize, - #[source] - source: WorkspaceGlobError, - }, -} - -fn substitute_run_variables( - variables: &HashMap, - settings: &mut WorkflowSettings, -) -> Result<(), RunVariableSubstitutionError> { - settings - .run - .substitute_variables(|name| variables.get(name).cloned())?; - for (index, pattern) in settings.run.artifacts.include.iter().enumerate() { - WorkspaceGlob::try_new(pattern) - .map_err(|source| RunVariableSubstitutionError::ArtifactGlob { index, source })?; - } - Ok(()) -} - async fn get_run_status( RequireRunManagementTarget(id, _actor): RequireRunManagementTarget, State(state): State>, @@ -1254,26 +1387,3 @@ fn build_command_log_response( }) .into_response() } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn rejects_artifact_glob_that_becomes_unsafe_after_interpolation() { - let variables = HashMap::from([("PATTERN".to_string(), "../outside/**".to_string())]); - let mut settings = WorkflowSettings::default(); - settings.run.artifacts.include = vec!["{{ vars.PATTERN }}".to_string()]; - - let error = substitute_run_variables(&variables, &mut settings) - .expect_err("interpolated parent traversal should be rejected"); - - assert!(matches!( - error, - RunVariableSubstitutionError::ArtifactGlob { - index: 0, - source: WorkspaceGlobError::ParentTraversal { .. }, - } - )); - } -} diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs index cbe469354..2ed62e655 100644 --- a/lib/apps/fabro-server/src/server/tests.rs +++ b/lib/apps/fabro-server/src/server/tests.rs @@ -3660,6 +3660,348 @@ async fn create_run_from_manifest_helper_persists_automation_metadata() { assert_eq!(summary.automation, Some(automation)); } +#[tokio::test] +async fn create_run_from_manifest_pins_compiled_and_persisted_behavior() { + let state = TestAppStateBuilder::new() + .runtime_settings( + default_test_server_settings(), + manifest_run_defaults_from_toml( + r#" +[run.metadata] +server-label = "server" +layer = "server" +"#, + ), + ) + .env_lookup(|_| None) + .vault_entries([(EnvVars::OPENAI_API_KEY, "test-openai-api-key")]) + .build(); + let run_id = RunId::new(); + let dot = r#"digraph CompilePin { + graph [goal="Graph goal", target="{{ inputs.target }}"] + start [shape=Mdiamond] + work [prompt="Ship {{ inputs.target }}", model="gpt-5.4"] + exit [shape=Msquare] + start -> work -> exit + }"#; + let mut manifest_json = minimal_manifest_json(dot); + manifest_json["title"] = json!(" Pinned create "); + manifest_json["goal"] = json!({ + "type": "value", + "text": "Inline release goal" + }); + manifest_json["args"] = json!({ + "model": "gpt-5.4", + "input": ["target=payments"] + }); + manifest_json["configs"] = json!([{ + "type": "project", + "path": "/tmp/project/.fabro/project.toml", + "source": r#" +_version = 1 + +[project] +name = "payments-project" + +[run.metadata] +project-label = "project" +layer = "project" +"# + }]); + manifest_json["cwd"] = json!("/tmp/project"); + manifest_json["git"] = json!({ + "origin_url": "https://github.com/acme/payments.git", + "branch": "feature/compiler", + "sha": "0123456789abcdef", + "dirty": "clean", + "push_outcome": { "type": "not_attempted" } + }); + let manifest: RunManifest = serde_json::from_value(manifest_json).unwrap(); + let submitted_manifest_bytes = serde_json::to_vec(&manifest).unwrap(); + let mut headers = HeaderMap::new(); + headers.insert( + header::USER_AGENT, + "fabro-cli/9.8.7".parse().expect("user agent should parse"), + ); + + let response = Box::pin(handler::runs::create_run_from_manifest( + Arc::clone(&state), + handler::runs::CreateRunFromManifestRequest { + manifest, + submitted_manifest_bytes: submitted_manifest_bytes.clone(), + explicit_run_id: Some(run_id), + explicit_title_supplied: true, + actor: Principal::System { + system_kind: SystemActorKind::Engine, + }, + headers, + automation: None, + }, + )) + .await; + + let body = response_json!(response, StatusCode::CREATED).await; + assert_eq!(body["id"], run_id.to_string()); + assert_eq!(body["title"], "Pinned create"); + assert_eq!(body["lifecycle"]["status"]["kind"], "submitted"); + + let run_store = state.stores.runs.open_run_reader(&run_id).await.unwrap(); + let events = run_store.list_events().await.unwrap(); + assert_eq!( + events + .iter() + .map(|envelope| envelope.event.event_name()) + .collect::>(), + vec!["run.created", "run.submitted"] + ); + let run_state = run_store.state().await.unwrap(); + let spec = &run_state.spec; + assert_eq!(spec.run_id, run_id); + assert_eq!(spec.graph.goal(), "Inline release goal"); + assert_eq!( + spec.graph.attrs.get("target").and_then(AttrValue::as_str), + Some("{{ inputs.target }}") + ); + assert_eq!( + spec.graph.nodes["work"] + .attrs + .get("prompt") + .and_then(AttrValue::as_str), + Some("Ship payments") + ); + assert_eq!( + spec.graph.nodes["work"] + .attrs + .get("model") + .and_then(AttrValue::as_str), + Some("gpt-5.4") + ); + assert_eq!( + spec.graph.nodes["work"] + .attrs + .get("provider") + .and_then(AttrValue::as_str), + Some("openai") + ); + assert_eq!(spec.settings.run.model.name.as_deref(), Some("gpt-5.4")); + assert_eq!(spec.settings.run.model.provider.as_deref(), Some("openai")); + assert_eq!( + spec.settings.run.inputs.get("target"), + Some(&toml::Value::String("payments".to_string())) + ); + assert_eq!( + spec.settings.project.name.as_deref(), + Some("payments-project") + ); + assert_eq!( + spec.labels.get("project-label").map(String::as_str), + Some("project") + ); + assert_eq!( + spec.labels.get("layer").map(String::as_str), + Some("project") + ); + assert_eq!( + spec.git.as_ref().map(|git| git.origin_url.as_str()), + Some("https://github.com/acme/payments.git") + ); + + let created = events[0].event.to_value().unwrap(); + assert_eq!(created["properties"]["title"], "Pinned create"); + assert_eq!(created["properties"]["labels"]["project-label"], "project"); + assert_eq!( + created["properties"]["provenance"]["client"]["user_agent"], + "fabro-cli/9.8.7" + ); + assert_eq!( + created["properties"]["provenance"]["subject"]["kind"], + "system" + ); + let manifest_blob = created["properties"]["manifest_blob"] + .as_str() + .expect("run.created should carry the submitted source blob") + .parse::() + .unwrap(); + let persisted_manifest = run_store + .read_blob(&manifest_blob) + .await + .unwrap() + .expect("submitted source blob should exist"); + assert_eq!(persisted_manifest.as_ref(), submitted_manifest_bytes); +} + +#[tokio::test] +async fn create_run_from_manifest_pins_compiler_http_error_mappings() { + let cases = [ + ( + { + let mut manifest = minimal_manifest_json(MINIMAL_DOT); + manifest["version"] = json!(2); + manifest + }, + "unsupported manifest version 2", + ), + ( + minimal_manifest_json( + r#"digraph Test { + graph [goal="Test"] + start [shape=Mdiamond] + work [prompt="Use {{ vars.MISSING }}"] + exit [shape=Msquare] + start -> work -> exit + }"#, + ), + "Validation failed", + ), + ( + { + let mut manifest = minimal_manifest_json( + r#"digraph Test { + graph [goal="Test"] + start [shape=Mdiamond] + work [prompt="Do work", model="gpt-5.4", provider="missing-provider"] + exit [shape=Msquare] + start -> work -> exit + }"#, + ); + manifest["args"] = json!({ + "model": "gpt-5.4", + "provider": "missing-provider" + }); + manifest + }, + "Model selection failed: unknown model provider 'missing-provider'", + ), + ]; + + for (manifest_json, expected_detail) in cases { + let state = TestAppStateBuilder::new() + .env_lookup(|_| None) + .vault_entries([(EnvVars::OPENAI_API_KEY, "test-openai-api-key")]) + .build(); + let manifest: RunManifest = serde_json::from_value(manifest_json).unwrap(); + let submitted_manifest_bytes = serde_json::to_vec(&manifest).unwrap(); + + let response = Box::pin(handler::runs::create_run_from_manifest( + state, + handler::runs::CreateRunFromManifestRequest { + manifest, + submitted_manifest_bytes, + explicit_run_id: Some(RunId::new()), + explicit_title_supplied: true, + actor: Principal::System { + system_kind: SystemActorKind::Engine, + }, + headers: HeaderMap::new(), + automation: None, + }, + )) + .await; + + let body = response_json!(response, StatusCode::BAD_REQUEST).await; + assert_eq!(body["errors"][0]["detail"], expected_detail); + } +} + +#[tokio::test] +async fn create_run_from_manifest_preserves_competing_preparation_error_precedence() { + let mut workflow_before_title = minimal_manifest_json(MINIMAL_DOT); + workflow_before_title["workflows"]["workflow.fabro"]["config"] = json!({ + "path": "workflow.toml", + "source": "_version = 1\n[run.unknown]\nvalue = true\n", + }); + workflow_before_title["title"] = json!(" "); + + let mut project_parse_before_later_path = minimal_manifest_json(MINIMAL_DOT); + project_parse_before_later_path["configs"] = json!([ + { + "type": "project", + "path": "/tmp/.fabro/project.toml", + "source": "_version = 1\n[run.unknown]\nvalue = true\n", + }, + { + "type": "project", + "source": "_version = 1\n", + }, + ]); + + for (manifest_json, expected_detail) in [ + (workflow_before_title, "Failed to parse run config TOML"), + ( + project_parse_before_later_path, + "Failed to parse run config TOML", + ), + ] { + let state = TestAppStateBuilder::new().build(); + let manifest: RunManifest = serde_json::from_value(manifest_json).unwrap(); + let submitted_manifest_bytes = serde_json::to_vec(&manifest).unwrap(); + + let response = Box::pin(handler::runs::create_run_from_manifest( + state, + handler::runs::CreateRunFromManifestRequest { + manifest, + submitted_manifest_bytes, + explicit_run_id: None, + explicit_title_supplied: true, + actor: Principal::System { + system_kind: SystemActorKind::Engine, + }, + headers: HeaderMap::new(), + automation: None, + }, + )) + .await; + + let body = response_json!(response, StatusCode::BAD_REQUEST).await; + assert_eq!(body["errors"][0]["detail"], expected_detail); + } +} + +#[tokio::test] +async fn create_run_from_manifest_resolves_generated_id_after_variable_snapshot() { + let state = TestAppStateBuilder::new() + .env_lookup(|_| None) + .vault_entries([(EnvVars::OPENAI_API_KEY, "test-openai-api-key")]) + .build(); + let variable = state + .stores + .variables + .set("OWNER", "payments", None) + .await + .expect("test variable should persist"); + let manifest: RunManifest = serde_json::from_value(minimal_manifest_json( + r#"digraph Test { + graph [goal="Test"] + start [shape=Mdiamond] + work [prompt="Ship {{ vars.OWNER }}"] + exit [shape=Msquare] + start -> work -> exit + }"#, + )) + .unwrap(); + let submitted_manifest_bytes = serde_json::to_vec(&manifest).unwrap(); + + let response = Box::pin(handler::runs::create_run_from_manifest( + state, + handler::runs::CreateRunFromManifestRequest { + manifest, + submitted_manifest_bytes, + explicit_run_id: None, + explicit_title_supplied: false, + actor: Principal::System { + system_kind: SystemActorKind::Engine, + }, + headers: HeaderMap::new(), + automation: None, + }, + )) + .await; + + let body = response_json!(response, StatusCode::CREATED).await; + let run_id = body["id"].as_str().unwrap().parse::().unwrap(); + assert!(run_id.created_at() >= variable.updated_at); +} + #[tokio::test] async fn fake_automation_materializer_injection_captures_input_and_returns_manifest() { let materialized_manifest: RunManifest = @@ -6366,31 +6708,36 @@ async fn create_unreadable_durable_run(state: &Arc, run_id: RunId) { workflow_event::append_event(&run_store, &run_id, &workflow_event::Event::RunRunning) .await .unwrap(); - let payload = fabro_store::EventPayload::new( - json!({ - "id": "evt-unreadable-run-completed", - "ts": "2026-05-05T20:46:33Z", - "run_id": run_id, - "event": "run.completed", - "properties": { - "timing": { - "wall_time_ms": 1, - "inference_time_ms": 0, - "tool_time_ms": 0, - "active_time_ms": 0 - }, - "artifact_count": 0, - "status": "legacy-status", - "reason": "completed", - }, - }), + let seq = run_store.last_event_seq().await.unwrap().unwrap() + 1; + let completed = workflow_event::to_run_event_at( &run_id, + &workflow_event::Event::WorkflowRunCompleted { + timing: fabro_types::RunTiming::wall_only(1), + artifact_count: 0, + status: "legacy-status".to_string(), + reason: SuccessReason::Completed, + total_usd_micros: None, + final_git_commit_sha: None, + final_patch: None, + diff_summary: None, + billing: None, + }, + "2026-05-05T20:46:33Z".parse().unwrap(), + None, + ); + let payload = workflow_event::build_redacted_event_payload(&completed, &run_id).unwrap(); + fabro_store::test_support::put_unvalidated_run_event( + &state.stores.runs, + &run_id, + seq, + payload.as_value(), ) + .await .unwrap(); let err = run_store - .append_event(&payload) + .state() .await - .expect_err("invalid projection event should be persisted but rejected by projection"); + .expect_err("poison event should make the run projection unreadable"); assert!( err.to_string().contains("invalid completed stage status"), "unexpected projection error: {err}" diff --git a/lib/components/fabro-llm/Cargo.toml b/lib/components/fabro-llm/Cargo.toml index b10321253..d7bcb8df7 100644 --- a/lib/components/fabro-llm/Cargo.toml +++ b/lib/components/fabro-llm/Cargo.toml @@ -21,6 +21,7 @@ anyhow.workspace = true thiserror.workspace = true serde.workspace = true serde_json.workspace = true +sha2.workspace = true strum.workspace = true tokio.workspace = true uuid.workspace = true diff --git a/lib/components/fabro-llm/src/adapter_registry.rs b/lib/components/fabro-llm/src/adapter_registry.rs index 1a09a31e7..a306c846b 100644 --- a/lib/components/fabro-llm/src/adapter_registry.rs +++ b/lib/components/fabro-llm/src/adapter_registry.rs @@ -303,6 +303,8 @@ mod tests { ("claude-sonnet-4-5", "claude-sonnet-4-5", T::Anthropic, C::AnthropicMessages, B::Anthropic, P::Anthropic), ("claude-sonnet-4-6", "claude-sonnet-4-6", T::Anthropic, C::AnthropicMessages, B::Anthropic, P::Anthropic), ("claude-sonnet-5", "claude-sonnet-5", T::Anthropic, C::AnthropicMessages, B::Anthropic, P::Claude5), + ("deepseek-v4-flash", "deepseek-v4-flash", T::OpenAiCompatible, C::OpenAiCompatible, B::OpenAi, P::OpenAi), + ("deepseek-v4-pro", "deepseek-v4-pro", T::OpenAiCompatible, C::OpenAiCompatible, B::OpenAi, P::OpenAi), ("gemini-3-flash-preview", "gemini-3-flash-preview", T::Gemini, C::GeminiGenerate, B::Gemini, P::Gemini), ("gemini-3.1-flash-lite", "gemini-3.1-flash-lite", T::Gemini, C::GeminiGenerate, B::Gemini, P::Gemini), ("gemini-3.1-pro-preview", "gemini-3.1-pro-preview", T::Gemini, C::GeminiGenerate, B::Gemini, P::Gemini), diff --git a/lib/components/fabro-llm/src/codec/bedrock_converse/decode.rs b/lib/components/fabro-llm/src/codec/bedrock_converse/decode.rs index 36ec5878d..f9cf0f87e 100644 --- a/lib/components/fabro-llm/src/codec/bedrock_converse/decode.rs +++ b/lib/components/fabro-llm/src/codec/bedrock_converse/decode.rs @@ -279,6 +279,21 @@ mod tests { assert!(decode_content_block(&serde_json::json!({"text": ""})).is_none()); } + #[test] + fn tool_use_names_are_preserved_verbatim() { + let block = serde_json::json!({ + "toolUse": { + "toolUseId": "tool-1", + "name": "search???", + "input": {} + } + }); + let Some(ContentPart::ToolCall(tool_call)) = decode_content_block(&block) else { + panic!("expected tool call"); + }; + assert_eq!(tool_call.name, "search???"); + } + #[test] fn reasoning_text_block_round_trips_signature() { let block = serde_json::json!({ diff --git a/lib/components/fabro-llm/src/codec/bedrock_converse/encode.rs b/lib/components/fabro-llm/src/codec/bedrock_converse/encode.rs index 39c9c5c10..bb1a4ba3b 100644 --- a/lib/components/fabro-llm/src/codec/bedrock_converse/encode.rs +++ b/lib/components/fabro-llm/src/codec/bedrock_converse/encode.rs @@ -4,6 +4,7 @@ use base64::Engine; use base64::engine::general_purpose::STANDARD as BASE64; use serde_json::{Map, Value, json}; +use super::sanitize; use crate::codec::{CodecCtx, EncodedRequest, extract_system_prompt, merge_named_provider_options}; use crate::error::Error; use crate::types::{ContentPart, Message, Request, Role, ToolChoice}; @@ -127,12 +128,11 @@ fn encode_message(message: &Message) -> Option { if blocks.is_empty() && message.role == Role::Tool { if let Some(tool_call_id) = &message.tool_call_id { let text = message.text(); - blocks.push(json!({ - "toolResult": { - "toolUseId": tool_call_id, - "content": [{ "text": text }], - } - })); + blocks.push(tool_result_block( + tool_call_id, + json!([{ "text": text }]), + false, + )); } } @@ -183,26 +183,18 @@ fn encode_content_part(part: &ContentPart) -> Option { Value::Object(_) => tool_call.arguments.clone(), _ => json!({}), }; - Some(json!({ - "toolUse": { - "toolUseId": tool_call.id, - "name": tool_call.name, - "input": input, - } - })) + Some(tool_use_block(&tool_call.id, &tool_call.name, input)) } ContentPart::ToolResult(result) => { let content = match &result.content { Value::String(text) => json!([{ "text": text }]), other => json!([{ "json": other }]), }; - let mut block = Map::new(); - block.insert("toolUseId".to_string(), json!(result.tool_call_id)); - block.insert("content".to_string(), content); - if result.is_error { - block.insert("status".to_string(), json!("error")); - } - Some(json!({ "toolResult": Value::Object(block) })) + Some(tool_result_block( + &result.tool_call_id, + content, + result.is_error, + )) } ContentPart::Thinking(thinking) => { if thinking.redacted { @@ -226,6 +218,28 @@ fn encode_content_part(part: &ContentPart) -> Option { } } +/// Build a `toolUse` block. All tool blocks must be constructed through +/// [`tool_use_block`] and [`tool_result_block`] so identifier sanitization +/// keeps `toolUse` and `toolResult` paired on the wire. +fn tool_use_block(id: &str, name: &str, input: Value) -> Value { + let mut block = Map::new(); + block.insert("toolUseId".to_string(), json!(sanitize::tool_use_id(id))); + block.insert("name".to_string(), json!(sanitize::tool_name(name))); + block.insert("input".to_string(), input); + json!({ "toolUse": Value::Object(block) }) +} + +/// Build a `toolResult` block; see [`tool_use_block`] for the pairing contract. +fn tool_result_block(id: &str, content: Value, is_error: bool) -> Value { + let mut block = Map::new(); + block.insert("toolUseId".to_string(), json!(sanitize::tool_use_id(id))); + block.insert("content".to_string(), content); + if is_error { + block.insert("status".to_string(), json!("error")); + } + json!({ "toolResult": Value::Object(block) }) +} + /// Convert common MIME types into Bedrock's media `format` enum values. fn media_format<'a>(media_type: Option<&str>, default: &'a str) -> &'a str { match media_type { @@ -522,6 +536,114 @@ mod tests { assert_eq!(tool_use["input"], json!({})); } + #[test] + fn historical_tool_names_are_sanitized_on_the_wire() { + let mut request = base_request("claude"); + request.messages = vec![Message { + role: Role::Assistant, + content: vec![ContentPart::ToolCall(ToolCall::new( + "tool-1", + "search???", + json!({}), + ))], + name: None, + tool_call_id: None, + }]; + + let encoded = encode_with(&request); + let tool_use = &encoded.body["messages"][0]["content"][0]["toolUse"]; + assert_eq!(tool_use["name"], sanitize::tool_name("search???")); + } + + #[test] + fn sanitized_tool_use_ids_remain_paired() { + for id in ["bad id!".to_string(), "x".repeat(100)] { + let mut request = base_request("claude"); + request.messages = vec![ + Message { + role: Role::Assistant, + content: vec![ContentPart::ToolCall(ToolCall::new( + &id, + "search", + json!({}), + ))], + name: None, + tool_call_id: None, + }, + Message { + role: Role::Tool, + content: vec![ContentPart::ToolResult(ToolResult::success( + &id, + json!("done"), + ))], + name: None, + tool_call_id: Some(id.clone()), + }, + ]; + + let encoded = encode_with(&request); + let tool_use_id = &encoded.body["messages"][0]["content"][0]["toolUse"]["toolUseId"]; + let tool_result_id = + &encoded.body["messages"][1]["content"][0]["toolResult"]["toolUseId"]; + assert_eq!(tool_use_id, tool_result_id); + assert!(tool_use_id.as_str().is_some_and(|value| value.len() <= 64)); + } + } + + #[test] + fn tool_role_fallback_sanitizes_the_tool_use_id() { + let mut request = base_request("claude"); + request.messages = vec![Message { + role: Role::Tool, + content: vec![], + name: None, + tool_call_id: Some("bad id!".to_string()), + }]; + + let encoded = encode_with(&request); + assert_eq!( + encoded.body["messages"][0]["content"][0]["toolResult"]["toolUseId"], + sanitize::tool_use_id("bad id!") + ); + } + + #[test] + fn overlength_tool_names_encode_within_the_bedrock_limit() { + let mut request = base_request("claude"); + request.messages = vec![Message { + role: Role::Assistant, + content: vec![ContentPart::ToolCall(ToolCall::new( + "tool-1", + "x".repeat(100), + json!({}), + ))], + name: None, + tool_call_id: None, + }]; + + let encoded = encode_with(&request); + let name = encoded.body["messages"][0]["content"][0]["toolUse"]["name"] + .as_str() + .unwrap(); + assert_eq!(name.len(), 64); + } + + #[test] + fn tool_definition_names_remain_unsanitized() { + let mut request = base_request("claude"); + request.tools = Some(vec![ToolDefinition::function( + "weird.name", + "Deliberately invalid for Bedrock", + json!({"type": "object"}), + )]); + + let encoded = encode_with(&request); + assert_eq!( + encoded.body["toolConfig"]["tools"][0]["toolSpec"]["name"], + "weird.name" + ); + } + #[test] fn thinking_parts_restructure_into_reasoning_text_blocks() { let mut request = base_request("claude"); diff --git a/lib/components/fabro-llm/src/codec/bedrock_converse/mod.rs b/lib/components/fabro-llm/src/codec/bedrock_converse/mod.rs index 12b315094..c55c0a3b3 100644 --- a/lib/components/fabro-llm/src/codec/bedrock_converse/mod.rs +++ b/lib/components/fabro-llm/src/codec/bedrock_converse/mod.rs @@ -12,6 +12,7 @@ mod decode; mod encode; +mod sanitize; mod stream; use crate::codec::{Codec, CodecCtx, EncodedRequest, StreamDecoder}; diff --git a/lib/components/fabro-llm/src/codec/bedrock_converse/sanitize.rs b/lib/components/fabro-llm/src/codec/bedrock_converse/sanitize.rs new file mode 100644 index 000000000..5ba139fc2 --- /dev/null +++ b/lib/components/fabro-llm/src/codec/bedrock_converse/sanitize.rs @@ -0,0 +1,122 @@ +//! Bedrock Converse tool identifier sanitization. +//! +//! Tool names must match `[a-zA-Z0-9_-]+`; tool-use IDs additionally allow +//! `.` and `:`. Both are limited to 64 characters. These helpers rewrite only +//! the Bedrock wire view: the canonical transcript retains provider output +//! verbatim. The encoder routes every tool block through its +//! `tool_use_block`/`tool_result_block` constructors so `toolUse` and +//! `toolResult` blocks remain paired. + +use sha2::{Digest, Sha256}; + +const MAX_LENGTH: usize = 64; +const HASH_HEX_LENGTH: usize = 16; +const PREFIX_LENGTH: usize = MAX_LENGTH - 1 - HASH_HEX_LENGTH; + +pub(super) fn tool_name(name: &str) -> String { + sanitize(name, "unknown_tool", is_tool_name_char) +} + +pub(super) fn tool_use_id(id: &str) -> String { + sanitize(id, "unknown_tool_use_id", is_tool_use_id_char) +} + +fn sanitize(value: &str, empty_fallback: &'static str, is_allowed: fn(char) -> bool) -> String { + if value.is_empty() { + return empty_fallback.to_string(); + } + + let sanitized: String = value + .chars() + .map(|character| { + if is_allowed(character) { + character + } else { + '_' + } + }) + .collect(); + + if sanitized.len() <= MAX_LENGTH { + sanitized + } else { + truncate_with_hash(&sanitized, value) + } +} + +fn is_tool_name_char(character: char) -> bool { + character.is_ascii_alphanumeric() || matches!(character, '_' | '-') +} + +fn is_tool_use_id_char(character: char) -> bool { + is_tool_name_char(character) || matches!(character, '.' | ':') +} + +fn truncate_with_hash(sanitized: &str, original: &str) -> String { + debug_assert!(sanitized.is_ascii()); + let digest = Sha256::digest(original.as_bytes()); + let digest_hex = format!("{digest:x}"); + format!( + "{}-{}", + &sanitized[..PREFIX_LENGTH], + &digest_hex[..HASH_HEX_LENGTH] + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn valid_values_pass_through_unchanged() { + for name in ["search", "TaskList", "a-b_c9"] { + assert_eq!(tool_name(name), name); + } + + let max_length = "a".repeat(64); + assert_eq!(tool_name(&max_length), max_length); + + let id = "functions.read_file:4"; + assert_eq!(tool_use_id(id), id); + assert_eq!(tool_name(id), "functions_read_file_4"); + } + + #[test] + fn invalid_characters_are_replaced() { + assert_eq!(tool_name("search???"), "search___"); + assert_eq!(tool_name("bad name"), "bad_name"); + assert_eq!(tool_use_id("bad id!"), "bad_id_"); + } + + #[test] + fn non_ascii_characters_become_single_underscores() { + let sanitized = tool_name("before🙂after"); + assert_eq!(sanitized, "before_after"); + assert!(sanitized.is_ascii()); + } + + #[test] + fn empty_values_use_nonempty_fallbacks() { + assert_eq!(tool_name(""), "unknown_tool"); + assert_eq!(tool_use_id(""), "unknown_tool_use_id"); + } + + #[test] + fn overlength_values_use_deterministic_hash_suffixes() { + let boundary = "a".repeat(65); + let first = tool_name(&boundary); + let second = tool_name(&boundary); + assert_eq!(first, second); + assert_eq!(first.len(), 64); + assert!( + first + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'_' | b'-')) + ); + + let shared_prefix = "x".repeat(99); + let left = tool_name(&format!("{shared_prefix}a")); + let right = tool_name(&format!("{shared_prefix}b")); + assert_ne!(left, right); + } +} diff --git a/lib/components/fabro-llm/src/codec/bedrock_converse/stream.rs b/lib/components/fabro-llm/src/codec/bedrock_converse/stream.rs index 0facf2583..0d413cabf 100644 --- a/lib/components/fabro-llm/src/codec/bedrock_converse/stream.rs +++ b/lib/components/fabro-llm/src/codec/bedrock_converse/stream.rs @@ -406,6 +406,22 @@ mod tests { assert!(!tool_call.arguments.is_null()); } + #[test] + fn streamed_tool_use_names_are_preserved_verbatim() { + let mut d = decoder(); + feed(&mut d, "messageStart", r#"{"role":"assistant"}"#); + feed( + &mut d, + "contentBlockStart", + r#"{"start":{"toolUse":{"toolUseId":"tool-1","name":"search???"}},"contentBlockIndex":0}"#, + ); + let stop = feed(&mut d, "contentBlockStop", r#"{"contentBlockIndex":0}"#); + let StreamEvent::ToolCallEnd { tool_call } = &stop[0] else { + panic!("expected ToolCallEnd"); + }; + assert_eq!(tool_call.name, "search???"); + } + #[test] fn tool_use_accumulates_string_input_fragments() { let mut d = decoder(); diff --git a/lib/components/fabro-llm/src/codec/openai_compatible/translate.rs b/lib/components/fabro-llm/src/codec/openai_compatible/translate.rs index dbef4831f..b32fac567 100644 --- a/lib/components/fabro-llm/src/codec/openai_compatible/translate.rs +++ b/lib/components/fabro-llm/src/codec/openai_compatible/translate.rs @@ -237,7 +237,9 @@ pub(super) fn translate_response_format(format: &ResponseFormat) -> serde_json:: #[cfg(test)] mod tests { use super::*; - use crate::types::{AudioData, ContentPart, DocumentData, Message, Role, ToolCall}; + use crate::types::{ + AudioData, ContentPart, DocumentData, Message, Role, ThinkingData, ToolCall, + }; #[test] fn translate_assistant_message_with_tool_calls_only() { @@ -291,6 +293,37 @@ mod tests { assert_eq!(tool_calls[0].function.name, "get_weather"); } + #[test] + fn translate_assistant_tool_call_replays_reasoning_content() { + let msg = Message { + role: Role::Assistant, + content: vec![ + ContentPart::Thinking(ThinkingData { + text: "I need the weather tool.".to_string(), + signature: None, + redacted: false, + }), + ContentPart::ToolCall(ToolCall::new( + "call_2", + "get_weather", + serde_json::json!({"city": "NYC"}), + )), + ], + name: None, + tool_call_id: None, + }; + + let translated = translate_messages(&[msg]); + + assert_eq!( + translated[0].reasoning_content.as_deref(), + Some("I need the weather tool.") + ); + assert_eq!(translated[0].tool_calls.as_ref().unwrap().len(), 1); + let json = serde_json::to_value(&translated[0]).unwrap(); + assert_eq!(json["reasoning_content"], "I need the weather tool."); + } + #[test] fn translate_assistant_message_with_raw_arguments() { let mut tc = ToolCall::new("call_3", "search", serde_json::json!({"q": "rust"})); diff --git a/lib/components/fabro-llm/src/codec/openai_compatible/wire.rs b/lib/components/fabro-llm/src/codec/openai_compatible/wire.rs index 3e1de949d..1b7ff3b33 100644 --- a/lib/components/fabro-llm/src/codec/openai_compatible/wire.rs +++ b/lib/components/fabro-llm/src/codec/openai_compatible/wire.rs @@ -44,7 +44,7 @@ pub(super) struct ChatMessage { #[serde(skip_serializing_if = "Option::is_none")] pub content: Option, /// Reasoning/thinking content echoed back for providers that require it - /// (Kimi). + /// during tool-call continuations (including Kimi and DeepSeek). #[serde(skip_serializing_if = "Option::is_none")] pub reasoning_content: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -300,6 +300,10 @@ pub(super) struct ApiUsage { pub cost: Option, #[serde(default)] pub prompt_tokens_details: Option, + /// DeepSeek-specific top-level count of prompt tokens served from its + /// automatic context cache. + #[serde(default)] + pub prompt_cache_hit_tokens: Option, #[serde(default)] pub completion_tokens_details: Option, } @@ -329,6 +333,7 @@ impl ApiUsage { .prompt_tokens_details .as_ref() .and_then(|d| d.cached_tokens) + .or(self.prompt_cache_hit_tokens) .unwrap_or(0); let cache_write_detail = self .prompt_tokens_details @@ -538,6 +543,23 @@ mod tests { }); } + #[test] + fn token_counts_accept_deepseek_cache_hit_field() { + let usage: ApiUsage = serde_json::from_value(serde_json::json!({ + "prompt_tokens": 53, + "completion_tokens": 11, + "prompt_cache_hit_tokens": 41 + })) + .unwrap(); + + assert_eq!(usage.token_counts(), TokenCounts { + input_tokens: 12, + output_tokens: 11, + cache_read_tokens: 41, + ..TokenCounts::default() + }); + } + #[test] fn reasoning_accepts_provider_and_openrouter_spellings() { let provider_response: ApiResponse = serde_json::from_value(serde_json::json!({ diff --git a/lib/components/fabro-llm/src/error.rs b/lib/components/fabro-llm/src/error.rs index 753c12e6f..b6fc67661 100644 --- a/lib/components/fabro-llm/src/error.rs +++ b/lib/components/fabro-llm/src/error.rs @@ -316,6 +316,9 @@ pub fn error_from_status_code( }; } 413 => ProviderErrorKind::ContextLength, + 429 if detail.error_code.as_deref() == Some("exceeded_current_quota_error") => { + ProviderErrorKind::QuotaExceeded + } 429 => ProviderErrorKind::RateLimit, 500..=599 => ProviderErrorKind::Server, // For ambiguous status codes (400, 422, etc.), use message-based classification @@ -603,6 +606,22 @@ mod tests { assert!(err.retryable()); } + #[test] + fn exceeded_current_quota_error_is_non_retryable_quota_failure() { + let err = error_from_status_code( + 429, + "Your account is suspended due to insufficient balance".into(), + "kimi".into(), + Some("exceeded_current_quota_error".into()), + None, + None, + ); + + assert_eq!(err.provider_kind(), Some(ProviderErrorKind::QuotaExceeded)); + assert!(!err.retryable()); + assert!(err.failover_eligible()); + } + #[test] fn error_message_classification_context_length() { let err = error_from_status_code( diff --git a/lib/components/fabro-llm/tests/integration.rs b/lib/components/fabro-llm/tests/integration.rs index 8362d101b..507913d1b 100644 --- a/lib/components/fabro-llm/tests/integration.rs +++ b/lib/components/fabro-llm/tests/integration.rs @@ -373,7 +373,7 @@ async fn openrouter_complete() { std::env::var(EnvVars::OPENROUTER_API_KEY).expect("OPENROUTER_API_KEY must be set"); let adapter = OpenAiCompatibleAdapter::new(api_key, "https://openrouter.ai/api/v1") .with_name("openrouter"); - let request = make_request("deepseek/deepseek-v4-flash"); + let request = make_request("deepseek/deepseek-v4-flash-0731"); let response = adapter.complete(&request).await.unwrap(); assert!( @@ -599,6 +599,39 @@ async fn fireworks_complete() { assert_eq!(response.provider, "fireworks"); } +#[fabro_macros::e2e_test(live("DEEPSEEK_API_KEY"))] +async fn deepseek_complete() { + let api_key = std::env::var(EnvVars::DEEPSEEK_API_KEY).expect("DEEPSEEK_API_KEY must be set"); + let adapter = + OpenAiCompatibleAdapter::new(api_key, "https://api.deepseek.com").with_name("deepseek"); + let request = Request { + // Thinking mode is enabled by default and shares this budget with the + // visible answer. + max_tokens: Some(1024), + ..make_request("deepseek-v4-flash") + }; + let response = adapter.complete(&request).await.unwrap(); + + assert!( + !response.text().is_empty(), + "response text should not be empty" + ); + assert!(response.usage.input_tokens > 0); + assert!(response.usage.output_tokens > 0 || response.usage.reasoning_tokens > 0); + assert_eq!(response.provider, "deepseek"); +} + +#[fabro_macros::e2e_test(live("DEEPSEEK_API_KEY"))] +async fn deepseek_v4_flash_deep_tool_round_trip() { + let api_key = std::env::var(EnvVars::DEEPSEEK_API_KEY).expect("DEEPSEEK_API_KEY must be set"); + let provider = ProviderId::new("deepseek"); + let catalog = enabled_provider_catalog(&provider, None); + let credential = ApiCredential::from_api_key(provider.clone(), api_key, &catalog) + .expect("DeepSeek credential should resolve from the catalog"); + + assert_deep_tool_round_trip(&catalog, &provider, "deepseek-v4-flash", credential).await; +} + #[fabro_macros::e2e_test(live("FIREWORKS_API_KEY"))] async fn fireworks_kimi_k2_7_code_deep_tool_round_trip() { let api_key = std::env::var(EnvVars::FIREWORKS_API_KEY).expect("FIREWORKS_API_KEY must be set"); diff --git a/lib/components/fabro-llm/tests/it/wire/openai_compatible.rs b/lib/components/fabro-llm/tests/it/wire/openai_compatible.rs index 490aff18a..0ff337cc5 100644 --- a/lib/components/fabro-llm/tests/it/wire/openai_compatible.rs +++ b/lib/components/fabro-llm/tests/it/wire/openai_compatible.rs @@ -185,8 +185,8 @@ async fn encode_tool_round_trip() { fabro_test::fabro_json_snapshot!(capture.body); } -/// Assistant thinking parts echo back as `reasoning_content` (Kimi-motivated, -/// applies to every compat assistant message). +/// Assistant thinking parts echo back as `reasoning_content` (required by +/// Kimi and DeepSeek during tool-call continuations). #[tokio::test] async fn encode_thinking_round_trip_as_reasoning_content() { let capture = encode_capture(&corpus_thinking_round_trip(MODEL)).await; diff --git a/lib/components/fabro-manifest/src/lib.rs b/lib/components/fabro-manifest/src/lib.rs index 13bae0051..91014a781 100644 --- a/lib/components/fabro-manifest/src/lib.rs +++ b/lib/components/fabro-manifest/src/lib.rs @@ -61,7 +61,6 @@ pub struct RunOverrideInput<'a> { pub model: Option<&'a str>, pub provider: Option<&'a str>, pub environment: Option<&'a str>, - pub docker_image: Option<&'a str>, pub preserve_sandbox: Option, pub dry_run: Option, pub auto_approve: Option, @@ -79,23 +78,19 @@ pub fn build_run_overrides(input: RunOverrideInput<'_>) -> RunLayer { fallbacks: MergeMap::default(), controls: None, }); - let environment = (input.environment.is_some() - || input.docker_image.is_some() - || input.preserve_sandbox.is_some()) - .then(|| RunEnvironmentLayer { - id: input.environment.map(ToOwned::to_owned), - image: input.docker_image.map(|image| EnvironmentImageLayer { - docker: Some(image.to_string()), - ..EnvironmentImageLayer::default() - }), - lifecycle: input - .preserve_sandbox - .map(|preserve| EnvironmentLifecycleLayer { - preserve: Some(preserve), - ..EnvironmentLifecycleLayer::default() - }), - ..RunEnvironmentLayer::default() - }); + let environment = + (input.environment.is_some() || input.preserve_sandbox.is_some()).then(|| { + RunEnvironmentLayer { + id: input.environment.map(ToOwned::to_owned), + lifecycle: input + .preserve_sandbox + .map(|preserve| EnvironmentLifecycleLayer { + preserve: Some(preserve), + ..EnvironmentLifecycleLayer::default() + }), + ..RunEnvironmentLayer::default() + } + }); let execution = (input.dry_run.is_some() || input.auto_approve.is_some()).then(|| RunExecutionLayer { mode: input.dry_run.map(|dry_run| { @@ -894,7 +889,6 @@ pub fn manifest_args_is_empty(args: &types::ManifestArgs) -> bool { && args.preserve_sandbox.is_none() && args.provider.is_none() && args.environment.is_none() - && args.docker_image.is_none() && args.input.is_empty() && args.verbose.is_none() } @@ -966,7 +960,6 @@ mod tests { model: Some("gpt-5.4-mini"), provider: Some("openai"), environment: Some("local"), - docker_image: None, preserve_sandbox: Some(true), dry_run: Some(true), auto_approve: Some(false), @@ -1028,6 +1021,40 @@ mod tests { ); } + #[test] + fn sparse_run_overrides_preserve_only_has_no_image() { + let overrides = build_sparse_run_overrides(RunOverrideInput { + preserve_sandbox: Some(true), + ..RunOverrideInput::default() + }) + .expect("preserve override"); + let environment = overrides.environment.expect("environment override"); + + assert!(environment.image.is_none()); + assert_eq!( + environment.lifecycle.expect("lifecycle override").preserve, + Some(true) + ); + } + + #[test] + fn sparse_run_overrides_environment_only_has_no_image() { + let overrides = build_sparse_run_overrides(RunOverrideInput { + environment: Some("local"), + ..RunOverrideInput::default() + }) + .expect("environment override"); + let environment = overrides.environment.expect("environment override"); + + assert_eq!(environment.id.as_deref(), Some("local")); + assert!(environment.image.is_none()); + } + + #[test] + fn sparse_run_overrides_default_is_empty() { + assert!(build_sparse_run_overrides(RunOverrideInput::default()).is_none()); + } + // Regression coverage for https://github.com/fabro-sh/fabro/issues/476. #[test] fn build_manifest_bundles_agent_output_schema_file() { diff --git a/lib/components/fabro-sandbox/src/daytona/mod.rs b/lib/components/fabro-sandbox/src/daytona/mod.rs index c157e7d52..6371f23d2 100644 --- a/lib/components/fabro-sandbox/src/daytona/mod.rs +++ b/lib/components/fabro-sandbox/src/daytona/mod.rs @@ -3305,7 +3305,7 @@ mod tests { } #[tokio::test] - async fn check_daytona_api_key_with_accepts_full_scopes() { + async fn check_daytona_api_key_with_accepts_full_scopes_and_new_scopes() { let server = MockServer::start_async().await; let auth = mock_auth_probe(&server, 200).await; let current_key = mock_current_key(&server, vec![ @@ -3313,6 +3313,9 @@ mod tests { "delete:snapshots", "write:sandboxes", "delete:sandboxes", + "manage:secrets", + "read:limits", + "manage:sso", ]) .await; diff --git a/lib/components/fabro-store/Cargo.toml b/lib/components/fabro-store/Cargo.toml index 796119e18..588e285a0 100644 --- a/lib/components/fabro-store/Cargo.toml +++ b/lib/components/fabro-store/Cargo.toml @@ -11,6 +11,9 @@ doctest = false [lints] workspace = true +[features] +test-support = [] + [dependencies] fabro-types = { path = "../../foundation/fabro-types" } fabro-util = { path = "../../foundation/fabro-util" } diff --git a/lib/components/fabro-store/src/error.rs b/lib/components/fabro-store/src/error.rs index 43c26b44a..df12e5994 100644 --- a/lib/components/fabro-store/src/error.rs +++ b/lib/components/fabro-store/src/error.rs @@ -14,6 +14,11 @@ pub enum Error { Io(#[from] std::io::Error), #[error("Invalid event payload: {0}")] InvalidEvent(String), + #[error("event rejected by run projection: {source}")] + EventRejected { + #[source] + source: Box, + }, #[error("Run not found: {0}")] RunNotFound(String), #[error("Run already exists: {0}")] @@ -35,7 +40,7 @@ pub enum Error { run_id: String, field: &'static str, }, - #[error("invalid status transition: {0}")] + #[error(transparent)] InvalidTransition(#[from] fabro_types::InvalidTransition), #[error("{0}")] Other(String), diff --git a/lib/components/fabro-store/src/lib.rs b/lib/components/fabro-store/src/lib.rs index 0eb246d04..e99eadd0f 100644 --- a/lib/components/fabro-store/src/lib.rs +++ b/lib/components/fabro-store/src/lib.rs @@ -10,8 +10,8 @@ mod run_state; mod run_summary_store; mod serializable_projection; mod slate; -#[cfg(test)] -mod test_util; +#[cfg(any(test, feature = "test-support"))] +pub mod test_support; mod types; pub use artifact_store::{ diff --git a/lib/components/fabro-store/src/run_summary_store.rs b/lib/components/fabro-store/src/run_summary_store.rs index d80936dc7..5db1a49b1 100644 --- a/lib/components/fabro-store/src/run_summary_store.rs +++ b/lib/components/fabro-store/src/run_summary_store.rs @@ -154,6 +154,11 @@ impl RunSummaryStore { Ok(()) } + #[cfg(test)] + pub(crate) async fn close_pool(&self) { + self.pool.close().await; + } + pub(crate) async fn reconcile(&self, entries: &[CachedRunProjection]) -> Result<()> { let mut transaction = self.pool.begin().await?; let stored_seqs: HashMap = @@ -571,7 +576,7 @@ mod tests { RunSummaryVisibility, }; use crate::slate::CachedRunProjection; - use crate::test_util; + use crate::test_support as store_test_support; fn dt(value: &str) -> DateTime { value.parse().unwrap() @@ -608,7 +613,7 @@ mod tests { } async fn store() -> (tempfile::TempDir, RunSummaryStore) { - test_util::sqlite_summary_store().await + store_test_support::sqlite_summary_store().await } fn sample_status(kind: RunStatusKind) -> RunStatus { diff --git a/lib/components/fabro-store/src/slate/mod.rs b/lib/components/fabro-store/src/slate/mod.rs index 275084dbb..893b0d8f0 100644 --- a/lib/components/fabro-store/src/slate/mod.rs +++ b/lib/components/fabro-store/src/slate/mod.rs @@ -322,6 +322,22 @@ impl Database { Ok(unreadable) } + #[cfg(any(test, feature = "test-support"))] + pub(crate) async fn put_unvalidated_run_event( + &self, + run_id: &RunId, + seq: u32, + payload: &serde_json::Value, + ) -> Result<()> { + let db = self.open_db().await?; + db.put( + keys::run_event_key(run_id, seq, 0), + serde_json::to_vec(payload)?, + ) + .await?; + Ok(()) + } + pub async fn get_cached_run(&self, run_id: &RunId) -> Result> { self.warm_projection_cache().await?; Ok(self.projection_cache.get(run_id).await) @@ -523,7 +539,7 @@ mod tests { use object_store::path::Path; use super::*; - use crate::{EventPayload, keys, test_util}; + use crate::{EventPayload, keys, test_support as store_test_support}; fn dt(value: &str) -> DateTime { value.parse().unwrap() @@ -572,7 +588,7 @@ mod tests { } async fn make_summary_store() -> (tempfile::TempDir, Arc) { - let (directory, store) = test_util::sqlite_summary_store().await; + let (directory, store) = store_test_support::sqlite_summary_store().await; (directory, Arc::new(store)) } @@ -684,6 +700,57 @@ mod tests { .unwrap(); } + async fn append_runnable(run: &RunDatabase, label: &str, created_at: DateTime) { + append_created(run, label, created_at).await; + run.append_event(&event_payload( + label, + "2026-03-27T12:00:01Z", + "run.submitted", + &serde_json::json!({}), + )) + .await + .unwrap(); + run.append_event(&event_payload( + label, + "2026-03-27T12:00:02Z", + "run.start_requested", + &serde_json::json!({ "resume": false }), + )) + .await + .unwrap(); + run.append_event(&event_payload( + label, + "2026-03-27T12:00:03Z", + "run.runnable", + &serde_json::json!({ "source": "start_requested" }), + )) + .await + .unwrap(); + } + + fn workflow_failure_payload(label: &str) -> EventPayload { + event_payload( + label, + "2026-03-27T12:00:04Z", + "run.failed", + &serde_json::json!({ + "failure": { + "reason": "workflow_error", + "detail": { + "message": "workflow failed", + "category": "deterministic" + } + }, + "timing": { + "wall_time_ms": 1, + "inference_time_ms": 0, + "tool_time_ms": 0, + "active_time_ms": 0 + }, + }), + ) + } + async fn append_completed(run: &RunDatabase, label: &str, created_at: DateTime) { append_running(run, label, created_at).await; run.append_event(&event_payload( @@ -849,6 +916,160 @@ mod tests { assert_eq!(run.list_events().await.unwrap().len(), 2); } + #[tokio::test] + async fn rejected_transition_writes_nothing_and_preserves_projection_cache() { + let (_object_store, store) = make_store(); + let run_id = test_run_id("run-1"); + let run = store.create_run(&run_id).await.unwrap(); + append_runnable(&run, "run-1", dt("2026-03-27T12:00:00Z")).await; + let events_before = run.list_events().await.unwrap(); + + let err = run + .append_event(&workflow_failure_payload("run-1")) + .await + .unwrap_err(); + + let Error::EventRejected { source } = err else { + panic!("expected event rejection"); + }; + assert!(matches!( + *source, + Error::InvalidTransition(fabro_types::InvalidTransition { + from: RunStatus::Runnable, + to: RunStatus::Failed { + reason: FailureReason::WorkflowError, + }, + }) + )); + assert_eq!(run.list_events().await.unwrap(), events_before); + assert_eq!(run.state().await.unwrap().status, RunStatus::Runnable); + let cached = store.get_cached_run(&run_id).await.unwrap().unwrap(); + assert_eq!(cached.last_seq, 4); + assert_eq!(cached.projection.status, RunStatus::Runnable); + } + + #[tokio::test] + async fn rejected_transition_leaves_reconciled_summary_present() { + let (_object_store, store) = make_store(); + let (_directory, summaries) = make_summary_store().await; + store.attach_run_summary_store(Arc::clone(&summaries)); + let run_id = test_run_id("run-1"); + let run = store.create_run(&run_id).await.unwrap(); + append_runnable(&run, "run-1", dt("2026-03-27T12:00:00Z")).await; + + let err = run + .append_event(&workflow_failure_payload("run-1")) + .await + .unwrap_err(); + assert!(matches!(err, Error::EventRejected { .. })); + + let entries = store + .list_cached_runs(&ListRunsQuery::default(), Utc::now()) + .await + .unwrap(); + summaries.reconcile(&entries).await.unwrap(); + let summary = summaries.get(&run_id, Utc::now()).await.unwrap().unwrap(); + assert_eq!(summary.lifecycle.status, RunStatus::Runnable); + } + + #[tokio::test] + async fn committed_append_succeeds_when_summary_update_fails_and_is_repairable() { + let (object_store, store) = make_store(); + let (directory, summaries) = make_summary_store().await; + store.attach_run_summary_store(Arc::clone(&summaries)); + let run_id = test_run_id("run-1"); + let run = store.create_run(&run_id).await.unwrap(); + append_created(&run, "run-1", dt("2026-03-27T12:00:00Z")).await; + summaries.close_pool().await; + + let result = run + .append_event_envelope(&event_payload( + "run-1", + "2026-03-27T12:00:01Z", + "run.title.updated", + &serde_json::json!({ "title": "Committed title" }), + )) + .await; + + assert!(result.is_ok(), "committed append returned {result:?}"); + assert_eq!(run.list_events().await.unwrap().len(), 2); + let cached = store.get_cached_run(&run_id).await.unwrap().unwrap(); + assert_eq!(cached.last_seq, 2); + assert_eq!(cached.summary.title, "Committed title"); + let stored = run.get_event(2).await.unwrap().unwrap(); + assert_eq!(stored.event, result.unwrap().event); + + let repaired_summaries = + Arc::new(store_test_support::sqlite_summary_store_at(directory.path()).await); + let stale = repaired_summaries + .get(&run_id, Utc::now()) + .await + .unwrap() + .unwrap(); + assert_ne!(stale.title, "Committed title"); + + let reopened = Database::new(object_store, "runs/", Duration::from_millis(1), None); + reopened.attach_run_summary_store(Arc::clone(&repaired_summaries)); + reopened.warm_projection_cache().await.unwrap(); + let repaired = repaired_summaries + .get(&run_id, Utc::now()) + .await + .unwrap() + .unwrap(); + assert_eq!(repaired.title, "Committed title"); + } + + #[tokio::test] + async fn first_event_is_validated_before_write() { + let (_object_store, store) = make_store(); + let run_id = test_run_id("run-1"); + let run = store.create_run(&run_id).await.unwrap(); + let invalid_first = event_payload( + "run-1", + "2026-03-27T12:00:00Z", + "run.title.updated", + &serde_json::json!({ "title": "Too early" }), + ); + + let err = run.append_event(&invalid_first).await.unwrap_err(); + + assert!(matches!(err, Error::EventRejected { .. })); + assert!(run.list_events().await.unwrap().is_empty()); + + append_created(&run, "run-1", dt("2026-03-27T12:00:01Z")).await; + assert_eq!(run.list_events().await.unwrap().len(), 1); + assert!(run.state().await.is_ok()); + } + + #[tokio::test] + async fn malformed_optional_envelope_field_is_rejected_before_write() { + let (_object_store, store) = make_store(); + let run_id = test_run_id("run-1"); + let run = store.create_run(&run_id).await.unwrap(); + let malformed = EventPayload::new( + serde_json::json!({ + "id": "evt-created", + "ts": "2026-03-27T12:00:00Z", + "run_id": run_id.to_string(), + "event": "run.created", + "node_id": 42, + "properties": { + "settings": WorkflowSettings::default(), + "graph": Graph::new("test"), + "run_dir": "/tmp/test", + "provenance": test_support::test_run_provenance(), + }, + }), + &run_id, + ) + .unwrap(); + + let err = run.append_event(&malformed).await.unwrap_err(); + + assert!(matches!(err, Error::InvalidEvent(_))); + assert!(run.list_events().await.unwrap().is_empty()); + } + #[tokio::test] async fn control_request_events_set_pending_control_without_overwriting_status() { let (_object_store, store) = make_store(); @@ -1321,13 +1542,14 @@ mod tests { .add(&bad_run_id) .await .unwrap(); - let db = store.open_db().await.unwrap(); - db.put( - keys::run_event_key(&bad_run_id, 1, 0), - br#"{"not":"a valid run event"}"#, - ) - .await - .unwrap(); + store + .put_unvalidated_run_event( + &bad_run_id, + 1, + &serde_json::json!({ "not": "a valid run event" }), + ) + .await + .unwrap(); let reopened = Database::new(object_store, "runs", Duration::from_millis(1), None); reopened.warm_projection_cache().await.unwrap(); @@ -1369,28 +1591,28 @@ mod tests { .and_then(serde_json::Value::as_object_mut) .unwrap(); run_settings.remove("integrations"); - let db = store.open_db().await.unwrap(); - db.put( - keys::run_event_key(&bad_run_id, 1, 0), - serde_json::to_vec(&serde_json::json!({ - "id": "evt-run-2-run.created", - "ts": "2026-03-27T12:00:10Z", - "run_id": bad_run_id, - "event": "run.created", - "properties": { - "settings": run_spec["settings"], - "graph": run_spec["graph"], - "workflow_slug": run_spec["workflow_slug"], - "source_directory": run_spec["source_directory"], - "run_dir": "/tmp/run-2", - "git": run_spec["git"], - "labels": run_spec["labels"], - }, - })) - .unwrap(), - ) - .await - .unwrap(); + store + .put_unvalidated_run_event( + &bad_run_id, + 1, + &serde_json::json!({ + "id": "evt-run-2-run.created", + "ts": "2026-03-27T12:00:10Z", + "run_id": bad_run_id, + "event": "run.created", + "properties": { + "settings": run_spec["settings"], + "graph": run_spec["graph"], + "workflow_slug": run_spec["workflow_slug"], + "source_directory": run_spec["source_directory"], + "run_dir": "/tmp/run-2", + "git": run_spec["git"], + "labels": run_spec["labels"], + }, + }), + ) + .await + .unwrap(); let reopened = Database::new(object_store, "runs", Duration::from_millis(1), None); let unreadable = reopened.list_unreadable_runs().await.unwrap(); @@ -1675,23 +1897,9 @@ mod tests { )) .await .unwrap(); - run.append_event(&event_payload( - "run-1", - "2026-03-27T12:00:04Z", - "run.failed", - &serde_json::json!({ - "failure": { - "reason": "workflow_error", - "detail": { - "message": "workflow failed", - "category": "deterministic" - } - }, - "timing": {"wall_time_ms": 1, "inference_time_ms": 0, "tool_time_ms": 0, "active_time_ms": 0}, - }), - )) - .await - .unwrap(); + run.append_event(&workflow_failure_payload("run-1")) + .await + .unwrap(); let reopened = Database::new( Arc::clone(&object_store), diff --git a/lib/components/fabro-store/src/slate/projection_cache.rs b/lib/components/fabro-store/src/slate/projection_cache.rs index 7cec9a204..79ff0c03b 100644 --- a/lib/components/fabro-store/src/slate/projection_cache.rs +++ b/lib/components/fabro-store/src/slate/projection_cache.rs @@ -5,8 +5,8 @@ use chrono::{DateTime, Utc}; use fabro_types::{Run, RunId, RunProjection}; use tokio::sync::Mutex; -use crate::run_state::{RunProjectionReducer, build_summary}; -use crate::{Error, EventEnvelope, ListRunsQuery, Result}; +use crate::ListRunsQuery; +use crate::run_state::build_summary; #[derive(Debug, Clone)] pub struct CachedRunProjection { @@ -85,26 +85,6 @@ impl RunProjectionCacheState { } } - fn update_parent_index( - &mut self, - run_id: RunId, - previous_parent_id: Option, - parent_id: Option, - ) { - if previous_parent_id == parent_id { - return; - } - if let Some(previous_parent_id) = previous_parent_id { - self.remove_parent_link(&previous_parent_id, &run_id); - } - if let Some(parent_id) = parent_id { - self.children_by_parent - .entry(parent_id) - .or_default() - .insert(run_id); - } - } - fn count_children(&self, run_id: &RunId) -> u64 { self.children_by_parent .get(run_id) @@ -222,51 +202,6 @@ impl RunProjectionCache { Some(entry.summary) } - pub(crate) async fn apply_event( - &self, - run_id: &RunId, - event: &EventEnvelope, - ) -> Result { - let mut state = self.state.lock().await; - let Some(entry) = state.entries.get(run_id) else { - if event.seq == 1 { - let projection = RunProjection::apply_events(std::slice::from_ref(event))?; - let entry = CachedRunProjection::from_projection(*run_id, projection, event.seq); - state.insert(entry.clone()); - return Ok(entry); - } - return Err(Error::InvalidEvent(format!( - "projection cache cannot initialize run {run_id} from event seq {}", - event.seq - ))); - }; - - let last_seq = entry.last_seq; - if event.seq <= last_seq { - return Ok(entry.clone()); - } - if event.seq != last_seq.saturating_add(1) { - return Err(Error::Other(format!( - "projection cache sequence gap for run {run_id}: last_seq={}, event_seq={}", - last_seq, event.seq - ))); - } - - let (previous_parent_id, parent_id, entry) = { - let entry = state - .entries - .get_mut(run_id) - .expect("entry was read from the same locked map"); - let previous_parent_id = entry.summary.parent_id; - Arc::make_mut(&mut entry.projection).apply_event(event)?; - entry.summary = build_summary(&entry.projection, run_id); - entry.last_seq = event.seq; - (previous_parent_id, entry.summary.parent_id, entry.clone()) - }; - state.update_parent_index(*run_id, previous_parent_id, parent_id); - Ok(entry) - } - pub(crate) async fn remove(&self, run_id: &RunId) { self.state.lock().await.remove(run_id); } diff --git a/lib/components/fabro-store/src/slate/run_store.rs b/lib/components/fabro-store/src/slate/run_store.rs index 9058627a6..be9ca78bf 100644 --- a/lib/components/fabro-store/src/slate/run_store.rs +++ b/lib/components/fabro-store/src/slate/run_store.rs @@ -9,7 +9,7 @@ use futures::Stream; use slatedb::{Db, DbIterator, DbRead}; use tokio::sync::{Mutex, broadcast, mpsc}; use tokio_stream::wrappers::UnboundedReceiverStream; -use tracing::{error, warn}; +use tracing::warn; use super::blob_store::BlobStore; use super::projection_cache::{CachedRunProjection, RunProjectionCache}; @@ -192,6 +192,15 @@ impl RunDatabase { } async fn projected_state_locked(&self) -> Result> { + self.projected_state_option_locked().await?.ok_or_else(|| { + Error::InvalidEvent(format!( + "run {} has no run.created event", + self.inner.run_id + )) + }) + } + + async fn projected_state_option_locked(&self) -> Result>> { let next_seq = { let cache = self.inner.projection_cache.lock().await; cache.last_seq.saturating_add(1) @@ -202,55 +211,61 @@ impl RunDatabase { apply_cached_projection_event(&mut cache.state, event)?; cache.last_seq = event.seq; } - cache.state.clone().ok_or_else(|| { - Error::InvalidEvent(format!( - "run {} has no run.created event", - self.inner.run_id - )) - }) + Ok(cache.state.clone()) } - async fn cache_event(&self, event: &EventEnvelope) -> Result<()> { + /// Current projection for validating an append allocated at `seq`. In the + /// steady state the local cache already sits at `seq - 1` because + /// `state_lock` serializes appends, so this skips the storage scan that + /// `projected_state_option_locked` issues. + async fn projected_state_for_append_locked( + &self, + seq: u32, + ) -> Result>> { { - let mut projection_cache = self.inner.projection_cache.lock().await; - if projection_cache.state.is_none() && event.seq > 1 { - drop(projection_cache); - self.rebuild_local_projection_cache_through(event.seq) - .await?; - } else { - apply_cached_projection_event(&mut projection_cache.state, event)?; - projection_cache.last_seq = event.seq; + let cache = self.inner.projection_cache.lock().await; + if cache.last_seq.saturating_add(1) == seq { + return Ok(cache.state.clone()); } } + self.projected_state_option_locked().await + } + + async fn install_in_memory_state_after_append( + &self, + event: &EventEnvelope, + cached: &CachedRunProjection, + ) { + { + let mut projection_cache = self.inner.projection_cache.lock().await; + projection_cache.state = Some(Arc::clone(&cached.projection)); + projection_cache.last_seq = event.seq; + } + self.inner + .shared_projection_cache + .replace(cached.clone()) + .await; + let mut recent_events = self.inner.recent_events.lock().await; recent_events.push_back(event.clone()); while recent_events.len() > self.inner.recent_event_limit { recent_events.pop_front(); } + drop(recent_events); let _ = self.inner.event_tx.send(event.clone()); - Ok(()) } - async fn rebuild_local_projection_cache_through(&self, seq: u32) -> Result<()> { - let events = list_events_from(&self.inner.db, &self.inner.run_id, 1).await?; - let Some(last_seq) = events.last().map(|event| event.seq) else { - return Err(Error::InvalidEvent(format!( - "run {} has no events while rebuilding projection cache", - self.inner.run_id - ))); - }; - if last_seq < seq { - return Err(Error::InvalidEvent(format!( - "run {} projection cache rebuild stopped at seq {last_seq}, before appended seq {seq}", - self.inner.run_id - ))); + async fn update_summary_after_committed_append(&self, cached: &CachedRunProjection) { + if let Some(store) = self.inner.run_summary_store.get() { + if let Err(err) = store.upsert_projection(cached).await { + warn!( + run_id = %self.inner.run_id, + source_last_seq = cached.last_seq, + error = ?err, + "failed to update SQLite run summary after committed append" + ); + } } - - let state = RunProjection::apply_events(&events)?; - let mut projection_cache = self.inner.projection_cache.lock().await; - projection_cache.state = Some(Arc::new(state)); - projection_cache.last_seq = last_seq; - Ok(()) } async fn cached_events_from(&self, start_seq: u32, limit: usize) -> Option> { @@ -270,12 +285,26 @@ impl RunDatabase { } impl RunDatabase { + /// Appends an event after validating it against the current run projection. + /// + /// A rejected event writes nothing. Every returned error means the event + /// was not committed and is safe to retry. Once the SlateDB write succeeds, + /// the append returns success even if a derived cache or SQLite summary + /// update fails; those failures are logged and repaired by later updates or + /// startup reconciliation. pub async fn append_event(&self, payload: &EventPayload) -> Result { Ok(self.append_event_envelope(payload).await?.seq) } /// Atomically appends `payload` when `predicate` matches the latest run /// projection. + /// + /// `Ok(None)` means the predicate rejected the append and nothing was + /// written. An invalid transition is also rejected before write, and every + /// returned error means the event was not committed and is safe to retry. + /// After the SlateDB write succeeds, derived cache and SQLite summary + /// updates are best-effort and cannot turn the committed append into an + /// error. pub async fn append_event_if( &self, payload: &EventPayload, @@ -285,95 +314,76 @@ impl RunDatabase { return Err(Error::ReadOnly); } payload.validate(&self.inner.run_id)?; - let _state_guard = self.inner.state_lock.lock().await; - let projection = self.projected_state_locked().await?; - if !predicate(&projection) { - return Ok(None); - } - Ok(Some(self.append_event_envelope_locked(payload).await?.seq)) + let (envelope, cached) = { + let _state_guard = self.inner.state_lock.lock().await; + let projection = self.projected_state_locked().await?; + if !predicate(&projection) { + return Ok(None); + } + let event = RunEvent::try_from(payload)?; + let event_bytes = serde_json::to_vec(payload)?; + self.append_event_envelope_locked(event, event_bytes) + .await? + }; + self.update_summary_after_committed_append(&cached).await; + Ok(Some(envelope.seq)) } + /// Appends and returns the stored event envelope after pre-write reduction. + /// + /// A rejected event writes nothing. Every returned error means the event + /// was not committed and is safe to retry. Once the SlateDB write succeeds, + /// derived cache and SQLite summary updates are best-effort: failures are + /// logged, and this method still returns the committed envelope. pub async fn append_event_envelope(&self, payload: &EventPayload) -> Result { if self.read_only { return Err(Error::ReadOnly); } payload.validate(&self.inner.run_id)?; - let _state_guard = self.inner.state_lock.lock().await; - self.append_event_envelope_locked(payload).await + let event = RunEvent::try_from(payload)?; + let event_bytes = serde_json::to_vec(payload)?; + let (envelope, cached) = { + let _state_guard = self.inner.state_lock.lock().await; + self.append_event_envelope_locked(event, event_bytes) + .await? + }; + self.update_summary_after_committed_append(&cached).await; + Ok(envelope) } - async fn append_event_envelope_locked(&self, payload: &EventPayload) -> Result { + async fn append_event_envelope_locked( + &self, + event: RunEvent, + event_bytes: Vec, + ) -> Result<(EventEnvelope, CachedRunProjection)> { let event_seq = self.inner.event_seq.as_ref().ok_or(Error::ReadOnly)?; - let seq = allocate_event_seq(event_seq)?; - let event = EventEnvelope { + let seq = next_event_seq(event_seq)?; + let envelope = EventEnvelope { seq, event }; + // Validation reduces through the exact code replay uses, so an event + // is written iff replay can reduce it. `Arc::make_mut` copy-on-writes, + // leaving the local projection cache untouched on rejection. + let mut next_state = self.projected_state_for_append_locked(seq).await?; + apply_cached_projection_event(&mut next_state, &envelope).map_err(event_rejected)?; + let next_projection = + next_state.expect("apply_cached_projection_event sets the state on success"); + let cached = CachedRunProjection::from_projection( + self.inner.run_id, + Arc::unwrap_or_clone(next_projection), seq, - event: RunEvent::try_from(payload)?, - }; + ); + reserve_event_seq(event_seq, seq)?; self.inner .db .put( keys::run_event_key(&self.inner.run_id, seq, Utc::now().timestamp_millis()), - serde_json::to_vec(payload)?, + event_bytes, ) .await?; - self.cache_event(&event).await?; - // Box::pin keeps append_event_envelope's future small enough for the - // clippy::large_futures budget of its many callers. - Box::pin(self.update_summary_projection_after_append(&event)).await?; - Ok(event) - } - - async fn update_summary_projection_after_append(&self, event: &EventEnvelope) -> Result<()> { - let cached = match self - .inner - .shared_projection_cache - .apply_event(&self.inner.run_id, event) - .await - { - Ok(entry) => entry, - Err(err) => { - match Self::build_cached_projection(&self.inner.db, &self.inner.run_id).await { - Ok(Some(entry)) => { - self.inner - .shared_projection_cache - .replace(entry.clone()) - .await; - entry - } - rebuild => { - self.inner - .shared_projection_cache - .remove(&self.inner.run_id) - .await; - if let Err(rebuild_err) = rebuild { - warn!( - run_id = %self.inner.run_id, - error = %rebuild_err, - "Failed to rebuild run projection cache after append" - ); - } - warn!( - run_id = %self.inner.run_id, - error = %err, - "Failed to update run projection cache after append" - ); - return Err(err); - } - } - } - }; - if let Some(store) = self.inner.run_summary_store.get() { - if let Err(err) = store.upsert_projection(&cached).await { - error!( - run_id = %self.inner.run_id, - source_last_seq = cached.last_seq, - error = %err, - "Failed to update SQLite run summary after append" - ); - return Err(err); - } - } - Ok(()) + // Box::pin keeps this future small enough for the + // clippy::large_futures budget of append_event_envelope's many + // callers. + Box::pin(self.install_in_memory_state_after_append(&envelope, &cached)).await; + Ok((envelope, cached)) } pub async fn list_events(&self) -> Result> { @@ -589,14 +599,27 @@ impl RunDatabase { } } -fn allocate_event_seq(event_seq: &AtomicU32) -> Result { - event_seq - .fetch_update(Ordering::SeqCst, Ordering::SeqCst, |seq| { - (seq <= keys::MAX_EVENT_SEQ).then_some(seq + 1) - }) - .map_err(|_| Error::EventSequenceExhausted { +fn event_rejected(error: Error) -> Error { + Error::EventRejected { + source: Box::new(error), + } +} + +fn next_event_seq(event_seq: &AtomicU32) -> Result { + let seq = event_seq.load(Ordering::SeqCst); + if seq > keys::MAX_EVENT_SEQ { + return Err(Error::EventSequenceExhausted { max_seq: keys::MAX_EVENT_SEQ, - }) + }); + } + Ok(seq) +} + +fn reserve_event_seq(event_seq: &AtomicU32, seq: u32) -> Result<()> { + event_seq + .compare_exchange(seq, seq + 1, Ordering::SeqCst, Ordering::SeqCst) + .map(|_| ()) + .map_err(|_| Error::Other("event sequence changed while append lock was held".to_string())) } fn apply_cached_projection_event( @@ -1288,6 +1311,29 @@ mod tests { assert_eq!(seqs, vec![5, 4, 3]); } + #[tokio::test] + async fn rejected_event_does_not_consume_last_available_sequence() { + let run = fresh_run().await; + let run_id = run.run_id(); + run.inner + .event_seq + .as_ref() + .unwrap() + .store(keys::MAX_EVENT_SEQ, Ordering::SeqCst); + + let err = run + .append_event(&run_created_payload(&run_id)) + .await + .unwrap_err(); + assert!(matches!(err, Error::EventRejected { .. })); + + let seq = run + .append_event(&stage_prompt_payload(&run_id, 1, Some("alpha"))) + .await + .unwrap(); + assert_eq!(seq, keys::MAX_EVENT_SEQ); + } + #[tokio::test] async fn append_event_rejects_sequences_beyond_key_order_limit() { let run = fresh_run().await; @@ -1304,6 +1350,7 @@ mod tests { .unwrap(); assert_eq!(seq, keys::MAX_EVENT_SEQ); + let events_before_error = run.list_events().await.unwrap(); let err = run .append_event(&stage_prompt_payload(&run_id, 2, Some("beta"))) .await @@ -1313,6 +1360,7 @@ mod tests { Error::EventSequenceExhausted { max_seq } if max_seq == keys::MAX_EVENT_SEQ )); + assert_eq!(run.list_events().await.unwrap(), events_before_error); assert!( run.get_event(keys::MAX_EVENT_SEQ + 1) .await diff --git a/lib/components/fabro-store/src/test_support/mod.rs b/lib/components/fabro-store/src/test_support/mod.rs new file mode 100644 index 000000000..7ae613652 --- /dev/null +++ b/lib/components/fabro-store/src/test_support/mod.rs @@ -0,0 +1,37 @@ +#[cfg(test)] +use std::path::Path; + +use fabro_types::RunId; + +#[cfg(test)] +use crate::RunSummaryStore; +use crate::{Database, Result}; + +/// Writes an event without append validation to model a log corrupted by an +/// older Fabro version. +pub async fn put_unvalidated_run_event( + database: &Database, + run_id: &RunId, + seq: u32, + payload: &serde_json::Value, +) -> Result<()> { + database + .put_unvalidated_run_event(run_id, seq, payload) + .await +} + +#[cfg(test)] +pub(crate) async fn sqlite_summary_store() -> (tempfile::TempDir, RunSummaryStore) { + let directory = tempfile::tempdir().unwrap(); + let store = sqlite_summary_store_at(directory.path()).await; + (directory, store) +} + +#[cfg(test)] +pub(crate) async fn sqlite_summary_store_at(directory: &Path) -> RunSummaryStore { + let database = fabro_db::Database::connect(directory.join("fabro.sqlite3")) + .await + .unwrap(); + database.migrate().await.unwrap(); + RunSummaryStore::new(database.clone_pool()) +} diff --git a/lib/components/fabro-store/src/test_util.rs b/lib/components/fabro-store/src/test_util.rs deleted file mode 100644 index bbc0b8717..000000000 --- a/lib/components/fabro-store/src/test_util.rs +++ /dev/null @@ -1,10 +0,0 @@ -use crate::RunSummaryStore; - -pub(crate) async fn sqlite_summary_store() -> (tempfile::TempDir, RunSummaryStore) { - let directory = tempfile::tempdir().unwrap(); - let database = fabro_db::Database::connect(directory.path().join("fabro.sqlite3")) - .await - .unwrap(); - database.migrate().await.unwrap(); - (directory, RunSummaryStore::new(database.clone_pool())) -} diff --git a/lib/components/fabro-store/src/types.rs b/lib/components/fabro-store/src/types.rs index 65235a55b..c91bbde56 100644 --- a/lib/components/fabro-store/src/types.rs +++ b/lib/components/fabro-store/src/types.rs @@ -57,7 +57,7 @@ impl TryFrom<&EventPayload> for RunEvent { type Error = Error; fn try_from(value: &EventPayload) -> Result { - Self::from_ref(value.as_value()) + Self::from_value(value.as_value().clone()) .map_err(|err| Error::InvalidEvent(format!("invalid stored event: {err}"))) } } diff --git a/lib/components/fabro-workflow/src/event/sink.rs b/lib/components/fabro-workflow/src/event/sink.rs index eb987f620..25d1896ba 100644 --- a/lib/components/fabro-workflow/src/event/sink.rs +++ b/lib/components/fabro-workflow/src/event/sink.rs @@ -163,14 +163,45 @@ impl RunEventLogger { let (tx, mut rx) = mpsc::unbounded_channel(); tokio::spawn(async move { + // A dropped run event is unrecoverable history loss, so the first + // one is an ERROR worth investigating. A broken sink fails for + // every event that follows, so report the rest as a count at flush + // instead of one ERROR per event. Flush runs per stage and per + // agent turn, so only losses since the last summary are reported. + let mut write_failures: u64 = 0; + let mut summarized_failures: u64 = 0; while let Some(command) = rx.recv().await { match command { RunEventCommand::Event(event) => { if let Err(err) = sink.write_run_event(&event).await { - tracing::warn!(error = %err, "Failed to write run event"); + write_failures += 1; + if write_failures == 1 { + tracing::error!( + run_id = %event.run_id, + event = %event.body.event_name(), + error = %err, + "Failed to write run event", + ); + } else { + tracing::debug!( + run_id = %event.run_id, + event = %event.body.event_name(), + failures = write_failures, + error = %err, + "Failed to write run event", + ); + } } } RunEventCommand::Flush(tx) => { + if write_failures > summarized_failures { + tracing::error!( + lost = write_failures - summarized_failures, + total = write_failures, + "Run events were lost to write failures", + ); + summarized_failures = write_failures; + } let _ = tx.send(()); } } diff --git a/lib/components/fabro-workflow/src/handler/command.rs b/lib/components/fabro-workflow/src/handler/command.rs index 6cf7d5741..a8ac957fe 100644 --- a/lib/components/fabro-workflow/src/handler/command.rs +++ b/lib/components/fabro-workflow/src/handler/command.rs @@ -289,7 +289,7 @@ fn schema_validation_failure_reason( let mut reason = format!("Script output failed output_schema validation: {script}"); for message in error.messages() { reason.push_str("\n- "); - reason.push_str(message); + reason.push_str(&message); } append_output_tail(&mut reason, output_text); reason diff --git a/lib/components/fabro-workflow/src/handler/llm/api.rs b/lib/components/fabro-workflow/src/handler/llm/api.rs index e3245eed6..099f568cd 100644 --- a/lib/components/fabro-workflow/src/handler/llm/api.rs +++ b/lib/components/fabro-workflow/src/handler/llm/api.rs @@ -1480,6 +1480,7 @@ impl CodergenBackend for AgentApiBackend { .as_ref() .map(structured_output::prompt_response_format); let mut repair_attempts = 0_i64; + let mut previous_validation_error = None; let mut total_usage = TokenCounts::default(); let mut total_cost = None; let mut inference_duration = Duration::ZERO; @@ -1527,8 +1528,11 @@ impl CodergenBackend for AgentApiBackend { structured_output::exhausted_failure_reason(node.output_retries()), )); } + let repair_message = + error.repair_message(schema, previous_validation_error.as_ref()); + previous_validation_error = Some(error); messages.push(Message::assistant(response_text)); - messages.push(Message::user(error.repair_message(schema))); + messages.push(Message::user(repair_message)); repair_attempts += 1; continue; } @@ -1716,6 +1720,7 @@ impl CodergenBackend for AgentApiBackend { let mut response = last_assistant_response(&live.session); if let Some(schema) = &output_schema { let mut repair_attempts = 0_i64; + let mut previous_validation_error = None; loop { let last_file_touched = last_touched_file(&live.file_tracking); match validate_agent_output_sources( @@ -1734,7 +1739,8 @@ impl CodergenBackend for AgentApiBackend { structured_output::exhausted_failure_reason(node.output_retries()), )); } - let repair_message = error.repair_message(schema); + let repair_message = + error.repair_message(schema, previous_validation_error.as_ref()); let repair_result = live .session .process_input_with_runtime( @@ -1745,6 +1751,11 @@ impl CodergenBackend for AgentApiBackend { live.record_input_timing(); match repair_result { Ok(()) => { + // Only once the model has actually seen the + // repair can a later identical failure mean it + // ignored the correction. Failover rebuilds the + // session from the original prompt instead. + previous_validation_error = Some(error); live.record_input_usage().await; repair_attempts += 1; response = last_assistant_response(&live.session); @@ -2241,6 +2252,13 @@ reasoning = false ) } + fn nested_output_schema_attr() -> AttrValue { + AttrValue::String( + r#"{"type":"object","required":["findings"],"properties":{"findings":{"type":"array","items":{"type":"object","required":["rationale"],"properties":{"rationale":{"type":"string"}}}}}}"# + .to_string(), + ) + } + #[test] fn agent_backend_stores_config() { let backend = AgentApiBackend::new( @@ -3733,6 +3751,76 @@ enabled = true assert_eq!(usage.tokens().output_tokens, 7); } + #[tokio::test] + async fn agent_run_identifies_a_schema_error_repeated_during_repair() { + let server = MockServer::start(); + let first = server.mock(|when, then| { + when.method(POST) + .path("/chat/completions") + .body_includes(r#""stream":true"#) + .body_excludes(r#""role":"assistant""#); + then.status(200) + .header("content-type", "text/event-stream") + .body(chat_completion_stream(r#"{"findings":[{}]}"#, 20, 3)); + }); + let first_repair = server.mock(|when, then| { + when.method(POST) + .path("/chat/completions") + .body_includes("JSON Pointer `/findings/0/rationale`") + .body_excludes("unchanged from your previous repair"); + then.status(200) + .header("content-type", "text/event-stream") + .body(chat_completion_stream(r#"{"findings":[{}]}"#, 21, 4)); + }); + let second_repair = server.mock(|when, then| { + when.method(POST) + .path("/chat/completions") + .body_includes("JSON Pointer `/findings/0/rationale`") + .body_includes("unchanged from your previous repair"); + then.status(200) + .header("content-type", "text/event-stream") + .body(chat_completion_stream( + r#"{"findings":[{"rationale":"done"}]}"#, + 22, + 5, + )); + }); + let backend = mock_api_backend(&server); + let mut node = Node::new("audit"); + node.attrs + .insert("output_schema".to_string(), nested_output_schema_attr()); + node.attrs + .insert("output_retries".to_string(), AttrValue::Integer(2)); + let context = Context::new(); + let emitter = Arc::new(Emitter::new(fabro_types::RunId::new())); + let workspace = tempfile::tempdir().unwrap(); + let sandbox: Arc = + Arc::new(LocalSandbox::new(workspace.path().to_path_buf())); + + let result = backend + .run(CodergenRunRequest { + node: &node, + prompt: "Audit the result", + context: &context, + thread_id: None, + emitter: &emitter, + sandbox: &sandbox, + tool_hooks: None, + cancel_token: CancellationToken::new(), + agent_tool_runtime: fabro_agent::AgentToolRuntime::default(), + }) + .await + .unwrap(); + + first.assert_calls(1); + first_repair.assert_calls(1); + second_repair.assert_calls(1); + let CodergenResult::Text { text, .. } = result else { + panic!("run should return text"); + }; + assert_eq!(text, r#"{"findings":[{"rationale":"done"}]}"#); + } + #[tokio::test] async fn agent_output_repair_continues_on_the_original_models_fallback_plan() { let server = MockServer::start(); diff --git a/lib/components/fabro-workflow/src/handler/structured_output.rs b/lib/components/fabro-workflow/src/handler/structured_output.rs index fc89f6581..845c7b070 100644 --- a/lib/components/fabro-workflow/src/handler/structured_output.rs +++ b/lib/components/fabro-workflow/src/handler/structured_output.rs @@ -1,8 +1,11 @@ +use std::fmt::Write as _; use std::sync::{Arc, LazyLock}; use fabro_graphviz::graph::Node; use fabro_llm::types::{ResponseFormat, ResponseFormatType}; -use jsonschema::Validator; +use jsonschema::error::ValidationErrorKind; +use jsonschema::paths::Location; +use jsonschema::{ValidationError, Validator}; use serde_json::Value; use crate::error::Error; @@ -45,24 +48,153 @@ pub(crate) enum StructuredOutputErrorKind { SchemaValidation, } +const MAX_SCHEMA_FRAGMENT_CHARS: usize = 320; + +/// `additionalProperties` errors carry one entry per unexpected key, and the +/// keys come from model output. Cap them so a wide object can't turn the repair +/// prompt into megabytes. +const MAX_UNEXPECTED_PROPERTIES: usize = 10; + +#[derive(Debug, Clone, PartialEq, Eq)] +struct SchemaValidationIssue { + instance_path: Location, + schema_path: Location, + detail: SchemaValidationIssueDetail, +} + +/// `Required` and `AdditionalProperties` get bespoke rendering because +/// `jsonschema` names the offending property without ever locating it. Every +/// other keyword already renders a message that names both the value and the +/// constraint, so it goes through `Other` with the schema fragment attached. +#[derive(Debug, Clone, PartialEq, Eq)] +enum SchemaValidationIssueDetail { + Required { + property: String, + }, + AdditionalProperties { + unexpected: Vec, + total: usize, + }, + Other { + message: String, + schema_fragment: Option, + }, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +enum StructuredOutputErrorDetails { + Message(String), + SchemaValidation(Vec), +} + #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct StructuredOutputError { - kind: StructuredOutputErrorKind, - messages: Vec, + kind: StructuredOutputErrorKind, + details: StructuredOutputErrorDetails, +} + +impl SchemaValidationIssue { + fn from_error(error: &ValidationError<'_>, schema: Option<&Value>) -> Self { + let detail = match error.kind() { + ValidationErrorKind::Required { property } => SchemaValidationIssueDetail::Required { + property: property + .as_str() + .map_or_else(|| property.to_string(), str::to_owned), + }, + ValidationErrorKind::AdditionalProperties { unexpected } => { + // `unexpected` arrives in the order the model emitted the keys, + // so sort before truncating. That keeps the retained subset and + // the rendered message stable, and lets two attempts that left + // the same keys in place compare equal whatever order they used. + let total = unexpected.len(); + let mut sorted = unexpected.clone(); + sorted.sort_unstable(); + sorted.truncate(MAX_UNEXPECTED_PROPERTIES); + SchemaValidationIssueDetail::AdditionalProperties { + unexpected: sorted, + total, + } + } + _ => SchemaValidationIssueDetail::Other { + message: error.to_string(), + schema_fragment: schema + .and_then(|schema| schema.pointer(error.schema_path().as_str())) + .map(bounded_json), + }, + }; + Self { + instance_path: error.instance_path().clone(), + schema_path: error.schema_path().clone(), + detail, + } + } + + fn render(&self) -> String { + let mut message = match &self.detail { + SchemaValidationIssueDetail::Required { property } => format!( + "Missing required property {} at JSON Pointer `{}`. Add it to the object at {}.", + Value::String(property.clone()), + self.instance_path.join(property), + pointer_phrase(&self.instance_path), + ), + SchemaValidationIssueDetail::AdditionalProperties { unexpected, total } => { + let mut properties = unexpected + .iter() + .map(|property| { + format!( + "{} at `{}`", + Value::String(property.clone()), + self.instance_path.join(property), + ) + }) + .collect::>() + .join(", "); + let remaining = total - unexpected.len(); + if remaining > 0 { + let _ = write!(properties, ", and {remaining} more"); + } + format!( + "Unexpected properties in the object at {}: {properties}.", + pointer_phrase(&self.instance_path), + ) + } + SchemaValidationIssueDetail::Other { message, .. } => format!( + "At {}: {}.", + pointer_phrase(&self.instance_path), + message.trim_end_matches('.'), + ), + }; + + let _ = write!( + message, + " Schema rule: {}", + pointer_phrase(&self.schema_path) + ); + if let SchemaValidationIssueDetail::Other { + schema_fragment: Some(fragment), + .. + } = &self.detail + { + message.push_str(": "); + message.push_str(fragment); + } + message.push('.'); + message + } } impl StructuredOutputError { fn new(kind: StructuredOutputErrorKind, message: impl Into) -> Self { Self { kind, - messages: vec![message.into()], + details: StructuredOutputErrorDetails::Message(message.into()), } } - fn validation(messages: Vec) -> Self { + fn validation(issues: Vec) -> Self { Self { - kind: StructuredOutputErrorKind::SchemaValidation, - messages, + kind: StructuredOutputErrorKind::SchemaValidation, + details: StructuredOutputErrorDetails::SchemaValidation(issues), } } @@ -73,8 +205,13 @@ impl StructuredOutputError { } #[must_use] - pub(crate) fn messages(&self) -> &[String] { - &self.messages + pub(crate) fn messages(&self) -> Vec { + match &self.details { + StructuredOutputErrorDetails::Message(message) => vec![message.clone()], + StructuredOutputErrorDetails::SchemaValidation(issues) => { + issues.iter().map(SchemaValidationIssue::render).collect() + } + } } #[must_use] @@ -87,7 +224,11 @@ impl StructuredOutputError { } #[must_use] - pub(crate) fn repair_message(&self, schema: &OutputSchemaKind) -> String { + pub(crate) fn repair_message( + &self, + schema: &OutputSchemaKind, + previous_error: Option<&Self>, + ) -> String { let expectation = match schema { OutputSchemaKind::Routing => format!( "Return a single JSON object with at least one routing field: {}.", @@ -98,18 +239,61 @@ impl StructuredOutputError { } }; let errors = self - .messages + .messages() .iter() .map(|message| format!("- {message}")) .collect::>() .join("\n"); - format!( - "Your previous response did not satisfy the node's output_schema.\n\n\ - Validation errors:\n{errors}\n\n\ - {expectation}\n\ - Do not include Markdown fences or explanatory prose; reply only with the corrected JSON object." - ) + let mut sections = + vec!["Your previous response did not satisfy the node's output_schema.".to_string()]; + if previous_error.is_some_and(|previous| self.shares_schema_issue_with(previous)) { + sections.push( + "At least one validation problem below is unchanged from your previous repair." + .to_string(), + ); + } + sections.push(format!("Validation errors:\n{errors}")); + sections.push(expectation); + if self.kind == StructuredOutputErrorKind::SchemaValidation { + sections.push( + "Apply each correction at the exact JSON Pointer shown and return the complete object." + .to_string(), + ); + } + sections.push( + "Do not include Markdown fences or explanatory prose; reply only with the corrected JSON object." + .to_string(), + ); + sections.join("\n\n") } + + fn shares_schema_issue_with(&self, other: &Self) -> bool { + let ( + StructuredOutputErrorDetails::SchemaValidation(current), + StructuredOutputErrorDetails::SchemaValidation(previous), + ) = (&self.details, &other.details) + else { + return false; + }; + current.iter().any(|issue| previous.contains(issue)) + } +} + +fn pointer_phrase(path: &Location) -> String { + if path.as_str().is_empty() { + "the document root".to_string() + } else { + format!("JSON Pointer `{path}`") + } +} + +fn bounded_json(value: &Value) -> String { + let mut rendered = value.to_string(); + if let Some((offset, _)) = rendered.char_indices().nth(MAX_SCHEMA_FRAGMENT_CHARS) { + rendered.truncate(offset); + rendered.push('…'); + } + rendered } #[derive(Debug, Clone, PartialEq)] @@ -202,8 +386,8 @@ pub(crate) fn validate_response_text( ) -> Result { match schema { OutputSchemaKind::Routing => validate_routing_response_text(text), - OutputSchemaKind::JsonSchema { validator, .. } => { - validate_custom_response_text(validator, text) + OutputSchemaKind::JsonSchema { schema, validator } => { + validate_custom_response_text(validator, schema, text) } } } @@ -324,7 +508,7 @@ fn validate_routing_response_text( if !contains_routing_field(obj) { continue; } - validate_value_against_validator(routing_validator(), &parsed)?; + validate_value_against_validator(routing_validator(), &parsed, None)?; return Ok(ValidatedStructuredOutput { value: parsed }); } @@ -339,6 +523,7 @@ fn validate_routing_response_text( fn validate_custom_response_text( validator: &Validator, + schema: &Value, text: &str, ) -> Result { // Prose after the object can contain braces, so the last candidate is not @@ -349,7 +534,7 @@ fn validate_custom_response_text( for candidate in candidates.iter().rev() { match serde_json::from_str::(candidate) { Ok(parsed) => { - validate_value_against_validator(validator, &parsed)?; + validate_value_against_validator(validator, &parsed, Some(schema))?; return Ok(ValidatedStructuredOutput { value: parsed }); } Err(err) if invalid_json.is_none() => invalid_json = Some(err.to_string()), @@ -372,16 +557,17 @@ fn validate_custom_response_text( fn validate_value_against_validator( validator: &Validator, value: &Value, + schema: Option<&Value>, ) -> Result<(), StructuredOutputError> { - let errors = validator + let issues = validator .iter_errors(value) - .map(|error| error.to_string()) .take(5) + .map(|error| SchemaValidationIssue::from_error(&error, schema)) .collect::>(); - if errors.is_empty() { + if issues.is_empty() { Ok(()) } else { - Err(StructuredOutputError::validation(errors)) + Err(StructuredOutputError::validation(issues)) } } @@ -682,6 +868,151 @@ mod tests { ); } + #[test] + fn missing_nested_property_reports_the_required_target_pointer() { + let schema = schema(serde_json::json!({ + "type": "object", + "required": ["findings"], + "properties": { + "findings": { + "type": "array", + "items": { + "type": "object", + "required": ["rationale"], + "properties": { + "rationale": { "type": "string" } + } + } + } + } + })); + + let error = validate_response_text(&schema, r#"{"findings":[{}]}"#).unwrap_err(); + + assert_eq!(error.messages(), vec![ + "Missing required property \"rationale\" at JSON Pointer `/findings/0/rationale`. \ + Add it to the object at JSON Pointer `/findings/0`. Schema rule: JSON Pointer \ + `/properties/findings/items/required`." + .to_string(), + ],); + } + + #[test] + fn type_and_enum_errors_report_instance_and_schema_pointers() { + let schema = schema(serde_json::json!({ + "type": "object", + "properties": { + "line": { "type": "integer" }, + "severity": { "enum": ["HIGH", "MEDIUM", "LOW"] } + } + })); + + let error = + validate_response_text(&schema, r#"{"line":"85","severity":"CRITICAL"}"#).unwrap_err(); + + assert_eq!(error.messages(), vec![ + "At JSON Pointer `/line`: \"85\" is not of type \"integer\". \ + Schema rule: JSON Pointer `/properties/line/type`: \"integer\"." + .to_string(), + "At JSON Pointer `/severity`: \"CRITICAL\" is not one of \"HIGH\", \"MEDIUM\" or \ + \"LOW\". Schema rule: JSON Pointer `/properties/severity/enum`: \ + [\"HIGH\",\"MEDIUM\",\"LOW\"]." + .to_string(), + ],); + } + + #[test] + fn additional_property_error_reports_each_property_pointer() { + let schema = schema(serde_json::json!({ + "type": "object", + "additionalProperties": false, + "properties": { + "findings": { "type": "array" } + } + })); + + let error = validate_response_text(&schema, r#"{"findings":[],"rationale":"wrong level"}"#) + .unwrap_err(); + + assert_eq!(error.messages(), vec![ + "Unexpected properties in the object at the document root: \"rationale\" at \ + `/rationale`. Schema rule: JSON Pointer `/additionalProperties`." + .to_string(), + ],); + } + + #[test] + fn repeated_schema_error_calls_out_the_unchanged_pointer() { + let schema = schema(serde_json::json!({ + "type": "object", + "required": ["findings"], + "properties": { + "findings": { + "type": "array", + "items": { + "type": "object", + "required": ["rationale"] + } + } + } + })); + let previous = validate_response_text(&schema, r#"{"findings":[{}]}"#).unwrap_err(); + let current = validate_response_text(&schema, r#"{"findings":[{}]}"#).unwrap_err(); + + let repair = current.repair_message(&schema, Some(&previous)); + + assert!( + repair.contains( + "At least one validation problem below is unchanged from your previous repair." + ), + "unexpected repair message: {repair}", + ); + assert!( + repair.contains("JSON Pointer `/findings/0/rationale`"), + "unexpected repair message: {repair}", + ); + } + + #[test] + fn the_same_unexpected_properties_in_a_new_order_are_still_unchanged() { + let schema = schema(serde_json::json!({ + "type": "object", + "additionalProperties": false, + "properties": { + "findings": { "type": "array" } + } + })); + let previous = validate_response_text(&schema, r#"{"beta":1,"alpha":1}"#).unwrap_err(); + let current = validate_response_text(&schema, r#"{"alpha":1,"beta":1}"#).unwrap_err(); + + let repair = current.repair_message(&schema, Some(&previous)); + + assert!( + repair.contains("unchanged from your previous repair"), + "unexpected repair message: {repair}", + ); + } + + #[test] + fn a_different_problem_at_the_same_location_is_not_called_unchanged() { + let schema = schema(serde_json::json!({ + "type": "object", + "additionalProperties": false, + "properties": { + "findings": { "type": "array" } + } + })); + let previous = validate_response_text(&schema, r#"{"stray":1}"#).unwrap_err(); + let current = validate_response_text(&schema, r#"{"different":1}"#).unwrap_err(); + + let repair = current.repair_message(&schema, Some(&previous)); + + assert!( + !repair.contains("unchanged from your previous repair"), + "unexpected repair message: {repair}", + ); + } + #[test] fn invalid_custom_schema_is_rejected_when_parsing_node_attr() { let mut node = Node::new("audit"); diff --git a/lib/components/fabro-workflow/src/operations/create.rs b/lib/components/fabro-workflow/src/operations/create.rs index 8451ef1df..780599b02 100644 --- a/lib/components/fabro-workflow/src/operations/create.rs +++ b/lib/components/fabro-workflow/src/operations/create.rs @@ -22,12 +22,10 @@ use tokio::task::spawn_blocking; use super::source::{ResolveWorkflowInput, WorkflowInput, resolve_workflow}; use crate::error::Error; use crate::event::{Event, append_event, to_run_event_at}; -use crate::file_resolver::FileResolver; use crate::pipeline::types::PersistOptions; use crate::pipeline::{self, Persisted, TransformOptions, Validated}; use crate::records::RunSpec; -use crate::run_lookup::default_scratch_base; -use crate::run_materialization::materialize_run; +use crate::run_materialization; use crate::transforms::{ModelResolutionTransform, RenderMode}; use crate::workflow_bundle::{RunDefinition, WorkflowBundle}; @@ -58,6 +56,90 @@ pub struct CreateRunInput { pub web_url: Option, } +impl CreateRunInput { + /// Split into the compile-stage input and the persistence metadata for + /// `run_id`, the two halves of the create pipeline. + fn into_stages( + self, + run_id: RunId, + storage_root: PathBuf, + ) -> (CreateRunCompileInput, CreateRunPersistenceMetadata) { + let Self { + workflow, + settings, + vars, + cwd, + workflow_slug, + workflow_path, + workflow_bundle, + submitted_manifest_bytes, + run_id: _, + title, + automation, + git, + fork_source_ref, + parent_id, + provenance, + configured_providers, + web_url, + } = self; + ( + CreateRunCompileInput { + workflow, + settings, + vars, + cwd, + workflow_path, + workflow_bundle, + configured_providers, + }, + CreateRunPersistenceMetadata { + run_id, + storage_root, + workflow_slug, + submitted_manifest_bytes, + title, + automation, + git, + fork_source_ref, + parent_id, + provenance, + web_url, + }, + ) + } +} + +/// Inputs needed to resolve and compile a workflow for run creation. +#[derive(Debug)] +pub struct CreateRunCompileInput { + pub workflow: WorkflowInput, + pub settings: WorkflowSettings, + pub vars: HashMap, + pub cwd: PathBuf, + pub workflow_path: Option, + pub workflow_bundle: Option, + pub configured_providers: Vec, +} + +/// Durable metadata joined to a materialized workflow before persistence. +/// `run_id` is already resolved, and `storage_root` is used to derive the +/// run's scratch directory during pure input assembly. +#[derive(Debug)] +pub struct CreateRunPersistenceMetadata { + pub run_id: RunId, + pub storage_root: PathBuf, + pub workflow_slug: Option, + pub submitted_manifest_bytes: Option>, + pub title: Option, + pub automation: Option, + pub git: Option, + pub fork_source_ref: Option, + pub parent_id: Option, + pub provenance: RunProvenance, + pub web_url: Option, +} + #[derive(Debug)] pub struct CreatedRun { pub persisted: Persisted, @@ -66,20 +148,97 @@ pub struct CreatedRun { pub dot_path: Option, } -struct PersistCreateOptions { +/// Result of resolving, preprocessing, validating, and promoting a workflow +/// for run creation. Model selectors in the graph are resolved, while the run +/// settings still reflect the compiled source and have not been materialized. +pub struct CompiledRun { + validated: Validated, settings: WorkflowSettings, - run_id: Option, - run_dir: Option, + raw_source: String, workflow_slug: Option, - source_name: Option, + workflow_config: Option, + dot_path: Option, + definition: Option, + source_directory: String, labels: HashMap, - source_directory: Option, - automation: Option, - git: Option, - fork_source_ref: Option, - provenance: RunProvenance, configured_providers: Vec, - catalog: Arc, +} + +impl CompiledRun { + pub fn validated(&self) -> &Validated { + &self.validated + } + + pub fn settings(&self) -> &WorkflowSettings { + &self.settings + } +} + +/// Compiled workflow with its run-level model settings materialized against +/// the same provider snapshot used during compilation. +pub struct MaterializedRun { + validated: Validated, + settings: WorkflowSettings, + raw_source: String, + workflow_slug: Option, + workflow_config: Option, + dot_path: Option, + definition: Option, + source_directory: String, + labels: HashMap, +} + +impl MaterializedRun { + pub fn settings(&self) -> &WorkflowSettings { + &self.settings + } +} + +/// Complete input for creating a durable run. The run ID and run directory +/// are resolved during assembly, before persistence begins. +pub struct CreateRunPersistenceInput { + materialized: MaterializedRun, + run_id: RunId, + run_dir: PathBuf, + workflow_slug: Option, + submitted_manifest_bytes: Option>, + title: Option, + automation: Option, + git: Option, + fork_source_ref: Option, + parent_id: Option, + provenance: RunProvenance, + web_url: Option, +} + +impl CreateRunPersistenceInput { + pub fn materialized(&self) -> &MaterializedRun { + &self.materialized + } + + pub fn run_id(&self) -> RunId { + self.run_id + } + + pub fn run_dir(&self) -> &Path { + &self.run_dir + } + + pub fn workflow_slug(&self) -> Option<&str> { + self.workflow_slug.as_deref() + } + + pub fn submitted_manifest_bytes(&self) -> Option<&[u8]> { + self.submitted_manifest_bytes.as_deref() + } + + pub fn automation(&self) -> Option<&AutomationRef> { + self.automation.as_ref() + } + + pub fn definition(&self) -> Option<&RunDefinition> { + self.materialized.definition.as_ref() + } } /// Resolve workflow inputs, normalize settings using the caller-provided @@ -90,96 +249,253 @@ pub async fn create( storage_root: PathBuf, catalog: Arc, ) -> Result { - let resolved = resolve_workflow(ResolveWorkflowInput { - workflow: request.workflow, - settings: request.settings, - cwd: request.cwd, + let run_id = request.run_id.unwrap_or_default(); + let persistence_input = spawn_blocking(move || { + let (compile_input, metadata) = request.into_stages(run_id, storage_root); + let compiled = compile_create_run(compile_input, Arc::clone(&catalog))?; + let materialized = materialize_create_run(compiled, catalog.as_ref())?; + Ok::<_, Error>(assemble_create_run_persistence_input( + materialized, + metadata, + )) }) - .map_err(|err| Error::Parse(err.to_string()))?; - let labels = resolved.settings.combined_labels(); - let settings = resolved.settings.clone(); + .await + .map_err(|err| Error::engine_with_source("workflow create task failed", err))??; - let CreateRunInput { - workflow: _, - settings: _, + Box::pin(persist_create_run(store, persistence_input)).await +} + +/// Resolve, preprocess, validate, and promote a workflow for run creation. +/// +/// This stage is synchronous and may read workflow files. Async callers must +/// run it on a blocking thread. +pub fn compile_create_run( + input: CreateRunCompileInput, + catalog: Arc, +) -> Result { + let CreateRunCompileInput { + workflow, + settings, vars, - cwd: _, - workflow_slug, + cwd, workflow_path, workflow_bundle, - submitted_manifest_bytes, + configured_providers, + } = input; + let resolved = resolve_workflow(ResolveWorkflowInput { + workflow, + settings, + cwd, + }) + .map_err(|err| Error::Parse(err.to_string()))?; + let settings = resolved.settings; + let labels = settings.combined_labels(); + let workflow_config = resolved + .workflow_toml_path + .as_deref() + .and_then(|path| std::fs::read_to_string(path).ok()); + let source_name = resolved + .dot_path + .as_ref() + .map(|path| path.display().to_string()); + let definition = match (workflow_path, workflow_bundle) { + (Some(workflow_path), Some(workflow_bundle)) => { + let bundled = workflow_bundle.workflow(&workflow_path).ok_or_else(|| { + Error::Parse("workflow path is missing from workflow bundle".to_string()) + })?; + if bundled.source != resolved.raw_source { + return Err(Error::Parse( + "resolved workflow does not match workflow bundle entrypoint".to_string(), + )); + } + Some(RunDefinition::new(workflow_path, workflow_bundle)) + } + (None, None) => None, + _ => { + return Err(Error::Parse( + "workflow path and workflow bundle must be provided together".to_string(), + )); + } + }; + let mut validated = preprocess_and_validate( + &resolved.raw_source, + resolved.goal_override.as_deref(), + &TransformOptions { + current_dir: resolved.current_dir.clone(), + file_resolver: resolved.file_resolver.clone(), + template_context: template_context(Some(&settings), vars), + source_name, + render_mode: RenderMode::Structural, + custom_transforms: Vec::new(), + model_resolution: Some( + ModelResolutionTransform::for_eligible( + catalog, + configured_providers.iter().cloned().collect(), + ) + .with_default_provider(configured_default_provider(&settings)), + ), + }, + )?; + + validated.promote_template_undefined_variables_to_errors(); + if validated.has_errors() { + return Err(Error::ValidationFailed { + diagnostics: validated.diagnostics().to_vec(), + }); + } + + Ok(CompiledRun { + validated, + settings, + raw_source: resolved.raw_source, + workflow_slug: resolved.workflow_slug, + workflow_config, + dot_path: resolved.dot_path, + definition, + source_directory: resolved.working_directory.to_string_lossy().to_string(), + labels, + configured_providers, + }) +} + +/// Materialize run-level model settings from a compiled workflow. +pub fn materialize_create_run( + compiled: CompiledRun, + catalog: &Catalog, +) -> Result { + let CompiledRun { + validated, + settings, + raw_source, + workflow_slug, + workflow_config, + dot_path, + definition, + source_directory, + labels, + configured_providers, + } = compiled; + let settings = run_materialization::materialize_run( + settings, + validated.graph(), + catalog, + &configured_providers, + )?; + Ok(MaterializedRun { + validated, + settings, + raw_source, + workflow_slug, + workflow_config, + dot_path, + definition, + source_directory, + labels, + }) +} + +/// Assemble all inputs needed for persistence without I/O or recompilation. +pub fn assemble_create_run_persistence_input( + materialized: MaterializedRun, + metadata: CreateRunPersistenceMetadata, +) -> CreateRunPersistenceInput { + let CreateRunPersistenceMetadata { run_id, + storage_root, + workflow_slug, + submitted_manifest_bytes, title, automation, git, fork_source_ref, parent_id, provenance, - configured_providers, web_url, - } = request; + } = metadata; + let run_dir = Storage::new(storage_root) + .run_scratch(&run_id) + .root() + .to_path_buf(); + let workflow_slug = workflow_slug.or_else(|| materialized.workflow_slug.clone()); - let run_id = run_id.unwrap_or_else(RunId::new); - let storage = Storage::new(storage_root); - let run_dir = storage.run_scratch(&run_id).root().to_path_buf(); - let source_directory = Some(resolved.working_directory.to_string_lossy().to_string()); + CreateRunPersistenceInput { + materialized, + run_id, + run_dir, + workflow_slug, + submitted_manifest_bytes, + title, + automation, + git, + fork_source_ref, + parent_id, + provenance, + web_url, + } +} - let goal_override = resolved.goal_override.clone(); - let current_dir = resolved.current_dir.clone(); - let file_resolver = resolved.file_resolver.clone(); - let resolved_workflow_slug = resolved.workflow_slug.clone(); +/// Persist one already-compiled and materialized run without recompiling it. +pub async fn persist_create_run( + store: &Database, + input: CreateRunPersistenceInput, +) -> Result { + let CreateRunPersistenceInput { + materialized, + run_id, + run_dir, + workflow_slug, + submitted_manifest_bytes, + title, + automation, + git, + fork_source_ref, + parent_id, + provenance, + web_url, + } = input; + let MaterializedRun { + validated, + settings, + raw_source, + workflow_slug: _, + workflow_config, + dot_path, + definition, + source_directory, + labels, + } = materialized; let persisted_run_dir = run_dir.clone(); - let accepted_definition = match (&workflow_path, &workflow_bundle) { - (Some(workflow_path), Some(workflow_bundle)) => Some(RunDefinition::new( - workflow_path.clone(), - workflow_bundle.clone(), - )), - _ => None, - }; - - let raw_source = resolved.raw_source.clone(); - let source_name = resolved - .dot_path - .as_ref() - .map(|path| path.display().to_string()); let persisted = spawn_blocking(move || { - create_from_source( - &raw_source, - vars, - PersistCreateOptions { - settings, - run_id: Some(run_id), - run_dir: Some(persisted_run_dir), - workflow_slug: workflow_slug.or(resolved_workflow_slug), - source_name, - labels, - source_directory, - automation, - git, - fork_source_ref, - provenance, - configured_providers, - catalog, - }, - current_dir, - file_resolver, - goal_override.as_deref(), - ) + let run_spec = RunSpec { + run_id, + settings, + graph: validated.graph().clone(), + graph_source: Some(validated.source().to_string()), + workflow_slug, + automation, + source_directory: Some(source_directory), + labels, + provenance, + manifest_blob: None, + definition_blob: None, + git, + fork_source_ref, + }; + pipeline::persist(validated, PersistOptions { + run_dir: persisted_run_dir, + run_spec, + }) }) .await .map_err(|err| Error::engine_with_source("workflow create task failed", err))??; - let workflow_config = resolved - .workflow_toml_path - .as_deref() - .and_then(|path| std::fs::read_to_string(path).ok()); persist_created_run( store, &persisted, - &resolved.raw_source, + &raw_source, workflow_config, submitted_manifest_bytes.as_deref(), - accepted_definition.as_ref(), + definition.as_ref(), title, parent_id, web_url, @@ -190,7 +506,7 @@ pub async fn create( persisted, run_id, run_dir, - dot_path: resolved.dot_path, + dot_path, }) } @@ -285,40 +601,6 @@ fn store_error(err: impl std::fmt::Display) -> Error { Error::engine(err.to_string()) } -fn create_from_source( - dot_source: &str, - vars: HashMap, - options: PersistCreateOptions, - current_dir: Option, - file_resolver: Option>, - goal_override: Option<&str>, -) -> Result { - let mut validated = preprocess_and_validate(dot_source, goal_override, &TransformOptions { - current_dir, - file_resolver, - template_context: template_context(Some(&options.settings), vars), - source_name: options.source_name.clone(), - render_mode: RenderMode::Structural, - custom_transforms: Vec::new(), - model_resolution: Some( - ModelResolutionTransform::for_eligible( - Arc::clone(&options.catalog), - options.configured_providers.iter().cloned().collect(), - ) - .with_default_provider(configured_default_provider(&options.settings)), - ), - })?; - - validated.promote_template_undefined_variables_to_errors(); - if validated.has_errors() { - return Err(Error::ValidationFailed { - diagnostics: validated.diagnostics().to_vec(), - }); - } - - persist_validated(validated, options) -} - /// Parse, transform, and validate `dot_source`. /// /// `options.model_resolution` drives both halves of catalog awareness: it @@ -376,59 +658,6 @@ fn apply_goal_override(graph: &mut Graph, goal_override: Option<&str>) { } } -fn persist_validated( - validated: Validated, - options: PersistCreateOptions, -) -> Result { - let PersistCreateOptions { - settings, - run_id, - run_dir, - workflow_slug, - source_name: _, - labels, - source_directory, - automation, - git, - fork_source_ref, - provenance, - configured_providers, - catalog, - } = options; - - let settings = materialize_run( - settings, - validated.graph(), - catalog.as_ref(), - &configured_providers, - )?; - - let run_id = run_id.unwrap_or_else(RunId::new); - let run_dir = run_dir.unwrap_or_else(|| default_run_dir(&run_id)); - - let run_spec = RunSpec { - run_id, - settings, - graph: validated.graph().clone(), - graph_source: Some(validated.source().to_string()), - workflow_slug, - automation, - source_directory, - labels, - provenance, - manifest_blob: None, - definition_blob: None, - git, - fork_source_ref, - }; - - pipeline::persist(validated, PersistOptions { run_dir, run_spec }) -} - -pub(crate) fn default_run_dir(run_id: &RunId) -> PathBuf { - make_run_dir(&default_scratch_base(), run_id) -} - pub fn make_run_dir(scratch_base: &Path, run_id: &RunId) -> PathBuf { fabro_config::RunScratch::for_run(scratch_base, run_id) .root() @@ -456,6 +685,7 @@ mod tests { use object_store::memory::InMemory; use super::*; + use crate::file_resolver::FileResolver; use crate::operations::{ValidateInput, validate, validate_with_catalog}; use crate::pipeline::types::{GOAL_SELF_REFERENCE_RULE, TEMPLATE_UNDEFINED_VARIABLE_RULE}; use crate::transforms::Transform; @@ -547,6 +777,24 @@ reasoning = false Catalog::builtin().all_provider_ids().into_iter().collect() } + fn compile_input(request: &CreateRunInput) -> CreateRunCompileInput { + let (compile_input, _) = request + .clone() + .into_stages(RunId::new(), PathBuf::from("/tmp/storage")); + compile_input + } + + fn persistence_metadata( + request: &CreateRunInput, + run_id: RunId, + storage_root: &Path, + ) -> CreateRunPersistenceMetadata { + let (_, metadata) = request + .clone() + .into_stages(run_id, storage_root.to_path_buf()); + metadata + } + fn validate_dot(dot_source: &str, settings: WorkflowSettings) -> Validated { validate_with_catalog( ValidateInput { @@ -1337,6 +1585,242 @@ reasoning = false ); } + #[test] + fn assemble_create_run_persistence_input_resolves_complete_durable_identity() { + let dir = tempfile::tempdir().unwrap(); + let storage_root = dir.path().join("storage"); + let automation = AutomationRef { + id: "nightly".to_string(), + name: Some("Nightly".to_string()), + trigger_id: Some("schedule_1".to_string()), + }; + let request = CreateRunInput { + workflow: WorkflowInput::DotSource { + source: MINIMAL_DOT.to_string(), + base_dir: None, + }, + settings: test_default_settings(), + vars: HashMap::new(), + cwd: dir.path().to_path_buf(), + workflow_slug: Some("request-slug".to_string()), + workflow_path: None, + workflow_bundle: None, + submitted_manifest_bytes: Some(b"submitted manifest".to_vec()), + run_id: Some(fixtures::RUN_1), + title: Some("Assembled run".to_string()), + automation: Some(automation.clone()), + git: None, + fork_source_ref: None, + parent_id: Some(fixtures::RUN_2), + provenance: test_support::test_run_provenance(), + configured_providers: test_provider_ids(), + web_url: Some("https://fabro.test/runs/1".to_string()), + }; + let catalog = test_catalog(); + let resolved_run_id = fixtures::RUN_64; + + let compiled = compile_create_run(compile_input(&request), Arc::clone(&catalog)).unwrap(); + let materialized = materialize_create_run(compiled, catalog.as_ref()).unwrap(); + let metadata = persistence_metadata(&request, resolved_run_id, &storage_root); + let input = assemble_create_run_persistence_input(materialized, metadata); + + assert_eq!(input.run_id(), resolved_run_id); + assert_eq!( + input.run_dir(), + Storage::new(&storage_root) + .run_scratch(&resolved_run_id) + .root() + ); + assert_eq!(input.workflow_slug(), Some("request-slug")); + assert_eq!( + input.submitted_manifest_bytes(), + Some(b"submitted manifest".as_slice()) + ); + assert_eq!(input.automation(), Some(&automation)); + assert_eq!( + input.materialized().settings().run.model.name.as_deref(), + Some("claude-sonnet-5") + ); + } + + #[test] + fn compile_create_run_rejects_mismatched_bundle_definition() { + let workflow_path = ManifestPath::from_wire("workflows/main.fabro").unwrap(); + let compiled_workflow = BundledWorkflow { + path: workflow_path.clone(), + source: MINIMAL_DOT.to_string(), + config: None, + files: HashMap::new(), + }; + let mismatched_bundle = + WorkflowBundle::new(HashMap::from([(workflow_path.clone(), BundledWorkflow { + source: MINIMAL_DOT.replace("Build feature", "Different goal"), + ..compiled_workflow.clone() + })])); + + let Err(error) = compile_create_run( + CreateRunCompileInput { + workflow: WorkflowInput::Bundled(compiled_workflow), + settings: test_default_settings(), + vars: HashMap::new(), + cwd: PathBuf::from("/tmp/project"), + workflow_path: Some(workflow_path), + workflow_bundle: Some(mismatched_bundle), + configured_providers: test_provider_ids(), + }, + test_catalog(), + ) else { + panic!("mismatched accepted definition should fail"); + }; + + assert!(matches!(error, Error::Parse(message) if message == + "resolved workflow does not match workflow bundle entrypoint")); + } + + #[test] + fn compile_create_run_exposes_resolved_metadata_and_definition() { + let workflow_path = ManifestPath::from_wire("workflows/main.fabro").unwrap(); + let bundled = BundledWorkflow { + path: workflow_path.clone(), + source: MINIMAL_DOT.to_string(), + config: None, + files: HashMap::new(), + }; + let bundle = WorkflowBundle::new(HashMap::from([(workflow_path.clone(), bundled.clone())])); + let compiled = compile_create_run( + CreateRunCompileInput { + workflow: WorkflowInput::Bundled(bundled), + settings: test_default_settings(), + vars: HashMap::new(), + cwd: PathBuf::from("/tmp/project"), + workflow_path: Some(workflow_path.clone()), + workflow_bundle: Some(bundle), + configured_providers: test_provider_ids(), + }, + test_catalog(), + ) + .unwrap(); + + assert_eq!(compiled.raw_source, MINIMAL_DOT); + assert_eq!(compiled.dot_path.as_deref(), Some(workflow_path.as_path())); + assert_eq!(compiled.labels, compiled.settings().combined_labels()); + let materialized = materialize_create_run(compiled, test_catalog().as_ref()).unwrap(); + let input = + assemble_create_run_persistence_input(materialized, CreateRunPersistenceMetadata { + run_id: fixtures::RUN_1, + storage_root: PathBuf::from("/tmp/storage"), + workflow_slug: None, + submitted_manifest_bytes: None, + title: None, + automation: None, + git: None, + fork_source_ref: None, + parent_id: None, + provenance: test_support::test_run_provenance(), + web_url: None, + }); + let definition = input + .definition() + .expect("bundled create input should retain a run definition"); + assert_eq!(definition.workflow_path, workflow_path); + } + + #[tokio::test] + async fn persist_create_run_uses_compiled_graph_without_recompiling_source() { + let dir = tempfile::tempdir().unwrap(); + let storage_root = dir.path().join("storage"); + let dot_path = dir.path().join("workflow.fabro"); + let compiled_source = MINIMAL_DOT.replace("Build feature", "Compiled goal"); + std::fs::write(&dot_path, &compiled_source).unwrap(); + let automation = AutomationRef { + id: "nightly".to_string(), + name: Some("Nightly".to_string()), + trigger_id: Some("schedule_1".to_string()), + }; + let request = CreateRunInput { + workflow: WorkflowInput::Path(dot_path.clone()), + settings: test_default_settings(), + vars: HashMap::new(), + cwd: dir.path().to_path_buf(), + workflow_slug: Some("compiled-slug".to_string()), + workflow_path: None, + workflow_bundle: None, + submitted_manifest_bytes: Some(b"submitted manifest".to_vec()), + run_id: Some(fixtures::RUN_2), + title: Some("Compiled run".to_string()), + automation: Some(automation.clone()), + git: None, + fork_source_ref: None, + parent_id: None, + provenance: test_support::test_run_provenance(), + configured_providers: test_provider_ids(), + web_url: None, + }; + let catalog = test_catalog(); + let workflow_config_path = dir.path().join("workflow.toml"); + std::fs::write( + &workflow_config_path, + "_version = 1\n[workflow]\ngraph = \"workflow.fabro\"\n", + ) + .unwrap(); + let compiled = compile_create_run(compile_input(&request), Arc::clone(&catalog)).unwrap(); + assert_eq!( + compiled.workflow_config.as_deref(), + Some("_version = 1\n[workflow]\ngraph = \"workflow.fabro\"\n") + ); + + std::fs::write(&dot_path, "this is no longer a graph").unwrap(); + std::fs::write(&workflow_config_path, "changed after compilation").unwrap(); + + let materialized = materialize_create_run(compiled, catalog.as_ref()).unwrap(); + let metadata = persistence_metadata(&request, fixtures::RUN_2, &storage_root); + let input = assemble_create_run_persistence_input(materialized, metadata); + let store = memory_store(); + let created = persist_create_run(store.as_ref(), input).await.unwrap(); + + assert_eq!(created.run_id, fixtures::RUN_2); + assert_eq!(created.dot_path.as_deref(), Some(dot_path.as_path())); + assert_eq!(created.persisted.graph().goal(), "Compiled goal"); + assert_eq!(created.persisted.source(), compiled_source); + + let run_store = store.open_run_reader(&fixtures::RUN_2).await.unwrap(); + let state = run_store.state().await.unwrap(); + assert_eq!(state.spec.graph.goal(), "Compiled goal"); + assert_eq!(state.spec.automation, Some(automation)); + let events = run_store.list_events().await.unwrap(); + assert_eq!( + events + .iter() + .map(|event| event.event.event_name()) + .collect::>(), + vec!["run.created", "run.submitted"] + ); + let EventBody::RunCreated(created) = &events[0].event.body else { + panic!("first durable event should be run.created"); + }; + assert_eq!( + created.workflow_source.as_deref(), + Some(compiled_source.as_str()) + ); + assert_eq!( + created.workflow_config.as_deref(), + Some("_version = 1\n[workflow]\ngraph = \"workflow.fabro\"\n") + ); + let manifest_blob = created + .manifest_blob + .as_ref() + .expect("submitted manifest should be persisted"); + assert_eq!( + run_store + .read_blob(manifest_blob) + .await + .unwrap() + .expect("submitted manifest blob should exist") + .as_ref(), + b"submitted manifest" + ); + } + #[tokio::test] async fn create_returns_validation_failed_with_diagnostics() { let dot = r#"digraph Test { diff --git a/lib/components/fabro-workflow/src/operations/mod.rs b/lib/components/fabro-workflow/src/operations/mod.rs index 9523c5ff5..57ed472f4 100644 --- a/lib/components/fabro-workflow/src/operations/mod.rs +++ b/lib/components/fabro-workflow/src/operations/mod.rs @@ -14,7 +14,12 @@ pub use archive::{ ArchiveOutcome, UnarchiveOutcome, archive, archived_rejection_message, ensure_not_archived, unarchive, }; -pub use create::{CreateRunInput, CreatedRun, create, make_run_dir}; +pub use create::{ + CompiledRun, CreateRunCompileInput, CreateRunInput, CreateRunPersistenceInput, + CreateRunPersistenceMetadata, CreatedRun, MaterializedRun, + assemble_create_run_persistence_input, compile_create_run, create, make_run_dir, + materialize_create_run, persist_create_run, +}; pub use fork::{ForkOutcome, ForkRunInput, ResolvedForkTarget, fork_run}; pub use resume::resume; pub use retry::{RetryOutcome, RetryRunInput, retry_run}; diff --git a/lib/foundation/fabro-api/tests/manifest_args_round_trip.rs b/lib/foundation/fabro-api/tests/manifest_args_round_trip.rs new file mode 100644 index 000000000..b03b95f6f --- /dev/null +++ b/lib/foundation/fabro-api/tests/manifest_args_round_trip.rs @@ -0,0 +1,12 @@ +use fabro_api::types; +use serde_json::json; + +#[test] +fn removed_docker_image_manifest_arg_is_ignored() { + let args: types::ManifestArgs = serde_json::from_value(json!({ + "docker_image": "ghcr.io/fabro/custom:latest" + })) + .unwrap(); + + assert_eq!(serde_json::to_value(args).unwrap(), json!({})); +} diff --git a/lib/foundation/fabro-model/src/catalog.rs b/lib/foundation/fabro-model/src/catalog.rs index 5c4bfaa10..a941cc5b7 100644 --- a/lib/foundation/fabro-model/src/catalog.rs +++ b/lib/foundation/fabro-model/src/catalog.rs @@ -3028,6 +3028,148 @@ enabled = true ); } + #[test] + fn builtin_deepseek_provider_routes_v4_models() { + let deepseek = ProviderId::new("deepseek"); + let catalog = Catalog::builtin(); + let provider = catalog + .provider(&deepseek) + .expect("DeepSeek provider should be active"); + + assert_eq!(provider.adapter, AdapterKind::OpenAiCompatible); + assert_eq!(provider.codec, CodecKind::OpenAiCompatible); + assert_eq!(provider.billing_policy, BillingPolicy::OpenAi); + assert_eq!( + provider.base_url.as_deref(), + Some("https://api.deepseek.com") + ); + assert_eq!(provider.priority, 75); + assert_eq!(provider.auth.as_ref().unwrap().credentials, vec![ + CredentialRef::Env("DEEPSEEK_API_KEY".to_string()), + CredentialRef::Vault("DEEPSEEK_API_KEY".to_string()), + ]); + assert_eq!( + catalog + .default_for_provider(&deepseek) + .map(|model| model.id.as_str()), + Some("deepseek-v4-flash") + ); + assert_eq!( + catalog + .small_default_for_provider(&deepseek) + .map(|model| model.id.as_str()), + Some("deepseek-v4-flash") + ); + assert_eq!( + catalog + .probe_for_provider(&deepseek) + .map(|model| model.id.as_str()), + Some("deepseek-v4-flash") + ); + + let expected = [ + ("deepseek-v4-flash", 0.14, 0.28, 0.0028), + ("deepseek-v4-pro", 0.435, 0.87, 0.003_625), + ]; + for (id, input, output, cache_read) in expected { + let model = catalog + .get_on_provider(&deepseek, id) + .unwrap_or_else(|| panic!("DeepSeek model '{id}' should be present")); + assert_eq!(model.family, "deepseek-v4", "{id}"); + assert_eq!(model.limits.context_window, 1_048_576, "{id}"); + assert_eq!(model.limits.max_output, Some(384_000), "{id}"); + assert!(model.features.tools, "{id}"); + assert!(!model.features.vision, "{id}"); + assert!(model.features.reasoning, "{id}"); + assert!(model.features.prompt_cache, "{id}"); + assert!(!model.features.sampling_params, "{id}"); + assert_eq!(model.costs.input_cost_per_mtok, Some(input), "{id}"); + assert_eq!(model.costs.output_cost_per_mtok, Some(output), "{id}"); + assert_eq!( + model.costs.cache_input_cost_per_mtok, + Some(cache_read), + "{id}" + ); + + let settings = catalog + .model_settings_on_provider(&deepseek, id) + .unwrap_or_else(|| panic!("DeepSeek settings for '{id}' should be present")); + assert_eq!(settings.api_id, id, "{id}"); + assert!(settings.reasoning_by_default, "{id}"); + } + } + + #[test] + fn builtin_deepseek_reasoning_controls_match_provider_dialects() { + let catalog = Catalog::from_builtin_with_overrides(&minimal_settings( + r" +[providers.fireworks] +enabled = true + +[providers.openrouter] +enabled = true +", + )) + .expect("DeepSeek gateway providers should build when enabled"); + + let expected = [ + (ProviderId::new("deepseek"), "deepseek-v4-flash", vec![ + ReasoningEffort::Low, + ReasoningEffort::High, + ReasoningEffort::Max, + ]), + (ProviderId::new("deepseek"), "deepseek-v4-pro", vec![ + ReasoningEffort::High, + ReasoningEffort::Max, + ]), + (ProviderId::new("fireworks"), "deepseek-v4-flash", vec![ + ReasoningEffort::High, + ReasoningEffort::Max, + ]), + (ProviderId::new("fireworks"), "deepseek-v4-pro", vec![ + ReasoningEffort::High, + ReasoningEffort::Max, + ]), + (ProviderId::new("openrouter"), "deepseek-v4-flash", vec![ + ReasoningEffort::Low, + ReasoningEffort::High, + ReasoningEffort::Max, + ]), + (ProviderId::new("openrouter"), "deepseek-v4-pro", vec![ + ReasoningEffort::High, + ReasoningEffort::XHigh, + ]), + ]; + + for (provider, id, efforts) in expected { + let model = catalog + .get_on_provider(&provider, id) + .unwrap_or_else(|| panic!("{provider}/{id} should be present")); + assert!(model.features.reasoning, "{provider}/{id}"); + assert_eq!( + model.features.reasoning_effort, + ReasoningEffortFeature::Levels, + "{provider}/{id}" + ); + assert_eq!(model.controls.reasoning_effort, efforts, "{provider}/{id}"); + assert!(!model.features.sampling_params, "{provider}/{id}"); + + let settings = catalog + .model_settings_on_provider(&provider, id) + .unwrap_or_else(|| panic!("{provider}/{id} settings should be present")); + assert_eq!( + settings.agent_profile, + AgentProfileKind::OpenAi, + "{provider}/{id}" + ); + assert!(settings.reasoning_by_default, "{provider}/{id}"); + assert_eq!( + settings.controls.reasoning_effort, efforts, + "{provider}/{id}" + ); + } + } + #[test] fn builtin_openrouter_provider_is_opt_in() { let openrouter = ProviderId::new("openrouter"); @@ -3071,6 +3213,27 @@ enabled = true .billing_policy, BillingPolicy::OpenAi ); + let deepseek = catalog + .get_on_provider(&openrouter, "deepseek-v4-flash") + .expect("DeepSeek V4 Flash should be present on OpenRouter"); + assert_eq!(deepseek.limits.max_output, Some(384_000)); + assert!(deepseek.features.prompt_cache); + assert_eq!(deepseek.costs.input_cost_per_mtok, Some(0.14)); + assert_eq!(deepseek.costs.output_cost_per_mtok, Some(0.28)); + assert_eq!(deepseek.costs.cache_input_cost_per_mtok, Some(0.0028)); + assert_eq!( + catalog.settings_for(deepseek).unwrap().api_id, + "deepseek/deepseek-v4-flash-0731" + ); + let deepseek_pro = catalog + .get_on_provider(&openrouter, "deepseek-v4-pro") + .expect("DeepSeek V4 Pro should be present on OpenRouter"); + assert_eq!(deepseek_pro.limits.max_output, Some(384_000)); + assert!(deepseek_pro.features.prompt_cache); + assert_eq!( + deepseek_pro.costs.cache_input_cost_per_mtok, + Some(0.003_625) + ); assert_eq!( catalog .default_for_provider(&openrouter) @@ -3316,8 +3479,8 @@ enabled = true for (selector, canonical_id) in [ ("deepseek-v4-pro", "deepseek-v4-pro"), - ("deepseek-v4", "deepseek-v4-pro"), - ("deepseek", "deepseek-v4-pro"), + ("deepseek-v4", "deepseek-v4-flash"), + ("deepseek", "deepseek-v4-flash"), ("deepseek-v4-flash", "deepseek-v4-flash"), ("deepseek-flash", "deepseek-v4-flash"), ] { @@ -3906,9 +4069,9 @@ enabled = true "accounts/fireworks/models/deepseek-v4-flash", "deepseek-v4", 1_048_576, - 16_384, - false, + 384_000, false, + true, 0.14, 0.28, 0.028, @@ -4028,7 +4191,7 @@ enabled = true } #[test] - fn builtin_fireworks_shared_slugs_are_portable_with_openrouter() { + fn builtin_deepseek_shared_slugs_are_portable_across_providers() { let catalog = Catalog::from_builtin_with_overrides(&minimal_settings( r" [providers.fireworks] @@ -4055,6 +4218,43 @@ enabled = true assert_eq!(model.provider, provider, "{provider}/{id}"); } } + + for provider in [ + ProviderId::new("deepseek"), + ProviderId::new("fireworks"), + ProviderId::new("openrouter"), + ] { + for id in ["deepseek-v4-pro", "deepseek-v4-flash"] { + let model = catalog + .get_on_provider(&provider, id) + .unwrap_or_else(|| panic!("'{id}' should resolve on provider '{provider}'")); + assert_eq!(model.id, id, "{provider}/{id}"); + assert_eq!(model.provider, provider, "{provider}/{id}"); + assert_eq!(model.limits.context_window, 1_048_576, "{provider}/{id}"); + } + for alias in ["deepseek", "deepseek-v4", "deepseek-flash"] { + let model = catalog + .resolve_on_provider(&provider, alias) + .unwrap_or_else(|error| { + panic!("'{alias}' should resolve on provider '{provider}': {error}") + }); + assert_eq!(model.id, "deepseek-v4-flash", "{provider}/{alias}"); + assert_eq!(model.provider, provider, "{provider}/{alias}"); + } + } + + let selected = catalog + .select( + "deepseek", + None, + &HashSet::from([ + ProviderId::new("deepseek"), + ProviderId::new("fireworks"), + ProviderId::new("openrouter"), + ]), + ) + .expect("direct DeepSeek should win portable DeepSeek selection"); + assert_eq!(selected.provider, ProviderId::new("deepseek")); } #[test] diff --git a/lib/foundation/fabro-model/src/catalog/providers/deepseek.toml b/lib/foundation/fabro-model/src/catalog/providers/deepseek.toml new file mode 100644 index 000000000..e97981178 --- /dev/null +++ b/lib/foundation/fabro-model/src/catalog/providers/deepseek.toml @@ -0,0 +1,74 @@ +[providers.deepseek] +display_name = "DeepSeek" +adapter = "openai_compatible" +api_key_url = "https://platform.deepseek.com/api_keys" +base_url = "https://api.deepseek.com" +priority = 75 + +[providers.deepseek.auth] +credentials = ["env:DEEPSEEK_API_KEY", "vault:DEEPSEEK_API_KEY"] + +# DeepSeek V4 uses thinking mode by default. The API accepts sampling +# parameters in that mode but ignores them, so Fabro omits those parameters. +# Prompt caching is automatic and usage reports prompt_cache_hit_tokens. +# Prices are from api-docs.deepseek.com/quick_start/pricing, verified +# 2026-07-31. + +[providers.deepseek.models."deepseek-v4-flash"] +display_name = "DeepSeek V4 Flash" +family = "deepseek-v4" +aliases = ["deepseek-v4", "deepseek", "deepseek-flash"] +# DeepSeek uses a general coding prompt and standard JSON function tools. +agent_profile = "openai" +default = true +small_default = true +probe = true + +[providers.deepseek.models."deepseek-v4-flash".limits] +context_window = 1048576 +max_output = 384000 + +[providers.deepseek.models."deepseek-v4-flash".features] +tools = true +vision = false +reasoning = true +reasoning_effort = "levels" +reasoning_by_default = true +prompt_cache = true +sampling_params = false + +[providers.deepseek.models."deepseek-v4-flash".controls] +reasoning_effort = ["low", "high", "max"] + +[providers.deepseek.models."deepseek-v4-flash".costs] +input_cost_per_mtok = 0.14 +output_cost_per_mtok = 0.28 +cache_input_cost_per_mtok = 0.0028 + +[providers.deepseek.models."deepseek-v4-pro"] +display_name = "DeepSeek V4 Pro" +family = "deepseek-v4" +agent_profile = "openai" + +[providers.deepseek.models."deepseek-v4-pro".limits] +context_window = 1048576 +max_output = 384000 + +[providers.deepseek.models."deepseek-v4-pro".features] +tools = true +vision = false +reasoning = true +reasoning_effort = "levels" +reasoning_by_default = true +prompt_cache = true +sampling_params = false + +[providers.deepseek.models."deepseek-v4-pro".controls] +# V4 Pro currently maps low to high. Keep only its distinct effort levels; +# DeepSeek says it plans to change Pro's mapping in early August 2026. +reasoning_effort = ["high", "max"] + +[providers.deepseek.models."deepseek-v4-pro".costs] +input_cost_per_mtok = 0.435 +output_cost_per_mtok = 0.87 +cache_input_cost_per_mtok = 0.003625 diff --git a/lib/foundation/fabro-model/src/catalog/providers/fireworks.toml b/lib/foundation/fabro-model/src/catalog/providers/fireworks.toml index 38b2a5231..553fd58ed 100644 --- a/lib/foundation/fabro-model/src/catalog/providers/fireworks.toml +++ b/lib/foundation/fabro-model/src/catalog/providers/fireworks.toml @@ -25,7 +25,7 @@ credentials = ["env:FIREWORKS_API_KEY", "vault:FIREWORKS_API_KEY"] # Prompt caching is automatic prefix caching (no cache_control breakpoints); # serverless responses report prompt_tokens_details.cached_tokens in the # usage body. Costs below are from docs.fireworks.ai/serverless/pricing -# (standard tier), verified 2026-07-24. +# (standard tier), verified 2026-07-31. [providers.fireworks.models."kimi-k2.7-code"] api_id = "accounts/fireworks/models/kimi-k2p7-code" @@ -72,6 +72,7 @@ cache_input_cost_per_mtok = 0.16 api_id = "accounts/fireworks/models/deepseek-v4-pro" display_name = "DeepSeek V4 Pro" family = "deepseek-v4" +agent_profile = "openai" [providers.fireworks.models."deepseek-v4-pro".limits] context_window = 1048576 @@ -81,7 +82,14 @@ max_output = 16384 tools = true vision = false reasoning = true +reasoning_effort = "levels" +reasoning_by_default = true prompt_cache = true +sampling_params = false + +[providers.fireworks.models."deepseek-v4-pro".controls] +# Fireworks promotes low/medium to high and xhigh to max for DeepSeek V4. +reasoning_effort = ["high", "max"] [providers.fireworks.models."deepseek-v4-pro".costs] input_cost_per_mtok = 1.74 @@ -92,16 +100,24 @@ cache_input_cost_per_mtok = 0.145 api_id = "accounts/fireworks/models/deepseek-v4-flash" display_name = "DeepSeek V4 Flash" family = "deepseek-v4" +aliases = ["deepseek-v4", "deepseek", "deepseek-flash"] +agent_profile = "openai" [providers.fireworks.models."deepseek-v4-flash".limits] context_window = 1048576 -max_output = 16384 +max_output = 384000 [providers.fireworks.models."deepseek-v4-flash".features] tools = true vision = false -reasoning = false +reasoning = true +reasoning_effort = "levels" +reasoning_by_default = true prompt_cache = true +sampling_params = false + +[providers.fireworks.models."deepseek-v4-flash".controls] +reasoning_effort = ["high", "max"] [providers.fireworks.models."deepseek-v4-flash".costs] input_cost_per_mtok = 0.14 diff --git a/lib/foundation/fabro-model/src/catalog/providers/openrouter.toml b/lib/foundation/fabro-model/src/catalog/providers/openrouter.toml index f57361b0f..d4ea987bb 100644 --- a/lib/foundation/fabro-model/src/catalog/providers/openrouter.toml +++ b/lib/foundation/fabro-model/src/catalog/providers/openrouter.toml @@ -416,39 +416,57 @@ output_cost_per_mtok = 1.20 api_id = "deepseek/deepseek-v4-pro" display_name = "DeepSeek V4 Pro" family = "deepseek-v4" -aliases = ["deepseek-v4", "deepseek"] +agent_profile = "openai" [providers.openrouter.models."deepseek-v4-pro".limits] -context_window = 1050000 -max_output = 16384 +context_window = 1048576 +max_output = 384000 [providers.openrouter.models."deepseek-v4-pro".features] tools = true vision = false reasoning = true +reasoning_effort = "levels" +reasoning_by_default = true +prompt_cache = true +sampling_params = false + +[providers.openrouter.models."deepseek-v4-pro".controls] +# OpenRouter names DeepSeek's max tier xhigh on this route. +reasoning_effort = ["high", "xhigh"] [providers.openrouter.models."deepseek-v4-pro".costs] input_cost_per_mtok = 0.435 output_cost_per_mtok = 0.87 +cache_input_cost_per_mtok = 0.003625 [providers.openrouter.models."deepseek-v4-flash"] -api_id = "deepseek/deepseek-v4-flash" +api_id = "deepseek/deepseek-v4-flash-0731" display_name = "DeepSeek V4 Flash" family = "deepseek-v4" -aliases = ["deepseek-flash"] +aliases = ["deepseek-v4", "deepseek", "deepseek-flash"] +agent_profile = "openai" [providers.openrouter.models."deepseek-v4-flash".limits] -context_window = 1050000 -max_output = 16384 +context_window = 1048576 +max_output = 384000 [providers.openrouter.models."deepseek-v4-flash".features] tools = true vision = false -reasoning = false +reasoning = true +reasoning_effort = "levels" +reasoning_by_default = true +prompt_cache = true +sampling_params = false + +[providers.openrouter.models."deepseek-v4-flash".controls] +reasoning_effort = ["low", "high", "max"] [providers.openrouter.models."deepseek-v4-flash".costs] -input_cost_per_mtok = 0.10 -output_cost_per_mtok = 0.20 +input_cost_per_mtok = 0.14 +output_cost_per_mtok = 0.28 +cache_input_cost_per_mtok = 0.0028 [providers.openrouter.models."kimi-k2.6"] api_id = "moonshotai/kimi-k2.6" diff --git a/lib/foundation/fabro-static/src/env_vars.rs b/lib/foundation/fabro-static/src/env_vars.rs index e6f00e3b9..bb8c5d078 100644 --- a/lib/foundation/fabro-static/src/env_vars.rs +++ b/lib/foundation/fabro-static/src/env_vars.rs @@ -48,6 +48,7 @@ impl EnvVars { pub const BEDROCK_API_KEY: &'static str = "BEDROCK_API_KEY"; pub const BRAVE_SEARCH_API_KEY: &'static str = "BRAVE_SEARCH_API_KEY"; pub const CHATGPT_ACCOUNT_ID: &'static str = "CHATGPT_ACCOUNT_ID"; + pub const DEEPSEEK_API_KEY: &'static str = "DEEPSEEK_API_KEY"; pub const FIREWORKS_API_KEY: &'static str = "FIREWORKS_API_KEY"; pub const GEMINI_API_KEY: &'static str = "GEMINI_API_KEY"; pub const GEMINI_BASE_URL: &'static str = "GEMINI_BASE_URL"; @@ -199,6 +200,7 @@ mod tests { EnvVars::BEDROCK_API_KEY, EnvVars::BRAVE_SEARCH_API_KEY, EnvVars::CHATGPT_ACCOUNT_ID, + EnvVars::DEEPSEEK_API_KEY, EnvVars::FIREWORKS_API_KEY, EnvVars::GEMINI_API_KEY, EnvVars::GEMINI_BASE_URL, diff --git a/lib/foundation/fabro-static/src/secret_registry.rs b/lib/foundation/fabro-static/src/secret_registry.rs index 8c823b112..22cf26177 100644 --- a/lib/foundation/fabro-static/src/secret_registry.rs +++ b/lib/foundation/fabro-static/src/secret_registry.rs @@ -19,6 +19,7 @@ const OPTIONAL_VAULT_SECRETS: &[&str] = &[ EnvVars::AWS_BEARER_TOKEN_BEDROCK, EnvVars::BEDROCK_API_KEY, EnvVars::BRAVE_SEARCH_API_KEY, + EnvVars::DEEPSEEK_API_KEY, EnvVars::FABRO_SLACK_APP_TOKEN, EnvVars::FABRO_SLACK_BOT_TOKEN, EnvVars::FIREWORKS_API_KEY, @@ -93,6 +94,7 @@ mod tests { EnvVars::ANTHROPIC_API_KEY, EnvVars::AWS_BEARER_TOKEN_BEDROCK, EnvVars::BEDROCK_API_KEY, + EnvVars::DEEPSEEK_API_KEY, EnvVars::FIREWORKS_API_KEY, EnvVars::GEMINI_API_KEY, EnvVars::INCEPTION_API_KEY, diff --git a/lib/packages/fabro-api-client/src/models/manifest-args.ts b/lib/packages/fabro-api-client/src/models/manifest-args.ts index 41fb58300..c0ae3985e 100644 --- a/lib/packages/fabro-api-client/src/models/manifest-args.ts +++ b/lib/packages/fabro-api-client/src/models/manifest-args.ts @@ -24,10 +24,6 @@ export interface ManifestArgs { * Named environment slug to select for the run. */ 'environment'?: string; - /** - * Per-run environment image override. - */ - 'docker_image'?: string; 'verbose'?: boolean; 'dry_run'?: boolean; 'auto_approve'?: boolean;