+ {/* While collapsed, the whole bar expands on click. Clicks that land on
+ a button (Interrupt, the chevron) keep their own behavior. The
+ chevron button stays the keyboard/assistive-tech toggle. */}
+
{
+ if ((event.target as HTMLElement | null)?.closest?.("button")) {
+ return;
+ }
+ onCollapsedChange(false);
+ }
+ : undefined
+ }
+ >
{/* A live region: the dock changing to a state that needs the
operator has to reach assistive tech, not only the eye. */}
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/app/components/steer-bar.test.tsx b/apps/fabro-web/app/components/steer-bar.test.tsx
index b13498af6..583a68750 100644
--- a/apps/fabro-web/app/components/steer-bar.test.tsx
+++ b/apps/fabro-web/app/components/steer-bar.test.tsx
@@ -98,11 +98,7 @@ describe("SteerBar", () => {
});
mountedRenderers.push(renderer);
- act(() => {
- renderer.root
- .findByProps({ "aria-label": "Collapse Steer running agent" })
- .props.onClick();
- });
+ // The dock starts collapsed by default.
expect(
renderer.root.findByProps({
"aria-label": "Expand Steer running agent",
diff --git a/apps/fabro-web/app/components/steer-bar.tsx b/apps/fabro-web/app/components/steer-bar.tsx
index 1b6e61f7c..4ec85bbcb 100644
--- a/apps/fabro-web/app/components/steer-bar.tsx
+++ b/apps/fabro-web/app/components/steer-bar.tsx
@@ -57,7 +57,9 @@ export function SteerBar({
ref,
}: SteerBarProps) {
const [errorMessage, setErrorMessage] = useState(null);
- const [collapsePreferred, setCollapsePreferred] = useState(false);
+ // Steering is an occasional control, so the dock starts collapsed and
+ // stays out of the way until the operator opens it.
+ const [collapsePreferred, setCollapsePreferred] = useState(true);
const textareaRef = useRef(null);
const steer = useSteerRun(runId);
const interrupt = useInterruptRun(runId);
diff --git a/apps/fabro-web/app/lib/api-client.test.ts b/apps/fabro-web/app/lib/api-client.test.ts
index 8d38323a1..5f0f44b19 100644
--- a/apps/fabro-web/app/lib/api-client.test.ts
+++ b/apps/fabro-web/app/lib/api-client.test.ts
@@ -6,6 +6,7 @@ import {
extractRequestId,
fetchAllPages,
generatedAxios,
+ runArtifactsDownloadUrl,
stageArtifactDownloadUrl,
} from "./api-client";
@@ -146,6 +147,14 @@ describe("stageArtifactDownloadUrl", () => {
});
});
+describe("runArtifactsDownloadUrl", () => {
+ test("builds the escaped ZIP download href", () => {
+ expect(runArtifactsDownloadUrl("run 1")).toBe(
+ "/api/v1/runs/run%201/artifacts/download",
+ );
+ });
+});
+
describe("extractRequestId", () => {
test("supports top-level, error-level, and detail-embedded request ids", () => {
expect(extractRequestId({ request_id: "top" })).toBe("top");
diff --git a/apps/fabro-web/app/lib/api-client.ts b/apps/fabro-web/app/lib/api-client.ts
index e77eb6d8a..bb59e3607 100644
--- a/apps/fabro-web/app/lib/api-client.ts
+++ b/apps/fabro-web/app/lib/api-client.ts
@@ -408,6 +408,13 @@ export function requestSignalOptions(request?: Request): RawAxiosRequestConfig {
return request?.signal ? { signal: request.signal } : {};
}
+/** Absolute href for a path under a run, for links the browser follows itself. */
+function runApiPath(id: string, suffix: string): string {
+ return `${generatedApiConfiguration.basePath ?? ""}/api/v1/runs/${
+ encodeURIComponent(id)
+ }${suffix}`;
+}
+
export function stageArtifactDownloadUrl(
id: string,
stageId: string,
@@ -418,7 +425,12 @@ export function stageArtifactDownloadUrl(
filename,
retry: String(retry),
});
- return `${generatedApiConfiguration.basePath ?? ""}/api/v1/runs/${
- encodeURIComponent(id)
- }/stages/${encodeURIComponent(stageId)}/artifacts/download?${searchParams}`;
+ return runApiPath(
+ id,
+ `/stages/${encodeURIComponent(stageId)}/artifacts/download?${searchParams}`,
+ );
+}
+
+export function runArtifactsDownloadUrl(id: string): string {
+ return runApiPath(id, "/artifacts/download");
}
diff --git a/apps/fabro-web/app/routes/run-artifacts.tsx b/apps/fabro-web/app/routes/run-artifacts.tsx
index a314973c9..0f75d6a2e 100644
--- a/apps/fabro-web/app/routes/run-artifacts.tsx
+++ b/apps/fabro-web/app/routes/run-artifacts.tsx
@@ -6,7 +6,10 @@ import type { RunArtifactEntry } from "@qltysh/fabro-api-client";
import { EmptyState, ErrorState, LoadingState } from "../components/state";
import { StageSidebar } from "../components/stage-sidebar";
-import { stageArtifactDownloadUrl } from "../lib/api-client";
+import {
+ runArtifactsDownloadUrl,
+ stageArtifactDownloadUrl,
+} from "../lib/api-client";
import { formatBytes } from "../lib/format";
import { plural } from "../lib/plural";
import { useRunArtifacts, useRunStages } from "../lib/queries";
@@ -117,7 +120,7 @@ function ArtifactList({ runId, files }: { runId: string; files: readonly Artifac
return (
-
+
{files.length} {plural(files.length, "file", "files")}
{versioned && (
@@ -127,11 +130,21 @@ function ArtifactList({ runId, files }: { runId: string; files: readonly Artifac
)}
-
- {versioned
- ? `${formatBytes(latestBytes)} latest · ${formatBytes(storedBytes)} stored`
- : `${formatBytes(latestBytes)} total`}
-
+
+
+ {versioned
+ ? `${formatBytes(latestBytes)} latest · ${formatBytes(storedBytes)} stored`
+ : `${formatBytes(latestBytes)} total`}
+
+
+
+
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 c806406f7..9f107d046 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..3c7e3403a 100644
--- a/docs/public/api-reference/fabro-api.yaml
+++ b/docs/public/api-reference/fabro-api.yaml
@@ -3282,6 +3282,53 @@ paths:
schema:
$ref: "#/components/schemas/ErrorResponse"
+ /api/v1/runs/{id}/artifacts/download:
+ get:
+ operationId: downloadRunArtifacts
+ tags: [Run Internals]
+ summary: Download Run Artifacts
+ description: |
+ Streams a ZIP archive with the latest captured version of each artifact path.
+ Stage order, retry number, and then stage ID determine the latest version, matching the artifacts page.
+ Captures from the graph's boundary nodes are excluded, identified by their `start` and `exit` handler type rather than by node name.
+
+ The archive streams, so the response status is sent before the first artifact is read.
+ A failure after that point aborts the transfer rather than returning `500`.
+ The ZIP central directory is written last, so a truncated download does not open as a valid archive.
+ parameters:
+ - $ref: "#/components/parameters/RunId"
+ responses:
+ "200":
+ description: ZIP archive containing the latest artifact files
+ headers:
+ Content-Disposition:
+ description: Attachment filename for the ZIP archive
+ schema:
+ type: string
+ content:
+ application/zip:
+ schema:
+ type: string
+ format: binary
+ "404":
+ description: Run not found
+ headers:
+ x-request-id:
+ $ref: "#/components/headers/XRequestId"
+ content:
+ application/json:
+ schema:
+ $ref: "#/components/schemas/ErrorResponse"
+ "500":
+ description: Artifact archive could not be prepared
+ headers:
+ x-request-id:
+ $ref: "#/components/headers/XRequestId"
+ content:
+ application/json:
+ schema:
+ $ref: "#/components/schemas/ErrorResponse"
+
/api/v1/runs/{id}/files:
get:
operationId: listRunFiles
@@ -9184,9 +9231,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 a4befa37f..84f00bb1e 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` | kimi | | 262K | $0.60 / $3.00 | 50 tok/s |
| `kimi-k3` | kimi | `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 feeea0c0a..ad4eedb0b 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 3486b35f5..adfb2d97c 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..8aea2bade 100644
--- a/lib/apps/fabro-server/Cargo.toml
+++ b/lib/apps/fabro-server/Cargo.toml
@@ -75,6 +75,7 @@ serde_json.workspace = true
serde_yaml = "0.9"
anyhow.workspace = true
async-trait.workspace = true
+async_zip.workspace = true
clap.workspace = true
toml.workspace = true
toml_edit.workspace = true
@@ -121,5 +122,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 fd9a1f50c..7c1a71951 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,
@@ -2150,7 +2067,6 @@ root = "/srv/fabro"
preserve_sandbox: None,
provider: None,
environment: None,
- docker_image: None,
input: Vec::new(),
verbose: None,
});
@@ -2183,7 +2099,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.rs b/lib/apps/fabro-server/src/server.rs
index 69220d2b9..bc5aeeeb3 100644
--- a/lib/apps/fabro-server/src/server.rs
+++ b/lib/apps/fabro-server/src/server.rs
@@ -137,6 +137,7 @@ use tokio_stream::StreamExt;
use tokio_stream::wrappers::{BroadcastStream, UnboundedReceiverStream};
use tokio_util::sync::CancellationToken;
use tower::{ServiceExt, service_fn};
+use tower_http::compression::predicate::{DefaultPredicate, NotForContentType, Predicate};
use tower_http::compression::{CompressionLayer, CompressionLevel};
use tracing::{Instrument, debug, error, info, warn};
use ulid::Ulid;
@@ -1922,12 +1923,14 @@ pub fn build_router_with_options(
/// Response-compression layer shared by the main and install-mode routers.
///
/// The default predicate skips streaming SSE (`text/event-stream`), gRPC,
-/// images, and tiny bodies. The quality is pinned because tower-http's
-/// default defers to each codec's own default, and brotli's is quality 11 —
-/// seconds of CPU on a multi-megabyte asset. Level 4 keeps both codecs fast
-/// at a near-optimal ratio.
-pub(crate) fn compression_layer() -> CompressionLayer {
- CompressionLayer::new().quality(CompressionLevel::Precise(4))
+/// images, ZIP archives, and tiny bodies. The quality is pinned because
+/// tower-http's default defers to each codec's own default, and brotli's is
+/// quality 11 — seconds of CPU on a multi-megabyte asset. Level 4 keeps both
+/// codecs fast at a near-optimal ratio.
+pub(crate) fn compression_layer() -> CompressionLayer {
+ CompressionLayer::new()
+ .quality(CompressionLevel::Precise(4))
+ .compress_when(DefaultPredicate::new().and(NotForContentType::const_new("application/zip")))
}
async fn http_log_middleware(mut req: axum_extract::Request, next: Next) -> Response {
diff --git a/lib/apps/fabro-server/src/server/handler/artifacts.rs b/lib/apps/fabro-server/src/server/handler/artifacts.rs
index 9d7b049fb..f426a0007 100644
--- a/lib/apps/fabro-server/src/server/handler/artifacts.rs
+++ b/lib/apps/fabro-server/src/server/handler/artifacts.rs
@@ -1,5 +1,22 @@
+use std::io;
use std::sync::Arc;
+use async_zip::base::write::ZipFileWriter;
+use async_zip::error::ZipError;
+use async_zip::{Compression, ZipEntryBuilder};
+use axum::http::HeaderValue;
+use fabro_store::{ArtifactStore, Error as StoreError};
+use fabro_types::RunProjection;
+use fabro_util::error::collect_chain;
+use futures_util::SinkExt as _;
+use futures_util::io::AsyncWriteExt as _;
+use tokio::io::{AsyncWrite, BufWriter};
+use tokio::sync::mpsc;
+use tokio_stream::wrappers::ReceiverStream;
+use tokio_util::io::{CopyToBytes, SinkWriter};
+use tokio_util::sync::PollSender;
+use tracing::warn;
+
use super::super::{
ApiError, AppState, ArtifactEntry, ArtifactKey, ArtifactListResponse, AsyncWriteExt, Body,
Bytes, DefaultBodyLimit, Digest, HashMap, HashSet, HeaderMap, IntoResponse, Json, NodeArtifact,
@@ -17,6 +34,7 @@ pub(super) fn routes() -> Router> {
.route("/runs/{id}/blobs", post(write_run_blob))
.route("/runs/{id}/blobs/{blobId}", get(read_run_blob))
.route("/runs/{id}/artifacts", get(list_run_artifacts))
+ .route("/runs/{id}/artifacts/download", get(download_run_artifacts))
.route(
"/runs/{id}/stages/{stageId}/artifacts",
get(list_stage_artifacts)
@@ -147,6 +165,210 @@ async fn list_run_artifacts(
}
}
+const ARCHIVE_STREAM_CHANNEL_CAPACITY: usize = 8;
+
+/// Deflate flushes its output in 8 KiB blocks, and every write below becomes
+/// its own allocation, channel send, and HTTP body frame. Batching to 64 KiB
+/// cuts all three by eight without meaningfully delaying the stream.
+const ARCHIVE_WRITE_BUFFER_BYTES: usize = 64 * 1024;
+
+#[derive(Debug, thiserror::Error)]
+enum ArtifactArchiveError {
+ #[error("archive output failed: {0}")]
+ Io(#[from] io::Error),
+ #[error("artifact read failed: {0}")]
+ Store(#[from] StoreError),
+ #[error("ZIP write failed: {0}")]
+ Zip(#[from] ZipError),
+}
+
+/// One entry per artifact path, holding the newest capture of that path.
+///
+/// Newest means latest stage, then latest retry, then highest stage ID. That
+/// last tiebreaker only decides between two stages the projection does not know
+/// about, which both sort oldest (`None` < `Some`); it is here so the winner
+/// does not depend on the order the store happens to list objects in. All three
+/// keys mirror the artifacts page, which sorts on the same triple and takes the
+/// last entry. Boundary stages are dropped: they run no work, so anything they
+/// captured was already in the workspace.
+///
+/// Paths are re-checked here rather than trusted: a path stored before a
+/// validation rule existed would otherwise be written straight into a ZIP that
+/// somebody extracts. An unsafe path is skipped, not fatal — one bad path must
+/// not cost the caller every other artifact.
+fn latest_run_artifacts(
+ entries: Vec,
+ projection: &RunProjection,
+) -> Vec {
+ let stage_order = projection
+ .iter_stages()
+ .enumerate()
+ .map(|(order, (stage_id, _))| (stage_id.clone(), order))
+ .collect::>();
+ // The stage ID compares as its serialized `node@visit` form, matching the
+ // string the artifacts page sorts on rather than `StageId`'s own ordering,
+ // which compares the visit numerically and would disagree.
+ let capture_rank = |artifact: &NodeArtifact| {
+ (
+ stage_order.get(&artifact.node).copied(),
+ artifact.retry,
+ artifact.node.to_string(),
+ )
+ };
+ let mut latest_by_path: HashMap = HashMap::new();
+
+ for artifact in entries {
+ if projection.is_boundary_stage(artifact.node.node_id()) {
+ continue;
+ }
+ if let Err(error) = ArtifactStore::validate_relative_path(&artifact.filename) {
+ warn!(
+ path = %artifact.filename,
+ %error,
+ "skipping artifact with an unsafe path"
+ );
+ continue;
+ }
+
+ match latest_by_path.get(&artifact.filename) {
+ Some(existing) if capture_rank(existing) >= capture_rank(&artifact) => {}
+ _ => {
+ latest_by_path.insert(artifact.filename.clone(), artifact);
+ }
+ }
+ }
+
+ let mut latest = latest_by_path.into_values().collect::>();
+ latest.sort_by(|left, right| left.filename.cmp(&right.filename));
+ latest
+}
+
+async fn write_artifact_archive(
+ writer: W,
+ artifact_store: ArtifactStore,
+ run_id: RunId,
+ artifacts: Vec,
+) -> Result<(), ArtifactArchiveError>
+where
+ W: AsyncWrite + Unpin,
+{
+ let mut archive = ZipFileWriter::with_tokio(writer);
+ for artifact in artifacts {
+ let key = ArtifactKey::new(artifact.node, artifact.retry, artifact.filename.clone());
+ let Some(mut source) = artifact_store.get_stream(&run_id, &key).await? else {
+ // Deleted between the listing and this read, which in practice means
+ // the run was pruned mid-download. Leave it out and keep going: an
+ // archive missing one file beats a truncated one missing the rest.
+ warn!(%run_id, path = %artifact.filename, "artifact vanished while archiving");
+ continue;
+ };
+ // Deflate, not Stored: artifacts are mostly logs and reports, and the
+ // response is excluded from transfer compression precisely because the
+ // archive already carries its own.
+ let entry = ZipEntryBuilder::new(artifact.filename.into(), Compression::Deflate);
+ // `destination` is a futures-io writer from async_zip, while `writer` at
+ // the end of this function is a tokio one, so both `AsyncWriteExt`
+ // traits are in scope and each call resolves to a different one.
+ let mut destination = archive.write_entry_stream(entry).await?;
+ while let Some(chunk) = source.next().await {
+ destination.write_all(&chunk?).await?;
+ }
+ destination.close().await?;
+ }
+ let mut writer = archive.close().await?.into_inner();
+ writer.shutdown().await?;
+ Ok(())
+}
+
+fn artifact_archive_body(
+ artifact_store: ArtifactStore,
+ run_id: RunId,
+ artifacts: Vec,
+) -> Body {
+ // A channel of `Result`, rather than `tokio::io::duplex`, so a failure
+ // partway through can poison the body. Dropping a duplex writer ends the
+ // response with a clean EOF, which would hand the caller a truncated ZIP
+ // that looks like a complete download. Sending an `Err` aborts the chunked
+ // body instead, so the caller sees a transfer error. Do not "simplify" this
+ // to a duplex without replacing that signal.
+ let (sender, receiver) =
+ mpsc::channel::>(ARCHIVE_STREAM_CHANNEL_CAPACITY);
+ let error_sender = sender.clone();
+ let sink = PollSender::new(sender)
+ .sink_map_err(|_| io::Error::from(io::ErrorKind::BrokenPipe))
+ .with(|chunk: Bytes| {
+ std::future::ready(Ok::, io::Error>(Ok(chunk)))
+ });
+ let writer = BufWriter::with_capacity(
+ ARCHIVE_WRITE_BUFFER_BYTES,
+ SinkWriter::new(CopyToBytes::new(sink)),
+ );
+
+ tokio::spawn(async move {
+ if let Err(error) = write_artifact_archive(writer, artifact_store, run_id, artifacts).await
+ {
+ // Log before signalling: the send fails when the caller has already
+ // gone away, and that is exactly when this log is the only record
+ // that the archive failed. The 200 went out long ago.
+ warn!(
+ %run_id,
+ error = %collect_chain(&error).join(": "),
+ "artifact archive stream failed"
+ );
+ let _ = error_sender
+ .send(Err(io::Error::other("artifact archive stream failed")))
+ .await;
+ }
+ });
+
+ Body::from_stream(ReceiverStream::new(receiver))
+}
+
+async fn download_run_artifacts(
+ _auth: RequiredUser,
+ State(state): State>,
+ Path(id): Path,
+) -> Response {
+ let id = match parse_run_id_path(&id) {
+ Ok(id) => id,
+ Err(response) => return response,
+ };
+ let cached = match state.cached_run(&id).await {
+ Ok(cached) => cached,
+ Err(error) => return error.into_response(),
+ };
+ let entries = match state.artifact_store.list_for_run(&id).await {
+ Ok(entries) => entries,
+ Err(error) => {
+ warn!(run_id = %id, %error, "failed to list artifacts for ZIP download");
+ return ApiError::new(
+ StatusCode::INTERNAL_SERVER_ERROR,
+ "Artifact archive could not be prepared.",
+ )
+ .into_response();
+ }
+ };
+ let artifacts = latest_run_artifacts(entries, &cached.projection);
+
+ let content_disposition = format!("attachment; filename=\"fabro-artifacts-{id}.zip\"");
+ let body = artifact_archive_body(state.artifact_store.clone(), id, artifacts);
+ let mut response = body.into_response();
+ response.headers_mut().insert(
+ header::CONTENT_TYPE,
+ HeaderValue::from_static("application/zip"),
+ );
+ response.headers_mut().insert(
+ header::CONTENT_DISPOSITION,
+ HeaderValue::from_str(&content_disposition)
+ .expect("run IDs produce valid attachment filenames"),
+ );
+ response.headers_mut().insert(
+ header::CACHE_CONTROL,
+ HeaderValue::from_static("private, no-store"),
+ );
+ response
+}
+
fn run_artifact_entry_from(entry: NodeArtifact) -> RunArtifactEntry {
RunArtifactEntry {
stage_id: entry.node.to_string(),
diff --git a/lib/apps/fabro-server/src/server/handler/billing.rs b/lib/apps/fabro-server/src/server/handler/billing.rs
index eaa53ba0f..38a22395c 100644
--- a/lib/apps/fabro-server/src/server/handler/billing.rs
+++ b/lib/apps/fabro-server/src/server/handler/billing.rs
@@ -170,7 +170,7 @@ fn live_billing_rows(projection: &RunProjection, now: DateTime) -> Vec bool {
|| !stage.usage.is_zero()
|| stage.started_at.is_some()
}
-
-fn is_boundary_stage(projection: &RunProjection, node_id: &str) -> bool {
- projection
- .spec()
- .graph()
- .nodes
- .get(node_id)
- .is_some_and(|node| matches!(node.handler_type(), Some("start" | "exit")))
-}
diff --git a/lib/apps/fabro-server/src/server/handler/mod.rs b/lib/apps/fabro-server/src/server/handler/mod.rs
index 75dd6507d..1f8edcdca 100644
--- a/lib/apps/fabro-server/src/server/handler/mod.rs
+++ b/lib/apps/fabro-server/src/server/handler/mod.rs
@@ -116,6 +116,7 @@ pub(super) fn demo_routes() -> Router> {
.route("/runs/{id}/graph/source", get(demo::get_run_graph_source))
.route("/runs/{id}/stages", get(demo::get_run_stages))
.route("/runs/{id}/artifacts", get(demo::list_run_artifacts_stub))
+ .route("/runs/{id}/artifacts/download", get(not_implemented))
.route("/runs/{id}/files", get(demo::list_run_files_stub))
.route("/runs/{id}/commits", get(demo::list_run_commits_stub))
.route(
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 b3da5be63..491fb2da9 100644
--- a/lib/apps/fabro-server/src/server/tests.rs
+++ b/lib/apps/fabro-server/src/server/tests.rs
@@ -7,6 +7,7 @@ use std::process::Stdio;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::{Arc as StdArc, Mutex as StdMutex};
+use async_zip::base::read::mem::ZipFileReader;
use axum::body::Body;
use axum::http::{Method, Request, header};
use chrono::{Duration as ChronoDuration, Utc};
@@ -3660,6 +3661,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 +6709,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}"
@@ -10589,6 +10937,190 @@ async fn stage_artifacts_keep_same_filename_per_retry() {
assert_eq!(&bytes[..], b"second");
}
+#[tokio::test]
+async fn run_artifacts_download_streams_latest_files_as_zip() {
+ let state = test_app_state();
+ let app = crate::test_support::build_test_router(Arc::clone(&state));
+ let run_id = create_run(&app, MINIMAL_DOT)
+ .await
+ .parse::()
+ .unwrap();
+
+ let run_store = state.stores.runs.open_run(&run_id).await.unwrap();
+ for event in [
+ workflow_event::Event::RunRunnable {
+ source: fabro_types::RunRunnableSource::StartRequested,
+ actor: None,
+ },
+ workflow_event::Event::RunStarting,
+ workflow_event::Event::RunRunning,
+ ] {
+ workflow_event::append_event(&run_store, &run_id, &event)
+ .await
+ .unwrap();
+ }
+ append_scoped_stage_event(
+ &state,
+ run_id,
+ "build",
+ 1,
+ &stage_started_event("build", "command"),
+ )
+ .await;
+ append_scoped_stage_event(
+ &state,
+ run_id,
+ "verify",
+ 1,
+ &stage_started_event("verify", "command"),
+ )
+ .await;
+
+ for (stage_id, retry, path, contents) in [
+ (
+ StageId::new("unknown", 1),
+ 99,
+ "reports/result.txt",
+ &b"unknown stage"[..],
+ ),
+ (
+ StageId::new("build", 1),
+ 1,
+ "reports/result.txt",
+ &b"build result"[..],
+ ),
+ (
+ StageId::new("verify", 1),
+ 1,
+ "reports/result.txt",
+ &b"first verify"[..],
+ ),
+ (
+ StageId::new("verify", 1),
+ 2,
+ "reports/result.txt",
+ &b"latest verify"[..],
+ ),
+ (
+ StageId::new("build", 1),
+ 1,
+ "logs/run.txt",
+ &b"build log"[..],
+ ),
+ (
+ StageId::new("start", 1),
+ 1,
+ "control-start.txt",
+ &b"excluded"[..],
+ ),
+ (
+ StageId::new("exit", 1),
+ 1,
+ "control-exit.txt",
+ &b"excluded"[..],
+ ),
+ // Neither stage reached the projection, so both rank equally on stage
+ // order and retry. The serialized stage ID breaks the tie the same way
+ // the artifacts page does: "unknown@2" sorts above "unknown@10".
+ (
+ StageId::new("unknown", 10),
+ 1,
+ "orphan.txt",
+ &b"visit ten"[..],
+ ),
+ (
+ StageId::new("unknown", 2),
+ 1,
+ "orphan.txt",
+ &b"visit two"[..],
+ ),
+ ] {
+ state
+ .artifact_store
+ .put(&run_id, &ArtifactKey::new(stage_id, retry, path), contents)
+ .await
+ .unwrap();
+ }
+
+ let response = app
+ .clone()
+ .oneshot(
+ Request::builder()
+ .method("GET")
+ .uri(api(&format!("/runs/{run_id}/artifacts/download")))
+ .header(header::ACCEPT_ENCODING, "gzip")
+ .body(Body::empty())
+ .unwrap(),
+ )
+ .await
+ .unwrap();
+ assert_eq!(
+ response
+ .headers()
+ .get(header::CONTENT_TYPE)
+ .and_then(|value| value.to_str().ok()),
+ Some("application/zip")
+ );
+ assert_eq!(
+ response
+ .headers()
+ .get(header::CONTENT_DISPOSITION)
+ .and_then(|value| value.to_str().ok()),
+ Some(format!("attachment; filename=\"fabro-artifacts-{run_id}.zip\"").as_str())
+ );
+ assert_eq!(
+ response
+ .headers()
+ .get(header::CACHE_CONTROL)
+ .and_then(|value| value.to_str().ok()),
+ Some("private, no-store")
+ );
+ assert!(response.headers().get(header::CONTENT_ENCODING).is_none());
+
+ let bytes = response_bytes!(response, StatusCode::OK).await;
+ let archive = ZipFileReader::new(bytes).await.unwrap();
+ let names = archive
+ .file()
+ .entries()
+ .iter()
+ .map(|entry| entry.filename().as_str().unwrap().to_string())
+ .collect::>();
+ assert_eq!(names, vec![
+ "logs/run.txt",
+ "orphan.txt",
+ "reports/result.txt"
+ ]);
+
+ let mut contents_by_name = HashMap::new();
+ for (index, name) in names.into_iter().enumerate() {
+ let mut entry = archive.reader_with_entry(index).await.unwrap();
+ let mut contents = Vec::new();
+ entry.read_to_end_checked(&mut contents).await.unwrap();
+ contents_by_name.insert(name, contents);
+ }
+ assert_eq!(contents_by_name["logs/run.txt"], b"build log");
+ assert_eq!(contents_by_name["reports/result.txt"], b"latest verify");
+ assert_eq!(contents_by_name["orphan.txt"], b"visit two");
+}
+
+#[tokio::test]
+async fn run_artifacts_download_returns_not_found_for_unknown_run() {
+ let state = test_app_state();
+ let app = crate::test_support::build_test_router(Arc::clone(&state));
+
+ let response = app
+ .oneshot(
+ Request::builder()
+ .method("GET")
+ .uri(api(&format!("/runs/{}/artifacts/download", RunId::new())))
+ .body(Body::empty())
+ .unwrap(),
+ )
+ .await
+ .unwrap();
+ assert_status!(response, StatusCode::NOT_FOUND).await;
+}
+
#[tokio::test]
async fn create_run_persists_run_spec() {
let state = test_app_state();
@@ -11316,6 +11848,7 @@ async fn worker_token_is_rejected_on_user_only_routes() {
(Method::GET, format!("/runs/{run_id}/graph/source")),
(Method::GET, format!("/runs/{run_id}/stages")),
(Method::GET, format!("/runs/{run_id}/artifacts")),
+ (Method::GET, format!("/runs/{run_id}/artifacts/download")),
(Method::GET, format!("/runs/{run_id}/files")),
(
Method::GET,
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 26ff6d70b..bb1b893ce 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/anthropic_messages/stream.rs b/lib/components/fabro-llm/src/codec/anthropic_messages/stream.rs
index 1f6a55fb8..bb8d79027 100644
--- a/lib/components/fabro-llm/src/codec/anthropic_messages/stream.rs
+++ b/lib/components/fabro-llm/src/codec/anthropic_messages/stream.rs
@@ -8,7 +8,7 @@
use super::SYNTHETIC_TOOL_NAME;
use super::decode::{convert_synthetic_tool_to_text, map_finish_reason, refusal_error};
use crate::codec::{RawEvent, StreamDecoder, parse_tool_arguments_or_empty};
-use crate::error::{Error, ProviderErrorDetail, ProviderErrorKind};
+use crate::error::{self, Error, ProviderErrorDetail, ProviderErrorKind};
use crate::types::{
ContentPart, FinishReason, Message, RateLimitInfo, Response, Role, StreamEvent, ThinkingData,
TokenCounts, ToolCall,
@@ -355,16 +355,11 @@ fn stream_error_event_to_provider_error(data: &serde_json::Value, provider_name:
.and_then(serde_json::Value::as_str)
.map(String::from);
- let kind = match error_code.as_deref() {
- Some("rate_limit_error") => ProviderErrorKind::RateLimit,
- Some("authentication_error") => ProviderErrorKind::Authentication,
- Some("permission_error") => ProviderErrorKind::AccessDenied,
- Some("not_found_error") => ProviderErrorKind::NotFound,
- Some("invalid_request_error") => ProviderErrorKind::InvalidRequest,
- Some("request_too_large") => ProviderErrorKind::ContextLength,
- // overloaded_error, api_error, and unknown stream errors are transient.
- _ => ProviderErrorKind::Server,
- };
+ // overloaded_error, api_error, and unknown stream errors are transient.
+ let kind = error_code
+ .as_deref()
+ .and_then(error::kind_from_error_code)
+ .unwrap_or(ProviderErrorKind::Server);
Error::Provider {
kind,
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 88fb02672..23cc0e260 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,
/// Modal reports reasoning tokens directly on `usage` instead of nesting
@@ -333,6 +337,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
@@ -609,6 +614,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/codec/openai_responses/stream.rs b/lib/components/fabro-llm/src/codec/openai_responses/stream.rs
index 9d0a06d4d..9bca719ec 100644
--- a/lib/components/fabro-llm/src/codec/openai_responses/stream.rs
+++ b/lib/components/fabro-llm/src/codec/openai_responses/stream.rs
@@ -11,7 +11,7 @@ use serde::Deserialize;
use super::decode::{map_finish_reason, token_counts_from_api_usage, tool_call_from_item};
use super::wire::ApiUsage;
use crate::codec::{CodecCtx, RawEvent, StreamDecoder};
-use crate::error::{Error, ProviderErrorDetail, ProviderErrorKind};
+use crate::error::{self, Error, ProviderErrorDetail, ProviderErrorKind};
use crate::types::{
ContentPart, FinishReason, Message, RateLimitInfo, Response, Role, StreamEvent, TokenCounts,
ToolCall,
@@ -36,35 +36,10 @@ fn provider_error_from_openai_error_json(error: &serde_json::Value, provider: &s
.filter(|message| !message.is_empty())
.map_or_else(|| "OpenAI stream error".to_string(), str::to_string);
- let kind = match classifier {
- Some("insufficient_quota" | "billing_hard_limit_reached") => {
- ProviderErrorKind::QuotaExceeded
- }
- Some("rate_limit_error" | "rate_limit_exceeded" | "too_many_requests") => {
- ProviderErrorKind::RateLimit
- }
- Some("authentication_error" | "invalid_api_key" | "invalid_authentication") => {
- ProviderErrorKind::Authentication
- }
- Some(
- "access_denied" | "account_deactivated" | "permission_denied" | "permission_error",
- ) => ProviderErrorKind::AccessDenied,
- Some("content_filter" | "content_policy_violation") => ProviderErrorKind::ContentFilter,
- Some("context_length_exceeded") => ProviderErrorKind::ContextLength,
- Some("server_error" | "internal_error" | "service_unavailable" | "engine_overloaded") => {
- ProviderErrorKind::Server
- }
- Some(code) if code.ends_with("_not_found") => ProviderErrorKind::NotFound,
- Some(code)
- if code.starts_with("invalid_")
- || code.starts_with("unsupported_")
- || code.ends_with("_too_large")
- || code.ends_with("_too_long") =>
- {
- ProviderErrorKind::InvalidRequest
- }
- Some(_) | None => ProviderErrorKind::Server,
- };
+ // Unrecognized and absent codes are treated as transient.
+ let kind = classifier
+ .and_then(error::kind_from_error_code)
+ .unwrap_or(ProviderErrorKind::Server);
Error::Provider {
kind,
diff --git a/lib/components/fabro-llm/src/error.rs b/lib/components/fabro-llm/src/error.rs
index 753c12e6f..6ec6d7886 100644
--- a/lib/components/fabro-llm/src/error.rs
+++ b/lib/components/fabro-llm/src/error.rs
@@ -284,6 +284,52 @@ impl Error {
}
}
+/// Provider error code to error kind mapping, for the codes that say more
+/// than the transport-level status or stream event type does.
+///
+/// Returns `None` when the code adds nothing, so each caller keeps its own
+/// default: the stream decoders treat an unrecognized code as transient,
+/// while [`error_from_status_code`] falls back to the HTTP status.
+///
+/// Every dialect classifies through this one table so a code such as
+/// `insufficient_quota` means the same thing whether it arrives in an HTTP
+/// error body or in a mid-stream error event.
+#[must_use]
+pub(crate) fn kind_from_error_code(code: &str) -> Option {
+ Some(match code {
+ // Out of credit, or over a billing cap. Distinct from RateLimit:
+ // backoff never clears it, but another provider has its own quota.
+ "insufficient_quota" | "billing_hard_limit_reached" | "exceeded_current_quota_error" => {
+ ProviderErrorKind::QuotaExceeded
+ }
+ "rate_limit_error" | "rate_limit_exceeded" | "too_many_requests" => {
+ ProviderErrorKind::RateLimit
+ }
+ "authentication_error" | "invalid_api_key" | "invalid_authentication" => {
+ ProviderErrorKind::Authentication
+ }
+ "access_denied" | "account_deactivated" | "permission_denied" | "permission_error" => {
+ ProviderErrorKind::AccessDenied
+ }
+ "content_filter" | "content_policy_violation" => ProviderErrorKind::ContentFilter,
+ // `request_too_large` is anthropic's oversized-input code, so it has
+ // to precede the `_too_large` suffix rule below.
+ "context_length_exceeded" | "request_too_large" => ProviderErrorKind::ContextLength,
+ "server_error" | "internal_error" | "service_unavailable" | "engine_overloaded" => {
+ ProviderErrorKind::Server
+ }
+ c if c == "not_found_error" || c.ends_with("_not_found") => ProviderErrorKind::NotFound,
+ c if c.starts_with("invalid_")
+ || c.starts_with("unsupported_")
+ || c.ends_with("_too_large")
+ || c.ends_with("_too_long") =>
+ {
+ ProviderErrorKind::InvalidRequest
+ }
+ _ => return None,
+ })
+}
+
/// HTTP status code to error type mapping (Section 6.4).
#[must_use]
pub fn error_from_status_code(
@@ -303,6 +349,8 @@ pub fn error_from_status_code(
raw,
};
+ let code_kind = detail.error_code.as_deref().and_then(kind_from_error_code);
+
// Check specific status codes first -- these always map to their designated
// error types
let kind = match status_code {
@@ -316,10 +364,16 @@ pub fn error_from_status_code(
};
}
413 => ProviderErrorKind::ContextLength,
+ // A 429 means rate limited unless the body reports a spent quota,
+ // which retrying will never clear.
+ 429 if code_kind == Some(ProviderErrorKind::QuotaExceeded) => {
+ ProviderErrorKind::QuotaExceeded
+ }
429 => ProviderErrorKind::RateLimit,
500..=599 => ProviderErrorKind::Server,
- // For ambiguous status codes (400, 422, etc.), use message-based classification
- _ => {
+ // For ambiguous status codes (400, 422, etc.), the provider's error
+ // code is the better signal; fall back to the message only without one
+ _ => code_kind.unwrap_or_else(|| {
let lower_msg = detail.message.to_lowercase();
if lower_msg.contains("not found") || lower_msg.contains("does not exist") {
ProviderErrorKind::NotFound
@@ -333,7 +387,7 @@ pub fn error_from_status_code(
} else {
ProviderErrorKind::InvalidRequest
}
- }
+ }),
};
Error::Provider {
@@ -603,6 +657,105 @@ mod tests {
assert!(err.retryable());
}
+ /// Every vendor spelling of "you are out of credit" arrives as a 429 and
+ /// has to classify as a spent quota, not as a rate limit.
+ #[test]
+ fn quota_codes_on_429_are_non_retryable_quota_failures() {
+ for (provider, code) in [
+ ("kimi", "exceeded_current_quota_error"),
+ ("openai", "insufficient_quota"),
+ ("openai", "billing_hard_limit_reached"),
+ ] {
+ let err = error_from_status_code(
+ 429,
+ "Your account has insufficient balance".into(),
+ provider.into(),
+ Some(code.into()),
+ None,
+ None,
+ );
+
+ assert_eq!(
+ err.provider_kind(),
+ Some(ProviderErrorKind::QuotaExceeded),
+ "{code}"
+ );
+ assert!(!err.retryable(), "{code}");
+ assert!(err.failover_eligible(), "{code}");
+ }
+ }
+
+ /// A 429 that is a genuine rate limit stays retryable, whether the body
+ /// names it, names something unrecognized, or carries no code at all.
+ #[test]
+ fn non_quota_429_stays_a_retryable_rate_limit() {
+ for code in [
+ Some("rate_limit_error"),
+ Some("rate_limit_reached_error"),
+ Some("invalid_request_error"),
+ None,
+ ] {
+ let err = error_from_status_code(
+ 429,
+ "slow down".into(),
+ "openai".into(),
+ code.map(String::from),
+ None,
+ None,
+ );
+
+ assert_eq!(
+ err.provider_kind(),
+ Some(ProviderErrorKind::RateLimit),
+ "{code:?}"
+ );
+ assert!(err.retryable(), "{code:?}");
+ }
+ }
+
+ /// For a status with no fixed meaning, the structured code beats guessing
+ /// from the message text.
+ #[test]
+ fn ambiguous_status_prefers_error_code_over_message() {
+ let err = error_from_status_code(
+ 402,
+ "Payment required".into(),
+ "openai".into(),
+ Some("insufficient_quota".into()),
+ None,
+ None,
+ );
+ assert_eq!(err.provider_kind(), Some(ProviderErrorKind::QuotaExceeded));
+ }
+
+ #[test]
+ fn kind_from_error_code_covers_every_dialect() {
+ for (code, expected) in [
+ ("insufficient_quota", ProviderErrorKind::QuotaExceeded),
+ ("rate_limit_error", ProviderErrorKind::RateLimit),
+ ("authentication_error", ProviderErrorKind::Authentication),
+ ("permission_error", ProviderErrorKind::AccessDenied),
+ ("content_policy_violation", ProviderErrorKind::ContentFilter),
+ ("context_length_exceeded", ProviderErrorKind::ContextLength),
+ ("engine_overloaded", ProviderErrorKind::Server),
+ // anthropic's oversized-input code beats the `_too_large` rule
+ ("request_too_large", ProviderErrorKind::ContextLength),
+ ("prompt_too_long", ProviderErrorKind::InvalidRequest),
+ ("invalid_request_error", ProviderErrorKind::InvalidRequest),
+ ("unsupported_parameter", ProviderErrorKind::InvalidRequest),
+ // both the anthropic and openai not-found spellings
+ ("not_found_error", ProviderErrorKind::NotFound),
+ ("model_not_found", ProviderErrorKind::NotFound),
+ ] {
+ assert_eq!(kind_from_error_code(code), Some(expected), "{code}");
+ }
+
+ // No opinion, so the caller keeps its own default.
+ assert_eq!(kind_from_error_code("overloaded_error"), None);
+ assert_eq!(kind_from_error_code("api_error"), None);
+ assert_eq!(kind_from_error_code(""), None);
+ }
+
#[test]
fn error_message_classification_context_length() {
let err = error_from_status_code(
diff --git a/lib/components/fabro-llm/src/providers/openai.rs b/lib/components/fabro-llm/src/providers/openai.rs
index 56eb76c3d..b56159c2c 100644
--- a/lib/components/fabro-llm/src/providers/openai.rs
+++ b/lib/components/fabro-llm/src/providers/openai.rs
@@ -520,6 +520,40 @@ mod tests {
assert!(matches!(err, Error::Configuration { .. }));
}
+ #[tokio::test]
+ async fn complete_classifies_insufficient_quota_as_quota_exceeded() {
+ let server = MockServer::start();
+ let mock = server.mock(|when, then| {
+ when.method(POST).path("/responses");
+ then.status(429)
+ .header("content-type", "application/json")
+ .json_body(serde_json::json!({
+ "error": {
+ "message": "You exceeded your current quota.",
+ "type": "insufficient_quota"
+ }
+ }));
+ });
+ let adapter = Adapter::new("sk-test").with_base_url(server.base_url());
+
+ let err = adapter
+ .complete(&minimal_request())
+ .await
+ .expect_err("spent quota should fail the completion");
+
+ mock.assert();
+ assert_eq!(err.provider_kind(), Some(ProviderErrorKind::QuotaExceeded));
+ assert_eq!(err.status_code(), Some(429));
+ assert!(!err.retryable());
+ assert!(err.failover_eligible());
+ match err {
+ Error::Provider { detail, .. } => {
+ assert_eq!(detail.error_code.as_deref(), Some("insufficient_quota"));
+ }
+ other => panic!("expected provider error, got {other:?}"),
+ }
+ }
+
#[tokio::test]
async fn codex_complete_via_stream_propagates_stream_errors() {
let server = MockServer::start();
diff --git a/lib/components/fabro-llm/tests/integration.rs b/lib/components/fabro-llm/tests/integration.rs
index 3c80ef796..b9790b5ad 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 1294e2e8a..1074cc222 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/artifact_store.rs b/lib/components/fabro-store/src/artifact_store.rs
index 5be00d792..072e0434c 100644
--- a/lib/components/fabro-store/src/artifact_store.rs
+++ b/lib/components/fabro-store/src/artifact_store.rs
@@ -4,6 +4,7 @@ use bytes::Bytes;
use chrono::Utc;
use fabro_types::RunId;
use futures::StreamExt;
+use futures::stream::BoxStream;
use object_store::ObjectStore;
use object_store::buffered::BufWriter;
use object_store::path::Path as ObjectPath;
@@ -122,6 +123,33 @@ impl ArtifactStore {
}
}
+ pub async fn get_stream(
+ &self,
+ run_id: &RunId,
+ key: &ArtifactKey,
+ ) -> Result>>> {
+ let path = self.artifact_path(run_id, key)?;
+ match self.object_store.get(&path).await {
+ Ok(result) => Ok(Some(
+ result
+ .into_stream()
+ .map(|chunk| chunk.map_err(Error::from))
+ .boxed(),
+ )),
+ Err(object_store::Error::NotFound { .. }) => Ok(None),
+ Err(err) => Err(err.into()),
+ }
+ }
+
+ /// Check a path against the same rules `put` enforces, without writing.
+ ///
+ /// Readers that hand artifact paths back to a client — the ZIP download,
+ /// for one — use this to re-check paths that were stored before a rule
+ /// existed.
+ pub fn validate_relative_path(relative_path: &str) -> Result<()> {
+ validate_filename_segments(relative_path).map(drop)
+ }
+
pub async fn list_for_run(&self, run_id: &RunId) -> Result> {
let prefix = self.run_prefix(run_id)?;
let mut stream = self.object_store.list(Some(&prefix));
@@ -220,12 +248,27 @@ impl ArtifactStore {
}
}
+/// Artifact filenames end up as paths on someone else's disk — extracted from a
+/// ZIP, written by a worker — so they must stay relative and portable. The
+/// backslash, NUL, and drive-letter rules are what make the path safe on
+/// Windows as well as Unix.
fn validate_filename_segments(filename: &str) -> Result> {
if filename.contains('\\') {
return Err(Error::Other(
"artifact filename must not contain backslashes".to_string(),
));
}
+ if filename.contains('\0') {
+ return Err(Error::Other(
+ "artifact filename must not contain NUL bytes".to_string(),
+ ));
+ }
+ let bytes = filename.as_bytes();
+ if bytes.first().is_some_and(u8::is_ascii_alphabetic) && bytes.get(1) == Some(&b':') {
+ return Err(Error::Other(
+ "artifact filename must not start with a drive letter".to_string(),
+ ));
+ }
let segments = filename.split('/').collect::>();
if segments.is_empty() || segments.iter().any(|segment| segment.is_empty()) {
return Err(Error::Other(
@@ -460,6 +503,22 @@ mod tests {
);
}
+ #[tokio::test]
+ async fn get_stream_round_trips_chunked_reads() {
+ let store = test_store();
+ let run_id = fixtures::RUN_1;
+ let key = ArtifactKey::new(StageId::new("build", 2), 1, "logs/output.txt");
+ store.put(&run_id, &key, b"hello world").await.unwrap();
+
+ let mut stream = store.get_stream(&run_id, &key).await.unwrap().unwrap();
+ let mut bytes = Vec::new();
+ while let Some(chunk) = stream.next().await {
+ bytes.extend_from_slice(&chunk.unwrap());
+ }
+
+ assert_eq!(bytes, b"hello world");
+ }
+
#[tokio::test]
async fn rejects_invalid_relative_filenames() {
let store = test_store();
@@ -468,15 +527,25 @@ mod tests {
for filename in [
"",
+ "/escape.txt",
"../escape.txt",
"logs//output.txt",
"logs/./output.txt",
r"logs\output.txt",
+ "C:/escape.txt",
+ "c:escape.txt",
+ "bad\0name.txt",
] {
let key = ArtifactKey::new(node.clone(), 1, filename);
let err = store.put(&run_id, &key, b"boom").await.unwrap_err();
- assert!(err.to_string().contains("artifact filename"));
+ assert!(
+ err.to_string().contains("artifact filename"),
+ "accepted unsafe artifact filename: {filename:?}"
+ );
+ assert!(ArtifactStore::validate_relative_path(filename).is_err());
}
+
+ assert!(ArtifactStore::validate_relative_path("nested/result.txt").is_ok());
}
#[tokio::test]
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_state.rs b/lib/components/fabro-store/src/run_state.rs
index 05945e33f..76f257401 100644
--- a/lib/components/fabro-store/src/run_state.rs
+++ b/lib/components/fabro-store/src/run_state.rs
@@ -1363,22 +1363,13 @@ pub(crate) fn projected_billing(state: &RunProjection) -> BilledTokenCounts {
let mut billing = BilledTokenCounts::default();
for (stage_id, stage) in state.iter_stages() {
- if !is_boundary_stage(state, stage_id.node_id()) {
+ if !state.is_boundary_stage(stage_id.node_id()) {
billing.add_counts(&stage.usage);
}
}
billing
}
-fn is_boundary_stage(projection: &RunProjection, node_id: &str) -> bool {
- projection
- .spec()
- .graph()
- .nodes
- .get(node_id)
- .is_some_and(|node| matches!(node.handler_type(), Some("start" | "exit")))
-}
-
fn run_models(state: &RunProjection) -> Vec {
let mut models = state
.iter_stages()
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/billing_rollup.rs b/lib/components/fabro-workflow/src/billing_rollup.rs
index 09256429f..986b541d4 100644
--- a/lib/components/fabro-workflow/src/billing_rollup.rs
+++ b/lib/components/fabro-workflow/src/billing_rollup.rs
@@ -52,7 +52,7 @@ pub fn billing_rollup_from_projection(
let mut billed_visit_count = 0_usize;
for (stage_id, stage) in projection.iter_stages() {
- if is_boundary_stage(projection, stage_id.node_id()) {
+ if projection.is_boundary_stage(stage_id.node_id()) {
continue;
}
let usage = stage.billed_usage(catalog);
@@ -124,15 +124,6 @@ pub fn billing_rollup_from_projection(
}
}
-fn is_boundary_stage(projection: &RunProjection, node_id: &str) -> bool {
- projection
- .spec()
- .graph()
- .nodes
- .get(node_id)
- .is_some_and(|node| matches!(node.handler_type(), Some("start" | "exit")))
-}
-
#[cfg(test)]
mod tests {
use std::collections::HashMap;
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 5dad09d34..acc35a52f 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