sync: merge upstream (51 commits) — {{ vars.* }} interpolation fixes (#524/#513/#492) - #1
Merged
Merged
Conversation
… clear error (D12) (fabro-sh#513) Implements the `inputs`-template-only half of **D12**. Independent off `main` — touches only `fabro-types` interp; no overlap with fabro-sh#511 or fabro-sh#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>
…ro-sh#520) ## 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>
…abro-sh#523) ## 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>
…abro-sh#524) ## 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>
…sh#521) ## 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 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>
…bro-sh#526) ## 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>
…fabro-sh#527) ## 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>
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>
…ro-sh#525) ## Summary Fixes fabro-sh#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>
## 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 - 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 - Updates direct web runtime dependency `react-router` from `7.12.0` to `7.15.1` in `apps/fabro-web`. - Regenerates the root Bun workspace lockfile. - Expected to resolve Dependabot alerts: - https://github.com/fabro-sh/fabro/security/dependabot/31 - https://github.com/fabro-sh/fabro/security/dependabot/32 - https://github.com/fabro-sh/fabro/security/dependabot/33 - https://github.com/fabro-sh/fabro/security/dependabot/34 - https://github.com/fabro-sh/fabro/security/dependabot/35 - https://github.com/fabro-sh/fabro/security/dependabot/36 - https://github.com/fabro-sh/fabro/security/dependabot/37 ## Grouping - Grouped these alerts because they all affect the same direct package, same manifest, same runtime scope, and same verification path. - Kept separate from the Rust `tar` alert because it touches a different ecosystem and lockfile. ## Verification - `bun pm why react-router` resolves `react-router@7.15.1` for `fabro-web`. - `cd apps/fabro-web && bun run typecheck` - `cd apps/fabro-web && bun test --isolate` (625 passed, 0 failed) - `cd apps/fabro-web && bun run build` - `git diff --check` ## Residual alerts - Rust `tar` alert 30 is handled separately in fabro-sh#534. Co-authored-by: Release Repro <release-repro@example.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)
…o-sh#532) ## 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>
…sed resolution (fabro-sh#528) ## 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>
…abro-sh#530) ## 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>
## 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`
## Summary
Adds a full CRUD management UI for server-managed MCP servers at
`/settings/mcps`, consuming the already-shipped `MCPServersApi` backend.
The implementation mirrors the existing `/settings/environments` pages
exactly in structure, naming, and component conventions.
## What changed
### Step 1 — Shared `KeyValueEditor` extracted
`KeyValueEditor`, `KeyValueEntry`, `entriesFromMap`, and
`mapFromEntries` are moved from `environment-form.tsx` into a new
`components/key-value-editor.tsx`. The component gains an optional
`renderEntryHint` prop so per-row warnings can be injected without
coupling the editor to credential logic. `Label` is promoted from
`environment-form.tsx` to `settings-panel.tsx` so both forms can use it.
### Step 2–4 — Query plumbing
- `query-keys.ts`: `mcpServers.{list, detail}` keys.
- `api-client.ts`: `mcpServersApi` instance (same pattern as
`environmentsApi`).
- `queries.ts`: `useMcpServers()` and `useMcpServer(id)` SWR hooks.
### Step 5 — Credential heuristics (`lib/credential-heuristics.ts`)
Pure functions `looksLikeCredential`, `secretNameForKey`,
`secretReference`. Key-name matching covers `authorization`, `password`,
`token`, `api[-_]?key`, `_key`/`_token`/`_secret` suffixes.
Value-entropy fallback fires for strings ≥ 20 chars, no spaces, mixed
case/digit classes. Template references (`{{ secrets.* }}`) are never
flagged.
### Step 6–7 — Form model + component (`components/mcp-server-form.tsx`)
- Flat `McpServerFormValues` discriminated on `McpTransportKind`.
- `defaultMcpServerFormValues`, `mcpServerToFormValues` (populates
`env`/`headers` from `env_keys`/`header_keys` with **empty values** —
the §5 write-only design), `createRequestFromForm`,
`replaceRequestFromForm`, `isMcpServerFormValid`, `credentialWarnings`.
- `McpServerFormFields` renders stdio / http / sandbox panels switching
on `values.transport`. Per-row credential nudge opens the secrets-new
page in a new tab and substitutes a `{{ secrets.NAME }}` reference; save
is never blocked by the heuristic.
- On edit, a row with a non-empty key and empty value blocks save with
an inline error (the intentional overwrite guard).
### Step 8–10 — Route pages
| File | Mirrors |
|---|---|
| `routes/settings-mcps.tsx` | `settings-environments.tsx` |
| `routes/settings-mcps-new.tsx` | `settings-environments-new.tsx` |
| `routes/settings-mcps-edit.tsx` | `settings-environments-edit.tsx` |
The edit page shows a write-only-values banner whenever the transport
has any `env_keys`/`header_keys`, uses `key={server.revision}` to
remount the form on external change, and translates 409 responses into
the `staleAwareMessage` pattern.
### Steps 11–12 — Router + nav
Three routes registered under `settings` children. `PuzzlePieceIcon` nav
entry added to the same section as Environments.
### Plan Summary
- Extract `KeyValueEditor` to shared component with hint-injection slot
- Credential heuristics library (pure, fully unit-tested)
- MCP form model: flat values ↔ discriminated API types, write-only-key
guard
- List / new / edit pages following environments pattern exactly
- Route registration and settings nav link
### Fabro Details
<details>
<summary>Ran 9 stages in 65m 58s for $20.54</summary>
| Stage | Duration | Cost | Retries |
|---|---|---|---|
| start | 0s | – | 0 |
| toolchain | 1s | – | 0 |
| preflight_compile | 2m 47s | – | 0 |
| preflight_lint | 4m 15s | – | 0 |
| implement | 26m 29s | $12.27 | 0 |
| simplify_opus | 7m 41s | $4.95 | 0 |
| simplify_gpt | 6m 51s | $2.69 | 0 |
| verify | 15m 44s | – | 0 |
| fixup | 1m 41s | $0.63 | 0 |
| **Total** | **65m 58s** | **$20.54** | **0** |
</details>
<details>
<summary>Ran <code>ImplementPlan.fabro</code> (11 nodes and 14
edges)</summary>
```dot
digraph ImplementPlan {
graph [
goal="Implement and simplify",
model_stylesheet="
* { model: claude-opus-4-8; }
"
]
rankdir=LR
start [shape=Mdiamond, label="Start"]
exit [shape=Msquare, label="Exit"]
toolchain [label="Toolchain", shape=parallelogram, script="command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1", max_retries=0]
preflight_compile [label="Preflight Compile", shape=parallelogram, script="cargo check -q --workspace 2>&1", max_retries=0]
preflight_lint [label="Preflight Lint", shape=parallelogram, script="cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1", max_retries=0]
fix_lints [label="Fix Lints", prompt="The preflight lint step failed. Read the build output from context and fix all clippy lint warnings.", max_visits=3]
implement [label="Implement", prompt="Read the plan file referenced in the goal and implement every step. Make all the code changes described in the plan. Use red/green TDD.", model="gpt-55", reasoning_effort="xhigh"]
simplify_opus [label="Simplify (Opus)", prompt="@prompts/simplify.md"]
simplify_gpt [label="Simplify (GPT-55)", prompt="@prompts/simplify.md", model="gpt-55"]
verify [label="Verify", shape=parallelogram, script="git fetch origin main 2>&1 && git merge --no-edit --no-stat origin/main 2>&1 && cargo +nightly-2026-04-14 fmt --all 2>&1 && cargo dev docs refresh 2>&1 && cargo +nightly-2026-04-14 fmt --check --all 2>&1 && { command -v rg >/dev/null 2>&1 || { echo 'rg is required for verify'; exit 127; }; } && ! rg -n 'AuthMode::Disabled|RunAuthMethod|RunSubjectProvenance|\bActorRef\b|\bActorKind\b|AuthenticatedSubject|AuthenticatedService|AuthorizeRunScoped|AuthorizeRunBlob|AuthorizeStageArtifact|AuthorizeCommandLog|auth_method\s*==\s*\"disabled\"' lib/crates apps lib/packages docs/public/api-reference/fabro-api.yaml 2>&1 && cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings 2>&1 && cargo nextest run --workspace --status-level slow --profile ci 2>&1 && cargo dev docs check 2>&1 && bun install --frozen-lockfile 2>&1 && (cd apps/fabro-web && bun run typecheck) 2>&1 && (cd apps/fabro-web && bun run test) 2>&1 && (cd lib/packages/fabro-api-client && bun run typecheck) 2>&1 && cargo dev build -- -p fabro-cli --release 2>&1", goal_gate=true, retry_target="fixup"]
fixup [label="Fixup", prompt="The verify step failed. Read the build output from context and fix all format, clippy, Rust test, docs, TypeScript typecheck/test, and build failures.", max_visits=3]
start -> toolchain
toolchain -> preflight_compile [condition="outcome=succeeded"]
toolchain -> exit
preflight_compile -> preflight_lint [condition="outcome=succeeded"]
preflight_compile -> exit
preflight_lint -> implement [condition="outcome=succeeded"]
preflight_lint -> fix_lints
fix_lints -> preflight_lint
implement -> simplify_opus -> simplify_gpt -> verify
verify -> exit [condition="outcome=succeeded"]
verify -> fixup
fixup -> verify
}
```
</details>
⚒️ Generated with [Fabro](https://fabro.sh)
---------
Co-authored-by: Fabro <noreply@fabro.sh>
…de… (fabro-sh#546) ## Summary `Sandbox::glob` worked correctly on the Local provider but silently returned empty results on Docker and Daytona for any pattern containing `/` or `**` (e.g. `*/SKILL.md`). This broke skill discovery on every remote sandbox — the production path — and degraded the agent's `Glob` tool for common patterns like `**/*.rs`. ## Root cause The remote providers delegated matching to `find -name <pattern>`, but `find -name` only matches the basename and rejects patterns containing `/`. So `find <base> -name "*/SKILL.md"` exits 0 with empty output while the file is sitting right there. ## Fix Glob is two distinct operations: **traversal** (needs filesystem access) and **matching** (pure string logic). The fix separates them cleanly: - A new `glob_match` module (`src/glob_match.rs`) provides `GlobMatcher` and `traversal_root` helpers, backed by the already-present `glob` crate's `Pattern` matcher with `require_literal_separator: true` so `*` stays within a single path segment. - Remote providers (Docker, Daytona) now run `find <root> -type f` (traversal only) and pass results through `GlobMatcher` on the host side. - Daytona additionally gains a `list_files_recursive` path that uses the Daytona filesystem API directly instead of shelling out, which is more robust when the shell is fail-closed. - Local is also rerouted through `GlobMatcher` with a `collect_local_files` walker, making all three providers share identical matching semantics by construction. mtime-based sort is preserved using metadata collected during traversal. ```mermaid flowchart TB caller["glob(pattern, path)"] traversal_root["traversal_root(base, pattern)\nextract literal prefix"] list["list files under root\n(find -type f / fs API / std::fs)"] matcher["GlobMatcher::new(base, pattern)\nglob::Pattern + MatchOptions"] filter["filter candidates"] sort["sort results"] caller --> traversal_root --> list --> filter caller --> matcher --> filter --> sort ``` ### Plan Summary - New `glob_match.rs` module: `GlobMatcher`, `traversal_root`, `join_path` utilities + unit tests proving parity with `glob::glob` on shared fixtures - Docker: replace `find -name` with `find -type f` + host-side `GlobMatcher` - Daytona: replace `find -name` with `list_files_recursive` (Daytona FS API) + `GlobMatcher` - Local: replace `glob::glob()` walk with `collect_local_files` (symlink-safe) + `GlobMatcher`; mtime sort preserved - New `LocalSandbox::glob` tests: relative path resolution, `**` depth, `*/SKILL.md` one-level semantics, symlink non-recursion ### Fabro Details <details> <summary>Ran 8 stages in 64m 31s for $14.97</summary> | Stage | Duration | Cost | Retries | |---|---|---|---| | start | 0s | – | 0 | | toolchain | 1s | – | 0 | | preflight_compile | 2m 21s | – | 0 | | preflight_lint | 2m 41s | – | 0 | | implement | 39m 47s | $11.23 | 0 | | simplify_opus | 7m 36s | $2.55 | 0 | | simplify_gpt | 3m 48s | $1.19 | 0 | | verify | 7m 47s | – | 0 | | **Total** | **64m 31s** | **$14.97** | **0** | </details> <details> <summary>Ran <code>ImplementPlan.fabro</code> (11 nodes and 14 edges)</summary> ```dot digraph ImplementPlan { graph [ goal="Implement and simplify", model_stylesheet=" * { model: claude-opus-4-8; } " ] rankdir=LR start [shape=Mdiamond, label="Start"] exit [shape=Msquare, label="Exit"] toolchain [label="Toolchain", shape=parallelogram, script="command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1", max_retries=0] preflight_compile [label="Preflight Compile", shape=parallelogram, script="cargo check -q --workspace 2>&1", max_retries=0] preflight_lint [label="Preflight Lint", shape=parallelogram, script="cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1", max_retries=0] fix_lints [label="Fix Lints", prompt="The preflight lint step failed. Read the build output from context and fix all clippy lint warnings.", max_visits=3] implement [label="Implement", prompt="Read the plan file referenced in the goal and implement every step. Make all the code changes described in the plan. Use red/green TDD.", model="gpt-55", reasoning_effort="xhigh"] simplify_opus [label="Simplify (Opus)", prompt="@prompts/simplify.md"] simplify_gpt [label="Simplify (GPT-55)", prompt="@prompts/simplify.md", model="gpt-55"] verify [label="Verify", shape=parallelogram, timeout="1800s", script="git fetch origin main 2>&1 && git merge --no-edit --no-stat origin/main 2>&1 && cargo +nightly-2026-04-14 fmt --all 2>&1 && cargo dev docs refresh 2>&1 && cargo +nightly-2026-04-14 fmt --check --all 2>&1 && { command -v rg >/dev/null 2>&1 || { echo 'rg is required for verify'; exit 127; }; } && ! rg -n 'AuthMode::Disabled|RunAuthMethod|RunSubjectProvenance|\bActorRef\b|\bActorKind\b|AuthenticatedSubject|AuthenticatedService|AuthorizeRunScoped|AuthorizeRunBlob|AuthorizeStageArtifact|AuthorizeCommandLog|auth_method\s*==\s*\"disabled\"' lib/crates apps lib/packages docs/public/api-reference/fabro-api.yaml 2>&1 && cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings 2>&1 && cargo nextest run --workspace --status-level slow --profile ci 2>&1 && cargo dev docs check 2>&1 && bun install --frozen-lockfile 2>&1 && (cd apps/fabro-web && bun run typecheck) 2>&1 && (cd apps/fabro-web && bun run test) 2>&1 && (cd lib/packages/fabro-api-client && bun run typecheck) 2>&1 && cargo dev build -- -p fabro-cli --release 2>&1", goal_gate=true, retry_target="fixup"] fixup [label="Fixup", prompt="The verify step failed. Read the build output from context and fix all format, clippy, Rust test, docs, TypeScript typecheck/test, and build failures.", max_visits=3] start -> toolchain toolchain -> preflight_compile [condition="outcome=succeeded"] toolchain -> exit preflight_compile -> preflight_lint [condition="outcome=succeeded"] preflight_compile -> exit preflight_lint -> implement [condition="outcome=succeeded"] preflight_lint -> fix_lints fix_lints -> preflight_lint implement -> simplify_opus -> simplify_gpt -> verify verify -> exit [condition="outcome=succeeded"] verify -> fixup fixup -> verify } ``` </details> ⚒️ Generated with [Fabro](https://fabro.sh) --------- Co-authored-by: Fabro <noreply@fabro.sh> Co-authored-by: Scott Werner <stwerner@vt.edu> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…abro-sh#543) ## What Adds the `vulnerability_alerts: write` fine-grained permission to the GitHub App manifest used when Fabro auto-creates a GitHub App, in **both** install flows: - `lib/crates/fabro-server/src/install.rs` (web-UI install) - `lib/crates/fabro-cli/src/commands/install.rs` (CLI install) `write` on `vulnerability_alerts` grants both read and write of Dependabot alerts (write implies read for fine-grained permissions). The two manifest builders are byte-for-byte identical by design, so both are updated together. A test assertion in the CLI install tests guards the new permission. ## Why We need auto-created Fabro apps to be able to read and manage Dependabot alerts. ## Note on rollout Manifest `default_permissions` are applied at **app-creation time**, so this only affects **newly** auto-created apps. Any app already created won't pick this up automatically — the owner must add the permission in the app's settings, and each existing installation must approve the new permission request. ## Test - `cargo nextest run -p fabro-cli -- manifest_includes_callback_urls_and_setup_url` passes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ects V2 (fabro-sh#544) ## What Adds the `organization_projects: write` permission to the GitHub App manifest used when Fabro auto-creates a GitHub App, in **both** install flows: - `lib/crates/fabro-server/src/install.rs` (web-UI install) - `lib/crates/fabro-cli/src/commands/install.rs` (CLI install) A test assertion in the CLI install tests guards the new permission. ## Why The GitHub Projects V2 tracker mints a scoped installation token requesting `{ "issues": "write", "organization_projects": "write" }` (`create_installation_access_token_for_projects`, `fabro-github/src/lib.rs`). GitHub only lets an installation token request a **subset** of the permissions the app was granted at install time — and `organization_projects` was never in the manifest. So on any auto-created Fabro app, the token request comes back **422** and the tracker fails before it can make a single GraphQL call. `issues: write` (also requested by that helper) is already covered by the manifest; `organization_projects` was the missing piece. ## Note on rollout Manifest `default_permissions` are applied at **app-creation time**, so this only affects **newly** auto-created apps. Existing apps need the permission added manually in their settings, and each installation must approve it. ## Follow-up (not in this PR) The `422` branch in `mint_installation_token_with_jwt` reports "GitHub App does not have access to repository {repo}" — which misattributes a missing-permission failure to repository access. Worth softening the message to mention permissions too; left out here to keep this PR focused on the scope change. ## Test - `cargo nextest run -p fabro-cli -- manifest_includes_callback_urls_and_setup_url` passes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tages tabs (fabro-sh#541) This change enables scrolling the stages sidebar on the run's overview/stages page. Without it, for long runs with lots of stages, the entire page scrolls, hiding the graph while it's running. https://github.com/user-attachments/assets/c5405a5b-8480-46f8-8d7c-4cd4914f6228 --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
) ## Summary Fixes the graph that was always being rendered as `left-to-right` even when the workflow's `rankdir` is `top-to-bottom` ## Test plan - [x] `bun run typecheck` (fabro-web) - [x] `bun test` (fabro-web, full suite — 625 pass) - [x] Manually load a run whose workflow declares `rankdir TB` and confirm the graph renders top-to-bottom on first load, with the toolbar's LR/TB buttons still working as manual overrides 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…oading (fabro-sh#550) ## Problem Loading the web UI from a remote server took **~11 seconds to first render on every refresh**. A HAR capture against a remote deployment showed the page downloading **13.5 MB of JavaScript across 356 files, uncompressed, on every single page load** — even though the assets are content-hashed and served with `Cache-Control: immutable`. Four compounding causes: 1. **`Pragma: no-cache` defeated the browser cache.** The security-headers middleware stamped `Pragma: no-cache` onto every response, including hashed assets that set a year-long immutable `Cache-Control`. Browsers treat a response `Pragma: no-cache` as `Cache-Control: no-cache` and check it *before* `max-age` (Chromium zeroes freshness on it), and since assets carried no validators, "revalidate" degraded into a full re-download. Empirically visible in the HAR: Google-Fonts woff2s served from cache (`transfer = 0`) during the same page load where all 356 of our assets re-downloaded in full. 2. **No response compression.** The server had no compression layer; 13.5 MB of JS compresses to ~2.5 MB with brotli. 3. **The HTML force-loaded every chunk.** `writeIndexHtml` emitted a `<script type="module">` tag for all 356 outputs. Only 2.9 MB is statically reachable from the entry; the other ~10.7 MB is dynamic-import-only code (syntax grammars, Graphviz WASM, xterm, diff file tree) that was being downloaded eagerly at high priority. 4. **The immutable heuristic over-matched.** Any dash in a filename counted as a content hash, so stable-named files (`pierre-diffs-worker/worker-portable.js`, `apple-touch-icon.png`) would be pinned in browser caches for a year across deploys once fix 1 made immutable caching effective. ## Changes - **`security_headers`**: apply the `no-store`/`Pragma: no-cache` defaults only when the handler didn't set its own `Cache-Control`. API responses keep the conservative defaults. - **Compression**: `tower-http` `CompressionLayer` (brotli + gzip) on both the main router and the install-mode router (install mode serves the same SPA bundle through a separate router). Default predicate keeps SSE (`text/event-stream`), gRPC, images, and tiny bodies identity-encoded. Quality pinned to `Precise(4)` — tower-http's default defers to the codec default, and brotli's default is quality 11 (seconds of CPU per multi-megabyte asset). - **Entry-only HTML**: `writeIndexHtml` emits script tags only for `kind === "entry-point"` outputs. The module graph pulls static imports (depth 1, so no waterfall); dynamic `import()` chunks load on demand. - **Cache-control classifier + validators**: only files matching the bundler's actual output shape (`assets/<stem>-<hash8>.js|css`, lowercase base-36) get `immutable`. Everything else is `no-cache` **with a strong ETag** and `If-None-Match` → `304` support, so index.html / app.css / the pierre worker revalidate in one cheap conditional request instead of a full re-download. ## Impact (measured on the built bundle) | | Before | After | |---|---|---| | Cold load, ~1 MB/s link | 13.5 MB raw ≈ **11–14 s** | ~0.8 MB compressed eager payload ≈ **~1 s** | | Refresh | full re-download, same 11–14 s | served from cache + one 304 ≈ **instant** | | Eager JS on first render | 13.56 MB / 356 files | 2.88 MB raw (0.79 MB gzip) / 6 files | ## Verification - 959 fabro-server tests pass (incl. new coverage); fmt + clippy clean; `bun run typecheck` passes (the 5 pre-existing bun test failures reproduce identically on `main` — missing `@pierre/diffs/dist/worker` fixture + flaky InstallApp timing tests). - New integration tests pin compression through **both** serving shapes that matter: regular routes and the SPA fallback service, each via tower `oneshot` **and** over a real TCP connection through hyper (raw-socket assertions, so no client auto-decompression can mask a regression). - Live-verified against a debug server: hashed assets get `immutable` + brotli and no `Pragma`; mutable assets get `no-cache` + ETag and answer conditionals with `304`; API responses keep `no-store`. - Headless Chrome boots the rebuilt SPA from the entry-only HTML and fully renders the UI. ## Notes for reviewers - The ETag is skipped for immutable assets deliberately — they never revalidate, so hashing multi-MB bodies per request would be pure overhead. - Install mode previously had **no** compression and shares the same bundle; it gets the same layer via a shared `compression_layer()` helper. - `bun test` has a pre-existing suite (`production build copies Pierre worker assets`) that fails without `@pierre/diffs/dist/worker` present locally; unrelated to this change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…abro-sh#542) Adds a `SecretRedactor` primitive to `fabro-redact` so that low-entropy secret values (e.g. environment names, short tokens) are redacted even when the existing content-based heuristics (`redact_string`, `redact_json_value`) would leave them alone. The type is a cheap, `Clone`-able handle backed by `Arc<RwLock<Vec<String>>>`, so a clone handed to another subsystem shares the same registry. `register` ignores empty/whitespace-only values to prevent a footgun that would blank all output. `redact_into` sorts and merges match regions before substituting, so a secret that is a prefix of another longer secret is handled correctly (longest wins via union). `redact_json` walks string leaves in objects and arrays; object keys are left intact. This is an inert library primitive — it changes no existing behavior and is wired up by Plan C. The existing `"REDACTED"` literal is extracted to a `pub(crate) REDACTION_MARKER` constant so both the old path and the new one stay in sync. ### Fabro Details <details> <summary>Ran 8 stages in 43m 24s for $5.69</summary> | Stage | Duration | Cost | Retries | |---|---|---|---| | start | 0s | – | 0 | | toolchain | 1s | – | 0 | | preflight_compile | 2m 23s | – | 0 | | preflight_lint | 2m 33s | – | 0 | | implement | 20m 1s | $3.09 | 0 | | simplify_opus | 4m 13s | $1.27 | 0 | | simplify_gpt | 7m 29s | $1.33 | 0 | | verify | 6m 16s | – | 0 | | **Total** | **43m 24s** | **$5.69** | **0** | </details> <details> <summary>Ran <code>ImplementPlan.fabro</code> (11 nodes and 14 edges)</summary> ```dot digraph ImplementPlan { graph [ goal="Implement and simplify", model_stylesheet=" * { model: claude-opus-4-8; } " ] rankdir=LR start [shape=Mdiamond, label="Start"] exit [shape=Msquare, label="Exit"] toolchain [label="Toolchain", shape=parallelogram, script="command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1", max_retries=0] preflight_compile [label="Preflight Compile", shape=parallelogram, script="cargo check -q --workspace 2>&1", max_retries=0] preflight_lint [label="Preflight Lint", shape=parallelogram, script="cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1", max_retries=0] fix_lints [label="Fix Lints", prompt="The preflight lint step failed. Read the build output from context and fix all clippy lint warnings.", max_visits=3] implement [label="Implement", prompt="Read the plan file referenced in the goal and implement every step. Make all the code changes described in the plan. Use red/green TDD.", model="gpt-55", reasoning_effort="xhigh"] simplify_opus [label="Simplify (Opus)", prompt="@prompts/simplify.md"] simplify_gpt [label="Simplify (GPT-55)", prompt="@prompts/simplify.md", model="gpt-55"] verify [label="Verify", shape=parallelogram, timeout="1800s", script="git fetch origin main 2>&1 && git merge --no-edit --no-stat origin/main 2>&1 && cargo +nightly-2026-04-14 fmt --all 2>&1 && cargo dev docs refresh 2>&1 && cargo +nightly-2026-04-14 fmt --check --all 2>&1 && { command -v rg >/dev/null 2>&1 || { echo 'rg is required for verify'; exit 127; }; } && ! rg -n 'AuthMode::Disabled|RunAuthMethod|RunSubjectProvenance|\bActorRef\b|\bActorKind\b|AuthenticatedSubject|AuthenticatedService|AuthorizeRunScoped|AuthorizeRunBlob|AuthorizeStageArtifact|AuthorizeCommandLog|auth_method\s*==\s*\"disabled\"' lib/crates apps lib/packages docs/public/api-reference/fabro-api.yaml 2>&1 && cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings 2>&1 && cargo nextest run --workspace --status-level slow --profile ci 2>&1 && cargo dev docs check 2>&1 && bun install --frozen-lockfile 2>&1 && (cd apps/fabro-web && bun run typecheck) 2>&1 && (cd apps/fabro-web && bun run test) 2>&1 && (cd lib/packages/fabro-api-client && bun run typecheck) 2>&1 && cargo dev build -- -p fabro-cli --release 2>&1", goal_gate=true, retry_target="fixup"] fixup [label="Fixup", prompt="The verify step failed. Read the build output from context and fix all format, clippy, Rust test, docs, TypeScript typecheck/test, and build failures.", max_visits=3] start -> toolchain toolchain -> preflight_compile [condition="outcome=succeeded"] toolchain -> exit preflight_compile -> preflight_lint [condition="outcome=succeeded"] preflight_compile -> exit preflight_lint -> implement [condition="outcome=succeeded"] preflight_lint -> fix_lints fix_lints -> preflight_lint implement -> simplify_opus -> simplify_gpt -> verify verify -> exit [condition="outcome=succeeded"] verify -> fixup fixup -> verify } ``` </details> ⚒️ Generated with [Fabro](https://fabro.sh) --------- Co-authored-by: Fabro <noreply@fabro.sh>
…abro-sh#545) ## Summary `{{ secrets.NAME }}` tokens in workflow config (MCP transport, prepare steps, run environment) now resolve from the server vault at the run boundary — the same late-binding point where `{{ env.* }}` tokens resolve. Secret values are never persisted and never left literal in resolved commands or env; a missing or non-Token secret aborts startup with a clear error. ## What changed **`fabro-types` — `run.rs`** - `resolve_env_string` (shared choke-point for MCP and prepare) gains a `secrets_lookup` parameter and routes through `ResolveCtx::new().with_env(...).with_secrets(...)` / `resolve_with`. - `McpServerSettings::resolve_transport_env` and `RunPrepareSettings::resolve_step_env` thread the new parameter through. - `RunEnvironmentSettings::resolve_env` becomes fallible (`Result<HashMap<_,_>, ResolveError>`). Per-value error handling preserves the historical env fallback for `Namespace::Env`-only errors while failing closed for `Namespace::Secrets` errors. The intentional `as_source()` fallback is gated behind its `#[expect(clippy::disallowed_methods)]` with an explicit reason. **`fabro-workflow` — `start.rs`** - A single vault read guard is acquired once at the top of `RunSession::new`, replacing the previous per-site reads (Daytona key, etc.). - `vault_token_lookup` wraps `fabro_auth::vault_get_token` — returning `Some(value)` only for `Token`-type secrets; `Oauth` and `File` secrets become `None` (fail-closed). - The shared `secret_lookup` closure is threaded into `runtime_mcp_server`, `runtime_setup_commands`, and `resolve_env`. `resolve_docker_config` gains the same parameter and now returns `Result`. **`fabro-sandbox` — `from_environment.rs`** - `docker_config_from_environment` (server-preflight path, no vault available) retains `resolve_or_source` behavior unchanged. - New `docker_config_from_environment_with_secrets` is the vault-backed variant used by `start.rs`. **`fabro-cli` — `exec.rs`** - `fabro exec` has no vault; passes `|_| None` for secrets, preserving existing behavior with updated call signature. ### Plan summary - **B.1** — Secret lookup threaded through `resolve_env_string` / `resolve_transport_env` / `resolve_step_env` / `resolve_env` in `fabro-types`. - **B.2** — Vault-backed `secret_lookup` closure built once in `RunSession::new` and passed to all boundary resolvers in `start.rs`. - **B.3** — Persistence invariant test: a created run's persisted `RunCreated` event still carries `{{ secrets.DEPLOY_TOKEN }}` in source form, not the resolved value. - **B.4** — Verification (fmt, clippy, nextest, release build) with hermetic temp-vault tests. ### Key design decisions - **Fail closed everywhere secrets are referenced** — no source fallback for secret tokens, even in `resolve_env` which otherwise keeps the env fallback. This is enforced by checking `value.references(Namespace::Secrets)` before the fallback branch. - **Token-only** — `vault_get_token` enforces this; `Oauth` and `File` secrets silently become `None` and then hard-error via the resolver, not a panic. - **Single vault read guard per `RunSession::new`** — acquired once, shared across MCP / prepare / env resolvers, then dropped before the struct is returned. Mirrors how the Daytona key was already read. - **`fabro exec` stays unchanged behaviorally** — the added `|_| None` secrets argument makes the new signature explicit about having no vault. ### Fabro Details <details> <summary>Ran 9 stages in 82m 55s for $24.43</summary> | Stage | Duration | Cost | Retries | |---|---|---|---| | start | 0s | – | 0 | | toolchain | 1s | – | 0 | | preflight_compile | 2m 21s | – | 0 | | preflight_lint | 2m 34s | – | 0 | | implement | 46m 58s | $15.83 | 0 | | simplify_opus | 12m 43s | $4.79 | 0 | | simplify_gpt | 6m 43s | $3.18 | 0 | | verify | 6m 41s | – | 0 | | fixup | 4m 33s | $0.63 | 0 | | **Total** | **82m 55s** | **$24.43** | **0** | </details> <details> <summary>Ran <code>ImplementPlan.fabro</code> (11 nodes and 14 edges)</summary> ```dot digraph ImplementPlan { graph [ goal="Implement and simplify", model_stylesheet=" * { model: claude-opus-4-8; } " ] rankdir=LR start [shape=Mdiamond, label="Start"] exit [shape=Msquare, label="Exit"] toolchain [label="Toolchain", shape=parallelogram, script="command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1", max_retries=0] preflight_compile [label="Preflight Compile", shape=parallelogram, script="cargo check -q --workspace 2>&1", max_retries=0] preflight_lint [label="Preflight Lint", shape=parallelogram, script="cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1", max_retries=0] fix_lints [label="Fix Lints", prompt="The preflight lint step failed. Read the build output from context and fix all clippy lint warnings.", max_visits=3] implement [label="Implement", prompt="Read the plan file referenced in the goal and implement every step. Make all the code changes described in the plan. Use red/green TDD.", model="gpt-55", reasoning_effort="xhigh"] simplify_opus [label="Simplify (Opus)", prompt="@prompts/simplify.md"] simplify_gpt [label="Simplify (GPT-55)", prompt="@prompts/simplify.md", model="gpt-55"] verify [label="Verify", shape=parallelogram, timeout="1800s", script="git fetch origin main 2>&1 && git merge --no-edit --no-stat origin/main 2>&1 && cargo +nightly-2026-04-14 fmt --all 2>&1 && cargo dev docs refresh 2>&1 && cargo +nightly-2026-04-14 fmt --check --all 2>&1 && { command -v rg >/dev/null 2>&1 || { echo 'rg is required for verify'; exit 127; }; } && ! rg -n 'AuthMode::Disabled|RunAuthMethod|RunSubjectProvenance|\bActorRef\b|\bActorKind\b|AuthenticatedSubject|AuthenticatedService|AuthorizeRunScoped|AuthorizeRunBlob|AuthorizeStageArtifact|AuthorizeCommandLog|auth_method\s*==\s*\"disabled\"' lib/crates apps lib/packages docs/public/api-reference/fabro-api.yaml 2>&1 && cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings 2>&1 && cargo nextest run --workspace --status-level slow --profile ci 2>&1 && cargo dev docs check 2>&1 && bun install --frozen-lockfile 2>&1 && (cd apps/fabro-web && bun run typecheck) 2>&1 && (cd apps/fabro-web && bun run test) 2>&1 && (cd lib/packages/fabro-api-client && bun run typecheck) 2>&1 && cargo dev build -- -p fabro-cli --release 2>&1", goal_gate=true, retry_target="fixup"] fixup [label="Fixup", prompt="The verify step failed. Read the build output from context and fix all format, clippy, Rust test, docs, TypeScript typecheck/test, and build failures.", max_visits=3] start -> toolchain toolchain -> preflight_compile [condition="outcome=succeeded"] toolchain -> exit preflight_compile -> preflight_lint [condition="outcome=succeeded"] preflight_compile -> exit preflight_lint -> implement [condition="outcome=succeeded"] preflight_lint -> fix_lints fix_lints -> preflight_lint implement -> simplify_opus -> simplify_gpt -> verify verify -> exit [condition="outcome=succeeded"] verify -> fixup fixup -> verify } ``` </details> ⚒️ Generated with [Fabro](https://fabro.sh) --------- Co-authored-by: Fabro <noreply@fabro.sh>
…o-sh#556) Multi-word workflow slugs typed into the New/Edit Automation form were being silently converted to snake_case (e.g. `patch-cves` → `patch_cves`), causing scheduled automations to resolve against a non-existent directory and **silently never fire**. ## What changed - `automation-form.tsx`: `onChange` for the Workflow slug field now calls `kebabify()` instead of the removed `snakeify()`. The "create from run" fallback prefill is updated the same way. Help text and placeholder are updated to reflect dash-separated slugs. - `snakeify()` is removed entirely (was only used in these two spots). - `kebabify()` is unexported (it was `export function`; it's now only used within the same file). - `automations-new.test.tsx`: updates the pre-populate assertion from `"fix_ci"` → `"fix-ci"`, adds a regression test that dashes are preserved and `"Patch CVEs"` → `"patch-cves"`, and adds a unit test for the `automationFormValuesFromRun` kebab fallback. ## Why kebab-case is correct Workflow slugs are derived from on-disk directory names (`.fabro/workflows/patch-cves/`), which are dash-separated by convention. The backend validator already accepts dashes; `AutomationId` actually forbids underscores. The snake_case behavior was a UI-only outlier present since the form's first draft with no documented rationale. No backend changes are needed. Existing automations with a stored snake_cased `workflow` selector will need a manual `PUT` to correct the value — that is an operational fix, out of scope here. ### Fabro Details <details> <summary>Ran 8 stages in 31m 57s for $6.85</summary> | Stage | Duration | Cost | Retries | |---|---|---|---| | start | 0s | – | 0 | | toolchain | 1s | – | 0 | | preflight_compile | 2m 20s | – | 0 | | preflight_lint | 2m 33s | – | 0 | | implement | 4m 28s | $2.46 | 0 | | simplify_fable | 8m 39s | $3.31 | 0 | | simplify_gpt | 2m 25s | $1.07 | 0 | | verify | 11m 2s | – | 0 | | **Total** | **31m 57s** | **$6.85** | **0** | </details> <details> <summary>Ran <code>ImplementPlan.fabro</code> (11 nodes and 14 edges)</summary> ```dot digraph ImplementPlan { graph [ goal="Implement and simplify", model_stylesheet=" * { model: claude-opus-4-8; } " ] rankdir=LR start [shape=Mdiamond, label="Start"] exit [shape=Msquare, label="Exit"] toolchain [label="Toolchain", shape=parallelogram, script="command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1", max_retries=0] preflight_compile [label="Preflight Compile", shape=parallelogram, script="cargo check -q --workspace 2>&1", max_retries=0] preflight_lint [label="Preflight Lint", shape=parallelogram, script="cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1", max_retries=0] fix_lints [label="Fix Lints", prompt="The preflight lint step failed. Read the build output from context and fix all clippy lint warnings.", max_visits=3] implement [label="Implement", prompt="Read the plan file referenced in the goal and implement every step. Make all the code changes described in the plan. Use red/green TDD.", model="gpt-55", reasoning_effort="xhigh"] simplify_fable [label="Simplify (Fable)", prompt="@prompts/simplify.md", model="claude-fable-5", reasoning_effort="xhigh"] simplify_gpt [label="Simplify (GPT-55)", prompt="@prompts/simplify.md", model="gpt-55"] verify [label="Verify", shape=parallelogram, timeout="1800s", script="git fetch origin main 2>&1 && git merge --no-edit --no-stat origin/main 2>&1 && cargo +nightly-2026-04-14 fmt --all 2>&1 && cargo dev docs refresh 2>&1 && cargo +nightly-2026-04-14 fmt --check --all 2>&1 && { command -v rg >/dev/null 2>&1 || { echo 'rg is required for verify'; exit 127; }; } && ! rg -n 'AuthMode::Disabled|RunAuthMethod|RunSubjectProvenance|\bActorRef\b|\bActorKind\b|AuthenticatedSubject|AuthenticatedService|AuthorizeRunScoped|AuthorizeRunBlob|AuthorizeStageArtifact|AuthorizeCommandLog|auth_method\s*==\s*\"disabled\"' lib/crates apps lib/packages docs/public/api-reference/fabro-api.yaml 2>&1 && cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings 2>&1 && cargo nextest run --workspace --status-level slow --profile ci 2>&1 && cargo dev docs check 2>&1 && bun install --frozen-lockfile 2>&1 && (cd apps/fabro-web && bun run typecheck) 2>&1 && (cd apps/fabro-web && bun run test) 2>&1 && (cd lib/packages/fabro-api-client && bun run typecheck) 2>&1 && cargo dev build -- -p fabro-cli --release 2>&1", goal_gate=true, retry_target="fixup"] fixup [label="Fixup", prompt="The verify step failed. Read the build output from context and fix all format, clippy, Rust test, docs, TypeScript typecheck/test, and build failures.", max_visits=3] start -> toolchain toolchain -> preflight_compile [condition="outcome=succeeded"] toolchain -> exit preflight_compile -> preflight_lint [condition="outcome=succeeded"] preflight_compile -> exit preflight_lint -> implement [condition="outcome=succeeded"] preflight_lint -> fix_lints fix_lints -> preflight_lint implement -> simplify_fable -> simplify_gpt -> verify verify -> exit [condition="outcome=succeeded"] verify -> fixup fixup -> verify } ``` </details> ⚒️ Generated with [Fabro](https://fabro.sh) --------- Co-authored-by: Fabro <noreply@fabro.sh>
## Summary
Adds the `rust-style-guide` Agent Skill under
`.fabro/skills/rust-style-guide/` so Fabro workflow agent stages can
apply the project's Rust conventions when writing or reviewing Rust
code.
Skills are discovered by convention from
`{git_root}/.fabro/skills/*/SKILL.md` at agent-session startup — there's
no manifest wiring or per-workflow declaration. Once present, every
agent stage lists the skill in its system prompt and registers the
`use_skill` tool, so an agent can load it (or a node prompt can
reference `/rust-style-guide`). Committing it here (rather than relying
on a local `~/.fabro/skills` copy) is what makes it available to
**remote, clone-based runs** (Docker/Daytona), which only see committed
+ pushed files.
## Contents (44 files)
- `SKILL.md` — entry point (with a short note pointing the agent at the
in-repo location of the supporting files, since Fabro hands the agent
the `SKILL.md` body and it reads the rest itself)
- `guidelines.md` + `guidelines/` — 38 Rust style policy pages
- `workflows/` — 4 procedure pages (new project, library release,
performance investigation, code review/refactor)
## Source / attribution
Vendored from https://github.com/brynary/rust-style-guide (commit
`8fd2a4f`), trimmed to the runtime skill payload; the upstream repo's
mdBook site and authoring scaffolding are omitted. Note: the upstream
repo has **no LICENSE file** — flagging for a call on
attribution/licensing before merge.
## Notes
- No behavior/code change — this is skill content only; nothing is
compiled or bundled.
- No `{{user_input}}` placeholder was added; agents reference the skill
in prose or via `use_skill`. (If we later want deterministic
`/rust-style-guide <task>` slash expansion in node prompts, add the
placeholder to `SKILL.md` then.)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds a `patch-cves` workflow that triages GitHub Dependabot alerts and opens verified dependency-patch PRs, one per alert group. Intended to be driven by a scheduled automation targeting this repo. ## What's included - **`.fabro/workflows/patch-cves/workflow.fabro`** — single agent stage pinned to `claude-opus-4-8`. - **`.fabro/workflows/patch-cves/prompts/patch-cves.md`** — the bundled prompt with the full CVE-patching procedure: query Dependabot alerts, rank and group them, choose the smallest safe fix, patch + regenerate lockfiles, verify (local gates + GitHub checks), and re-query alerts. Ecosystem rules cover Rust/Cargo and TypeScript/Bun (Bun only — never npm/npx/yarn/pnpm). Treats all advisory/package/log text as untrusted data. - **`.fabro/workflows/patch-cves/workflow.toml`** — requests the GitHub App installation-token permissions the run needs: `vulnerability_alerts=read`, `contents=write`, `pull_requests=write`, `checks=read`. Sets `run.pull_request.enabled = false` so fabro's run-branch finalization PR doesn't race the per-group PRs the agent opens directly via `gh`. ## Design The instructions ship as a bundled prompt file (`@prompts/patch-cves.md`) that travels in the run manifest, so the workflow is fully self-contained — no external skill or runtime discovery involved. Validated with `fabro validate patch-cves` (OK). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## What On **Runs → Overview**, the workflow graph now supports the standard Figma/Excalidraw canvas interactions: - **Two-finger scroll → pan** - **⌘/Ctrl + scroll → zoom**, anchored under the cursor (mac trackpad pinch works too — the browser delivers it as `ctrl+wheel`) The graph already had drag-to-pan, stepped zoom (toolbar +/−), and fit-to-window. This adds the missing wheel/trackpad input on top of that existing transform state. https://github.com/user-attachments/assets/15eac98b-2603-44c9-b438-7ee27034ccd7 ## How - **`app/lib/graph-viewport.ts`** (new) — pure, framework-free zoom math: `zoomAtPoint` keeps the point under the cursor fixed while scaling; `clampZoom` + zoom constants. Zoom becomes a continuous float (was a discrete step index) so ⌘-scroll is smooth instead of jumping between steps. Unit-tested (`graph-viewport.test.ts`), including the cursor-anchor invariant. - **`useElementEvent` in `hooks/effects.ts`** (new) — element-scoped, non-passive listener, a sibling to the existing `useWindowEvent`/`useDocumentEvent`. Non-passive is required so the handler can `preventDefault()` the browser's own ⌘-zoom; a JSX `onWheel` can't. - **`routes/run-overview.tsx`** — coalesces zoom+pan into one `view` state (atomic cursor-anchored updates), adds the wheel handler (plain scroll → pan, ⌘/Ctrl → zoom), and `touch-none overscroll-contain` so a horizontal swipe can't trigger browser back-nav. - **`components/graph-toolbar.tsx`** — presentational continuous interface; +/− buttons reuse `zoomAtPoint` (center-anchored). Deletes the now-dead `graph-toolbar-constants.ts`. ## Testing - `bun run typecheck` clean; `bun test` green (incl. 4 new viewport tests). - Verified live against a real 10-node run graph via Chrome DevTools: two-finger pan tracks the scroll delta; ⌘+wheel zoom is cursor-anchored (confirmed even with the cursor over a node); toolbar +/− step ×1.25 and clamp/disable at 200%; fit-to-window sets a continuous scale; node click/hover unaffected. ## Non-goals - **Playground canvas** (`components/playground/canvas`) shares the same hand-rolled pan/zoom pattern and also lacks wheel support — deliberately out of scope; `graph-viewport.ts` is the seam to adopt it later. - **No persistence** — zoom/pan stays ephemeral per visit, as it was before. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
….prepare) (fabro-sh#554) ## Summary Runs created before fabro-sh#530 disappear from the run list after upgrading, because their persisted `run.created` event can no longer be deserialized. fabro-sh#530 renamed `RunPrepareSettings`'s field from `commands: Vec<String>` to `steps: Vec<PreparedStep>`. That struct is persisted inside the `run.created` event (`WorkflowSettings.run.prepare`). Events written by older versions carry a `prepare` object with a `commands` key and **no** `steps` key. Because `steps` had no serde default, deserializing such an event fails with: ``` Serialization error: missing field `steps` ``` `warm_projection_cache` catches that error per-run and **skips** the run (`fabro_store::slate: Skipping run during projection cache warmup`), so every pre-fabro-sh#530 run silently vanishes from the run list. The event data is intact on disk — it just can't be read back. This is an event-schema back-compat break: any type persisted in an event must stay readable across the field renames/additions that happen after it was written. ## Fix Add `#[serde(default)]` at the container level on `RunPrepareSettings`, so a `prepare` object missing `steps` (and/or `timeout_ms`) falls back to the existing `Default` impl (empty steps, product-default timeout) instead of failing the whole run. The unknown legacy `commands` key is ignored (the struct has no `deny_unknown_fields`). - New runs always serialize explicit `steps`, so nothing changes for them — the fabro-sh#530 feature is unaffected. - Pre-fabro-sh#530 runs load again with an empty prepare phase, which is faithful: those runs already executed; this only rebuilds a read model for display. `#[serde(default)]` is already the evolution idiom in this same struct tree (e.g. `RunModelSettings.controls`). ## Test plan - [x] `cargo test -p fabro-types` — added two regression tests that deserialize the exact pre-fabro-sh#530 event shape (`{ commands, timeout_ms }`, no `steps`) and an empty object, asserting both load instead of erroring. - [x] Built the patched server and pointed it at a real `~/.fabro/storage` that had 119 pre-fabro-sh#530 runs being skipped. After the fix, 0 runs are skipped and all 119 appear in `GET /api/v1/runs`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Bryan Helmkamp <bryan@brynary.com>
Created from HEAD commit 6746bff.
Switching from a run's Overview tab to another tab and back reset the graph zoom and position to the default. Now it holds. ## Why The viewport (pan and zoom) lived in `RunOverview` component state. Overview and Stages are sibling routes under `runs/:id`, so switching tabs unmounts Overview and drops that state. ## Fix `apps/fabro-web/app/routes/run-overview.tsx`: cache the viewport per run outside the component so it survives the remount, and reset it when the run id changes, since the route instance is reused when only the id changes. Added two tests: viewport restores on remount for the same run, and does not carry across runs. Does not persist across a full page reload (in-memory only). --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Bryan Helmkamp <bryan@brynary.com>
## What Adds `pr-simplify`, a Fabro workflow that runs a "simplify" code-review pass over an existing PR and updates that same PR in place. ## How it works - **One agent, three parallel reviews.** A single agent node runs the pass and uses `spawn_agent` to fan out three reviewers — code reuse, code quality, and efficiency — concurrently, then aggregates their findings. Sub-agent results return directly to the orchestrator, which is the clean way to aggregate multiple perspectives. (A fork + `tripleoctagon` fan-in was the wrong primitive here: fan-in selects a single "best" branch and merges only its worktree, so it would silently drop two of the three reviews.) - **Updates the existing PR — no new PR.** The agent runs `gh pr checkout` on the PR's branch, applies the fixes, commits, and pushes — landing one fixup commit on the existing PR, plus a summary comment and a `simplify:<model>` label. `[run.pull_request] enabled = false` keeps Fabro from opening a second PR from its run branch. - **Fable by default, overridable.** The graph sets `default_model=claude-fable-5`, which floors the orchestrator and all three reviewers to Fable. `--model <id>` wins over it per run (`configured model → graph default_model → catalog default`), and the label reflects whatever actually ran. ## Usage ```bash fabro run pr-simplify -I pr=<number> # Fable (default) fabro run pr-simplify -I pr=<number> --model gpt-55 # override the model ``` Requires GitHub token permissions `contents` / `pull_requests` / `issues` = write (declared in the workflow) so it can push the commit, comment, and label. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Bryan Helmkamp <19+brynary@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Bryan Helmkamp <19+brynary@users.noreply.github.com>
fabro-sh#552) ## Problem The post-node run-branch checkpoint commit runs repository commit hooks unless `skip_git_hooks` is enabled, but its sandbox command timeout was hardcoded to 30 seconds. Consumers whose hooks run a multi-minute gate cannot complete a checkpoint. ## Change Adds `commit_timeout_ms` to the existing `[run.checkpoint]` table. - Defaults to `30000`, preserving existing behavior. - Threads the value through config raw layer -> merge -> resolve -> resolved settings -> `RunOptions` -> `GitState` -> both checkpoint call sites. - Applies the configured timeout to checkpoint `git add -A` and `git commit`. - Keeps old serialized run manifests compatible via serde default. ## Testing - `cargo +nightly-2026-04-14 fmt --all` - `cargo +nightly-2026-04-14 clippy --locked --workspace --all-targets -- -D warnings` - `cargo nextest run --locked -p fabro-config -p fabro-types -p fabro-workflow` - 1795 passed, 31 skipped - `cargo nextest run --locked -p fabro-cli attach_json_errors_without_prompting_for_human_input` - `cargo nextest run --locked --workspace --status-level slow --profile ci --no-fail-fast` - 6951 passed, 3 timed out, 187 skipped - The 3 timeouts are preexisting on clean `upstream/main`: verified by running `CARGO_TARGET_DIR=/data/projects/fabro/target cargo nextest run --locked -p fabro-cli --profile ci --no-fail-fast workflow::acp::acp` from a detached worktree at `upstream/main` (`8c7d5dc7d`), which timed out the same three tests: - `workflow::acp::acp_artifacts_are_listed_when_touched_file_mtime_precedes_attempt_start` - `workflow::acp::acp_backend_does_not_inject_registered_provider_credentials` - `workflow::acp::acp_backend_workflow` ## Compatibility No behavior change without explicit opt-in. Omitted config resolves to the existing 30 second timeout, and old serialized run manifests deserialize unchanged. --------- Co-authored-by: thewoolleyman <chad@thewoolleyman.com> Co-authored-by: Bryan Helmkamp <bryan@brynary.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…#562) The `Resolved` / `Provenance` types in `fabro-types` interp tracked which env vars and secrets contributed to a resolved value, but no production code ever read `.provenance` — every caller immediately discarded it with `.map(|r| r.value)`. The redaction design this metadata anticipated was superseded by per-run exact-value registration (`fabro_redact::SecretRedactor`); origin-tagging on resolved strings can't reach the surfaces where secrets actually leak (subprocess output, diffs, tool output), so it added no coverage. This PR removes the dead scaffolding with zero behavior change: - `resolve` / `resolve_with` now return `Result<String, ResolveError>` directly; `Resolved` and `Provenance` are deleted along with the name-accumulation logic inside `resolve_with`. - All call sites drop the now-unnecessary `.map(|r| r.value)` unwrap. - Provenance assertions in tests are removed; all value/error assertions are preserved. - The module doc is updated to describe the actual model: secret values are intended to be registered into a per-run exact-value redactor at resolution time; sensitivity is not tracked on resolved strings. - A comment on `ResolvedRunGoal` / `ResolvedGoalSource` (an unrelated run-metadata concept sharing the word "provenance") is rephrased to avoid confusion with the deleted type. `Provenance` no longer appears in `fabro-types/src/settings/mod.rs` exports. The unrelated `RunClientProvenance` / `RunServerProvenance` run-spec types are untouched. ### Fabro Details <details> <summary>Ran 8 stages in 39m 23s for $7.58</summary> | Stage | Duration | Cost | Retries | |---|---|---|---| | start | 0s | – | 0 | | toolchain | 1s | – | 0 | | preflight_compile | 2m 17s | – | 0 | | preflight_lint | 2m 33s | – | 0 | | implement | 11m 6s | $4.23 | 0 | | simplify_fable | 8m 27s | $1.60 | 0 | | simplify_gpt | 5m 58s | $1.75 | 0 | | verify | 8m 36s | – | 0 | | **Total** | **39m 23s** | **$7.58** | **0** | </details> <details> <summary>Ran <code>ImplementPlan.fabro</code> (11 nodes and 14 edges)</summary> ```dot digraph ImplementPlan { graph [ goal="Implement and simplify", model_stylesheet=" * { model: claude-opus-4-8; } " ] rankdir=LR start [shape=Mdiamond, label="Start"] exit [shape=Msquare, label="Exit"] toolchain [label="Toolchain", shape=parallelogram, script="command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1", max_retries=0] preflight_compile [label="Preflight Compile", shape=parallelogram, script="cargo check -q --workspace 2>&1", max_retries=0] preflight_lint [label="Preflight Lint", shape=parallelogram, script="cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1", max_retries=0] fix_lints [label="Fix Lints", prompt="The preflight lint step failed. Read the build output from context and fix all clippy lint warnings.", max_visits=3] implement [label="Implement", prompt="Read the plan file referenced in the goal and implement every step. Make all the code changes described in the plan. Use red/green TDD. Be sure to use the rust-style-guide skill to help you follow this repo's Rust style conventions.", model="gpt-55", reasoning_effort="xhigh"] simplify_fable [label="Simplify (Fable)", prompt="@prompts/simplify.md", model="claude-fable-5", reasoning_effort="xhigh"] simplify_gpt [label="Simplify (GPT-55)", prompt="@prompts/simplify.md", model="gpt-55"] verify [label="Verify", shape=parallelogram, timeout="1800s", script="git fetch origin main 2>&1 && git merge --no-edit --no-stat origin/main 2>&1 && cargo +nightly-2026-04-14 fmt --all 2>&1 && cargo dev docs refresh 2>&1 && cargo +nightly-2026-04-14 fmt --check --all 2>&1 && { command -v rg >/dev/null 2>&1 || { echo 'rg is required for verify'; exit 127; }; } && ! rg -n 'AuthMode::Disabled|RunAuthMethod|RunSubjectProvenance|\bActorRef\b|\bActorKind\b|AuthenticatedSubject|AuthenticatedService|AuthorizeRunScoped|AuthorizeRunBlob|AuthorizeStageArtifact|AuthorizeCommandLog|auth_method\s*==\s*\"disabled\"' lib/crates apps lib/packages docs/public/api-reference/fabro-api.yaml 2>&1 && cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings 2>&1 && cargo nextest run --workspace --status-level slow --profile ci 2>&1 && cargo dev docs check 2>&1 && bun install --frozen-lockfile 2>&1 && (cd apps/fabro-web && bun run typecheck) 2>&1 && (cd apps/fabro-web && bun run test) 2>&1 && (cd lib/packages/fabro-api-client && bun run typecheck) 2>&1 && cargo dev build -- -p fabro-cli --release 2>&1", goal_gate=true, retry_target="fixup"] fixup [label="Fixup", prompt="The verify step failed. Read the build output from context and fix all format, clippy, Rust test, docs, TypeScript typecheck/test, and build failures.", max_visits=3] start -> toolchain toolchain -> preflight_compile [condition="outcome=succeeded"] toolchain -> exit preflight_compile -> preflight_lint [condition="outcome=succeeded"] preflight_compile -> exit preflight_lint -> implement [condition="outcome=succeeded"] preflight_lint -> fix_lints fix_lints -> preflight_lint implement -> simplify_fable -> simplify_gpt -> verify verify -> exit [condition="outcome=succeeded"] verify -> fixup fixup -> verify } ``` </details> ⚒️ Generated with [Fabro](https://fabro.sh) --------- Co-authored-by: Fabro <noreply@fabro.sh>
…gs (fabro-sh#564) ## Summary Provider `extra_headers` previously required values to be typed TOML tables (`{ env = "X" }`, `{ literal = "Y" }`, `{ vault = "Z" }`). This PR migrates them to the project's standard interpolation string format: plain text for literals, `{{ env.NAME }}` tokens for environment variables, and `{{ secrets.NAME }}` tokens for vault secrets. This brings `extra_headers` in line with the rest of the interpolation system and unlocks mixed-segment values like `Bearer {{ secrets.GATEWAY_TOKEN }}`. ### What changed and why **Config authoring surface (`fabro-config`):** `ProviderSettings.extra_headers` changes from `Option<HashMap<String, HeaderValueRef>>` to `Option<HashMap<String, InterpString>>`. The `Combine` impl and all re-exports are updated accordingly. **Catalog layer (`fabro-model`):** `ProviderCatalogSettings.extra_headers` and `CatalogProvider.extra_headers` become `HashMap<String, String>` — raw interpolation source strings. This is required by the crate dependency direction: `fabro-types` (which owns `InterpString`) depends on `fabro-model`, so `fabro-model` cannot hold `InterpString` without creating a cycle. The source string is re-parsed and resolved in `fabro-auth` at credential-build time. **Credential resolution (`fabro-auth`):** Both `CredentialResolver` (vault-backed) and `EnvCredentialSource` (env-only) are rewritten to parse each header source string as an `InterpString` and resolve it with a `ResolveCtx` scoped to `env` + `secrets` only. A new `resolve_extra_headers` helper is shared between the two paths. Vault resolution uses `vault_token_lookup`, which wraps `vault_get_token` and maps any non-Token vault entry to `None` — so file and OAuth vault entries fail closed rather than resolving incorrectly. `vars.*` and `inputs.*` tokens are not in scope and produce `Unavailable` errors automatically. **New error variant:** `ResolveError::Interpolation { provider, source }` surfaces header resolution failures as diagnosable auth issues. The inner `source` (an `InterpResolveError`) names only the token namespace and name — never a resolved value. **`{ literal = "..." }` guardrail removed:** `HeaderValueRef` deliberately rejected bare string header values to discourage pasting credentials. `InterpString` accepts any string. This is an intentional change; the mitigation is documentation — use `{{ secrets.NAME }}` for credential-shaped values, not bare literals. **Redactor registration gap (noted, not fixed here):** Secrets resolved into provider headers at the credential boundary do not flow through the run boundary's exact-match redaction registry. Exposure is low (headers are host-side and outbound-only, never logged), but a follow-up should thread a registering lookup through `VaultCredentialSource`. A code comment at the resolution site marks the gap. ### Breaking change Existing `extra_headers` config using `{ env = "X" }`, `{ literal = "Y" }`, or `{ vault = "Z" }` table syntax **will fail to parse** after this change. Users must migrate to the token form: plain strings for literals, `{{ env.X }}` for env vars, `{{ secrets.X }}` for vault secrets. A changelog entry is included. ### Plan Summary - Update `ProviderSettings.extra_headers` → `InterpString` in `fabro-config` - Collapse authoring `InterpString` → source `String` in `provider_settings_to_catalog` (allowlisted `as_source()` call) - Delete `HeaderValueRef` and its serde/display/parse machinery from `fabro-model` - Rewrite both auth resolution paths to use `InterpString::parse + resolve_with`; add `Interpolation` error variant - Add `vault_token_lookup` helper for token-only fail-closed vault resolution - Update test TOML in `fabro-llm`, builtin catalog comment in `openrouter.toml`, and all hand-written + generated docs ### Fabro Details <details> <summary>Ran 8 stages in 108m 43s for $34.44</summary> | Stage | Duration | Cost | Retries | |---|---|---|---| | start | 0s | – | 0 | | toolchain | 1s | – | 0 | | preflight_compile | 5m 33s | – | 0 | | preflight_lint | 5m 59s | – | 0 | | implement | 43m 23s | $17.34 | 0 | | simplify_fable | 32m 49s | $13.09 | 0 | | simplify_gpt | 6m 25s | $4.01 | 0 | | verify | 13m 58s | – | 0 | | **Total** | **108m 43s** | **$34.44** | **0** | </details> <details> <summary>Ran <code>ImplementPlan.fabro</code> (11 nodes and 14 edges)</summary> ```dot digraph ImplementPlan { graph [ goal="Implement and simplify", model_stylesheet=" * { model: claude-opus-4-8; } " ] rankdir=LR start [shape=Mdiamond, label="Start"] exit [shape=Msquare, label="Exit"] toolchain [label="Toolchain", shape=parallelogram, script="command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1", max_retries=0] preflight_compile [label="Preflight Compile", shape=parallelogram, script="cargo check -q --workspace 2>&1", max_retries=0] preflight_lint [label="Preflight Lint", shape=parallelogram, script="cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1", max_retries=0] fix_lints [label="Fix Lints", prompt="The preflight lint step failed. Read the build output from context and fix all clippy lint warnings.", max_visits=3] implement [label="Implement", prompt="Read the plan file referenced in the goal and implement every step. Make all the code changes described in the plan. Use red/green TDD. Be sure to use the rust-style-guide skill to help you follow this repo's Rust style conventions.", model="gpt-55", reasoning_effort="xhigh"] simplify_fable [label="Simplify (Fable)", prompt="@prompts/simplify.md", model="claude-fable-5", reasoning_effort="xhigh"] simplify_gpt [label="Simplify (GPT-55)", prompt="@prompts/simplify.md", model="gpt-55"] verify [label="Verify", shape=parallelogram, timeout="1800s", script="git fetch origin main 2>&1 && git merge --no-edit --no-stat origin/main 2>&1 && cargo +nightly-2026-04-14 fmt --all 2>&1 && cargo dev docs refresh 2>&1 && cargo +nightly-2026-04-14 fmt --check --all 2>&1 && { command -v rg >/dev/null 2>&1 || { echo 'rg is required for verify'; exit 127; }; } && ! rg -n 'AuthMode::Disabled|RunAuthMethod|RunSubjectProvenance|\bActorRef\b|\bActorKind\b|AuthenticatedSubject|AuthenticatedService|AuthorizeRunScoped|AuthorizeRunBlob|AuthorizeStageArtifact|AuthorizeCommandLog|auth_method\s*==\s*\"disabled\"' lib/crates apps lib/packages docs/public/api-reference/fabro-api.yaml 2>&1 && cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings 2>&1 && cargo nextest run --workspace --status-level slow --profile ci 2>&1 && cargo dev docs check 2>&1 && bun install --frozen-lockfile 2>&1 && (cd apps/fabro-web && bun run typecheck) 2>&1 && (cd apps/fabro-web && bun run test) 2>&1 && (cd lib/packages/fabro-api-client && bun run typecheck) 2>&1 && cargo dev build -- -p fabro-cli --release 2>&1", goal_gate=true, retry_target="fixup"] fixup [label="Fixup", prompt="The verify step failed. Read the build output from context and fix all format, clippy, Rust test, docs, TypeScript typecheck/test, and build failures.", max_visits=3] start -> toolchain toolchain -> preflight_compile [condition="outcome=succeeded"] toolchain -> exit preflight_compile -> preflight_lint [condition="outcome=succeeded"] preflight_compile -> exit preflight_lint -> implement [condition="outcome=succeeded"] preflight_lint -> fix_lints fix_lints -> preflight_lint implement -> simplify_fable -> simplify_gpt -> verify verify -> exit [condition="outcome=succeeded"] verify -> fixup fixup -> verify } ``` </details> ⚒️ Generated with [Fabro](https://fabro.sh) --------- Co-authored-by: Fabro <noreply@fabro.sh>
…#524/fabro-sh#513/fabro-sh#492) Post-merge, prompts/goals interpolate {{ vars.NAME }} (server-managed run variables) alongside {{ inputs.* }} and {{ goal }} — implemented in fabro-template and fully documented upstream at docs/public/workflows/variables.mdx. This adds a short fork-local pointer plus the corrected qa-pipeline.toml goal-line example (previously unresolved $repo_name/$sha shell-style placeholders). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Owner
Author
|
Adversarial review (worker fabro-3) — verdict: SOUND.
Merging (squash). Deployment of ~/fabro-run is a separate operator step. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
upstream/main(fabro-sh/fabro) into our fork — 51 commits, clean merge, no conflicts. Our 2 fork-only CI commits (0d1c85347ci: main-only mirror,32f685becci: gate) survive as ancestors of the merge commit.docs/TEMPLATING.md: a short fork-local pointer to upstream's owndocs/public/workflows/variables.mdx(merged in via this sync) plus a migration example for our qa-pipeline goal line. Per "prefer upstream docs, do not duplicate" — the full contract is not re-explained here.The new templating contract (post fabro-sh#524/fabro-sh#513/fabro-sh#492)
Prompts/goals render exactly three top-level template namespaces via
fabro-template(lib/crates/fabro-template/src/lib.rs):{{ goal }}— the workflow goal{{ inputs.NAME }}— typed[run.inputs]values, overridable with-I/--input{{ vars.NAME }}— server-managed run variables (strings), set withfabro variable set NAME value, read withfabro variable get/listReferencing an unknown
inputs.*/vars.*member is a strict render error, not silent-empty (outside structural/lenient passes).{{ env.* }}is available in config strings/HTTP hook headers but NOT in goal/prompt.Corrected qa-pipeline goal line
Our deployed qa-pipeline predates all three upstream PRs and used shell-style placeholders that were never interpolated:
(This PR does not touch any deployed pipeline config — out of scope per the task. It documents the corrected line for whoever migrates that config.)
Test results
cargo build --release: exit 0, 645 crates compiled clean.cargo test --workspace: 431 passed, 8 failed, 1 ignored (excluding the successful sub-suites tallied separately above that line). All 8 failures are pre-existing/environmental, not merge-introduced:fabro-cli::tests::pre_tracing_bootstrap_*) are a parallel-test env-var race (FABRO_LOG_DESTINATION) — confirmed fixed when re-run single-threaded (cargo test -p fabro-cli --bins -- --test-threads=1), all pass.server_client::tests::http_target_transport_times_out_when_peer_accepts_without_http_response) is because~/.fabro/doesn't exist on this box (missing fixture directory), not a code bug.git diff 32f685bec...upstream/main -- lib/crates/fabro-cli/src/main.rs lib/crates/fabro-cli/src/server_client.rsis empty — these files are byte-identical between our pre-merge tip and upstream/main, so nothing in the merge touched this code.cargo nextest run --workspace --profile e2e --run-ignored only): could not run meaningfully — no.envpresent on this box, so the dual-mode#[e2e_test(twin, live(...))]tests fall through to the real Anthropic API with no key (401 x-api-key header is required) rather than the twin mock. This is an environment/credentials gap on this box, not evaluated further here.Test plan
cargo nextest run --profile e2e --run-ignored onlyon a box with.envpopulated, to get real live/twin e2e coverage before merge$var-style placeholders🤖 Generated with Claude Code