diff --git a/.github/workflows/engine-freeze.yml b/.github/workflows/engine-freeze.yml deleted file mode 100644 index be8b84f07..000000000 --- a/.github/workflows/engine-freeze.yml +++ /dev/null @@ -1,53 +0,0 @@ -name: Engine freeze - -# The engine half of fabro-workflow takes bug fixes only; new engine behaviour -# goes to Petri. A pull request that adds lines under the frozen paths fails -# here unless it carries the `bugfix` label. The frozen paths live in -# `scripts/check-engine-freeze.sh`, which runs locally the same way. -# Labeling re-runs the check so a label added after a failure clears it. - -on: - pull_request: - branches: [main] - types: [opened, synchronize, reopened, labeled, unlabeled] - paths: - - "lib/components/fabro-workflow/src/**" - - "scripts/check-engine-freeze.sh" - - "scripts/check-engine-freeze-test.sh" - - ".github/workflows/engine-freeze.yml" - -concurrency: - group: ${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: true - -permissions: {} - -jobs: - freeze: - name: Engine half takes bug fixes only - runs-on: ubuntu-24.04 - permissions: - contents: read - steps: - - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - persist-credentials: false - # The check diffs against the merge base with the base branch. - fetch-depth: 0 - - name: Self-test the check - run: scripts/check-engine-freeze-test.sh - - name: Check the frozen paths - env: - BASE_REF: ${{ github.base_ref }} - HAS_BUGFIX_LABEL: ${{ contains(github.event.pull_request.labels.*.name, 'bugfix') }} - run: | - git fetch --no-tags origin "$BASE_REF" - if scripts/check-engine-freeze.sh "origin/$BASE_REF"; then - exit 0 - fi - if [ "$HAS_BUGFIX_LABEL" = "true" ]; then - echo "The 'bugfix' label waives the engine freeze for this pull request." - exit 0 - fi - echo "::error::This pull request adds lines to the frozen engine half of fabro-workflow (see the log for the paths). New engine behaviour goes to Petri (lib/components/fabro-petri). A bug fix needs the 'bugfix' label." - exit 1 diff --git a/AGENTS.md b/AGENTS.md index e77e01f92..ef19a66f8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -136,9 +136,124 @@ Fabro is an AI-powered workflow orchestration platform. Workflows are defined as - **fabro-telemetry** — CLI analytics (Segment) and crash reporting (Sentry), with anonymous IDs, command sanitization, and detached subprocess delivery - **fabro-util** — Shared utilities (redaction, terminal formatting) -### Engine freeze +### TypeScript (fabro-web) +- `cd apps/fabro-web && bun run dev` — rebuild web assets on change for the Rust server; refresh the browser manually +- `cd apps/fabro-web && bun test` — run tests +- `cd apps/fabro-web && bun run typecheck` — type check +- `cd apps/fabro-web && bun run build` — production build (writes to `apps/fabro-web/dist/` only; does NOT update the bundled SPA that ships in the Rust binary) +- `cargo dev build [-- ]` — refreshes the embedded SPA assets from the production build, verifies SPA asset budgets, and then runs `cargo build` with forwarded args. The embedded assets are gitignored except for `.gitkeep`; use this when building a Rust binary that should include a populated SPA bundle. `bun run dev` for local development is unchanged because debug builds prefer `apps/fabro-web/dist/` on disk via the server fallback. -The engine half of `fabro-workflow` takes bug fixes only: `handler/`, `lifecycle/`, `pipeline/execute` (the file and the directory), `graph/routing.rs`, `node_handler.rs`, `retry.rs`, `condition.rs`, `context.rs` and `model_fallback.rs` under `lib/components/fabro-workflow/src/`. New engine behaviour goes to Petri through `fabro-petri`. The `Engine freeze` CI check (`.github/workflows/engine-freeze.yml`) fails a pull request that adds lines under those paths unless it carries the `bugfix` label. The path list lives in `scripts/check-engine-freeze.sh`, which runs locally as `scripts/check-engine-freeze.sh origin/main` and reports the added lines; `scripts/check-engine-freeze-test.sh` is its self-test. +### Docker image +- `cargo dev docker-build` — builds the local Docker image from the current tree using the release pipeline's cargo-zigbuild approach. Honors `--arch amd64|arm64`, `--tag ` (default `fabro-sh/fabro`), `--compile-only` (stages `tmp/docker-context//fabro` without `docker build`), and `--dry-run` (prints the Docker commands without running them). Prefer this over writing a throwaway Dockerfile; the release pipeline, `Dockerfile`, and this command share the same binary layout. + +### Docker sandbox provider +- Docker is the default runtime sandbox provider from `defaults.toml`. The Fabro process must have a working Docker client environment (`DOCKER_HOST`, socket access, Docker Desktop behavior, TLS settings, groups/permissions, and any remote daemon policy are operator responsibilities). +- The packaged compose service mounts `/var/run/docker.sock` so the server can create sibling run containers on the host daemon. This is host-root-equivalent under Docker's security model; only use it in the trusted, single-tenant deployment model described by the sandbox code/docs. +- Docker and Daytona are clone-based providers. When a run manifest has a GitHub origin, they clone it into the provider workspace. Present non-GitHub origins fail unless the provider has `skip_clone = true`; absent origins or `skip_clone = true` create an empty workspace without repository files. For an exact commit, the submitted branch names the working branch and the syntactically valid SHA is requested directly. No layer proves branch/SHA ancestry: a fetchable commit is checked out, an unavailable commit fails setup, and branch HEAD is never substituted. +- The sandbox layer also accepts an optional exact commit for future admitted + runs. An exact commit always requires a non-empty branch. The sandbox driver + performs the pin the same way on every provider: it initializes an empty + repository, fetches the SHA directly at the requested depth, and attaches + the admitted branch to it, so the workspace reports the admitted branch + name. Daytona's native toolbox clone serves plain branch clones only; its + commit pin checks the branch head out first, so the driver does not use + it. A successful clone has the pin checked out; the driver's + conformance suite verifies that on every provider, and fabro does not + re-verify HEAD. Never fall back to a newer branch HEAD, and do not wire + this capability directly from legacy `GitContext.sha`. The sandbox layer + does not verify that the commit is reachable from the branch; admission + owns that check. Current production callers remain branch-only until the + RunIntent admission cutover supplies a validated branch/SHA pair. + +### Release automation +- `cargo dev release` — creates the next stable release tag. Use `cargo dev release --nightly` for a nightly prerelease. Use `--dry-run` to print planned commands without mutating git or running Cargo, `--skip-tests` only after running the release-mode smoke yourself, and `--release-date YYYY-MM-DD` or `FABRO_RELEASE_DATE` for deterministic version computation. + +### Marketing site (apps/marketing) +- `cd apps/marketing && bun run dev` — start Astro dev server +- `cd apps/marketing && bun run build` — production build +- `cd apps/marketing && bunx vercel --prod` — deploy to Vercel (project: website, domain: fabro.sh) + +### Dev servers +1. `fabro server start` — starts the Rust API server (demo mode is per-request via `X-Fabro-Demo: 1` header) +2. `cd apps/fabro-web && bun run dev` — rebuilds web assets on change; refresh the browser manually +3. Mintlify docs dev server (requires Docker — `mintlify dev` needs Node LTS which may not match the host): + ``` + docker run --rm -d -p 3333:3333 -v $(pwd)/docs/public:/docs -w /docs --name mintlify-dev node:22-slim \ + bash -c "npx mintlify dev --host 0.0.0.0 --port 3333" + ``` + Then open http://localhost:3333. Stop with `docker stop mintlify-dev`. + +## API workflow + +The OpenAPI spec at `docs/public/api-reference/fabro-api.yaml` is the source of truth for the fabro-api HTTP interface. + +1. Edit `docs/public/api-reference/fabro-api.yaml` +2. `cargo build -p fabro-api` — build.rs regenerates Rust types and client via progenitor +3. Write/update handler in `lib/apps/fabro-server/src/server.rs`, add route to `build_router()` +4. `cargo nextest run -p fabro-server` — conformance test catches spec/router drift +5. `cd lib/packages/fabro-api-client && bun run generate` — regenerates TypeScript Axios client + +### API type ownership + +- Treat OpenAPI as the source of truth for the wire contract, not as the automatic owner of Rust types. +- Before adding or keeping a generated schema type, search the workspace for an existing hand-written Rust type with the same product meaning. +- If the schema and an existing Rust type have the same semantics and serde shape, reuse the existing type via `lib/foundation/fabro-api/build.rs` `with_replacement(...)` instead of generating a parallel API type. +- If two types are close but not identical, prefer proposing changes that align them into one canonical type rather than accepting small drift. It is usually better to iterate the API now than to create permanently split Rust/API types. +- Keep a separate API DTO only when the API is intentionally a projection, summary, or presentation-specific view of internal state. In that case, give it a distinct API-facing name instead of reusing the internal concept name. +- Treat `ApiFoo` aliases and `foo_to_api` / `foo_from_api` adapters as a smell unless they represent a real semantic boundary. They should not exist only to bridge accidental duplicate types. +- If a type is shared across crates and is part of the core product vocabulary, move it to a shared crate first, then make `fabro-api` reuse it. +- For every new `with_replacement(...)`, add a `fabro-api` test that proves type identity and JSON parity with the OpenAPI schema. + +## Test support boundaries + +Test-only helpers, fixture constructors, fake credentials, in-memory stores, panic-heavy setup code, and test environment shims must not be exposed from production modules or linked into normal builds. + +Put shared test helpers in a dedicated `test_support` module gated behind tests or an explicit feature: + +```rust +#[cfg(any(test, feature = "test-support"))] +pub mod test_support; +``` + +If another crate's tests need those helpers, enable the feature only through a dev-dependency using Cargo's dual-listing pattern: + +```toml +[dependencies] +fabro-server = { path = "../fabro-server" } + +[dev-dependencies] +fabro-server = { path = "../fabro-server", features = ["test-support"] } +``` + +Do not enable `test-support` in default features, production dependencies, release builds, or binaries. + +Use names that make the boundary obvious: `test_app_state`, `test_store_bundle`, `test_auth_mode`, and similar. Avoid production-looking names such as `create_app_state` for test fixtures. `#[doc(hidden)]` is not a substitute for feature-gating; hidden public APIs still compile, link, and can be used accidentally. + +Before merging changes that add or move shared test helpers, verify: + +- `cargo build --workspace` succeeds without `test-support` +- relevant tests compile and run with `test-support` +- `rg -n "create_app_state|test-only-name"` does not show production call sites +- release/debug artifacts do not contain fake secrets, fixture tokens, or test helper symbols when built without `test-support` + +## Architecture + +Fabro is an AI-powered workflow orchestration platform. Workflows are defined as Graphviz graphs, where each node is a stage (agent, prompt, command, conditional, human, parallel, etc.) executed by the workflow engine. + +### Rust crates (`lib/apps/`, `lib/components/`, and `lib/foundation/`) +- **fabro-cli** — CLI entry point. Commands: `run`, `exec`, `serve`, `validate`, `parse`, `cp`, `model`, `doctor`, `install`, `ps`, `system prune` +- **fabro-workflow** — Core workflow engine. Parses Graphviz graphs, runs stages, manages checkpoints/resume, hooks, and human-in-the-loop interactions +- **fabro-sandbox** — Local, Docker, and Daytona sandbox providers. `RunSandbox` is also the `Environment` pebble's coding agent runs its tools through; agent stages, Ask Fabro, hook evaluators, and `fabro exec` all run on the `pebble-coding-agent` crate (pinned by rev in the workspace `Cargo.toml`). `RunSandbox` is also the `Environment` pebble's coding agent runs its tools through; agent stages, Ask Fabro, hook evaluators, and `fabro exec` all run on the `pebble-coding-agent` crate (pinned by rev in the workspace `Cargo.toml`). Docker is the default runtime provider and creates clone-based `/workspace` containers through the operator's Docker daemon; Daytona uses the same GitHub-only clone-source contract. Docker daemon access is host-root-equivalent and assumes trusted callers/payloads. +- **fabro-petri** — Fabro's adapters over Petri, the workflow engine: the one crate that imports the Petri packages (pinned by rev in the workspace `Cargo.toml`), holding the run store over SQLite and the platform adapters +- **fabro-server** — Axum HTTP server. Routes for runs, sessions, models, completions, usage. SSE event streaming. Demo mode via header +- **fabro-llm** — Unified LLM client with providers: Anthropic, OpenAI, Gemini, OpenAI-compatible, plus retry/middleware/streaming +- **fabro-api** — Auto-generated Rust types and reqwest HTTP client from OpenAPI spec (build.rs + progenitor) +- **fabro-github** — GitHub App auth (JWT signing, installation tokens, PR creation) +- **fabro-mcp** — Model Context Protocol client/server +- **fabro-slack** — Slack integration (socket mode, blocks API) +- **fabro-checkpoint** — Git checkpoint author identity and commit trailers +- **fabro-telemetry** — CLI analytics (Segment) and crash reporting (Sentry), with anonymous IDs, command sanitization, and detached subprocess delivery +- **fabro-util** — Shared utilities (redaction, terminal formatting) ### TypeScript (`apps/` and `lib/packages/`) - **apps/fabro-web** — React 19 + React Router + Tailwind CSS frontend, bundled by a custom Bun script (`apps/fabro-web/scripts/build.ts`), not Vite diff --git a/Cargo.lock b/Cargo.lock index 12c0c95ae..179c4fcd1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2401,7 +2401,6 @@ dependencies = [ "fabro-environment", "fabro-github", "fabro-graphviz", - "fabro-hooks", "fabro-http", "fabro-install", "fabro-interview", @@ -2642,31 +2641,6 @@ dependencies = [ "thiserror 2.0.18", ] -[[package]] -name = "fabro-hooks" -version = "0.357.0-nightly.0" -dependencies = [ - "async-trait", - "fabro-auth", - "fabro-http", - "fabro-llm", - "fabro-redact", - "fabro-sandbox", - "fabro-types", - "fabro-util", - "httpmock", - "lithos-llm", - "pebble-agent", - "pebble-coding-agent", - "regex", - "serde", - "serde_json", - "tokio", - "tokio-util", - "toml 0.8.23", - "tracing", -] - [[package]] name = "fabro-http" version = "0.357.0-nightly.0" @@ -3005,7 +2979,6 @@ dependencies = [ "fabro-environment", "fabro-github", "fabro-graphviz", - "fabro-hooks", "fabro-http", "fabro-install", "fabro-interview", diff --git a/docs/public/reference/sdk.mdx b/docs/public/reference/sdk.mdx index 7e0cca8dc..3f3843dc8 100644 --- a/docs/public/reference/sdk.mdx +++ b/docs/public/reference/sdk.mdx @@ -221,7 +221,7 @@ Fabro stores every one of these as an `agent.*` run event whose properties are t ### Tool middleware -Implement pebble's `ToolMiddleware` to intercept tool calls for approval, logging, or transformation, and install it with the builder's `.tool_middleware(...)`. Fabro's `fabro_hooks::WorkflowToolHookCallback` is one: it runs the workflow's `pre_tool_use` hooks before each call and the `post_tool_use` hooks after. +Implement pebble's `ToolMiddleware` to intercept tool calls for approval, logging, or transformation, and install it with the builder's `.tool_middleware(...)`. Petri's Attractor agent step installs one for Fabro's `pre_tool_use` and `post_tool_use` hooks. ```rust use async_trait::async_trait; diff --git a/lib/apps/fabro-cli/Cargo.toml b/lib/apps/fabro-cli/Cargo.toml index 7f7c1fa55..b2e0aa74a 100644 --- a/lib/apps/fabro-cli/Cargo.toml +++ b/lib/apps/fabro-cli/Cargo.toml @@ -29,7 +29,6 @@ pebble-coding-agent.workspace = true pebble-cli-core.workspace = true sandbox-driver.workspace = true fabro-dump = { path = "../../components/fabro-dump" } -fabro-hooks = { path = "../../components/fabro-hooks" } fabro-install = { path = "../../components/fabro-install" } fabro-interview = { path = "../../components/fabro-interview" } fabro-mcp = { path = "../../components/fabro-mcp" } diff --git a/lib/apps/fabro-server/Cargo.toml b/lib/apps/fabro-server/Cargo.toml index 9135bc4c3..0a51955ab 100644 --- a/lib/apps/fabro-server/Cargo.toml +++ b/lib/apps/fabro-server/Cargo.toml @@ -28,7 +28,6 @@ fabro-spa = { path = "../fabro-spa" } fabro-config = { path = "../../foundation/fabro-config" } fabro-environment.workspace = true fabro-graphviz = { path = "../../components/fabro-graphviz" } -fabro-hooks = { path = "../../components/fabro-hooks" } fabro-interview = { path = "../../components/fabro-interview" } fabro-slack = { path = "../../components/fabro-slack" } fabro-workflow = { path = "../../components/fabro-workflow" } diff --git a/lib/components/fabro-hooks/Cargo.toml b/lib/components/fabro-hooks/Cargo.toml deleted file mode 100644 index b214d80f3..000000000 --- a/lib/components/fabro-hooks/Cargo.toml +++ /dev/null @@ -1,38 +0,0 @@ -[package] -name = "fabro-hooks" -edition.workspace = true -version.workspace = true -publish = false -license.workspace = true -description = "User-defined lifecycle hooks for Fabro workflows" - -[lib] -doctest = false - -[lints] -workspace = true - -[dependencies] -fabro-auth = { path = "../../foundation/fabro-auth" } -fabro-llm = { path = "../fabro-llm" } -fabro-sandbox = { path = "../fabro-sandbox" } -pebble-agent.workspace = true -pebble-coding-agent.workspace = true -fabro-redact.workspace = true -fabro-types = { path = "../../foundation/fabro-types" } -lithos-llm = { workspace = true, features = ["runtime"] } -fabro-util = { path = "../../foundation/fabro-util" } -fabro-http.workspace = true -serde.workspace = true -serde_json.workspace = true -tokio.workspace = true -async-trait.workspace = true -regex.workspace = true -tracing.workspace = true -tokio-util.workspace = true - -[dev-dependencies] -fabro-auth = { path = "../../foundation/fabro-auth", features = ["test-support"] } -httpmock = "0.8" -tokio = { workspace = true, features = ["test-util", "macros"] } -toml.workspace = true diff --git a/lib/components/fabro-hooks/src/bridge.rs b/lib/components/fabro-hooks/src/bridge.rs deleted file mode 100644 index 6d9f65b8b..000000000 --- a/lib/components/fabro-hooks/src/bridge.rs +++ /dev/null @@ -1,349 +0,0 @@ -use std::sync::Arc; - -use async_trait::async_trait; -use fabro_sandbox::RunSandbox; -use fabro_types::{RunId, tool_call_arguments}; -use pebble_agent::{ - ToolCallNext, ToolCallRequest, ToolErrorKind, ToolMiddleware, ToolOutcome, ToolSystemError, -}; - -use crate::runner::HookRunner; -use crate::types::{HookContext, HookDecision, HookEvent, HookExecutionContext}; - -/// Bridge between the workflow hook system and pebble's tool pipeline. -/// -/// Created per-node in the workflow engine, capturing the `HookRunner` and -/// context needed to build `HookContext` for tool-level events. A blocking -/// `pre_tool_use` decision denies the call before it runs; `post_tool_use` -/// and `post_tool_use_failure` fire after the tool finishes, on success and -/// on failure respectively. -pub struct WorkflowToolHookCallback { - pub hook_runner: Arc, - pub sandbox: Arc, - pub run_id: RunId, - pub workflow_name: String, - pub hook_execution_context: HookExecutionContext, - pub node_id: String, -} - -impl WorkflowToolHookCallback { - fn base_context(&self, event: HookEvent, tool_name: &str) -> HookContext { - let mut ctx = HookContext::new(event, self.run_id, self.workflow_name.clone()); - ctx.node_id = Some(self.node_id.clone()); - ctx.tool_name = Some(tool_name.to_string()); - ctx - } - - async fn run_hook(&self, ctx: &HookContext) -> HookDecision { - self.hook_runner - .run( - ctx, - self.sandbox.clone(), - self.hook_execution_context.clone(), - ) - .await - } - - /// Whether a `pre_tool_use` hook blocks the call, and why. - pub async fn pre_tool_use( - &self, - tool_name: &str, - tool_input: &serde_json::Value, - ) -> Option { - let mut ctx = self.base_context(HookEvent::PreToolUse, tool_name); - ctx.tool_input = Some(tool_input.clone()); - - match self.run_hook(&ctx).await { - HookDecision::Block { reason } => { - Some(reason.unwrap_or_else(|| "Blocked by hook".to_string())) - } - _ => None, - } - } - - pub async fn post_tool_use(&self, tool_name: &str, tool_call_id: &str, tool_output: &str) { - let mut ctx = self.base_context(HookEvent::PostToolUse, tool_name); - ctx.tool_call_id = Some(tool_call_id.to_string()); - ctx.tool_output = Some(tool_output.to_string()); - - self.run_hook(&ctx).await; - } - - pub async fn post_tool_use_failure(&self, tool_name: &str, tool_call_id: &str, error: &str) { - let mut ctx = self.base_context(HookEvent::PostToolUseFailure, tool_name); - ctx.tool_call_id = Some(tool_call_id.to_string()); - ctx.error_message = Some(error.to_string()); - - self.run_hook(&ctx).await; - } -} - -#[async_trait] -impl ToolMiddleware for WorkflowToolHookCallback { - async fn call( - &self, - request: ToolCallRequest, - next: ToolCallNext<'_>, - ) -> Result { - let tool_name = request.call().name.clone(); - let tool_call_id = request.call().id.clone(); - let tool_input = tool_call_arguments(request.call()); - - if let Some(reason) = self.pre_tool_use(&tool_name, &tool_input).await { - return Ok(ToolOutcome::failure(ToolErrorKind::Denied, reason)); - } - - let outcome = next.run(request).await?; - match &outcome { - ToolOutcome::Success { output, .. } => { - self.post_tool_use(&tool_name, &tool_call_id, &output.text()) - .await; - } - ToolOutcome::Failure { message, .. } => { - self.post_tool_use_failure(&tool_name, &tool_call_id, message) - .await; - } - // `ToolOutcome` is non-exhaustive; an outcome this build does not - // know is neither a success nor a failure the hooks describe. - _ => {} - } - Ok(outcome) - } -} - -#[cfg(test)] -mod tests { - use std::path::PathBuf; - use std::sync::Mutex; - - use fabro_llm::credentials::CredentialProvider; - use fabro_llm::lithos_catalog::Catalog; - use fabro_types::fixtures; - - use super::*; - use crate::config::{HookDefinition, HookSettings}; - use crate::executor::HookExecutor; - use crate::types::{HookContext, HookResult}; - - struct CapturingExecutor { - captured_contexts: Arc>>, - captured_execution_contexts: Arc>>, - decision: HookDecision, - } - - #[async_trait::async_trait] - impl HookExecutor for CapturingExecutor { - async fn execute( - &self, - _definition: &HookDefinition, - context: &HookContext, - _sandbox: Arc, - execution_context: &HookExecutionContext, - _llm_source: Arc, - _catalog: Arc, - ) -> HookResult { - self.captured_contexts.lock().unwrap().push(context.clone()); - self.captured_execution_contexts - .lock() - .unwrap() - .push(execution_context.clone()); - HookResult { - hook_name: None, - decision: self.decision.clone(), - duration_ms: 1, - } - } - } - - fn make_hook(event: HookEvent) -> HookDefinition { - HookDefinition { - name: Some("test-hook".into()), - event, - command: Some("echo test".into()), - hook_type: None, - matcher: None, - blocking: None, - timeout_ms: None, - sandbox: Some(false), - } - } - - async fn make_sandbox() -> Arc { - Arc::new( - fabro_sandbox::local_sandbox(std::env::current_dir().unwrap()) - .await - .unwrap(), - ) - } - - fn make_bridge( - hook_runner: Arc, - sandbox: Arc, - hook_execution_context: HookExecutionContext, - ) -> WorkflowToolHookCallback { - WorkflowToolHookCallback { - hook_runner, - sandbox, - run_id: fixtures::RUN_1, - workflow_name: "test-wf".into(), - hook_execution_context, - node_id: "plan".into(), - } - } - - #[tokio::test] - async fn pre_tool_use_builds_correct_context() { - let captured = Arc::new(Mutex::new(Vec::new())); - let executor = Arc::new(CapturingExecutor { - captured_contexts: captured.clone(), - captured_execution_contexts: Arc::new(Mutex::new(Vec::new())), - decision: HookDecision::Proceed, - }); - let config = HookSettings { - hooks: vec![make_hook(HookEvent::PreToolUse)], - }; - let runner = Arc::new(HookRunner::with_executor(config, executor)); - let sandbox = make_sandbox().await; - let bridge = make_bridge(runner, sandbox, HookExecutionContext::default()); - - bridge - .pre_tool_use("shell", &serde_json::json!({"command": "ls"})) - .await; - - let contexts = captured.lock().unwrap(); - assert_eq!(contexts.len(), 1); - assert_eq!(contexts[0].event, HookEvent::PreToolUse); - assert_eq!(contexts[0].tool_name.as_deref(), Some("shell")); - assert_eq!( - contexts[0].tool_input, - Some(serde_json::json!({"command": "ls"})) - ); - assert_eq!(contexts[0].run_id, fixtures::RUN_1); - assert_eq!(contexts[0].node_id.as_deref(), Some("plan")); - } - - #[tokio::test] - async fn pre_tool_use_maps_block_decision() { - let executor = Arc::new(CapturingExecutor { - captured_contexts: Arc::new(Mutex::new(Vec::new())), - captured_execution_contexts: Arc::new(Mutex::new(Vec::new())), - decision: HookDecision::Block { - reason: Some("forbidden".into()), - }, - }); - let config = HookSettings { - hooks: vec![make_hook(HookEvent::PreToolUse)], - }; - let runner = Arc::new(HookRunner::with_executor(config, executor)); - let sandbox = make_sandbox().await; - let bridge = make_bridge(runner, sandbox, HookExecutionContext::default()); - - let decision = bridge.pre_tool_use("shell", &serde_json::json!({})).await; - assert_eq!(decision.as_deref(), Some("forbidden")); - } - - #[tokio::test] - async fn pre_tool_use_maps_proceed() { - let executor = Arc::new(CapturingExecutor { - captured_contexts: Arc::new(Mutex::new(Vec::new())), - captured_execution_contexts: Arc::new(Mutex::new(Vec::new())), - decision: HookDecision::Proceed, - }); - let config = HookSettings { - hooks: vec![make_hook(HookEvent::PreToolUse)], - }; - let runner = Arc::new(HookRunner::with_executor(config, executor)); - let sandbox = make_sandbox().await; - let bridge = make_bridge(runner, sandbox, HookExecutionContext::default()); - - let decision = bridge.pre_tool_use("shell", &serde_json::json!({})).await; - assert_eq!(decision, None); - } - - #[tokio::test] - async fn post_tool_use_builds_context_with_output() { - let captured = Arc::new(Mutex::new(Vec::new())); - let executor = Arc::new(CapturingExecutor { - captured_contexts: captured.clone(), - captured_execution_contexts: Arc::new(Mutex::new(Vec::new())), - decision: HookDecision::Proceed, - }); - let config = HookSettings { - hooks: vec![make_hook(HookEvent::PostToolUse)], - }; - let runner = Arc::new(HookRunner::with_executor(config, executor)); - let sandbox = make_sandbox().await; - let bridge = make_bridge(runner, sandbox, HookExecutionContext::default()); - - bridge - .post_tool_use("shell", "call_1", "file1.txt\nfile2.txt") - .await; - - let contexts = captured.lock().unwrap(); - assert_eq!(contexts.len(), 1); - assert_eq!(contexts[0].event, HookEvent::PostToolUse); - assert_eq!(contexts[0].tool_name.as_deref(), Some("shell")); - assert_eq!(contexts[0].tool_call_id.as_deref(), Some("call_1")); - assert_eq!( - contexts[0].tool_output.as_deref(), - Some("file1.txt\nfile2.txt") - ); - } - - #[tokio::test] - async fn post_tool_use_failure_builds_context_with_error() { - let captured = Arc::new(Mutex::new(Vec::new())); - let executor = Arc::new(CapturingExecutor { - captured_contexts: captured.clone(), - captured_execution_contexts: Arc::new(Mutex::new(Vec::new())), - decision: HookDecision::Proceed, - }); - let config = HookSettings { - hooks: vec![make_hook(HookEvent::PostToolUseFailure)], - }; - let runner = Arc::new(HookRunner::with_executor(config, executor)); - let sandbox = make_sandbox().await; - let bridge = make_bridge(runner, sandbox, HookExecutionContext::default()); - - bridge - .post_tool_use_failure("shell", "call_1", "command not found") - .await; - - let contexts = captured.lock().unwrap(); - assert_eq!(contexts.len(), 1); - assert_eq!(contexts[0].event, HookEvent::PostToolUseFailure); - assert_eq!(contexts[0].tool_name.as_deref(), Some("shell")); - assert_eq!(contexts[0].tool_call_id.as_deref(), Some("call_1")); - assert_eq!( - contexts[0].error_message.as_deref(), - Some("command not found") - ); - } - - #[tokio::test] - async fn pre_tool_use_passes_supplied_hook_execution_context() { - let captured_contexts = Arc::new(Mutex::new(Vec::new())); - let captured_execution_contexts = Arc::new(Mutex::new(Vec::new())); - let executor = Arc::new(CapturingExecutor { - captured_contexts, - captured_execution_contexts: Arc::clone(&captured_execution_contexts), - decision: HookDecision::Proceed, - }); - let config = HookSettings { - hooks: vec![make_hook(HookEvent::PreToolUse)], - }; - let runner = Arc::new(HookRunner::with_executor(config, executor)); - let sandbox = make_sandbox().await; - let hook_execution_context = HookExecutionContext { - host_source_dir: Some(PathBuf::from("/host/source")), - sandbox_work_dir: Some(PathBuf::from("/supplied/sandbox")), - }; - let bridge = make_bridge(runner, sandbox, hook_execution_context.clone()); - - bridge.pre_tool_use("shell", &serde_json::json!({})).await; - - assert_eq!(captured_execution_contexts.lock().unwrap().as_slice(), &[ - hook_execution_context - ]); - } -} diff --git a/lib/components/fabro-hooks/src/config.rs b/lib/components/fabro-hooks/src/config.rs deleted file mode 100644 index 72ac52f2a..000000000 --- a/lib/components/fabro-hooks/src/config.rs +++ /dev/null @@ -1,44 +0,0 @@ -//! Hook configuration runtime settings. - -pub use fabro_types::settings::run::{HookDefinition, HookEvent, HookType, TlsMode}; -use serde::{Deserialize, Serialize}; - -/// Top-level hook configuration: a list of hook definitions. -#[derive(Debug, Clone, Default, Deserialize, PartialEq, Serialize)] -pub struct HookSettings { - #[serde(default)] - pub hooks: Vec, -} - -impl HookSettings { - /// Merge with another config. Concatenates lists; on name collisions, - /// `other` wins. - #[must_use] - pub fn merge(self, other: Self) -> Self { - let mut by_name: std::collections::HashMap = - std::collections::HashMap::new(); - let mut order: Vec = Vec::new(); - - for hook in self.hooks { - let name = hook.effective_name(); - if !by_name.contains_key(&name) { - order.push(name.clone()); - } - by_name.insert(name, hook); - } - for hook in other.hooks { - let name = hook.effective_name(); - if !by_name.contains_key(&name) { - order.push(name.clone()); - } - by_name.insert(name, hook); - } - - let hooks = order - .into_iter() - .filter_map(|name| by_name.remove(&name)) - .collect(); - - Self { hooks } - } -} diff --git a/lib/components/fabro-hooks/src/executor.rs b/lib/components/fabro-hooks/src/executor.rs deleted file mode 100644 index 0bd6fac70..000000000 --- a/lib/components/fabro-hooks/src/executor.rs +++ /dev/null @@ -1,1536 +0,0 @@ -use std::borrow::Cow; -use std::collections::HashMap; -use std::sync::{Arc, LazyLock}; -use std::time::Instant; - -use async_trait::async_trait; -use fabro_llm::credentials::CredentialProvider; -use fabro_llm::lithos_catalog::Catalog; -use fabro_llm::{Client, ClientOptions, Request}; -use fabro_redact::redacted_url_for_log; -use fabro_sandbox::{ExecResultExt as _, RunSandbox, SecretRedactor}; -use fabro_types::PermissionLevel; -use fabro_types::settings::{InterpString, ResolveCtx, ResolveError}; -use pebble_coding_agent::extensions::{ - SystemPromptContext, SystemPromptDecision, SystemPromptTransform, -}; -use pebble_coding_agent::{CodingAgent, CodingAgentOptions, Error as AgentError, ShutdownReason}; -use tokio::process::Command as TokioCommand; -use tokio::time::timeout as tokio_timeout; - -use crate::config::{HookDefinition, HookType, TlsMode}; -use crate::types::{ - HookContext, HookDecision, HookExecutionContext, HookResult, PromptHookResponse, -}; - -const HOOK_EVALUATOR_SYSTEM_PROMPT: &str = "You are a hook evaluator for a workflow engine. Given context about a workflow event, evaluate the condition."; - -/// How many tool rounds an agent hook may run when its definition names none. -const DEFAULT_MAX_TOOL_ROUNDS: u32 = 50; - -static HOOK_RESPONSE_SCHEMA: LazyLock = LazyLock::new(|| { - serde_json::json!({ - "type": "object", - "properties": { - "ok": { "type": "boolean" }, - "reason": { "type": "string" } - }, - "required": ["ok"], - "additionalProperties": false - }) -}); - -/// Replaces the profile's system prompt with the hook evaluator's. An agent -/// hook is not a coding session: no memory, no skills, no environment -/// preamble, just the evaluation contract. -struct HookEvaluatorPrompt; - -impl SystemPromptTransform for HookEvaluatorPrompt { - fn transform(&self, _context: SystemPromptContext<'_>) -> SystemPromptDecision { - SystemPromptDecision::Replace(HOOK_EVALUATOR_SYSTEM_PROMPT.to_owned()) - } -} - -fn duration_ms(duration: std::time::Duration) -> u64 { - u64::try_from(duration.as_millis()).unwrap_or(u64::MAX) -} - -/// Trait for executing hooks via different transports. -#[async_trait] -pub trait HookExecutor: Send + Sync { - async fn execute( - &self, - definition: &HookDefinition, - context: &HookContext, - sandbox: Arc, - execution_context: &HookExecutionContext, - llm_source: Arc, - catalog: Arc, - ) -> HookResult; -} - -/// Resolve a typed [`InterpString`] hook segment at fire time. -/// -/// No namespace is wired here. `{{ vars.* }}` is already substituted -/// server-side when the run is created, so a literal value resolves unchanged -/// and any remaining token — `secrets`, `inputs`, `env` — surfaces as -/// `Unavailable`. That is a hard error, so a hook referencing one fails closed -/// rather than firing with a half-resolved value. -/// -/// The value stays typed end-to-end: it is carried as an `InterpString` -/// through the config resolve layer and resolved here from its segments — -/// there is no `InterpString -> String -> InterpString` re-parse. -/// -/// Returns the typed [`ResolveError`] so callers keep the source until the -/// decision boundary renders it; do not flatten it to a `String` here. -fn resolve_interp(value: &InterpString) -> Result { - value.resolve_with(&mut ResolveCtx::new()) -} - -#[expect( - clippy::disallowed_methods, - reason = "hook HTTP logs use the unresolved token source, not the resolved URL; \ - redacted_url_for_log masks literal credentials in parseable source URLs and \ - replaces unparseable sources with a placeholder" -)] -fn safe_url_source_for_log(url: &InterpString) -> String { - redacted_url_for_log(&url.as_source()) -} - -/// Executes hooks via shell commands or HTTP POST. -pub struct HookExecutorImpl; - -impl HookExecutorImpl { - /// Parse a hook decision from JSON stdout and exit code. - fn parse_decision(exit_code: i32, stdout: &str) -> HookDecision { - if exit_code == 0 { - // Try parsing JSON response for explicit decision - if let Ok(decision) = serde_json::from_str::(stdout.trim()) { - return decision; - } - HookDecision::Proceed - } else if exit_code == 2 { - // Exit 2 = block/skip - if let Ok(decision) = serde_json::from_str::(stdout.trim()) { - return decision; - } - HookDecision::Block { - reason: Some("hook exited with code 2".to_string()), - } - } else { - HookDecision::Block { - reason: Some(format!("hook exited with code {exit_code}")), - } - } - } - - /// Resolve the prompt and optional model segments at fire time. - /// - /// Fail-closed: an unresolved token is a hard error so the hook never - /// fires with a half-resolved value. The caller turns the error into a - /// `Block` decision, matching the command-hook behavior. - fn resolve_prompt_and_model( - prompt: &InterpString, - model: Option<&InterpString>, - ) -> Result<(String, Option), ResolveError> { - let prompt = resolve_interp(prompt)?; - let model = model.map(resolve_interp).transpose()?; - Ok((prompt, model)) - } - - /// Execute a command hook (sandbox or host). - async fn execute_command( - definition: &HookDefinition, - command: &InterpString, - context: &HookContext, - sandbox: &Arc, - execution_context: &HookExecutionContext, - ) -> HookDecision { - let command = match resolve_interp(command) { - Ok(command) => command, - Err(error) => { - return HookDecision::Block { - reason: Some(error.to_string()), - }; - } - }; - let context_json = serde_json::to_string(context).unwrap_or_default(); - let timeout_ms = duration_ms(definition.timeout()); - - let mut env_vars = HashMap::new(); - env_vars.insert("FABRO_EVENT".to_string(), context.event.to_string()); - env_vars.insert("FABRO_RUN_ID".to_string(), context.run_id.to_string()); - env_vars.insert("FABRO_WORKFLOW".to_string(), context.workflow_name.clone()); - if let Some(ref node_id) = context.node_id { - env_vars.insert("FABRO_NODE_ID".to_string(), node_id.clone()); - } - - if definition.runs_in_sandbox() { - let ctx_path = format!( - "/tmp/fabro-hook-context-{}.json", - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap_or_default() - .as_nanos() - ); - if sandbox.write_file(&ctx_path, &context_json).await.is_ok() { - env_vars.insert("FABRO_HOOK_CONTEXT".to_string(), ctx_path.clone()); - } - let sandbox_work_dir = execution_context - .command_cwd_for(definition) - .map(|path| path.to_string_lossy().to_string()); - match sandbox - .exec_command( - &command, - timeout_ms, - sandbox_work_dir.as_deref(), - Some(&env_vars), - None, - ) - .await - { - Ok(result) => Self::parse_decision( - result.program_exit_code().unwrap_or(-1), - &result.stdout_lossy(), - ), - Err(e) => HookDecision::Block { - reason: Some(format!("sandbox exec failed: {e}")), - }, - } - } else { - let mut cmd = TokioCommand::new("sh"); - cmd.arg("-c").arg(&command); - if let Some(wd) = execution_context.command_cwd_for(definition) { - cmd.current_dir(wd); - } - for (k, v) in &env_vars { - cmd.env(k, v); - } - cmd.stdin(std::process::Stdio::piped()); - cmd.stdout(std::process::Stdio::piped()); - cmd.stderr(std::process::Stdio::piped()); - - match cmd.spawn() { - Ok(mut child) => { - if let Some(mut stdin) = child.stdin.take() { - use tokio::io::AsyncWriteExt; - let _ = stdin.write_all(context_json.as_bytes()).await; - } - match child.wait_with_output().await { - Ok(output) => { - let exit_code = output.status.code().unwrap_or(1); - let stdout = String::from_utf8_lossy(&output.stdout); - Self::parse_decision(exit_code, &stdout) - } - Err(e) => HookDecision::Block { - reason: Some(format!("command wait failed: {e}")), - }, - } - } - Err(e) => HookDecision::Block { - reason: Some(format!("command spawn failed: {e}")), - }, - } - } - } - - /// Strip markdown code fences from LLM responses. - /// - /// LLMs often wrap JSON in ```json ... ``` blocks. - fn strip_code_fences(text: &str) -> &str { - let trimmed = text.trim(); - let inner = trimmed - .strip_prefix("```json") - .or_else(|| trimmed.strip_prefix("```")) - .unwrap_or(trimmed); - let inner = inner.strip_suffix("```").unwrap_or(inner); - inner.trim() - } - - /// Parse a prompt/agent hook LLM response into a `HookDecision`. - /// - /// Fail-open: invalid JSON or missing fields → `Proceed`. - pub fn parse_prompt_response(response_text: &str) -> HookDecision { - let cleaned = Self::strip_code_fences(response_text); - match serde_json::from_str::(cleaned) { - Ok(resp) if resp.ok => HookDecision::Proceed, - Ok(resp) => HookDecision::Block { - reason: resp.reason, - }, - Err(e) => { - tracing::warn!(error = %e, "prompt hook response parse failed, proceeding"); - HookDecision::Proceed - } - } - } - - /// Keep the requested selector intact so the ready-provider-aware LLM - /// client can resolve aliases at dispatch time. - fn resolve_model(model: Option<&str>) -> String { - model.unwrap_or("haiku").to_string() - } - - /// Build the user message for prompt/agent hooks. - fn build_hook_user_message(prompt: &str, context: &HookContext) -> String { - let context_json = serde_json::to_string(context).unwrap_or_default(); - format!("Hook prompt: {prompt}\n\nEvent context:\n{context_json}") - } - - /// Execute an LLM hook with a timeout, failing open on error or timeout. - async fn execute_llm_with_timeout( - timeout: std::time::Duration, - hook_kind: &str, - f: F, - ) -> HookDecision - where - F: FnOnce() -> Fut, - Fut: std::future::Future, - { - if let Ok(decision) = tokio_timeout(timeout, f()).await { - decision - } else { - tracing::warn!("{hook_kind} hook timed out, proceeding"); - HookDecision::Proceed - } - } - - /// Execute a prompt hook: single-turn LLM call returning ok/block. - async fn execute_prompt( - definition: &HookDefinition, - prompt: &InterpString, - model: Option<&InterpString>, - context: &HookContext, - llm_source: Arc, - catalog: Arc, - ) -> HookDecision { - let (prompt, model) = match Self::resolve_prompt_and_model(prompt, model) { - Ok(resolved) => resolved, - Err(error) => { - tracing::error!(error = %error, "prompt hook interpolation failed, not firing"); - return HookDecision::Block { - reason: Some(error.to_string()), - }; - } - }; - - let resolved_model = Self::resolve_model(model.as_deref()); - let user_msg = Self::build_hook_user_message(&prompt, context); - - Self::execute_llm_with_timeout(definition.timeout(), "prompt", || async move { - let client = match Self::build_client(catalog, llm_source).await { - Ok(client) => client, - Err(e) => { - tracing::warn!(error = %e, "prompt hook client creation failed, proceeding"); - return HookDecision::Proceed; - } - }; - - let request = Request::builder() - .model(&resolved_model) - .system(HOOK_EVALUATOR_SYSTEM_PROMPT) - .user(user_msg) - .max_output_tokens(1024) - .build(); - let request = match request { - Ok(request) => request, - Err(e) => { - tracing::warn!(error = %e, "prompt hook request invalid, proceeding"); - return HookDecision::Proceed; - } - }; - - match client - .complete_object(request, "hook_response", HOOK_RESPONSE_SCHEMA.clone()) - .await - { - Ok(completion) => { - match serde_json::from_value::(completion.object) { - Ok(resp) if resp.ok => HookDecision::Proceed, - Ok(resp) => HookDecision::Block { - reason: resp.reason, - }, - Err(e) => { - tracing::warn!(error = %e, "prompt hook response deserialize failed, proceeding"); - HookDecision::Proceed - } - } - } - Err(e) => { - tracing::warn!(error = %e, "prompt hook LLM call failed, proceeding"); - HookDecision::Proceed - } - } - }) - .await - } - - /// Execute an agent hook: a coding agent evaluates the condition with the - /// sandbox's tools and answers with the same `{ok, reason}` object as a - /// prompt hook. - /// - /// The agent runs pebble's full tool set at `PermissionLevel::Full`, with - /// no memory or skills, the evaluator system prompt in place of the - /// profile's, and `max_tool_rounds` as pebble's tool-round budget. - /// Exhausting the budget, an LLM failure, or a timeout all fail open. - /// - /// Fabro's `max_tool_rounds` names how many model turns the hook may - /// take, executing the tools each asks for, before it proceeds on a turn - /// that still asks for tools. Pebble's budget of `rounds` lets `rounds` - /// tool turns run and refuses the next one without running its tools, so - /// `max_tool_rounds - 1` reaches the same decision at the same turn and - /// spares the last, useless tool execution. Zero is a loop that never - /// asks the model: the hook proceeds without an agent. - async fn execute_agent( - definition: &HookDefinition, - prompt: &InterpString, - model: Option<&InterpString>, - max_tool_rounds: Option, - context: &HookContext, - sandbox: Arc, - llm_source: Arc, - catalog: Arc, - ) -> HookDecision { - let (prompt, model) = match Self::resolve_prompt_and_model(prompt, model) { - Ok(resolved) => resolved, - Err(error) => { - tracing::error!(error = %error, "agent hook interpolation failed, not firing"); - return HookDecision::Block { - reason: Some(error.to_string()), - }; - } - }; - - let resolved_model = Self::resolve_model(model.as_deref()); - let user_msg = Self::build_hook_user_message(&prompt, context); - let Some(rounds) = max_tool_rounds - .unwrap_or(DEFAULT_MAX_TOOL_ROUNDS) - .checked_sub(1) - else { - tracing::warn!("agent hook allows no tool rounds, proceeding"); - return HookDecision::Proceed; - }; - let rounds = usize::try_from(rounds).unwrap_or(usize::MAX); - - Self::execute_llm_with_timeout(definition.timeout(), "agent", || async move { - let client = match Self::build_client(catalog, llm_source).await { - Ok(c) => c, - Err(e) => { - tracing::warn!(error = %e, "agent hook client creation failed, proceeding"); - return HookDecision::Proceed; - } - }; - - let options = CodingAgentOptions::default() - .with_context_compaction(false) - .with_max_tool_rounds(rounds); - let mut agent = match CodingAgent::builder(client, sandbox) - .model(resolved_model) - .permission_level(PermissionLevel::Full) - .system_prompt_transform(Arc::new(HookEvaluatorPrompt)) - .redactor(Arc::new(SecretRedactor)) - .options(options) - .build() - .await - { - Ok(agent) => agent, - Err(e) => { - tracing::warn!(error = %e, "agent hook agent build failed, proceeding"); - return HookDecision::Proceed; - } - }; - - let report = agent.prompt(user_msg).await; - let decision = match report.result { - Ok(output) => Self::parse_prompt_response(output.text.as_deref().unwrap_or("")), - Err(AgentError::ToolRoundsExhausted { .. }) => { - tracing::warn!("agent hook exhausted max tool rounds, proceeding"); - HookDecision::Proceed - } - Err(e) => { - tracing::warn!(error = %e, "agent hook did not complete, proceeding"); - HookDecision::Proceed - } - }; - if let Err(e) = agent.shutdown(ShutdownReason::Completed).await { - tracing::debug!(error = %e, "agent hook session did not shut down cleanly"); - } - decision - }) - .await - } - - /// The LLM client hooks dispatch through: every provider the source can - /// serve, with standard retries. - async fn build_client( - catalog: Arc, - llm_source: Arc, - ) -> Result { - fabro_llm::build_client( - Catalog::clone(&catalog), - llm_source, - ClientOptions::standard(), - ) - .await - .map(|built| built.client) - } - - /// Build an HTTP client for the given TLS mode. - fn build_http_client(tls: TlsMode) -> fabro_http::HttpClient { - let accept_invalid = matches!(tls, TlsMode::NoVerify | TlsMode::Off); - #[cfg(test)] - { - fabro_http::HttpClientBuilder::new() - .danger_accept_invalid_certs(accept_invalid) - .no_proxy() - .build() - .expect("hook HTTP client should build") - } - #[cfg(not(test))] - { - fabro_http::HttpClientBuilder::new() - .danger_accept_invalid_certs(accept_invalid) - .build() - .expect("hook HTTP client should build") - } - } - - /// Execute an HTTP hook: POST context JSON and parse the response. - /// - /// Token resolution is fail-closed: a missing or out-of-scope token in the - /// URL or a header is a hard `Block`, so the hook never fires with a - /// half-resolved URL or an empty credential header. Transport outcomes - /// (non-2xx, connection errors, unparseable body) stay fail-open and - /// return `Proceed`. - async fn execute_http( - client: &fabro_http::HttpClient, - url: &InterpString, - headers: Option<&HashMap>, - tls: &TlsMode, - context: &HookContext, - timeout: std::time::Duration, - ) -> HookDecision { - let resolved_url = match resolve_interp(url) { - Ok(url) => url, - Err(error) => { - tracing::error!( - url_source = %safe_url_source_for_log(url), - error = %error, - "HTTP hook URL interpolation failed, not firing" - ); - return HookDecision::Block { - reason: Some(error.to_string()), - }; - } - }; - - // Enforce URL scheme based on TLS mode - match tls { - TlsMode::Verify | TlsMode::NoVerify => { - if !resolved_url.starts_with("https://") { - return HookDecision::Block { - reason: Some(format!( - "HTTP hook URL must use https:// (tls mode is {tls:?})" - )), - }; - } - } - TlsMode::Off => {} - } - - let mut request = client.post(&resolved_url).timeout(timeout).json(context); - - if let Some(hdrs) = headers { - for (key, value) in hdrs { - let interpolated = match resolve_interp(value) { - Ok(rendered) => rendered, - Err(error) => { - tracing::error!( - url_source = %safe_url_source_for_log(url), - header = %key, - error = %error, - "HTTP hook header interpolation failed, not firing" - ); - return HookDecision::Block { - reason: Some(error.to_string()), - }; - } - }; - request = request.header(key, interpolated); - } - } - - let response = match request.send().await { - Ok(resp) => resp, - Err(e) => { - tracing::warn!( - url_source = %safe_url_source_for_log(url), - error = %e, - "HTTP hook request failed, proceeding" - ); - return HookDecision::Proceed; - } - }; - - if !response.status().is_success() { - tracing::warn!( - url_source = %safe_url_source_for_log(url), - status = response.status().as_u16(), - "HTTP hook returned non-2xx, proceeding" - ); - return HookDecision::Proceed; - } - - let body = match response.text().await { - Ok(text) => text, - Err(e) => { - tracing::warn!( - url_source = %safe_url_source_for_log(url), - error = %e, - "HTTP hook body read failed, proceeding" - ); - return HookDecision::Proceed; - } - }; - - if body.trim().is_empty() { - return HookDecision::Proceed; - } - - match serde_json::from_str::(body.trim()) { - Ok(decision) => decision, - Err(e) => { - tracing::warn!( - url_source = %safe_url_source_for_log(url), - error = %e, - "HTTP hook response parse failed, proceeding" - ); - HookDecision::Proceed - } - } - } -} - -/// Cached HTTP clients keyed by TLS mode. -struct HttpClientCache { - verify: fabro_http::HttpClient, - no_verify: fabro_http::HttpClient, - off: fabro_http::HttpClient, -} - -impl HttpClientCache { - fn new() -> Self { - Self { - verify: HookExecutorImpl::build_http_client(TlsMode::Verify), - no_verify: HookExecutorImpl::build_http_client(TlsMode::NoVerify), - off: HookExecutorImpl::build_http_client(TlsMode::Off), - } - } - - fn get(&self, tls: TlsMode) -> &fabro_http::HttpClient { - match tls { - TlsMode::Verify => &self.verify, - TlsMode::NoVerify => &self.no_verify, - TlsMode::Off => &self.off, - } - } -} - -impl Default for HttpClientCache { - fn default() -> Self { - Self::new() - } -} - -#[async_trait] -impl HookExecutor for HookExecutorImpl { - async fn execute( - &self, - definition: &HookDefinition, - context: &HookContext, - sandbox: Arc, - execution_context: &HookExecutionContext, - llm_source: Arc, - catalog: Arc, - ) -> HookResult { - use std::sync::OnceLock; - static HTTP_CLIENTS: OnceLock = OnceLock::new(); - - let start = Instant::now(); - - let decision = match definition.resolved_hook_type() { - Some( - Cow::Borrowed(HookType::Command { ref command }) - | Cow::Owned(HookType::Command { ref command }), - ) => { - Self::execute_command(definition, command, context, &sandbox, execution_context) - .await - } - Some( - Cow::Borrowed(HookType::Http { - ref url, - ref headers, - ref tls, - }) - | Cow::Owned(HookType::Http { - ref url, - ref headers, - ref tls, - }), - ) => { - let clients = HTTP_CLIENTS.get_or_init(HttpClientCache::new); - Self::execute_http( - clients.get(*tls), - url, - headers.as_ref(), - tls, - context, - definition.timeout(), - ) - .await - } - Some( - Cow::Borrowed(HookType::Prompt { - ref prompt, - ref model, - }) - | Cow::Owned(HookType::Prompt { - ref prompt, - ref model, - }), - ) => { - Self::execute_prompt( - definition, - prompt, - model.as_ref(), - context, - llm_source, - Arc::clone(&catalog), - ) - .await - } - Some( - Cow::Borrowed(HookType::Agent { - ref prompt, - ref model, - ref max_tool_rounds, - }) - | Cow::Owned(HookType::Agent { - ref prompt, - ref model, - ref max_tool_rounds, - }), - ) => { - Self::execute_agent( - definition, - prompt, - model.as_ref(), - *max_tool_rounds, - context, - sandbox, - llm_source, - Arc::clone(&catalog), - ) - .await - } - None => HookDecision::Block { - reason: Some("no hook type specified".into()), - }, - }; - - let duration_ms = duration_ms(start.elapsed()); - HookResult { - hook_name: definition.name.clone(), - decision, - duration_ms, - } - } -} - -#[cfg(test)] -mod tests { - use fabro_auth::test_support; - use fabro_llm::credentials::CredentialProvider; - use fabro_types::fixtures; - use fabro_types::settings::ResolveErrorKind; - - use super::*; - use crate::config::HookType; - use crate::types::HookEvent; - - fn make_context() -> HookContext { - HookContext::new(HookEvent::StageStart, fixtures::RUN_1, "test-wf".into()) - } - - async fn make_sandbox() -> Arc { - Arc::new( - fabro_sandbox::local_sandbox(std::env::current_dir().unwrap()) - .await - .unwrap(), - ) - } - - fn test_llm_source() -> Arc { - test_support::vault_only_credential_source() - } - - fn test_catalog() -> Arc { - Arc::new(fabro_llm::default_catalog()) - } - - fn test_http_client() -> fabro_http::HttpClient { - HookExecutorImpl::build_http_client(TlsMode::Off) - } - - fn make_definition(command: &str) -> HookDefinition { - HookDefinition { - name: Some("test-hook".into()), - event: HookEvent::StageStart, - command: Some(command.into()), - hook_type: None, - matcher: None, - blocking: None, - timeout_ms: Some(5000), - sandbox: Some(false), // host execution for tests - } - } - - #[test] - fn parse_decision_exit_0_proceed() { - assert_eq!( - HookExecutorImpl::parse_decision(0, ""), - HookDecision::Proceed - ); - } - - #[test] - fn parse_decision_exit_0_with_json() { - let json = r#"{"decision": "skip", "reason": "not needed"}"#; - assert_eq!( - HookExecutorImpl::parse_decision(0, json), - HookDecision::Skip { - reason: Some("not needed".into()), - } - ); - } - - #[test] - fn parse_decision_exit_2_block() { - assert!(matches!( - HookExecutorImpl::parse_decision(2, ""), - HookDecision::Block { .. } - )); - } - - #[test] - fn parse_decision_exit_2_with_json() { - let json = r#"{"decision": "skip", "reason": "skipping"}"#; - assert_eq!( - HookExecutorImpl::parse_decision(2, json), - HookDecision::Skip { - reason: Some("skipping".into()), - } - ); - } - - #[test] - fn parse_decision_exit_1_block() { - assert!(matches!( - HookExecutorImpl::parse_decision(1, ""), - HookDecision::Block { .. } - )); - } - - #[test] - fn parse_decision_exit_0_override() { - let json = r#"{"decision": "override", "edge_to": "node_b"}"#; - assert_eq!( - HookExecutorImpl::parse_decision(0, json), - HookDecision::Override { - edge_to: "node_b".into(), - } - ); - } - - #[tokio::test] - async fn command_executor_host_success() { - let executor = HookExecutorImpl; - let def = make_definition("exit 0"); - let ctx = make_context(); - let sandbox = make_sandbox().await; - let source = test_llm_source(); - let result = executor - .execute( - &def, - &ctx, - sandbox, - &HookExecutionContext::default(), - Arc::clone(&source), - test_catalog(), - ) - .await; - assert_eq!(result.decision, HookDecision::Proceed); - assert_eq!(result.hook_name.as_deref(), Some("test-hook")); - } - - #[tokio::test] - async fn command_executor_host_failure() { - let executor = HookExecutorImpl; - let def = make_definition("exit 1"); - let ctx = make_context(); - let sandbox = make_sandbox().await; - let source = test_llm_source(); - let result = executor - .execute( - &def, - &ctx, - sandbox, - &HookExecutionContext::default(), - Arc::clone(&source), - test_catalog(), - ) - .await; - assert!(matches!(result.decision, HookDecision::Block { .. })); - } - - #[tokio::test] - async fn command_executor_host_skip_via_exit_2() { - let executor = HookExecutorImpl; - let def = make_definition("exit 2"); - let ctx = make_context(); - let sandbox = make_sandbox().await; - let source = test_llm_source(); - let result = executor - .execute( - &def, - &ctx, - sandbox, - &HookExecutionContext::default(), - Arc::clone(&source), - test_catalog(), - ) - .await; - assert!(matches!(result.decision, HookDecision::Block { .. })); - } - - #[tokio::test] - async fn command_executor_host_json_decision() { - let executor = HookExecutorImpl; - let def = make_definition(r#"echo '{"decision": "skip", "reason": "test skip"}'"#); - let ctx = make_context(); - let sandbox = make_sandbox().await; - let source = test_llm_source(); - let result = executor - .execute( - &def, - &ctx, - sandbox, - &HookExecutionContext::default(), - Arc::clone(&source), - test_catalog(), - ) - .await; - assert_eq!(result.decision, HookDecision::Skip { - reason: Some("test skip".into()), - }); - } - - #[tokio::test] - async fn command_executor_env_vars_set() { - let executor = HookExecutorImpl; - // Print env vars to stdout for verification - let def = make_definition("echo $ARC_EVENT:$ARC_RUN_ID:$ARC_WORKFLOW"); - let mut ctx = make_context(); - ctx.node_id = Some("plan".into()); - let sandbox = make_sandbox().await; - let source = test_llm_source(); - let result = executor - .execute( - &def, - &ctx, - sandbox, - &HookExecutionContext::default(), - Arc::clone(&source), - test_catalog(), - ) - .await; - assert_eq!(result.decision, HookDecision::Proceed); - } - - #[tokio::test] - async fn no_hook_type_blocks() { - let executor = HookExecutorImpl; - let def = HookDefinition { - name: None, - event: HookEvent::StageStart, - command: None, - hook_type: None, - matcher: None, - blocking: None, - timeout_ms: None, - sandbox: Some(false), - }; - let ctx = make_context(); - let sandbox = make_sandbox().await; - let source = test_llm_source(); - let result = executor - .execute( - &def, - &ctx, - sandbox, - &HookExecutionContext::default(), - Arc::clone(&source), - test_catalog(), - ) - .await; - assert!(matches!(result.decision, HookDecision::Block { .. })); - } - - // --- parse_prompt_response tests --- - - #[test] - fn parse_prompt_response_ok_true() { - assert_eq!( - HookExecutorImpl::parse_prompt_response(r#"{"ok": true}"#), - HookDecision::Proceed, - ); - } - - #[test] - fn parse_prompt_response_ok_false() { - assert_eq!( - HookExecutorImpl::parse_prompt_response(r#"{"ok": false, "reason": "tests failing"}"#), - HookDecision::Block { - reason: Some("tests failing".into()), - }, - ); - } - - #[test] - fn parse_prompt_response_ok_false_no_reason() { - assert_eq!( - HookExecutorImpl::parse_prompt_response(r#"{"ok": false}"#), - HookDecision::Block { reason: None }, - ); - } - - #[test] - fn parse_prompt_response_invalid_json() { - assert_eq!( - HookExecutorImpl::parse_prompt_response("not json"), - HookDecision::Proceed, - ); - } - - #[test] - fn parse_prompt_response_strips_code_fences() { - assert_eq!( - HookExecutorImpl::parse_prompt_response( - "```json\n{\"ok\": false, \"reason\": \"no\"}\n```" - ), - HookDecision::Block { - reason: Some("no".into()), - }, - ); - } - - #[test] - fn strip_code_fences_plain() { - assert_eq!( - HookExecutorImpl::strip_code_fences(r#"{"ok": true}"#), - r#"{"ok": true}"# - ); - } - - #[test] - fn strip_code_fences_json() { - assert_eq!( - HookExecutorImpl::strip_code_fences("```json\n{\"ok\": true}\n```"), - "{\"ok\": true}" - ); - } - - #[test] - fn strip_code_fences_bare() { - assert_eq!( - HookExecutorImpl::strip_code_fences("```\n{\"ok\": true}\n```"), - "{\"ok\": true}" - ); - } - - // --- hook segment resolution helpers --- - - fn interp(value: &str) -> InterpString { - InterpString::parse(value) - } - - #[test] - fn safe_url_source_for_log_redacts_parseable_url_source() { - let safe = safe_url_source_for_log(&interp( - "https://user:secret@example.com/hook?token=literal&keep=value", - )); - - assert_eq!( - safe, - "https://user:****@example.com/hook?token=****&keep=value" - ); - } - - #[test] - fn safe_url_source_for_log_hides_unparseable_url_source() { - let safe = safe_url_source_for_log(&interp("{{ env.FABRO_TEST_HOOK_URL }}")); - - assert_eq!(safe, ""); - } - - /// Hook values are resolved from their typed segments at fire time, never - /// via a String -> InterpString re-parse. `{{ vars.* }}` is already - /// substituted server-side, so a literal value passes straight through. - #[test] - fn resolve_interp_passes_through_literal_values() { - assert_eq!(resolve_interp(&interp("plain text")).unwrap(), "plain text"); - assert_eq!( - resolve_interp(&interp("Bearer already-substituted")).unwrap(), - "Bearer already-substituted" - ); - } - - /// Fail-closed: a token that survived to fire time can never resolve, so a - /// hook referencing one blocks rather than sending a half-rendered header. - #[test] - fn resolve_interp_errors_on_an_unresolved_token() { - let err = resolve_interp(&interp("a{{ env.FABRO_TEST_NOEXIST }}-b")).unwrap_err(); - assert_eq!(err.name, "FABRO_TEST_NOEXIST"); - assert_eq!(err.kind, ResolveErrorKind::Unavailable); - - let err = resolve_interp(&interp("Bearer {{ secrets.API_KEY }}")).unwrap_err(); - assert_eq!(err.name, "API_KEY"); - assert_eq!(err.kind, ResolveErrorKind::Unavailable); - } - - // --- HTTP hook execution tests --- - - #[tokio::test] - async fn http_hook_posts_json_and_parses_decision() { - let server = httpmock::MockServer::start_async().await; - let mock = server - .mock_async(|when, then| { - when.method("POST") - .path("/hook") - .header("content-type", "application/json"); - then.status(200) - .body(r#"{"decision": "skip", "reason": "not needed"}"#); - }) - .await; - - let client = test_http_client(); - let decision = HookExecutorImpl::execute_http( - &client, - &interp(&server.url("/hook")), - None, - &TlsMode::Off, - &make_context(), - std::time::Duration::from_secs(5), - ) - .await; - - mock.assert_async().await; - assert_eq!(decision, HookDecision::Skip { - reason: Some("not needed".into()), - }); - } - - #[tokio::test] - async fn http_hook_empty_2xx_returns_proceed() { - let server = httpmock::MockServer::start_async().await; - let mock = server - .mock_async(|when, then| { - when.method("POST").path("/hook"); - then.status(200).body(""); - }) - .await; - - let client = test_http_client(); - let decision = HookExecutorImpl::execute_http( - &client, - &interp(&server.url("/hook")), - None, - &TlsMode::Off, - &make_context(), - std::time::Duration::from_secs(5), - ) - .await; - - mock.assert_async().await; - assert_eq!(decision, HookDecision::Proceed); - } - - #[tokio::test] - async fn http_hook_non_2xx_returns_proceed() { - let server = httpmock::MockServer::start_async().await; - let mock = server - .mock_async(|when, then| { - when.method("POST").path("/hook"); - then.status(500).body("Internal Server Error"); - }) - .await; - - let client = test_http_client(); - let decision = HookExecutorImpl::execute_http( - &client, - &interp(&server.url("/hook")), - None, - &TlsMode::Off, - &make_context(), - std::time::Duration::from_secs(5), - ) - .await; - - mock.assert_async().await; - assert_eq!(decision, HookDecision::Proceed); - } - - #[tokio::test] - async fn http_hook_connection_failure_returns_proceed() { - let client = test_http_client(); - let decision = HookExecutorImpl::execute_http( - &client, - &interp("http://127.0.0.1:1"), - None, - &TlsMode::Off, - &make_context(), - std::time::Duration::from_secs(1), - ) - .await; - - assert_eq!(decision, HookDecision::Proceed); - } - - /// `{{ vars.* }}` is substituted server-side, so a header arrives literal - /// and is sent as-is. - #[tokio::test] - async fn http_hook_sends_substituted_headers() { - let server = httpmock::MockServer::start_async().await; - let mock = server - .mock_async(|when, then| { - when.method("POST") - .path("/hook") - .header("authorization", "Bearer my-secret"); - then.status(200).body(""); - }) - .await; - - let headers = HashMap::from([("Authorization".to_string(), interp("Bearer my-secret"))]); - - let client = test_http_client(); - let decision = HookExecutorImpl::execute_http( - &client, - &interp(&server.url("/hook")), - Some(&headers), - &TlsMode::Off, - &make_context(), - std::time::Duration::from_secs(5), - ) - .await; - - mock.assert_async().await; - assert_eq!(decision, HookDecision::Proceed); - } - - /// Fail-closed: `{{ env.* }}` no longer resolves anywhere, so a header - /// referencing one must block rather than send a half-rendered credential. - #[tokio::test] - async fn http_hook_env_header_token_blocks_without_firing() { - let server = httpmock::MockServer::start_async().await; - let mock = server - .mock_async(|when, then| { - when.method("POST").path("/hook"); - then.status(200).body(""); - }) - .await; - - let headers = HashMap::from([( - "Authorization".to_string(), - interp("Bearer {{ env.FABRO_TEST_TOKEN }}"), - )]); - - let client = test_http_client(); - let decision = HookExecutorImpl::execute_http( - &client, - &interp(&server.url("/hook")), - Some(&headers), - &TlsMode::Off, - &make_context(), - std::time::Duration::from_secs(5), - ) - .await; - - assert_eq!(mock.calls_async().await, 0); - match decision { - HookDecision::Block { reason } => { - assert!( - reason - .as_deref() - .is_some_and(|reason| reason.contains("FABRO_TEST_TOKEN")), - "block reason should name the token, got: {reason:?}" - ); - } - other => panic!("expected Block on env header token, got {other:?}"), - } - } - - #[tokio::test] - async fn http_hook_dispatches_a_substituted_url() { - let server = httpmock::MockServer::start_async().await; - let mock = server - .mock_async(|when, then| { - when.method("POST").path("/hook"); - then.status(200).body(""); - }) - .await; - - let client = test_http_client(); - let decision = HookExecutorImpl::execute_http( - &client, - &interp(&server.url("/hook")), - None, - &TlsMode::Off, - &make_context(), - std::time::Duration::from_secs(5), - ) - .await; - - mock.assert_async().await; - assert_eq!(decision, HookDecision::Proceed); - } - - #[tokio::test] - async fn http_hook_missing_url_token_blocks_without_firing() { - let server = httpmock::MockServer::start_async().await; - let mock = server - .mock_async(|when, then| { - when.method("POST").path("/hook"); - then.status(200).body(""); - }) - .await; - - let client = test_http_client(); - let decision = HookExecutorImpl::execute_http( - &client, - &interp("{{ env.FABRO_TEST_MISSING_URL }}/hook"), - None, - &TlsMode::Off, - &make_context(), - std::time::Duration::from_secs(5), - ) - .await; - - // Fail-closed: the missing token must not fire the hook at all. - assert_eq!(mock.calls_async().await, 0); - match decision { - HookDecision::Block { reason } => { - assert!( - reason - .as_deref() - .is_some_and(|reason| reason.contains("FABRO_TEST_MISSING_URL")), - "block reason should name the missing token, got: {reason:?}" - ); - } - other => panic!("expected Block on missing url token, got {other:?}"), - } - } - - #[tokio::test] - async fn http_hook_missing_header_token_blocks_without_firing() { - let server = httpmock::MockServer::start_async().await; - let mock = server - .mock_async(|when, then| { - when.method("POST").path("/hook"); - then.status(200).body(""); - }) - .await; - - let headers = HashMap::from([( - "Authorization".to_string(), - interp("Bearer {{ env.FABRO_TEST_MISSING_HEADER }}"), - )]); - - let client = test_http_client(); - let decision = HookExecutorImpl::execute_http( - &client, - &interp(&server.url("/hook")), - Some(&headers), - &TlsMode::Off, - &make_context(), - std::time::Duration::from_secs(5), - ) - .await; - - // Fail-closed: a missing header token must not fire the hook with an - // empty credential header. - assert_eq!(mock.calls_async().await, 0); - assert!(matches!(decision, HookDecision::Block { .. })); - } - - // --- TLS mode enforcement tests --- - - #[tokio::test] - async fn http_hook_rejects_http_url_when_tls_verify() { - let client = test_http_client(); - let decision = HookExecutorImpl::execute_http( - &client, - &interp("http://example.com/hook"), - None, - &TlsMode::Verify, - &make_context(), - std::time::Duration::from_secs(5), - ) - .await; - - assert!(matches!(decision, HookDecision::Block { .. })); - } - - #[tokio::test] - async fn http_hook_rejects_http_url_when_tls_no_verify() { - let client = test_http_client(); - let decision = HookExecutorImpl::execute_http( - &client, - &interp("http://example.com/hook"), - None, - &TlsMode::NoVerify, - &make_context(), - std::time::Duration::from_secs(5), - ) - .await; - - assert!(matches!(decision, HookDecision::Block { .. })); - } - - #[tokio::test] - async fn http_hook_allows_http_url_when_tls_off() { - let server = httpmock::MockServer::start_async().await; - let mock = server - .mock_async(|when, then| { - when.method("POST").path("/hook"); - then.status(200).body(""); - }) - .await; - - let client = test_http_client(); - let decision = HookExecutorImpl::execute_http( - &client, - &interp(&server.url("/hook")), - None, - &TlsMode::Off, - &make_context(), - std::time::Duration::from_secs(5), - ) - .await; - - mock.assert_async().await; - assert_eq!(decision, HookDecision::Proceed); - } - - #[tokio::test] - async fn executor_dispatches_http_hook() { - let server = httpmock::MockServer::start_async().await; - let mock = server - .mock_async(|when, then| { - when.method("POST").path("/hook"); - then.status(200).body(r#"{"decision": "proceed"}"#); - }) - .await; - - let executor = HookExecutorImpl; - let def = HookDefinition { - name: Some("http-test".into()), - event: HookEvent::StageStart, - command: None, - hook_type: Some(HookType::Http { - url: interp(&server.url("/hook")), - headers: None, - tls: TlsMode::Off, - }), - matcher: None, - blocking: None, - timeout_ms: Some(5000), - sandbox: Some(false), - }; - let ctx = make_context(); - let sandbox = make_sandbox().await; - let source = test_llm_source(); - let result = executor - .execute( - &def, - &ctx, - sandbox, - &HookExecutionContext::default(), - Arc::clone(&source), - test_catalog(), - ) - .await; - - mock.assert_async().await; - assert_eq!(result.decision, HookDecision::Proceed); - assert_eq!(result.hook_name.as_deref(), Some("http-test")); - } - - #[tokio::test] - async fn command_hook_missing_env_blocks() { - let sandbox = make_sandbox().await; - let decision = HookExecutorImpl::execute_command( - &make_definition("echo {{ env.MISSING_HOOK_VALUE }}"), - &interp("echo {{ env.MISSING_HOOK_VALUE }}"), - &make_context(), - &sandbox, - &HookExecutionContext::default(), - ) - .await; - - assert!(matches!(decision, HookDecision::Block { .. })); - } - - // Fail-closed: a prompt hook with a missing token does not fire the LLM - // call; it blocks with the resolution error, matching command hooks. - #[tokio::test] - async fn prompt_hook_missing_env_blocks() { - let decision = HookExecutorImpl::execute_prompt( - &make_definition("unused"), - &interp("{{ env.MISSING_HOOK_VALUE }}"), - None, - &make_context(), - test_llm_source(), - test_catalog(), - ) - .await; - - match decision { - HookDecision::Block { reason } => { - assert!( - reason - .as_deref() - .is_some_and(|reason| reason.contains("MISSING_HOOK_VALUE")), - "block reason should name the missing token, got: {reason:?}" - ); - } - other => panic!("expected Block on missing prompt token, got {other:?}"), - } - } - - // Fail-closed: an agent hook with a missing token blocks instead of firing. - #[tokio::test] - async fn agent_hook_missing_env_blocks() { - let decision = HookExecutorImpl::execute_agent( - &make_definition("unused"), - &interp("{{ env.MISSING_HOOK_VALUE }}"), - None, - Some(1), - &make_context(), - make_sandbox().await, - test_llm_source(), - test_catalog(), - ) - .await; - - assert!(matches!(decision, HookDecision::Block { .. })); - } -} diff --git a/lib/components/fabro-hooks/src/lib.rs b/lib/components/fabro-hooks/src/lib.rs deleted file mode 100644 index 79a77bb98..000000000 --- a/lib/components/fabro-hooks/src/lib.rs +++ /dev/null @@ -1,13 +0,0 @@ -pub mod bridge; -pub mod config; -pub mod executor; -pub mod runner; -pub mod types; - -pub use bridge::WorkflowToolHookCallback; -pub use config::{HookDefinition, HookSettings, HookType, TlsMode}; -// Re-exported because the interpolatable fields of `HookType` are typed as -// `InterpString`; constructing a hook definition requires it. -pub use fabro_types::settings::InterpString; -pub use runner::HookRunner; -pub use types::{HookContext, HookDecision, HookEvent, HookExecutionContext}; diff --git a/lib/components/fabro-hooks/src/runner.rs b/lib/components/fabro-hooks/src/runner.rs deleted file mode 100644 index 683ce160b..000000000 --- a/lib/components/fabro-hooks/src/runner.rs +++ /dev/null @@ -1,504 +0,0 @@ -use std::collections::HashMap; -use std::sync::Arc; - -#[cfg(test)] -use fabro_auth::test_support; -use fabro_llm::credentials::CredentialProvider; -use fabro_llm::lithos_catalog::Catalog; -use fabro_sandbox::RunSandbox; - -use crate::config::{HookDefinition, HookSettings}; -use crate::executor::{HookExecutor, HookExecutorImpl}; -use crate::types::{HookContext, HookDecision, HookExecutionContext}; - -/// Central orchestrator: filters matching hooks, executes them, merges -/// decisions. -pub struct HookRunner { - config: HookSettings, - executor: Arc, - llm_source: Arc, - catalog: Arc, - /// Pre-compiled regexes keyed by matcher pattern string. - compiled_matchers: HashMap, -} - -impl HookRunner { - #[must_use] - pub fn new( - config: HookSettings, - llm_source: Arc, - catalog: Arc, - ) -> Self { - let compiled_matchers = Self::compile_matchers(&config); - Self { - config, - executor: Arc::new(HookExecutorImpl), - llm_source, - catalog, - compiled_matchers, - } - } - - /// Create a HookRunner with a custom executor (for testing). - #[cfg(test)] - pub fn with_executor(config: HookSettings, executor: Arc) -> Self { - let compiled_matchers = Self::compile_matchers(&config); - Self { - config, - executor, - llm_source: test_support::vault_only_credential_source(), - catalog: Arc::new(fabro_llm::default_catalog()), - compiled_matchers, - } - } - - fn compile_matchers(config: &HookSettings) -> HashMap { - let mut map = HashMap::new(); - for hook in &config.hooks { - if let Some(ref pattern) = hook.matcher { - if !map.contains_key(pattern) { - if let Ok(re) = regex::Regex::new(pattern) { - map.insert(pattern.clone(), re); - } - } - } - } - map - } - - /// Run all matching hooks for the given event and return the merged - /// decision. - pub async fn run( - &self, - context: &HookContext, - sandbox: Arc, - execution_context: HookExecutionContext, - ) -> HookDecision { - let matching = self.filter_hooks(context); - if matching.is_empty() { - return HookDecision::Proceed; - } - - let hooks_matched = matching.len(); - tracing::info!( - event = %context.event, - hooks_matched, - "Running hooks" - ); - - let any_blocking = matching.iter().any(|h| h.is_blocking()); - - let decision = if any_blocking { - // Sequential execution for blocking hooks, short-circuit on first Block - self.run_sequential(&matching, context, sandbox, &execution_context) - .await - } else { - // Non-blocking: run all, ignore decisions - self.run_non_blocking(&matching, context, sandbox, &execution_context) - .await - }; - - tracing::info!( - event = %context.event, - decision = ?decision, - "Hooks complete" - ); - - decision - } - - /// Filter hooks that match the given event and context. - fn filter_hooks(&self, context: &HookContext) -> Vec<&HookDefinition> { - self.config - .hooks - .iter() - .filter(|h| h.event == context.event) - .filter(|h| self.matches(h, context)) - .collect() - } - - /// Check if a hook's matcher applies to this context. - fn matches(&self, hook: &HookDefinition, context: &HookContext) -> bool { - let Some(ref pattern) = hook.matcher else { - return true; - }; - let Some(re) = self.compiled_matchers.get(pattern) else { - // Pattern failed to compile during construction — already warned - return false; - }; - [ - context.node_id.as_deref(), - context.handler_type.as_deref(), - context.edge_to.as_deref(), - context.edge_from.as_deref(), - context.tool_name.as_deref(), - ] - .iter() - .any(|field| field.is_some_and(|v| re.is_match(v))) - } - - async fn run_sequential( - &self, - hooks: &[&HookDefinition], - context: &HookContext, - sandbox: Arc, - execution_context: &HookExecutionContext, - ) -> HookDecision { - let mut merged = HookDecision::Proceed; - for hook in hooks { - tracing::debug!( - hook = %hook.effective_name(), - event = %context.event, - "Executing hook" - ); - let result = self - .executor - .execute( - hook, - context, - sandbox.clone(), - execution_context, - Arc::clone(&self.llm_source), - Arc::clone(&self.catalog), - ) - .await; - tracing::debug!( - hook = %hook.effective_name(), - duration_ms = result.duration_ms, - decision = ?result.decision, - "Hook complete" - ); - - if hook.is_blocking() { - merged = merged.merge(result.decision); - // Short-circuit on Block - if matches!(merged, HookDecision::Block { .. }) { - tracing::error!( - hook = %hook.effective_name(), - event = %context.event, - decision = ?merged, - "Hook blocked execution" - ); - return merged; - } - } else if !result.decision.is_proceed() { - tracing::warn!( - hook = %hook.effective_name(), - event = %context.event, - decision = ?result.decision, - "Non-blocking hook returned non-proceed, ignoring" - ); - } - } - merged - } - - async fn run_non_blocking( - &self, - hooks: &[&HookDefinition], - context: &HookContext, - sandbox: Arc, - execution_context: &HookExecutionContext, - ) -> HookDecision { - for hook in hooks { - tracing::debug!( - hook = %hook.effective_name(), - event = %context.event, - "Executing hook" - ); - let result = self - .executor - .execute( - hook, - context, - sandbox.clone(), - execution_context, - Arc::clone(&self.llm_source), - Arc::clone(&self.catalog), - ) - .await; - tracing::debug!( - hook = %hook.effective_name(), - duration_ms = result.duration_ms, - decision = ?result.decision, - "Hook complete" - ); - if !result.decision.is_proceed() { - tracing::warn!( - hook = %hook.effective_name(), - event = %context.event, - decision = ?result.decision, - "Non-blocking hook failed, continuing" - ); - } - } - HookDecision::Proceed - } -} - -#[cfg(test)] -mod tests { - use fabro_types::fixtures; - - use super::*; - use crate::config::HookSettings; - use crate::types::{HookContext, HookEvent, HookResult}; - - struct MockExecutor { - decision: HookDecision, - } - - #[async_trait::async_trait] - impl HookExecutor for MockExecutor { - async fn execute( - &self, - definition: &HookDefinition, - _context: &HookContext, - _sandbox: Arc, - _execution_context: &HookExecutionContext, - _llm_source: Arc, - _catalog: Arc, - ) -> HookResult { - HookResult { - hook_name: definition.name.clone(), - decision: self.decision.clone(), - duration_ms: 1, - } - } - } - - async fn make_sandbox() -> Arc { - Arc::new( - fabro_sandbox::local_sandbox(std::env::current_dir().unwrap()) - .await - .unwrap(), - ) - } - - fn make_context(event: HookEvent) -> HookContext { - HookContext::new(event, fixtures::RUN_1, "test-wf".into()) - } - - fn test_llm_source() -> Arc { - test_support::vault_only_credential_source() - } - - fn test_catalog() -> Arc { - Arc::new(fabro_llm::default_catalog()) - } - - fn make_hook(event: HookEvent, name: &str) -> HookDefinition { - HookDefinition { - name: Some(name.into()), - event, - command: Some("echo test".into()), - hook_type: None, - matcher: None, - blocking: None, - timeout_ms: None, - sandbox: Some(false), - } - } - - #[tokio::test] - async fn no_hooks_returns_proceed() { - let runner = HookRunner::new(HookSettings::default(), test_llm_source(), test_catalog()); - let ctx = make_context(HookEvent::RunStart); - let sandbox = make_sandbox().await; - let decision = runner - .run(&ctx, sandbox.clone(), HookExecutionContext::default()) - .await; - assert_eq!(decision, HookDecision::Proceed); - } - - #[tokio::test] - async fn filters_by_event() { - let config = HookSettings { - hooks: vec![ - make_hook(HookEvent::RunStart, "a"), - make_hook(HookEvent::StageStart, "b"), - ], - }; - let runner = HookRunner::with_executor( - config, - Arc::new(MockExecutor { - decision: HookDecision::Proceed, - }), - ); - let ctx = make_context(HookEvent::RunStart); - let matching = runner.filter_hooks(&ctx); - assert_eq!(matching.len(), 1); - assert_eq!(matching[0].name.as_deref(), Some("a")); - } - - #[tokio::test] - async fn matcher_filters_by_node_id() { - let mut hook = make_hook(HookEvent::StageStart, "filtered"); - hook.matcher = Some("agent".into()); - let config = HookSettings { hooks: vec![hook] }; - let runner = HookRunner::with_executor( - config, - Arc::new(MockExecutor { - decision: HookDecision::Proceed, - }), - ); - - // No node_id — no match - let ctx = make_context(HookEvent::StageStart); - assert!(runner.filter_hooks(&ctx).is_empty()); - - // Matching node_id - let mut ctx = make_context(HookEvent::StageStart); - ctx.node_id = Some("agent_step".into()); - assert_eq!(runner.filter_hooks(&ctx).len(), 1); - - // Non-matching node_id - let mut ctx = make_context(HookEvent::StageStart); - ctx.node_id = Some("start".into()); - assert!(runner.filter_hooks(&ctx).is_empty()); - } - - #[tokio::test] - async fn matcher_filters_by_handler_type() { - let mut hook = make_hook(HookEvent::StageStart, "filtered"); - hook.matcher = Some("^agent$".into()); - let config = HookSettings { hooks: vec![hook] }; - let runner = HookRunner::with_executor( - config, - Arc::new(MockExecutor { - decision: HookDecision::Proceed, - }), - ); - - let mut ctx = make_context(HookEvent::StageStart); - ctx.handler_type = Some("agent".into()); - assert_eq!(runner.filter_hooks(&ctx).len(), 1); - - let mut ctx = make_context(HookEvent::StageStart); - ctx.handler_type = Some("command".into()); - assert!(runner.filter_hooks(&ctx).is_empty()); - } - - #[tokio::test] - async fn matcher_filters_by_tool_name() { - let mut hook = make_hook(HookEvent::PreToolUse, "tool-filter"); - hook.matcher = Some("shell".into()); - let config = HookSettings { hooks: vec![hook] }; - let runner = HookRunner::with_executor( - config, - Arc::new(MockExecutor { - decision: HookDecision::Proceed, - }), - ); - - // Matches tool_name "shell" - let mut ctx = make_context(HookEvent::PreToolUse); - ctx.tool_name = Some("shell".into()); - assert_eq!(runner.filter_hooks(&ctx).len(), 1); - - // Does not match tool_name "read_file" - let mut ctx = make_context(HookEvent::PreToolUse); - ctx.tool_name = Some("read_file".into()); - assert!(runner.filter_hooks(&ctx).is_empty()); - } - - #[tokio::test] - async fn blocking_hook_block_decision() { - let config = HookSettings { - hooks: vec![make_hook(HookEvent::RunStart, "blocker")], - }; - let runner = HookRunner::with_executor( - config, - Arc::new(MockExecutor { - decision: HookDecision::Block { - reason: Some("denied".into()), - }, - }), - ); - let ctx = make_context(HookEvent::RunStart); - let sandbox = make_sandbox().await; - let decision = runner - .run(&ctx, sandbox.clone(), HookExecutionContext::default()) - .await; - assert!(matches!(decision, HookDecision::Block { .. })); - } - - #[tokio::test] - async fn blocking_hook_skip_decision() { - let mut hook = make_hook(HookEvent::StageStart, "skipper"); - hook.blocking = Some(true); - let config = HookSettings { hooks: vec![hook] }; - let runner = HookRunner::with_executor( - config, - Arc::new(MockExecutor { - decision: HookDecision::Skip { - reason: Some("skip it".into()), - }, - }), - ); - let ctx = make_context(HookEvent::StageStart); - let sandbox = make_sandbox().await; - let decision = runner - .run(&ctx, sandbox.clone(), HookExecutionContext::default()) - .await; - assert!(matches!(decision, HookDecision::Skip { .. })); - } - - #[tokio::test] - async fn non_blocking_hook_doesnt_block() { - let mut hook = make_hook(HookEvent::StageComplete, "observer"); - hook.blocking = Some(false); - let config = HookSettings { hooks: vec![hook] }; - let runner = HookRunner::with_executor( - config, - Arc::new(MockExecutor { - decision: HookDecision::Block { - reason: Some("ignored".into()), - }, - }), - ); - let ctx = make_context(HookEvent::StageComplete); - let sandbox = make_sandbox().await; - let decision = runner - .run(&ctx, sandbox.clone(), HookExecutionContext::default()) - .await; - // Non-blocking hooks don't affect the decision - assert_eq!(decision, HookDecision::Proceed); - } - - #[tokio::test] - async fn executor_integration_success() { - let config = HookSettings { - hooks: vec![{ - let mut h = make_hook(HookEvent::RunStart, "echo-hook"); - h.command = Some("exit 0".into()); - h - }], - }; - let runner = HookRunner::new(config, test_llm_source(), test_catalog()); - let ctx = make_context(HookEvent::RunStart); - let sandbox = make_sandbox().await; - let decision = runner - .run(&ctx, sandbox.clone(), HookExecutionContext::default()) - .await; - assert_eq!(decision, HookDecision::Proceed); - } - - #[tokio::test] - async fn executor_integration_block() { - let config = HookSettings { - hooks: vec![{ - let mut h = make_hook(HookEvent::RunStart, "fail-hook"); - h.command = Some("exit 1".into()); - h - }], - }; - let runner = HookRunner::new(config, test_llm_source(), test_catalog()); - let ctx = make_context(HookEvent::RunStart); - let sandbox = make_sandbox().await; - let decision = runner - .run(&ctx, sandbox.clone(), HookExecutionContext::default()) - .await; - assert!(matches!(decision, HookDecision::Block { .. })); - } -} diff --git a/lib/components/fabro-hooks/src/types.rs b/lib/components/fabro-hooks/src/types.rs deleted file mode 100644 index 8a1cba192..000000000 --- a/lib/components/fabro-hooks/src/types.rs +++ /dev/null @@ -1,356 +0,0 @@ -use std::path::{Path, PathBuf}; - -use fabro_types::RunId; -use serde::{Deserialize, Serialize}; - -use crate::config::HookDefinition; -pub use crate::config::HookEvent; - -/// Rich JSON payload sent to hooks. -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct HookContext { - pub event: HookEvent, - pub run_id: RunId, - pub workflow_name: String, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub cwd: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub node_id: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub node_label: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub handler_type: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub status: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub edge_from: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub edge_to: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub edge_label: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub failure_reason: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub attempt: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub max_attempts: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub tool_name: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub tool_input: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub tool_call_id: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub tool_output: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub error_message: Option, -} - -impl HookContext { - #[must_use] - pub fn new(event: HookEvent, run_id: RunId, workflow_name: String) -> Self { - Self { - event, - run_id, - workflow_name, - cwd: None, - node_id: None, - node_label: None, - handler_type: None, - status: None, - edge_from: None, - edge_to: None, - edge_label: None, - failure_reason: None, - attempt: None, - max_attempts: None, - tool_name: None, - tool_input: None, - tool_call_id: None, - tool_output: None, - error_message: None, - } - } -} - -/// Response returned by prompt/agent hooks from the LLM. -#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] -pub struct PromptHookResponse { - pub ok: bool, - #[serde(default)] - pub reason: Option, -} - -/// Decision returned by blocking hooks. -#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] -#[serde(tag = "decision", rename_all = "snake_case")] -pub enum HookDecision { - #[default] - Proceed, - Skip { - #[serde(default)] - reason: Option, - }, - Block { - #[serde(default)] - reason: Option, - }, - Override { - edge_to: String, - }, -} - -impl HookDecision { - /// Merge two decisions. Block > Skip/Override > Proceed. - #[must_use] - pub fn merge(self, other: Self) -> Self { - match (&self, &other) { - (Self::Block { .. }, _) => self, - (_, Self::Block { .. }) => other, - (Self::Skip { .. } | Self::Override { .. }, _) => self, - (_, Self::Skip { .. } | Self::Override { .. }) => other, - _ => Self::Proceed, - } - } - - #[must_use] - pub fn is_proceed(&self) -> bool { - matches!(self, Self::Proceed) - } -} - -/// Realm-specific locations available to hook execution. -#[derive(Clone, Debug, Default, PartialEq, Eq)] -pub struct HookExecutionContext { - pub host_source_dir: Option, - pub sandbox_work_dir: Option, -} - -impl HookExecutionContext { - #[must_use] - pub fn command_cwd_for(&self, definition: &HookDefinition) -> Option<&Path> { - if definition.runs_in_sandbox() { - self.sandbox_work_dir.as_deref() - } else { - self.host_source_dir.as_deref() - } - } -} - -/// Result from executing a single hook. -#[derive(Debug, Clone)] -pub struct HookResult { - pub hook_name: Option, - pub decision: HookDecision, - pub duration_ms: u64, -} - -#[cfg(test)] -mod tests { - use std::path::{Path, PathBuf}; - - use fabro_types::fixtures; - - use super::*; - - fn command_hook(sandbox: bool) -> HookDefinition { - HookDefinition { - name: Some("cwd-test".into()), - event: HookEvent::RunStart, - command: Some("pwd".into()), - hook_type: None, - matcher: None, - blocking: None, - timeout_ms: None, - sandbox: Some(sandbox), - } - } - - #[test] - fn hook_context_serde_round_trip() { - let ctx = HookContext { - event: HookEvent::StageStart, - run_id: fixtures::RUN_1, - workflow_name: "test-wf".into(), - cwd: Some("/tmp".into()), - node_id: Some("plan".into()), - node_label: Some("Plan".into()), - handler_type: Some("agent".into()), - status: None, - edge_from: None, - edge_to: None, - edge_label: None, - failure_reason: None, - attempt: Some(1), - max_attempts: Some(3), - tool_name: None, - tool_input: None, - tool_call_id: None, - tool_output: None, - error_message: None, - }; - let json = serde_json::to_string(&ctx).unwrap(); - let back: HookContext = serde_json::from_str(&json).unwrap(); - assert_eq!(back.event, HookEvent::StageStart); - assert_eq!(back.run_id, fixtures::RUN_1); - assert_eq!(back.node_id.as_deref(), Some("plan")); - } - - #[test] - fn hook_context_omits_none_fields() { - let ctx = HookContext::new(HookEvent::RunStart, fixtures::RUN_1, "wf".into()); - let json = serde_json::to_string(&ctx).unwrap(); - assert!(!json.contains("node_id")); - assert!(!json.contains("failure_reason")); - } - - #[test] - fn hook_execution_context_returns_host_source_dir_for_host_command() { - let context = HookExecutionContext { - host_source_dir: Some(PathBuf::from("/host/project")), - sandbox_work_dir: Some(PathBuf::from("/workspace/project")), - }; - - assert_eq!( - context.command_cwd_for(&command_hook(false)), - Some(Path::new("/host/project")) - ); - } - - #[test] - fn hook_execution_context_returns_sandbox_work_dir_for_sandbox_command() { - let context = HookExecutionContext { - host_source_dir: Some(PathBuf::from("/host/project")), - sandbox_work_dir: Some(PathBuf::from("/workspace/project")), - }; - - assert_eq!( - context.command_cwd_for(&command_hook(true)), - Some(Path::new("/workspace/project")) - ); - } - - #[test] - fn hook_execution_context_returns_none_when_matching_dir_is_missing() { - let context = HookExecutionContext { - host_source_dir: Some(PathBuf::from("/host/project")), - sandbox_work_dir: None, - }; - - assert_eq!(context.command_cwd_for(&command_hook(true)), None); - } - - #[test] - fn hook_decision_serde_round_trip() { - let decisions = [ - HookDecision::Proceed, - HookDecision::Skip { - reason: Some("not needed".into()), - }, - HookDecision::Block { - reason: Some("forbidden".into()), - }, - HookDecision::Override { - edge_to: "node_b".into(), - }, - ]; - for decision in decisions { - let json = serde_json::to_string(&decision).unwrap(); - let back: HookDecision = serde_json::from_str(&json).unwrap(); - assert_eq!(decision, back); - } - } - - #[test] - fn hook_decision_merge_block_wins() { - let block = HookDecision::Block { - reason: Some("no".into()), - }; - let skip = HookDecision::Skip { - reason: Some("skip".into()), - }; - let proceed = HookDecision::Proceed; - - assert!(matches!( - proceed.clone().merge(block.clone()), - HookDecision::Block { .. } - )); - assert!(matches!( - block.clone().merge(skip.clone()), - HookDecision::Block { .. } - )); - assert!(matches!( - skip.clone().merge(block.clone()), - HookDecision::Block { .. } - )); - } - - #[test] - fn hook_decision_merge_skip_over_proceed() { - let skip = HookDecision::Skip { - reason: Some("skip".into()), - }; - let proceed = HookDecision::Proceed; - - assert!(matches!( - proceed.clone().merge(skip.clone()), - HookDecision::Skip { .. } - )); - assert!(matches!(skip.merge(proceed), HookDecision::Skip { .. })); - } - - #[test] - fn hook_decision_merge_first_non_proceed_wins() { - let skip = HookDecision::Skip { - reason: Some("a".into()), - }; - let override_d = HookDecision::Override { - edge_to: "x".into(), - }; - // First non-Proceed wins when no Block - assert!(matches!(skip.merge(override_d), HookDecision::Skip { .. })); - } - - #[test] - fn hook_decision_default_is_proceed() { - assert_eq!(HookDecision::default(), HookDecision::Proceed); - } - - #[test] - fn prompt_hook_response_ok_true() { - let resp: PromptHookResponse = serde_json::from_str(r#"{"ok": true}"#).unwrap(); - assert!(resp.ok); - assert_eq!(resp.reason, None); - } - - #[test] - fn prompt_hook_response_ok_false_with_reason() { - let resp: PromptHookResponse = - serde_json::from_str(r#"{"ok": false, "reason": "not ready"}"#).unwrap(); - assert!(!resp.ok); - assert_eq!(resp.reason.as_deref(), Some("not ready")); - } - - #[test] - fn hook_context_with_tool_fields() { - let mut ctx = HookContext::new(HookEvent::PreToolUse, fixtures::RUN_1, "wf".into()); - ctx.tool_name = Some("shell".into()); - ctx.tool_input = Some(serde_json::json!({"command": "ls"})); - ctx.tool_call_id = Some("call_123".into()); - let json = serde_json::to_string(&ctx).unwrap(); - assert!(json.contains("\"tool_name\":\"shell\"")); - assert!(json.contains("\"tool_call_id\":\"call_123\"")); - assert!(json.contains("\"tool_input\"")); - } - - #[test] - fn hook_context_tool_output_serializes() { - let mut ctx = HookContext::new(HookEvent::PostToolUse, fixtures::RUN_1, "wf".into()); - ctx.tool_name = Some("shell".into()); - ctx.tool_output = Some("file1.txt\nfile2.txt".into()); - let json = serde_json::to_string(&ctx).unwrap(); - assert!(json.contains("\"tool_output\"")); - // error_message should be omitted - assert!(!json.contains("\"error_message\"")); - } -} diff --git a/lib/components/fabro-hooks/tests/host_command_hooks.rs b/lib/components/fabro-hooks/tests/host_command_hooks.rs deleted file mode 100644 index 21178750b..000000000 --- a/lib/components/fabro-hooks/tests/host_command_hooks.rs +++ /dev/null @@ -1,84 +0,0 @@ -use std::path::Path; -use std::sync::Arc; - -use fabro_auth::test_support; -use fabro_hooks::{ - HookContext, HookDecision, HookDefinition, HookEvent, HookExecutionContext, HookRunner, - HookSettings, InterpString, -}; -use fabro_llm::credentials::CredentialProvider; -use fabro_llm::lithos_catalog::Catalog; -use fabro_sandbox::{RunSandbox, local_sandbox}; -use fabro_types::RunId; -use tokio::fs; - -fn test_llm_source() -> Arc { - test_support::vault_only_credential_source() -} - -fn test_catalog() -> Arc { - Arc::new(fabro_llm::default_catalog()) -} - -async fn test_sandbox() -> Arc { - Arc::new( - local_sandbox(std::env::current_dir().expect("test process should have a cwd")) - .await - .expect("local sandbox should be created"), - ) -} - -#[tokio::test] -async fn host_command_hook_uses_host_workdir_not_sandbox_workdir() { - let host_work_dir = - std::env::temp_dir().join(format!("fabro-host-hook-cwd-{}", std::process::id())); - let _ = fs::remove_dir_all(&host_work_dir).await; - fs::create_dir_all(&host_work_dir) - .await - .expect("test should create host hook cwd"); - let container_only_work_dir = Path::new("/workspace/fabro-host-hook-repro-missing"); - assert!( - !container_only_work_dir.exists(), - "reproduction requires a container-only cwd that does not exist on the host" - ); - - let runner = HookRunner::new( - HookSettings { - hooks: vec![HookDefinition { - name: Some("host-marker".to_string()), - event: HookEvent::RunStart, - command: Some(InterpString::parse("printf ran > marker.txt")), - hook_type: None, - matcher: None, - blocking: Some(true), - timeout_ms: Some(5000), - sandbox: Some(false), - }], - }, - test_llm_source(), - test_catalog(), - ); - let context = HookContext::new( - HookEvent::RunStart, - RunId::new(), - "host-hook-cwd".to_string(), - ); - - let decision = runner - .run(&context, test_sandbox().await, HookExecutionContext { - host_source_dir: Some(host_work_dir.clone()), - sandbox_work_dir: Some(container_only_work_dir.to_path_buf()), - }) - .await; - - assert_eq!(decision, HookDecision::Proceed); - assert_eq!( - fs::read_to_string(host_work_dir.join("marker.txt")) - .await - .expect("host hook should create marker file"), - "ran" - ); - fs::remove_dir_all(&host_work_dir) - .await - .expect("test should clean up host hook cwd"); -} diff --git a/lib/components/fabro-petri/README.md b/lib/components/fabro-petri/README.md index f338a88ae..14b23f3bb 100644 --- a/lib/components/fabro-petri/README.md +++ b/lib/components/fabro-petri/README.md @@ -10,18 +10,6 @@ member that lists them as dependencies. Every other Fabro crate reaches the engine through what this crate exports. A Petri pin move is therefore a change to this crate and the lockfile, nothing else. -## Engine freeze - -The engine half of `fabro-workflow` (`handler/`, `lifecycle/`, -`pipeline/execute`, `graph/routing.rs`, `node_handler.rs`, `retry.rs`, -`condition.rs`, `context.rs` and `model_fallback.rs` under its `src/`) takes -bug fixes only. New engine behaviour goes to Petri and reaches Fabro through -this crate. The `Engine freeze` CI check -(`.github/workflows/engine-freeze.yml`) fails a pull request that adds lines -under those paths unless it carries the `bugfix` label. The path list is in -`scripts/check-engine-freeze.sh`; run it locally as -`scripts/check-engine-freeze.sh origin/main` to see what a branch adds there. - ## What it holds Every adapter the integration plan describes lands here. diff --git a/lib/foundation/fabro-dev/tests/it/policy.rs b/lib/foundation/fabro-dev/tests/it/policy.rs index 6f1693b86..cd66f9201 100644 --- a/lib/foundation/fabro-dev/tests/it/policy.rs +++ b/lib/foundation/fabro-dev/tests/it/policy.rs @@ -13,8 +13,6 @@ const TEMPLATE_RENDER_ALLOWED_PATH_FRAGMENTS: &[&str] = &[ "lib/foundation/fabro-template/src/lib.rs", // Workflow-definition rendering must stay centralized here. "lib/components/fabro-workflow/src/transforms/variable_expansion.rs", - // Hook header/env interpolation is a separate system. - "lib/components/fabro-hooks/src/executor.rs", // This policy test names the forbidden patterns. "/tests/it/policy.rs", ]; diff --git a/scripts/check-engine-freeze-test.sh b/scripts/check-engine-freeze-test.sh deleted file mode 100755 index 8a8de3f53..000000000 --- a/scripts/check-engine-freeze-test.sh +++ /dev/null @@ -1,65 +0,0 @@ -#!/usr/bin/env bash -# Self-test for scripts/check-engine-freeze.sh against a synthetic repository: -# an added line under a frozen path exits 1, and deletions or additions -# elsewhere exit 0. -set -euo pipefail - -CHECK="$(cd "$(dirname "$0")" && pwd)/check-engine-freeze.sh" -SRC="lib/components/fabro-workflow/src" - -export GIT_AUTHOR_NAME=test GIT_AUTHOR_EMAIL=test@example.com -export GIT_COMMITTER_NAME=test GIT_COMMITTER_EMAIL=test@example.com - -repo="$(mktemp -d)" -trap 'rm -rf "$repo"' EXIT -cd "$repo" -git init -q -b main -mkdir -p "$SRC/handler" "$SRC/pipeline/execute" "$SRC/transforms" -printf 'a\nb\nc\n' > "$SRC/handler/agent.rs" -printf 'a\nb\n' > "$SRC/pipeline/execute/tests.rs" -printf 'a\n' > "$SRC/retry.rs" -printf 'a\n' > "$SRC/transforms/preamble.rs" -printf 'a\n' > "$SRC/pipeline/finalize.rs" -git add -A -git commit -q -m base - -failures=0 -expect() { - local name="$1" want="$2" - git checkout -q -b "$name" main - "case_$name" - git add -A - git commit -q --allow-empty -m "$name" - local got=0 - "$CHECK" main > /dev/null || got=$? - if [ "$got" -eq "$want" ]; then - echo "ok $name (exit $got)" - else - echo "FAIL $name: expected exit $want, got $got" - failures=$((failures + 1)) - fi - git checkout -q main -} - -case_adds_to_frozen_file() { echo d >> "$SRC/handler/agent.rs"; } -case_adds_to_frozen_dir() { echo c >> "$SRC/pipeline/execute/tests.rs"; } -case_rewrites_frozen_line() { printf 'a\nB\nc\n' > "$SRC/handler/agent.rs"; } -case_deletes_from_frozen_file() { printf 'a\n' > "$SRC/handler/agent.rs"; } -case_adds_outside_freeze() { - echo b >> "$SRC/transforms/preamble.rs" - echo b >> "$SRC/pipeline/finalize.rs" -} -case_no_change() { :; } - -expect adds_to_frozen_file 1 -expect adds_to_frozen_dir 1 -expect rewrites_frozen_line 1 -expect deletes_from_frozen_file 0 -expect adds_outside_freeze 0 -expect no_change 0 - -if [ "$failures" -ne 0 ]; then - echo "$failures case(s) failed" - exit 1 -fi -echo "all cases passed" diff --git a/scripts/check-engine-freeze.sh b/scripts/check-engine-freeze.sh deleted file mode 100755 index 03fcc73a5..000000000 --- a/scripts/check-engine-freeze.sh +++ /dev/null @@ -1,80 +0,0 @@ -#!/usr/bin/env bash -# Report added lines under the frozen engine half of fabro-workflow. -# -# The engine half of `lib/components/fabro-workflow` takes bug fixes only; -# new engine behaviour goes to Petri (`lib/components/fabro-petri`). This -# script lists every frozen file the current branch adds lines to, compared -# with the base ref, and exits 1 when there is at least one. It knows nothing -# about pull request labels: the CI job (`.github/workflows/engine-freeze.yml`) -# waives a failure when the pull request carries the `bugfix` label. -# -# Usage: scripts/check-engine-freeze.sh [] (default: origin/main) -set -euo pipefail - -BASE_REF="${1:-origin/main}" -LABEL="bugfix" - -# The frozen paths, relative to the fabro-workflow crate's `src/`. A directory -# freezes everything under it. `preamble` in the integration plan is -# `handler/llm/preamble.rs`, which `handler/` covers; `transforms/preamble.rs` -# is a graph transform and is not frozen. -FROZEN=( - handler - lifecycle - pipeline/execute.rs - pipeline/execute - graph/routing.rs - node_handler.rs - retry.rs - condition.rs - context.rs - model_fallback.rs -) - -if [ "${FREEZE_LIST_ONLY:-}" = "1" ]; then - printf '%s\n' "${FROZEN[@]}" - exit 0 -fi - -ROOT="$(git rev-parse --show-toplevel)" -CRATE_SRC="lib/components/fabro-workflow/src" - -paths=() -for entry in "${FROZEN[@]}"; do - paths+=("$CRATE_SRC/$entry") -done - -# Three dots: the changes since the merge base, which is what a pull request -# adds to its base branch. -numstat="$(git -C "$ROOT" diff --numstat "$BASE_REF...HEAD" -- "${paths[@]}")" - -offenders=() -while IFS=$'\t' read -r added _deleted path; do - [ -n "${path:-}" ] || continue - case "$added" in - ''|*[!0-9]*) continue ;; # binary files report '-' - esac - if [ "$added" -gt 0 ]; then - offenders+=("$added $path") - fi -done <<< "$numstat" - -if [ "${#offenders[@]}" -eq 0 ]; then - echo "engine freeze: no lines added under the frozen paths of fabro-workflow since $BASE_REF" - exit 0 -fi - -echo "engine freeze: this branch adds lines to the frozen engine half of fabro-workflow (since $BASE_REF):" -for line in "${offenders[@]}"; do - echo " +${line%% *} ${line#* }" -done -echo -echo "The engine half of lib/components/fabro-workflow takes bug fixes only." -echo "New engine behaviour goes to Petri (lib/components/fabro-petri)." -echo "Frozen paths under $CRATE_SRC/:" -for entry in "${FROZEN[@]}"; do - echo " $entry" -done -echo -echo "A bug fix passes CI when the pull request carries the '$LABEL' label." -exit 1