From 325fe0c4bbd1ed4716317bdc8d8f54d6732740f3 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Mon, 14 Sep 2026 09:41:05 -0600 Subject: [PATCH] Fix the twin-mode hook and arc e2e tests The four hook tests and arc_e2e_with_real_llm in workflow/hooks.rs failed in twin mode for three reasons, all in the test fixtures. The hooked workflows were written as `.toml` beside `.fabro`. Version packaging accepts a config only as `workflow.toml` beside its graph (44dccfa3d), so `fabro run` failed at collection. Each hooked workflow now lives in its own `/` directory as `workflow.toml`. The isolated server never learned the twin's base URL. The CLI command carried `OPENAI_BASE_URL`, but the run executes in the server, which does not see the test process environment, so it called the real OpenAI API with the namespace as its key. The twin-mode server settings now repoint the `openai` provider at the twin through the operator `[llm]` overlay, the same way `run_uses_vault_credentials_for_worker_execution` does. With the server reaching the twin, the hook scenarios were consumed by the wrong request: the server asks the model for a run title in the same namespace before the hook fires, and the scenarios had no matcher. The block test then saw the twin's default response and the hook failed open, so the run succeeded. Hook scenarios now match on the `Hook prompt:` prefix of the evaluator's user message. Co-Authored-By: Claude Fable 5.1 --- lib/apps/fabro-cli/tests/it/workflow/hooks.rs | 78 ++++++++++--------- 1 file changed, 40 insertions(+), 38 deletions(-) diff --git a/lib/apps/fabro-cli/tests/it/workflow/hooks.rs b/lib/apps/fabro-cli/tests/it/workflow/hooks.rs index 504825e1f..8354aa846 100644 --- a/lib/apps/fabro-cli/tests/it/workflow/hooks.rs +++ b/lib/apps/fabro-cli/tests/it/workflow/hooks.rs @@ -66,26 +66,6 @@ fn twin_server_storage_dir(context: &fabro_test::TestContext) -> std::path::Path context.temp_dir.join("hook-server-storage") } -/// The twin-mode server settings: a private storage root and dev-token auth. -/// Live mode runs against the developer's own settings. -fn write_server_settings(context: &fabro_test::TestContext) { - if !TestMode::from_env().is_twin() { - return; - } - context.write_home( - ".fabro/settings.toml", - format!( - r#"[server.storage] -root = "{}" - -[server.auth] -methods = ["dev-token"] -"#, - toml_path(&twin_server_storage_dir(context)), - ), - ); -} - fn seed_openai_vault(storage_dir: &std::path::Path, api_key: &str) { let mut vault = Vault::load(Storage::new(storage_dir).secrets_path()).expect("test vault should load"); @@ -94,15 +74,44 @@ fn seed_openai_vault(storage_dir: &std::path::Path, api_key: &str) { .expect("OpenAI credential should store in test vault"); } +/// The twin-mode server: a private storage root, dev-token auth, and the +/// `openai` provider repointed at the twin through the operator `[llm]` +/// overlay. The run executes in the isolated server, which never sees the +/// test process environment, so `OPENAI_BASE_URL` on the CLI command alone +/// would leave the server calling the real API with the namespace as its +/// key. Live mode runs against the developer's own settings. fn configure_twin_server( context: &mut fabro_test::TestContext, - _twin: &TwinOpenAi, + twin: &TwinOpenAi, namespace: &str, ) { + context.write_home( + ".fabro/settings.toml", + format!( + r#"[server.storage] +root = "{}" + +[server.auth] +methods = ["dev-token"] + +[llm.providers.openai] +base_url = "{}" +"#, + toml_path(&twin_server_storage_dir(context)), + twin.base_url, + ), + ); seed_openai_vault(&twin_server_storage_dir(context), namespace); context.isolated_server(); } +/// A twin scenario the hook evaluator's request matches. The server also +/// asks the model for a run title in the same namespace before the hook +/// fires, so an unscoped scenario would answer the title request instead. +fn hook_scenario() -> TwinScenario { + TwinScenario::responses("gpt-5.4-mini").input_contains("Hook prompt:") +} + fn write_workflow(context: &fabro_test::TestContext, name: &str, dot: &str) -> std::path::PathBuf { context.write_temp(name, dot); context.temp_dir.join(name) @@ -110,7 +119,9 @@ fn write_workflow(context: &fabro_test::TestContext, name: &str, dot: &str) -> s /// A workflow config that bundles the graph `.fabro` with `hooks`, /// which is where run hooks live: `fabro run` does not transmit `run` -/// settings from the user's settings file. Returns the config path to run. +/// settings from the user's settings file. The pair lives in its own +/// `/` directory because version packaging accepts a config only as +/// `workflow.toml` beside its graph. Returns the config path to run. fn write_hooked_workflow( context: &fabro_test::TestContext, name: &str, @@ -118,8 +129,8 @@ fn write_hooked_workflow( hooks: &str, ) -> std::path::PathBuf { let graph = format!("{name}.fabro"); - context.write_temp(&graph, dot); - let config = format!("{name}.toml"); + context.write_temp(format!("{name}/{graph}"), dot); + let config = format!("{name}/workflow.toml"); context.write_temp( &config, format!( @@ -161,7 +172,6 @@ async fn conclusion_status(context: &fabro_test::TestContext) -> String { #[fabro_macros::e2e_test(twin, live("ANTHROPIC_API_KEY"))] async fn hook_prompt_proceed_allows_run() { let mut context = test_context!(); - write_server_settings(&context); let workflow = write_hooked_workflow( &context, "hook_prompt_proceed", @@ -186,7 +196,7 @@ model = "{model}" let twin = twin_openai().await; let namespace = format!("{}::{}", module_path!(), line!()); TwinScenarios::new(namespace.clone()) - .scenario(TwinScenario::responses("gpt-5.4-mini").text(r#"{"ok":true}"#)) + .scenario(hook_scenario().text(r#"{"ok":true}"#)) .load(twin) .await; configure_twin_server(&mut context, twin, &namespace); @@ -208,7 +218,6 @@ model = "{model}" #[fabro_macros::e2e_test(twin, live("ANTHROPIC_API_KEY"))] async fn hook_prompt_block_prevents_run() { let mut context = test_context!(); - write_server_settings(&context); let workflow = write_hooked_workflow( &context, "hook_prompt_block", @@ -233,10 +242,7 @@ model = "{model}" let twin = twin_openai().await; let namespace = format!("{}::{}", module_path!(), line!()); TwinScenarios::new(namespace.clone()) - .scenario( - TwinScenario::responses("gpt-5.4-mini") - .text(r#"{"ok":false,"reason":"math check failed"}"#), - ) + .scenario(hook_scenario().text(r#"{"ok":false,"reason":"math check failed"}"#)) .load(twin) .await; configure_twin_server(&mut context, twin, &namespace); @@ -262,7 +268,6 @@ model = "{model}" #[fabro_macros::e2e_test(twin, live("ANTHROPIC_API_KEY"))] async fn hook_agent_proceed_allows_run() { let mut context = test_context!(); - write_server_settings(&context); let workflow = write_hooked_workflow( &context, "hook_agent_proceed", @@ -289,7 +294,7 @@ agent = "enabled" let twin = twin_openai().await; let namespace = format!("{}::{}", module_path!(), line!()); TwinScenarios::new(namespace.clone()) - .scenario(TwinScenario::responses("gpt-5.4-mini").text(r#"{"ok":true}"#)) + .scenario(hook_scenario().text(r#"{"ok":true}"#)) .load(twin) .await; configure_twin_server(&mut context, twin, &namespace); @@ -313,7 +318,6 @@ async fn hook_agent_with_tool_use() { let mut context = test_context!(); let marker = context.temp_dir.join("hook_check.txt"); std::fs::write(&marker, "READY").unwrap(); - write_server_settings(&context); let workflow = write_hooked_workflow( &context, "hook_agent_tools", @@ -342,10 +346,9 @@ agent = "enabled" let namespace = format!("{}::{}", module_path!(), line!()); TwinScenarios::new(namespace.clone()) .scenario( - TwinScenario::responses("gpt-5.4-mini") - .tool_call(TwinToolCall::read_file(marker.display().to_string())), + hook_scenario().tool_call(TwinToolCall::read_file(marker.display().to_string())), ) - .scenario(TwinScenario::responses("gpt-5.4-mini").text(r#"{"ok":true}"#)) + .scenario(hook_scenario().text(r#"{"ok":true}"#)) .load(twin) .await; configure_twin_server(&mut context, twin, &namespace); @@ -367,7 +370,6 @@ agent = "enabled" #[fabro_macros::e2e_test(twin, live("ANTHROPIC_API_KEY"))] async fn arc_e2e_with_real_llm() { let mut context = test_context!(); - write_server_settings(&context); let hello = context.temp_dir.join("hello.txt"); let workflow = write_workflow( &context,