## Summary
Move server-managed environments from sibling TOML files into SQLite,
matching the storage model already used by variables and secrets.
This adds:
- an `environments` SQLite table with DB-level validation for IDs,
revisions, providers, network modes, booleans, and JSON fields
- a SQLite-backed `EnvironmentStore` with cached synchronous reads,
transactional create/replace/delete, synthetic unpersisted `local`, and
`default` as an ordinary seeded row users can delete
- one-time legacy import from `environments/*.toml` next to the active
server `settings.toml`, including relative Dockerfile path inlining and
backup rename to `environments.imported-<timestamp>.bak`
- install/test/CLI seeding of `default` directly into SQLite instead of
writing `environments/default.toml`
- docs updates for API/SQLite-managed server environments and legacy
import behavior
The REST API shape is unchanged; path Dockerfile sources remain rejected
over the environments API.
## Testing
- `cargo nextest run -p fabro-db -p fabro-environment` - 15 passed
- `cargo nextest run -p fabro-server --features test-support
environments` - 16 passed
- `cargo nextest run -p fabro-server --features test-support install` -
60 passed
- `cargo nextest run -p fabro-server --features test-support
create_run_rejects_disabled_sandbox_provider` - 1 passed
- `cargo nextest run -p fabro-server --features test-support
system_sandbox_provider` - 2 passed
- `cargo nextest run -p fabro-cli install` - 132 passed
- `cargo +nightly-2026-04-14 fmt --check --all`
- `cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D
warnings`
## What
Per-step environment in `run.prepare.steps[].env` was parsed and then
**dropped** before it reached the resolved run settings, so prepare
steps could never see their declared env. This PR carries that env all
the way through to the executor, resolves prepare-step interpolation at
the run boundary, and fixes an argv-quoting bug.
Three things:
1. **Per-step env is carried through.** `RunPrepareSettings` now holds
`steps: Vec<PreparedStep>` (command plus per-step `env`) instead of a
flat `commands: Vec<String>`. The per-step env reaches `exec_command`,
which already accepts per-command env vars, and is merged on top of the
base sandbox environment.
2. **Interpolation resolves at the run boundary.** Prepare-step
`script`/`command` and per-step `env` values are carried in source form
out of the portable config resolve layer (so `fabro validate` stays
portable and never requires env to be set). Their `{{ env.* }}` tokens
resolve in the process that actually runs the steps, via
`RunPrepareSettings::resolve_step_env` — mirroring the existing MCP
transport env resolution. A missing env var is a **hard error**
(fail-closed); there is no fallback to the unresolved literal.
3. **Argv is shell-quoted.** Argv-style prepare steps were assembled
with `join(" ")`, so an argument containing spaces or quotes was
re-split by the shell. They are now shell-quoted per element with the
shared `shell_quote()` helper. `script` steps stay verbatim because they
are raw shell snippets.
## How
- `RunPrepareSettings.commands: Vec<String>` becomes
`RunPrepareSettings.steps: Vec<PreparedStep>` where `PreparedStep {
command, env }`. The server-side `{{ vars.* }}` substitution pass now
walks each step's command and env.
- New `RunPrepareSettings::resolve_step_env(env_lookup)` resolves `{{
env.* }}` in each step's command and env values, returning a hard error
on a missing var (and a loud `Unavailable` error for reserved
`secrets`/`inputs` tokens).
- The run boundary (`fabro_workflow::operations::start`) gains
`runtime_setup_commands`, the prepare-step counterpart to
`runtime_mcp_server`. `LifecycleOptions` now carries `Vec<SetupCommand>`
(command + env), and the initialize phase passes each step's env to
`exec_command`.
- `resolve_prepare` shell-quotes each argv element and carries per-step
env in source form. The stale lint suppression on the resolved fields is
rewritten to describe the deliberate source preservation that now
resolves at the run boundary.
- The shell-quoting helper moves to a shared `fabro_util::shell` module
(backed by `shlex`); `fabro_sandbox::shell_quote` delegates to it so the
config resolve layer and sandbox code share one audited implementation.
- The OpenAPI `RunPrepareSettings` schema and the generated TypeScript
client are updated to the new `steps`/`PreparedStep` shape.
## Testing
- `cargo build --workspace`
- `cargo +nightly-2026-04-14 fmt --check --all`
- `cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D
warnings`
- `cargo nextest run` for `fabro-util`, `fabro-types`, `fabro-config`,
`fabro-sandbox`, `fabro-api`, `fabro-workflow`, `fabro-server`,
`fabro-cli` (provider keys stripped) — all green.
- `cd lib/packages/fabro-api-client && bun run typecheck` — clean.
New tests cover: per-step env carried through resolution; script/command
+ env resolved at the run boundary; a missing env var is a hard error
(in both the command and a per-step env value); reserved `secrets`
tokens surface as `Unavailable`; argv elements are shell-quoted (an arg
with spaces/quotes is correctly quoted) while a `script` stays verbatim;
and an end-to-end check that per-step env reaches the executed setup
command (with a negative control proving the success is attributable to
the per-step env).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## What
Makes hook interpolation typed end-to-end and fail-closed, and removes
the bespoke template engine on HTTP-hook headers.
- **Typed end-to-end.** Hook `command`, `url`, header values, `prompt`,
and `model` are now carried as a typed `InterpString` from the config
resolve layer all the way to the executor. The executor resolves each
segment at hook fire time from the typed value instead of collapsing it
to a `String` and re-parsing it. This mirrors the MCP transport env
resolution boundary (`resolve_transport_env` / `runtime_mcp_server`).
- **Narrow header tokens.** HTTP-hook headers previously ran through
MiniJinja with an env allowlist
(`TemplateContext::with_env_lookup_allowed`). They now resolve through
the same narrow `{{ ns.NAME }}` token resolver as every other hook field
— no template engine, no allowlist.
- **Fail-closed everywhere.** A missing or out-of-scope `{{ env.* }}` /
`{{ secrets.* }}` token in a command, URL, header, prompt, or model is
now a hard error that blocks the hook rather than firing it with a
half-resolved or empty value. Previously command hooks failed closed but
http/prompt/agent hooks failed open (warned and proceeded), which could
dispatch an HTTP request with an empty credential header or run an LLM
call against a half-rendered prompt. Transport-level outcomes (non-2xx
responses, connection errors, unparseable bodies) stay fail-open.
A follow-up cleanup commit removes the template engine's `env` namespace
(`with_env_lookup` / `with_env_lookup_allowed` / the `EnvLookup`
object), which the header path was the last consumer of.
## How
- `fabro-types` and `fabro-hooks` `HookType` / `HookDefinition` now type
the interpolatable fields as `InterpString`. `InterpString` serializes
as its raw source, so persisted run specs and checkpoints round-trip
unchanged.
- The `fabro-config` resolve layer clones the typed `InterpString`
through instead of calling `as_source()`, so the fields no longer leak
unresolved template text — the old "source preservation" `#[expect]`
annotations on the hook resolvers are gone.
- The executor's single `resolve_interp` helper resolves a typed
`InterpString` and is shared by the command, http, prompt, and agent
paths; resolution failure maps to `HookDecision::Block`, which the
runner already reports loudly (error for blocking hooks, warn for
non-blocking).
## Testing
- New unit tests: fire-time resolution from the typed value (no
re-parse), narrow-token header resolution, and fail-closed behavior for
HTTP url, HTTP header, and prompt hooks on a missing variable (the hook
does not fire and the resolution error surfaces).
- Existing hook tests updated and kept green.
- Gates: `cargo build --workspace`, `cargo +nightly-2026-04-14 fmt
--check --all`, `cargo +nightly-2026-04-14 clippy --workspace
--all-targets -- -D warnings`, and `cargo nextest run` for the touched
crates (`fabro-hooks`, `fabro-types`, `fabro-config`, `fabro-template`,
`fabro-workflow`, `fabro-server`, and the `fabro-cli` hook/config
tests), all green.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## What
Adds the **mcp-servers HTTP API**: `GET/POST /api/v1/mcp-servers` and
`GET/PUT/DELETE /api/v1/mcp-servers/{id}` on top of the merged
`fabro-mcp-store` foundation and OpenAPI spec.
This includes the AppState wiring needed for the catalog to work end to
end: `McpServerStore` construction from `{active-config-dir}/mcps/`, an
`AppState` accessor, the `fabro-server` dependency, and route
registration for list/create/get/replace/delete handlers.
The API mirrors the automations concurrency pattern with ETags on
read/write responses and required `If-Match` headers for replace/delete.
## Resolved before merge
- **Credential-omitting read model:** read responses now return
`McpServerView` / `McpTransportView`, so stored env/header values are
not exposed by GET/list/create/replace responses. Responses include only
`env_keys` / `header_keys`; persisted values remain available to runtime
execution.
- **Manifest catalog references:** run manifest validation, graph
rendering, preflight, and run creation now resolve server-managed MCP
catalog references such as `[run.agent.mcps.<name>] id = "..."`.
- **Schema strictness:** unknown MCP transport fields are rejected,
aligning the reused Rust domain type with the OpenAPI
`additionalProperties: false` contract.
- **Create response headers:** the `POST /mcp-servers` 201 response now
documents its `ETag` header in OpenAPI.
## Follow-up intentionally left out
Credential-literal validation remains structural only: create/replace
currently accept literal env/header values and persist them for runtime
use. The warn-vs-hard-reject UX is a separate follow-up for the settings
UI; it is not a response-omission issue.
## Testing
Current PR checks are green:
- Rust: format, clippy, generated docs, Linux tests
- TypeScript: build, test, typecheck
Local checks run during the simplify/CI-fix pass:
- `cargo +nightly-2026-04-14 fmt --check --all`
- `cargo +nightly-2026-04-14 clippy --locked --workspace --all-targets
-- -D warnings`
- `cargo nextest run -p fabro-config run_agent_mcps`
- `cargo nextest run -p fabro-mcp-store`
- `cargo nextest run -p fabro-api --test mcp_server_round_trip`
- `cargo build -p fabro-api`
- `cargo nextest run -p fabro-server --features test-support
system_sandbox_provider`
- `cargo nextest run -p fabro-server --features test-support --test it
mcp_servers`
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
## Summary
This moves workflow-visible variables from JSON file storage into
SQLite-backed storage, establishing the first durable SQL table while
preserving the existing variable API behavior.
## What Changed
- Added a `fabro-db` crate with bundled SQLite, an embedded migration
for the `variables` table, and a `Database` owner for `connect()`,
`migrate()`, `health_check()`, and pool access.
- Replaced the `fabro-variable` JSON file store with an async
SQLx-backed `VariableStore` that preserves sorted listing,
case-sensitive names, empty string values, name validation, and
description-preserving upserts.
- Wired server startup to create `<storage>/db/fabro.sqlite3`, run
SQLite migrations, import legacy variables when needed, and pass the
shared pool into server state.
- Grouped live server stores under `AppStores` so runs, variables,
vault, environments, and automations share one state boundary while
artifacts remain separate.
- Updated variable handlers, run creation, validation, and test support
for async SQLite-backed variable access.
- Added schema, store-level, legacy import, and API-level persistence
coverage for variables.
## Legacy JSON Migration
On startup, Fabro looks for `<storage>/variables.json`. If it is
missing, startup is a no-op for legacy variables.
If the file exists, Fabro parses and validates the full file before
mutating SQLite. Valid entries are inserted with `ON CONFLICT(name) DO
NOTHING`, so existing SQLite values remain authoritative and only
missing names are imported from the legacy file.
After a successful import transaction, the source file is renamed to a
timestamped backup such as `variables.json.imported-<timestamp>.bak`. A
later startup naturally skips the import because the original source
path no longer exists. Invalid JSON or invalid variable names leave the
source file in place for operator repair.
Variable values are not logged during import. Logs include only safe
metadata such as source/backup paths, row counts, and variable names.
## Verification
- `cargo nextest run -p fabro-db -p fabro-variable`
- `cargo nextest run -p fabro-server --features test-support variables`
- `cargo +nightly-2026-04-14 fmt --check --all`
- `cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D
warnings`
---
[](https://github.com/EveryInc/compound-engineering-plugin)
Generated with GPT-5 via [Codex](https://openai.com/codex)
## Summary
- Updates transitive Rust dependency `tar` from `0.4.45` to `0.4.46` in
`Cargo.lock`.
- Expected to resolve Dependabot alert:
https://github.com/fabro-sh/fabro/security/dependabot/30
- Dependency path: `fabro-sandbox` -> `tar`.
## Grouping
- Kept this separate from the web alerts because it is a Rust
lockfile-only patch with a separate verification path.
## Verification
- `cargo tree -i tar` resolves `tar v0.4.46`.
- `cargo build --workspace`
- `cargo nextest run --workspace` (6860 passed, 185 skipped; nextest
reported 1 leaky test warning as non-fatal)
- `git diff --check`
## Residual alerts
- React Router alerts 31-37 are intentionally handled in a separate web
PR.
Co-authored-by: Release Repro <release-repro@example.com>
## Summary
`fabro provider login --server ... --provider openrouter` now asks the
selected Fabro server for provider metadata before reading, validating,
and storing API keys, so server-enabled providers are accepted even when
the local CLI catalog does not know them.
This adds a server-side credential test endpoint that validates
submitted API keys against the server's effective catalog without
persisting them, then keeps saving the resulting secret to the selected
target server. OpenAI Codex device login remains client-side for the
browser/device flow, with the resulting OAuth credential stored on the
selected server.
The OpenRouter docs and model docs are updated to use the current
`--provider openrouter` login syntax and clarify that remote deployments
need the server host settings updated.
## Testing
- `cargo nextest run -p fabro-client -p fabro-server -p fabro-cli
provider`
- `cargo +nightly-2026-04-14 fmt --check --all`
- `cargo +nightly-2026-04-14 clippy -p fabro-client -p fabro-server -p
fabro-cli --all-targets -- -D warnings`
- `rg -n "provider login openrouter|fabro provider login [a-z]"
docs/public lib/crates/fabro-cli/tests lib/crates/fabro-cli/src -g
'*.md' -g '*.mdx' -g '*.rs'`
---
[](https://github.com/EveryInc/compound-engineering-plugin)
🤖 Generated with GPT-5 (context compacted, extended thinking) via
[Codex](https://openai.com/codex)
## Summary
Fixes#501.
Adds a Docker sandbox diagnostics check so `fabro doctor` verifies the
Docker daemon when the Docker sandbox provider is enabled. Disabled
Docker providers are reported as disabled without touching the local
daemon.
## What changed
- Added `DockerSandboxProvider::check_daemon()` using Bollard `ping()`
only, with no container/image side effects.
- Added a `Docker Sandbox` check to server diagnostics with
pass/error/timeout handling and operator remediation.
- Updated demo diagnostics and doctor/server test fixtures so tests that
do not exercise Docker explicitly disable the provider.
- Added deterministic tests for enabled success, enabled failure,
enabled timeout, and disabled skip paths.
## Verification
- `cargo check -p fabro-server -p fabro-sandbox -p fabro-cli`
- `cargo test -p fabro-server docker_sandbox --lib`
- `cargo test -p fabro-server --features test-support
diagnostics_reports_under_scoped_daytona_api_key --lib`
- `cargo test -p fabro-cli --test it cmd::doctor`
- `git diff --check`
Not run locally: pinned nightly `fmt`/`clippy` because this environment
has Homebrew Rust only and no `rustup` for `nightly-2026-04-14`.
---------
Co-authored-by: Bryan Helmkamp <bryan@brynary.com>
Adds `openrouter.svg` so OpenRouter renders its brand mark on
`/settings/models` instead of the letter-initial fallback. The icon is
the official OpenRouter mark (monochrome, `currentColor`), normalized to
match the other provider logos. No code change needed — the route
already resolves `/images/providers/<provider.id>.svg`, and the catalog
provider id is `openrouter`.
---
[](https://github.com/EveryInc/compound-engineering-plugin)
🤖 Generated with Claude Opus 4.8 (1M context, extended thinking) via
[Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## What
Introduces an `ImportableTemplate` type that unifies the "inline content
**or**
`@path` file import" concept used by node `prompt`s, the graph `goal`,
and
`output_schema`. This is the last template-side piece of the
interpolation
unification: a single named type now owns the `@`-classification and
static-reference validation that was previously hand-rolled in three
places.
This is a **behavior-preserving refactor** — no user-visible change.
## How
- New `ImportableTemplate { Inline(String), Import { path } }` in
`transforms/importable_template.rs`, with `parse` (classifies a value —
a
leading `@` marks a file import), `import_path`, and `validate` (rejects
template syntax in an import path). Callers of templated fields classify
the
**already-rendered** string, because a leading `@` can be produced by
rendering (e.g. `{{ inputs.prompt_file }}` → `@prompts/work.md`).
- `prompt` + `goal`: render the inline value, then — if it's an `@file`
import —
load and render the file contents via the type. The missing-file →
literal
passthrough is preserved.
- `output_schema`: shares the same classification but is loaded
**verbatim** (it
is intentionally not a template), keeping its hard-error-on-missing-file
behavior.
- Deletes the dead `resolve_file_ref` helper (no non-test callers) and
inlines
the trivial `render_file_contents` wrapper.
- Migrates the `FilesystemFileResolver` coverage (tilde, `..`,
fallback-dir
precedence, missing file) — which previously only existed through
`resolve_file_ref`'s tests — onto direct `file_resolver` tests.
`TemplateTransform` and the import transform are untouched, so
goal-before-
prompts ordering and the goal-self-reference guard are preserved
exactly.
## Scope
Covers the DOT node `prompt` + graph `goal` `@file` path. The
settings-layer
`run.goal` resolution is intentionally left as-is — it uses a different
model
(interpolates env into the file path and does not render file contents),
so
folding it in would be a semantic change, not a refactor. That
convergence can
be a deliberate follow-up.
## Testing
- `cargo nextest run -p fabro-workflow` — 1182 passed (31
e2e/credentialed
skipped). New unit tests on the type (classification, validation) and
the
migrated `FilesystemFileResolver` tests.
- Regression net kept green: file-inlining (prompt/goal, output_schema
verbatim/error/routing, `{% include %}` rooting, fallback dir), the
`TemplateTransform` goal/self-reference/ordering tests, and the
cross-pass
`reports_goal_self_reference_once_across_passes`.
- `cargo +nightly fmt --check --all` and nightly
`clippy --workspace --all-targets -- -D warnings` clean.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## What
Changes `RunAgentSettings.mcps` from `HashMap<String,
McpServerSettings>` to `HashMap<String, ResolvedMcpEntry>`, a two-state
enum:
- `Resolved(McpServerSettings)` — an inline, fully-resolved MCP server
(every code path produces this today).
- `Reference { id, enabled }` — an unresolved reference to a named
server in the MCP catalog.
This is the **type-shape foundation only**: every current path still
produces `Resolved`, and no reference parsing or catalog lookup is added
here. It unblocks a later server-side pass that swaps `Reference` →
`Resolved` against the MCP server store before a run spec is persisted,
so persisted runs stay self-contained snapshots.
## Why this shape
- `ResolvedMcpEntry` is `#[serde(untagged)]` with `Resolved` first, so a
resolved entry (de)serializes as a bare `McpServerSettings` with no enum
tag — preserving backward compatibility with run specs persisted before
the enum existed.
- `McpServerRef` uses `deny_unknown_fields`, so the two variants can
never collide (`McpServerSettings` requires `name` + `transport`, which
a reference rejects).
- `McpServerRef.id` is a plain `String`, keeping `fabro-types` decoupled
from the MCP store crate.
## Consumers updated
- **fabro-config** `resolve_agent`: wraps each enabled inline entry as
`Resolved`, reusing the shared `resolve_enabled_mcps` enable-filter.
- **fabro-types** `RunNamespace::substitute_variables`: only walks
`Resolved` entries (references carry no templates).
- **fabro-workflow** `operations/start.rs`: extracts `Resolved` at the
post-persistence worker-startup consumer; a surviving `Reference` is an
invariant violation, guarded with `debug_assert!` plus a hard error.
- **fabro-cli** `exec.rs`: the `run.agent.mcps` fallback for `fabro
exec` keeps only `Resolved` inline servers; catalog references are
run-only on this CLI-direct path (no server-side resolver).
## Tests
- Back-compat round-trip proving old-format bare-`McpServerSettings`
maps (JSON and TOML) deserialize as all-`Resolved`.
- A `{ id, enabled }` value parses as `Reference` while a full server
config parses as `Resolved`.
- `Resolved` serializes back out as a bare `McpServerSettings`.
Independent of the in-flight MCP server store and OpenAPI-spec PRs;
mergeable on its own.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
## What
Adds the HTTP contract for managing server-defined MCP servers. The
handler implementation follows in a later change.
- New `/api/v1/mcp-servers` paths: `list`, `create`, `retrieve`,
`replace`, `delete`, with ETag / `If-Match` optimistic concurrency
mirroring the automations conventions.
- New schemas: `McpServer`, `CreateMcpServerRequest`,
`ReplaceMcpServerRequest`, `McpServerListResponse`.
- **Collapsed a duplicate `McpTransport` schema** into the single
canonical one and gave it a proper `discriminator` plus the
previously-missing optional `protocol` field (`streamable_http` |
`sse`). This also fixes a latent gap in the existing run-config
projection and is non-breaking (`protocol` is `#[serde(default)]`).
## Testing
- `cargo build -p fabro-api` is green — progenitor generates the client
methods and types cleanly from the new spec.
## Notes / follow-ups for the handler change
- Recommended `with_replacement` mapping (reuse, no parallel DTOs):
`McpServer` → `McpServerDefinition`, create/replace →
`McpServerDraft`/`McpServerReplace`, transport → existing
`fabro_types::McpTransport`/`McpHttpProtocol`; list envelopes become
small DTOs.
- Parity caveat: progenitor emits `i64` for the `u64` timeouts and `i32`
for the `u16 port`; harmless under `with_replacement`, but the handler
change must add identity/JSON-parity tests and not skip
`with_replacement` for those types.
- `createMcpServer` returns ETag on 201 (Environments convention) so the
UI gets the fresh revision.
- The "warn vs hard-reject credential-looking literal values" question
is recorded in the request-schema descriptions and intentionally not
enforced.
- Part of a short series adding server-managed MCP servers.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
## What
Adds the storage foundation for server-managed MCP servers: a durable
store plus its domain model. No server wiring, HTTP API, or UI yet —
this is standalone scaffolding that later PRs build on.
- New **`fabro-mcp-store`** crate: a concrete, filesystem-backed
`McpServerStore` — one TOML file per definition under
`{active-config-dir}/mcps/`, an in-memory cache, and a SHA-256
content-hash revision for optimistic concurrency. Modeled directly on
`AutomationStore`. Includes an id-only `ids()` accessor for cheap
listing that avoids cloning the (potentially sensitive) env/header maps
a full definition carries.
- New **`McpServerDefinition` / `McpServerDraft` / `McpServerReplace`**
domain model (plus `McpServerId` / `McpServerRevision` and structural
validation) in `fabro-types`, reusing the existing `McpTransport`. These
stay persistence-independent; the on-disk TOML DTO and the filesystem
plumbing live in `fabro-mcp-store`.
Nothing in the workspace depends on the new crate yet. Wiring
`McpServerStore` into the server, the HTTP API, and the UI are follow-up
PRs.
## Testing
- `fabro-mcp-store`: 7/7 (empty/missing dir, non-TOML ignored,
malformed/invalid-filename fail load, CRUD round-trip, stale-revision
and duplicate-create rejected).
- `fabro-types`: `mcp_store` validation and round-trip tests pass.
`cargo build --workspace`, fmt, and clippy all green.
## Notes
- The domain model derives `PartialEq` but not `Eq` because
`McpTransport` carries `HashMap`s (differs from `Automation*`, matches
the transport's capabilities).
- Validation is structural for now (id format, non-empty name,
well-formed transport); credential-literal validation is deliberately
deferred to the API layer (flagged TODO).
- The store is concrete by design (no trait): a future move off per-file
TOML is a one-time migration, not a runtime backend choice. The revision
is currently derived from the canonical TOML bytes — the one
storage-coupled detail to revisit if that move happens.
- Part of a short series adding server-managed MCP servers; independent
of the sibling PRs.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
## What
Threads the run's variable store through the workflow transform pipeline
so
node `prompt`s and the graph `goal` can interpolate `{{ vars.* }}`.
Until now `{{ vars.* }}` only resolved in settings-level fields (e.g.
`run.goal`) via the server-side `substitute_variables` pass. Node
prompts are
DOT graph attributes that pass never touched, so `{{ vars.* }}` in a
prompt
rendered as undefined. This closes that gap.
Builds on the earlier template-context slice (adds `vars` to
`TemplateContext`); this PR wires it end to end.
## How
- `TransformOptions` carries a `vars` map, threaded into the import,
file-inlining, and template transforms — and propagated into imported
subgraphs, so imported prompts interpolate vars too. Every prompt/goal
render
context gains the variable map.
- The create API accepts `vars` (`CreateRunInput` →
`preprocess_and_validate` →
`TransformOptions`).
- The server snapshots its `VariableStore` at run creation
(`VariableStore::value_map()`) and passes it in — the same store the
settings-goal substitution already reads.
## Scope decisions
- Goal `@file` contents interpolate vars too; **import paths stay
inputs-only**
(structural file resolution, conceptually outside the prompt/goal
scope).
- Offline / CLI / `fabro validate` render with an empty var map, so
`{{ vars.* }}` is undefined there: a warning at validate, a hard error
at
run-create — identical to how `inputs` behaves offline.
## Testing
- Transform-level: node-prompt and goal interpolation; unknown-var
warning.
- Create-pipeline: vars resolve; an unknown var warns at validate and
promotes
to a hard error at run-create.
- End-to-end server test: `POST /variables` + `POST /runs`, asserting
the
rendered prompt in the persisted `run.created` event.
Verified: `cargo +nightly fmt --check`, nightly `clippy -D warnings`
(including
the `test-support`-gated server integration binary), the tests above,
and a
full-workspace `cargo check`.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## What
Foundational refactor toward running a workflow that lives in one repo
against a *different* workspace repo (shared / external workflows). No
public API surface and no behavior change for automations — it only
reshapes internals behind a reusable seam.
- **New `git_checkout` module.** Lifts the git-clone +
manifest-from-checkout machinery out of `automation_materializer`:
`GitRepoCache` (cached bare clone + per-call worktree), the git command
plans, credential resolution/redaction, and GitHub owner/repo slug
parsing/validation. All `pub(crate)`; no module is exported.
- **Split the workflow source from the git context.**
`build_manifest_from_checkout` now takes the *workflow-source checkout*
(which workflow to bundle) and the *git context* (which repo the run
clones and executes in) as separate inputs. Automations are the case
where both coincide. This is the seam a future external-workflow
resolver needs.
- **Decoupled the builder input.** `ManifestFromCheckoutInput` no longer
embeds `AutomationRunMaterializeInput`; it takes only the fields it
needs plus a caller-supplied error context, so it's reusable without
automation-specific types.
## Review fixes folded in
- **Error type points the right way.** The shared materialize error
moved into `git_checkout` as the provider-neutral `RunMaterializeError`
(same variants, neutral messages). The foundation module no longer
depends back on its consumer, and a bad workflow-source slug no longer
reports "invalid automation target".
- **Required git context, not `Option`.** No caller omits it today;
widening to optional later is backwards-compatible if a real case
appears.
## Testing
- `cargo build -p fabro-server`, pinned-nightly `fmt --all` and `clippy
-p fabro-server --all-targets -D warnings`: clean.
- `cargo nextest run -p fabro-server`: 729/732 pass. The 3 failures are
graphviz SVG-render-subprocess tests (`get_graph_returns_svg`,
`render_graph_from_manifest_*`) that fail identically on the clean
baseline in this environment — pre-existing and unrelated.
- The rewritten unit test proves the split: a manifest built from a
workflow-source checkout while `manifest.git` points at a *different*
repo and ref.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## What
Two latent fixes to MCP server config handling, independent of any new
feature:
1. **`enabled = false` is now honored for inline MCP servers.** Entries
under `[run.agent.mcps.*]` and `[cli.exec.agent.mcps.*]` accepted an
`enabled` flag that resolution silently ignored, so a disabled server
still started. Disabled entries are now dropped from the resolved set.
Absent `enabled` still means enabled.
2. **Explicitly configured empty `cli.exec.agent.mcps` sets are
preserved.** If every `cli.exec` MCP entry is disabled, `fabro exec` now
treats that as an intentional empty override instead of falling back to
`run.agent.mcps`.
3. **Per-server `tool_timeout_secs` now applies to MCP tool calls.** The
value was carried through config but never reached the call path. The
connection manager now owns each server timeout and applies it when
calling tools.
## Testing
- New and updated tests cover StickyMap same-key replacement across
layers, `enabled = false` skipped for run and `cli.exec`, absent
`enabled` kept, higher-layer disable shadowing, explicit empty
`cli.exec` MCP overrides, and configured tool timeout behavior.
- `cargo +nightly-2026-04-14 fmt --check --all`
- `cargo nextest run -p fabro-config -p fabro-agent -p fabro-mcp`: 737
passed, 93 skipped.
- `cargo +nightly-2026-04-14 clippy -p fabro-config -p fabro-agent -p
fabro-mcp -p fabro-cli --all-targets -- -D warnings`
- `cargo test --locked -p fabro-workflow --test it --no-run`
## Notes
- **Behavior change** worth a changelog entry: disabled inline MCPs are
now actually disabled, explicit empty `cli.exec` MCP overrides are
respected, and per-server tool timeouts now take effect.
- First of a short series adding server-managed MCP servers; this PR is
self-contained and independent of the others.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Implements the `inputs`-template-only half of **D12**. Independent off
`main` — touches only `fabro-types` interp; no overlap with #511 or
#512.
## What changes for users
`{{ inputs.* }}` in an `InterpString` field (command, script, header,
env, URL — MCP transports, prepare steps, hooks, server settings) now
fails with a **clear, actionable message**:
> `{{ inputs.X }}` is only available in prompts and goals, not in
command, script, header, env, or URL fields
It *already* failed there (no resolve context ever provided an inputs
lookup, so it errored as a generic "unavailable"); this makes the
rejection explicit and points the user at where `inputs` belongs.
## How
- **`ResolveCtx` drops its unused `inputs` lookup** (`with_inputs` had
zero production callers). The type now structurally cannot resolve
`inputs` in an `InterpString` field; `lookup_for(Inputs)` returns
`None`.
- The `Unavailable` error message is `inputs`-specific and points to
prompts/goals.
- **`substitute_with` still preserves `inputs` tokens**
(unknown-namespace passthrough), so `run.goal` — an `InterpString` that
feeds a template — keeps forwarding `{{ inputs.* }}` to its prompt/goal
render. This is the load-bearing behavior that makes "inputs works in
goals" coexist with "inputs rejected in InterpString fields", and it's
covered by an existing test
(`substitute_variables_preserves_late_bound_tokens`).
- Module docs updated: three resolvable namespaces in `InterpString`
(`env`/`vars`/`secrets`); `inputs` is template-only.
## Note on timing
The rejection fires at **resolve time** (use-time / run boundary), not
at `fabro validate`. That matches how the other late-bound namespaces
behave and keeps this PR small; a validate-time fail-fast would need to
distinguish goal (forwards inputs) from pure-`InterpString` fields and
is a larger, separate change if we want it.
## Tests
`resolve_with_rejects_inputs_as_template_only` (rejection + friendly
message); `substitute_variables_preserves_late_bound_tokens` confirms
goal forwarding is unaffected.
Verified: `cargo build --workspace`, nightly `clippy --workspace
--all-targets -D warnings`, `fmt`, `cargo nextest run --workspace`
(**6796 passed**).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Implements the goal self-reference behavior for interpolation
unification. Independent off `main` — no dependency on the other interp
PRs (touches only the template/goal-render path).
## What changes for users
A graph `goal` is a template that interpolates `{{ inputs.* }}`
(unchanged). A node `prompt` can reference the rendered goal via `{{
goal }}` (unchanged). **New:** a goal can **no longer reference itself**
— `{{ goal }}` *inside* a goal was previously a silent passthrough (left
as the literal text `{{ goal }}`); it's now a clear error.
```
graph [goal="Refine {{ goal }}"] # error: a goal cannot reference itself
work [prompt="Work on {{ goal }}"] # fine: prompts reference the rendered goal
```
## How
- **Structural guarantee:** the goal renders with **no `goal` key in
scope** (`TemplateContext::new().with_inputs(..)` instead of the
`for_input_scan` passthrough), so a self-reference can't resolve.
- **Friendly lint:** before rendering, `resolved_goal` checks the goal
template for a top-level `goal` reference — new
`fabro_template::references_top_level_variable`, backed by MiniJinja
`undeclared_variables` — and emits a dedicated `goal_self_reference`
diagnostic (`Severity::Error`) with a clear message and fix-it, instead
of a generic "undefined variable `goal`". Fails `fabro validate` and
run-create alike.
The goal is resolved in two transform passes (FileInlining +
TemplateTransform); the diagnostic is emitted **once** (FileInlining
discards its goal-resolution diagnostics; TemplateTransform is the
canonical emitter).
## Behavior change (release notes)
A goal containing `{{ goal }}` now **errors** instead of passing through
as literal text. The error message is the migration signal.
## Tests
- `references_top_level_variable` detection
- transform-level rejection (`Severity::Error`)
- single-emission across the two passes
- end-to-end `validate` rejection
- existing goal/prompt tests still green (prompts reference goal; goal
interpolates inputs)
## Verification
- `cargo build --workspace`
- `cargo +nightly clippy --workspace --all-targets -- -D warnings`
- `cargo +nightly fmt --check`
- `cargo nextest run --workspace`: 6800 passed
Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
First **enhancing** PR of the interpolation unification: now that the
reducing PRs have pinned interpolation to the workflow config language,
this adds real `{{ env.* }}` resolution for MCP server transports — at
**both** the `fabro run` and `fabro exec` boundaries.
Independent off `main` — **no dependency on #510** (zero file overlap;
#510 touches the control-plane server settings). Builds on the
already-merged InterpString foundation (#472).
## What users get
MCP server transport fields now interpolate `{{ env.* }}` tokens,
resolved **at the boundary where the server is actually launched**:
- **stdio / sandbox**: `command`, `args`, and per-server `env` values
- **http**: `url` and `headers`
A literal value passes through unchanged; a `{{ env.NAME }}` token is
resolved against the launching process's environment. **Missing env var
is a hard error** (D3) instead of the previous behavior where the raw
token leaked downstream as literal text. Reserved `secrets`/`inputs`
tokens (no resolver here yet) surface as a loud `Unavailable` error
rather than passing through.
Resolution happens at the run/exec boundary, not in the shared config
resolve layer, so `fabro validate` stays portable (env presence is a
runtime concern, not a validation one).
## Both consumers, one resolver
`fabro run` and `fabro exec` read the **same** MCP representation —
`run.agent.mcps` and `cli.exec.agent.mcps` both parse through
`McpEntryLayer` (InterpString) and collapse via the same
`resolve_mcp_entry`. Originally only the run boundary resolved env, so a
file-sourced `[cli.exec.agent.mcps.*.env] KEY = "{{ env.X }}"` (from
`~/.fabro/settings.toml`) resolved under `run` but **leaked the raw
token under `exec`** — a silent asymmetry that would generate confusing
bug reports.
This PR closes that by moving the resolution onto the type as
`McpServerSettings::resolve_transport_env` (in `fabro-types`, next to
the `vars` half `substitute_mcp_transport`), so both consumers share one
resolver with no drift:
- `runtime_mcp_server` (run worker) → resolves against the worker
process env
- `fabro exec` → resolves against the CLI process env
`runtime_mcp_server` becomes a thin wrapper that just adds the server
name to the error.
## Tests
- `fabro-types`: 5 `resolve_transport_env` unit tests — literal
passthrough, stdio command+env, http url+headers, sandbox env,
missing-env hard error, and the reserved-`secrets` loud-fail case.
- `fabro-workflow`: the 5 existing `runtime_mcp_server_*` tests are
unchanged and now exercise the shared resolver through the wrapper.
Files: `fabro-config/src/resolve/run.rs`,
`fabro-types/src/settings/run.rs`,
`fabro-workflow/src/operations/start.rs`,
`fabro-cli/src/commands/exec.rs`.
Verified: `cargo build`, nightly `clippy --all-targets -D warnings`,
nightly `fmt --check`, and `cargo nextest run -p fabro-types -p
fabro-workflow -p fabro-config -p fabro-cli` (2679 passed with ambient
provider keys stripped; the one failure otherwise is the pre-existing
ambient-`*_API_KEY` flake, unrelated to MCP).
> Note: the shared resolver takes `Resolved.value` and drops interp
`Provenance` (consistent with every other resolved path today —
`Provenance` currently has zero consumers, and resolved MCP transport
values never surface in logs/events/API). Whether MCP env should carry
provenance for precise redaction vs. relying on content-based
`fabro-redact` is tracked as an open decision under D4.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Third reducing PR of the interpolation unification (**D11 resolution
(c)**): the control plane never interpolates. `InterpString` is now
strictly the user-facing workflow config language; server identity,
storage, listen, object-store, and GitHub App identifiers are plain
`String`, consumed where needed with no resolution point.
## Demoted to `String` (was `InterpString`)
Both the layer and resolved types:
- `server.listen.unix.path`, `server.api.url`, `server.web.url`
- `server.storage.root`, `server.artifacts.prefix`,
`server.slatedb.prefix`
- object store: `Local.root`, S3 `bucket` / `region` / `endpoint`
(shared by artifacts + slatedb)
- `github.app_id` / `client_id` / `slug`
**Kept `InterpString`:** `slack.default_channel` (run-time consumption —
the one server-defined survivor). `server.listen.tcp.address` stays the
`SocketAddr` `parsed_value` special case.
## Native `FABRO_WEB_URL` read
Deployment-time late binding now goes through a native env read instead
of a `{{ env.* }}` token: `FABRO_WEB_URL` overrides `server.web.url`
(**env override > settings literal > default**), applied in
`canonical_origin` and reused by the JWT issuer, cookie-secure check,
and system-info. `docker/split-web` no longer ferries the value through
a settings token (compose still sets the env var). `canonical_origin`'s
error message now advertises a knob that is actually true for everyone.
## Behavior change (release notes)
- `{{ env.* }}` / `{{ vars.* }}` tokens in the demoted server fields are
now **literal text**, not interpolated. The resolve layer emits
`warn_if_demoted_template` for every demoted field, so operators with
tokens still in server config **fail loud** rather than silently
treating the token as a literal.
- Operators who relied on env-based storage location should use the
existing native `FABRO_STORAGE_DIR` (`--storage-dir`) override.
`FABRO_STORAGE_ROOT` promotion is intentionally deferred (not a proven
need).
## Cleanup
`fabro-server`'s `crate::interp` shrinks to just the process-env lookup
facade; `resolve_interp` / `_path` / `_with` and the
`AppState::resolve_interp` seam are deleted (nothing resolves
server-scope `InterpString` anymore).
## Verification
- `cargo build --workspace` ✅
- `cargo +nightly clippy --workspace --all-targets -- -D warnings` ✅
(incl. the `as_source` gate)
- `cargo +nightly fmt --check --all` ✅
- `cargo nextest run --workspace`: 6305 passed; added two tests covering
the `FABRO_WEB_URL` override precedence (env-wins and settings-literal
fallback).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
# Demote non-category leak fields to plain `String`
Second slice of the interpolation unification — the first **reducing**
PR
stacked on the foundation (#472), per the reduce-first sequencing:
narrowing
changes land before capability additions. (The other reducing slice, the
DOT
de-templating, already landed independently as #474.)
## Why
The target model gives `InterpString` to fields in five categories —
`command` / `script` / `headers` / `env` / `url` — wherever they appear.
A
handful of fields were typed `InterpString` but are *identifiers or
commit
content*, not in any category:
- `run.model.provider` / `run.model.name`
- `cli.exec.model.provider` / `cli.exec.model.name`
- `run.git.author.name` / `run.git.author.email`
- `run.scm.owner` / `run.scm.repository`
Their consumers never resolved them — they leaked raw source text via
`as_source()`. This PR demotes them to plain `String` (layer and
resolved
structs) with **no interpolation**.
## The principle: only `InterpString` fields access variables
These fields are dropped from the variable substitute pass entirely, so
both
`{{ vars.* }}` and `{{ env.* }}` are now literal text. This **removes an
incidental behavior**: run-scoped plain-`String` fields used to get
`{{ vars.* }}` substituted via the String pass (a lucky accident), while
`env` always leaked literally. Variable access becomes deliberate and
typed
rather than accidental; if any of these fields should support variables
later, that's a controlled promotion back to `InterpString`.
## Behavior changes (honest list)
- **The incidental run-scoped `{{ vars.* }}` substitution on these eight
fields stops working.** To keep the removal visible rather than silent,
a
`tracing::warn!` fires at resolve time when a demoted field still
contains
claimed template tokens (`warn_if_demoted_template`). Unclaimed `{{ ...
}}`
text (jq programs, Go templates) never interpolated and does not warn.
- `{{ env.* }}` / `{{ secrets.* }}` / `{{ inputs.* }}` never resolved on
these fields, so nothing else changes.
## Added in review: D11 demotions (separate commit, revertable)
The rule got refined during review: a field is `InterpString` iff it is
in one
of the five categories **and resolved at the run boundary** (the only
point
where `vars`/`secrets`/`inputs` exist — they're server state, so
connect-time
and startup-time fields can't reach them even in principle). A separate
commit
applies the clean subset so it can be cherry-picked out if we change
course:
- `cli.target.http.url` / `cli.target.unix.path` — consumed at CLI
connect
time; consumers only ever leaked raw source, so nothing working is
removed.
- `run.working_dir` — **the outlier; see the PR comment.** Its `{{
vars.* }}`
substitution worked; demoted on the category test alone.
## What's deliberately NOT here
- The **control-plane fields** (`server.storage.root` /
`listen.unix.path` /
S3 fields / `github.app_id/client_id/slug` / `server.api.url` /
`server.web.url`) — untouched here, demoted in a follow-up PR.
**Resolved during review** (see the resolution comment): `InterpString`
was
conflating the user-facing workflow language with the internal control
plane. Control-plane fields never interpolate; the few deployment knobs
that need late binding (e.g. `FABRO_WEB_URL`, whose only real usage is
the
split-web PoC ferrying a compose env var across a file mount) become
explicit native `EnvVars` reads, and `fabro-server/src/interp.rs`
shrinks
to deletion. `slack.default_channel` stays `InterpString` (consumed with
run context).
## Implementation notes
- Consumers move from `as_source()` to direct `String` access; the
foundation's `#[expect(disallowed_methods, ... demotion pending ...)]`
annotations for these fields are removed (no longer `InterpString`).
- `fabro-checkpoint`'s author plumbing and `fabro-manifest`'s scm fields
simplify accordingly.
## Verification
- `cargo build --workspace`
- `cargo nextest run --workspace` → 6684 passed, 181 skipped
- `cargo +nightly fmt --check --all`
- `cargo +nightly clippy --workspace --all-targets -- -D warnings` →
clean
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
