From ae0cae3a224275dafe524892759ba555f51fb098 Mon Sep 17 00:00:00 2001 From: "ilblackdragon@gmail.com" Date: Fri, 27 Mar 2026 22:16:36 -0700 Subject: [PATCH] feat(engine): non-blocking auth signal, NeedAuthentication flow, timeout safety MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the HTTP tool detects a missing credential for a registered host: 1. EffectBridgeAdapter emits SSE AuthRequired event (best-effort, for connected frontends — silently dropped for missions/background threads) 2. Error flows back to LLM as normal ActionResult (non-blocking) 3. LLM tells the user to authenticate This avoids the blocking interruption approach which would hang mission threads and sub-threads that have no channel context. Engine additions: - EngineError::NeedAuthentication variant for structured auth failures - ThreadOutcome::NeedAuthentication for batch interruption when needed - structured.rs handles NeedAuthentication by interrupting the batch (stops subsequent calls, returns outcome to orchestrator) - Auth callback on EffectBridgeAdapter (optional, set by router for SSE) - extract_credential_name parser for HTTP tool error messages - routine_* tools added to is_v1_only_tool blocklist Safety: added 5-minute timeout to await_thread_outcome to prevent infinite hangs (e.g. after denied tool approval where thread fails to resume). Tests: 3 structured executor tests (NeedAuthentication interrupts batch, stops subsequent calls, regular errors don't interrupt) + 7 effect adapter tests (credential extraction, callback firing, v1-only tools). Also adds Linear API skill (skills/linear/SKILL.md). Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/executor/structured.rs | 205 ++++++++++++++++++ .../src/runtime/conversation.rs | 12 + .../ironclaw_engine/src/runtime/messaging.rs | 8 + crates/ironclaw_engine/src/types/error.rs | 8 + skills/linear/SKILL.md | 91 ++++++++ src/bridge/effect_adapter.rs | 147 ++++++++++++- src/bridge/router.rs | 44 ++++ 7 files changed, 512 insertions(+), 3 deletions(-) create mode 100644 skills/linear/SKILL.md diff --git a/crates/ironclaw_engine/src/executor/structured.rs b/crates/ironclaw_engine/src/executor/structured.rs index 0e554624..a9dd8047 100644 --- a/crates/ironclaw_engine/src/executor/structured.rs +++ b/crates/ironclaw_engine/src/executor/structured.rs @@ -141,6 +141,30 @@ pub async fn execute_action_calls( }); results.push(action_result); } + Err(crate::types::error::EngineError::NeedAuthentication { + credential_name, + action_name, + call_id, + parameters, + }) => { + // Interrupt the batch — thread should pause for authentication. + events.push(EventKind::ActionFailed { + step_id: context.step_id, + action_name: action_name.clone(), + call_id: call_id.clone(), + error: format!("authentication required for credential '{credential_name}'"), + }); + return Ok(ActionBatchResult { + results, + events, + need_approval: Some(ThreadOutcome::NeedAuthentication { + credential_name, + action_name, + call_id, + parameters, + }), + }); + } Err(e) => { let error_result = ActionResult { call_id: call.id.clone(), @@ -407,6 +431,187 @@ mod tests { assert_eq!(result.results[1].call_id, "id_bbbb"); } + // ── NeedAuthentication tests ───────────────────────────── + + #[tokio::test] + async fn need_authentication_interrupts_batch() { + let thread = Thread::new( + "test", + ThreadType::Foreground, + ProjectId::new(), + ThreadConfig::default(), + ); + let effects: Arc = Arc::new(MockEffects::new( + vec![test_action("http")], + vec![Err(EngineError::NeedAuthentication { + credential_name: "github_token".into(), + action_name: "http".into(), + call_id: "call_auth_1".into(), + parameters: serde_json::json!({"url": "https://api.github.com/repos"}), + })], + )); + let leases = Arc::new(LeaseManager::new()); + let policy = Arc::new(PolicyEngine::new()); + let ctx = make_exec_context(&thread); + + leases.grant(thread.id, "tools", vec![], None, None).await; + + let calls = vec![ActionCall { + id: "call_auth_1".into(), + action_name: "http".into(), + parameters: serde_json::json!({"url": "https://api.github.com/repos"}), + }]; + + let result = execute_action_calls(&calls, &thread, &effects, &leases, &policy, &ctx, &[]) + .await + .unwrap(); + + // Batch should be interrupted with NeedAuthentication outcome + assert!( + result.need_approval.is_some(), + "NeedAuthentication should interrupt the batch" + ); + match result.need_approval.unwrap() { + ThreadOutcome::NeedAuthentication { + credential_name, + action_name, + .. + } => { + assert_eq!(credential_name, "github_token"); + assert_eq!(action_name, "http"); + } + other => panic!("expected NeedAuthentication, got {:?}", other), + } + + // ActionFailed event should be emitted + assert!( + result + .events + .iter() + .any(|e| matches!(e, EventKind::ActionFailed { .. })), + "should emit ActionFailed event" + ); + } + + #[tokio::test] + async fn need_authentication_stops_before_subsequent_calls() { + // Two calls: first needs auth, second should never execute + let thread = Thread::new( + "test", + ThreadType::Foreground, + ProjectId::new(), + ThreadConfig::default(), + ); + let effects: Arc = Arc::new(MockEffects::new( + vec![test_action("http"), test_action("echo")], + vec![ + Err(EngineError::NeedAuthentication { + credential_name: "api_key".into(), + action_name: "http".into(), + call_id: "call_1".into(), + parameters: serde_json::json!({}), + }), + // This should never be called + Ok(ActionResult { + call_id: String::new(), + action_name: "echo".into(), + output: serde_json::json!("should not appear"), + is_error: false, + duration: Duration::from_millis(1), + }), + ], + )); + let leases = Arc::new(LeaseManager::new()); + let policy = Arc::new(PolicyEngine::new()); + let ctx = make_exec_context(&thread); + + leases.grant(thread.id, "tools", vec![], None, None).await; + + let calls = vec![ + ActionCall { + id: "call_1".into(), + action_name: "http".into(), + parameters: serde_json::json!({}), + }, + ActionCall { + id: "call_2".into(), + action_name: "echo".into(), + parameters: serde_json::json!({}), + }, + ]; + + let result = execute_action_calls(&calls, &thread, &effects, &leases, &policy, &ctx, &[]) + .await + .unwrap(); + + // Second call should NOT have executed + assert!( + result.results.is_empty(), + "no results should be returned before the interrupted call" + ); + assert!(result.need_approval.is_some()); + } + + /// Regular EngineError::Effect (not NeedAuthentication) should NOT interrupt — + /// it becomes a normal error result and execution continues. + #[tokio::test] + async fn regular_effect_error_does_not_interrupt() { + let thread = Thread::new( + "test", + ThreadType::Foreground, + ProjectId::new(), + ThreadConfig::default(), + ); + let effects: Arc = Arc::new(MockEffects::new( + vec![test_action("http"), test_action("echo")], + vec![ + Err(EngineError::Effect { + reason: "connection timeout".into(), + }), + Ok(ActionResult { + call_id: String::new(), + action_name: "echo".into(), + output: serde_json::json!("second call ran"), + is_error: false, + duration: Duration::from_millis(1), + }), + ], + )); + let leases = Arc::new(LeaseManager::new()); + let policy = Arc::new(PolicyEngine::new()); + let ctx = make_exec_context(&thread); + + leases.grant(thread.id, "tools", vec![], None, None).await; + + let calls = vec![ + ActionCall { + id: "call_1".into(), + action_name: "http".into(), + parameters: serde_json::json!({}), + }, + ActionCall { + id: "call_2".into(), + action_name: "echo".into(), + parameters: serde_json::json!({}), + }, + ]; + + let result = execute_action_calls(&calls, &thread, &effects, &leases, &policy, &ctx, &[]) + .await + .unwrap(); + + // Both calls should have results (error does not interrupt) + assert_eq!(result.results.len(), 2); + assert!(result.results[0].is_error); + assert!(!result.results[1].is_error); + assert!( + result.need_approval.is_none(), + "no interruption for regular errors" + ); + } + + // ── call_id preservation (OpenAI/Mistral) ───────────────── + /// Provider-specific: OpenAI rejects empty string call_id. Verify no result /// ever has an empty call_id when the ActionCall provided one. #[tokio::test] diff --git a/crates/ironclaw_engine/src/runtime/conversation.rs b/crates/ironclaw_engine/src/runtime/conversation.rs index a90a7c63..9f65d1c7 100644 --- a/crates/ironclaw_engine/src/runtime/conversation.rs +++ b/crates/ironclaw_engine/src/runtime/conversation.rs @@ -249,6 +249,18 @@ impl ConversationManager { )); // Thread stays active — waiting for approval } + ThreadOutcome::NeedAuthentication { + credential_name, + action_name: _, + call_id: _, + parameters: _, + } => { + conv.add_entry(ConversationEntry::system_for_thread( + thread_id, + format!("Authentication required for credential: {credential_name}"), + )); + // Thread stays active — waiting for OAuth completion + } } self.store.save_conversation(conv).await?; } diff --git a/crates/ironclaw_engine/src/runtime/messaging.rs b/crates/ironclaw_engine/src/runtime/messaging.rs index 2aea75ba..490d34ad 100644 --- a/crates/ironclaw_engine/src/runtime/messaging.rs +++ b/crates/ironclaw_engine/src/runtime/messaging.rs @@ -38,6 +38,14 @@ pub enum ThreadOutcome { call_id: String, parameters: serde_json::Value, }, + /// An action needs a credential that requires user authentication (e.g. OAuth). + /// The thread pauses until the credential is available, then resumes. + NeedAuthentication { + credential_name: String, + action_name: String, + call_id: String, + parameters: serde_json::Value, + }, } /// A mailbox for sending signals to a running thread. diff --git a/crates/ironclaw_engine/src/types/error.rs b/crates/ironclaw_engine/src/types/error.rs index 9c46dfc3..61f50fa6 100644 --- a/crates/ironclaw_engine/src/types/error.rs +++ b/crates/ironclaw_engine/src/types/error.rs @@ -58,6 +58,14 @@ pub enum EngineError { #[error("skill error: {reason}")] Skill { reason: String }, + + #[error("authentication required for credential '{credential_name}'")] + NeedAuthentication { + credential_name: String, + action_name: String, + call_id: String, + parameters: serde_json::Value, + }, } use crate::types::project::ProjectId; diff --git a/skills/linear/SKILL.md b/skills/linear/SKILL.md new file mode 100644 index 00000000..6d892681 --- /dev/null +++ b/skills/linear/SKILL.md @@ -0,0 +1,91 @@ +--- +name: linear +version: "1.0.0" +description: Linear issue tracker API integration +activation: + keywords: + - "linear" + - "ticket" + - "sprint" + - "backlog" + - "roadmap" + exclude_keywords: + - "jira" + - "asana" + patterns: + - "(?i)(create|list|show|assign|close|update)\\s.*(issue|ticket|task|bug)" + - "(?i)linear\\.app" + tags: + - "project-management" + - "issue-tracking" + max_context_tokens: 2000 +credentials: + - name: linear_api_key + provider: linear + location: + type: bearer + hosts: + - "api.linear.app" + setup_instructions: "Create an API key at https://linear.app/settings/api" +--- + +# Linear API Skill + +You have access to the Linear GraphQL API via the `http` tool. Credentials are automatically injected — **never construct Authorization headers manually**. When the URL host is `api.linear.app`, the system injects `Authorization: Bearer {linear_api_key}` transparently. + +## API Patterns + +Linear uses a single GraphQL endpoint: `https://api.linear.app/graphql` + +All requests are `POST` with a JSON body containing `query` and optional `variables`. + +### List Issues + +``` +http(method="POST", url="https://api.linear.app/graphql", body={"query": "{ issues(first: 20, orderBy: updatedAt) { nodes { id identifier title state { name } assignee { name } priority priorityLabel createdAt } } }"}) +``` + +### Get Issue by Identifier + +``` +http(method="POST", url="https://api.linear.app/graphql", body={"query": "query($id: String!) { issue(id: $id) { id identifier title description state { name } assignee { name } labels { nodes { name } } comments { nodes { body user { name } createdAt } } } }", "variables": {"id": "ISSUE_ID"}}) +``` + +### Search Issues + +``` +http(method="POST", url="https://api.linear.app/graphql", body={"query": "query($term: String!) { issueSearch(query: $term, first: 10) { nodes { id identifier title state { name } priorityLabel } } }", "variables": {"term": "SEARCH_TERM"}}) +``` + +### Create Issue + +``` +http(method="POST", url="https://api.linear.app/graphql", body={"query": "mutation($input: IssueCreateInput!) { issueCreate(input: $input) { success issue { id identifier title url } } }", "variables": {"input": {"title": "...", "description": "...", "teamId": "TEAM_ID", "priority": 2}}}) +``` + +### List Teams (to get teamId for issue creation) + +``` +http(method="POST", url="https://api.linear.app/graphql", body={"query": "{ teams { nodes { id name key } } }"}) +``` + +### Update Issue State + +``` +http(method="POST", url="https://api.linear.app/graphql", body={"query": "mutation($id: String!, $stateId: String!) { issueUpdate(id: $id, input: { stateId: $stateId }) { success issue { id identifier title state { name } } } }", "variables": {"id": "ISSUE_UUID", "stateId": "STATE_UUID"}}) +``` + +## Response Handling + +- Linear returns `{"data": {...}}` on success, `{"errors": [...]}` on failure. +- Issue identifiers look like `ENG-123` (team key + number). +- Always check for `errors` in the response before processing `data`. +- GraphQL errors include a `message` and optional `extensions` with error codes. + +## Common Mistakes + +- Do NOT add an `Authorization` header — it is injected automatically. +- Always use `POST` method — Linear's API is GraphQL only. +- The `id` field is a UUID, the `identifier` field is human-readable (e.g., `ENG-42`). +- Use `issueSearch` for text search, not `issues` with a filter (text search is separate). +- When creating issues, you MUST provide `teamId`. List teams first if unknown. diff --git a/src/bridge/effect_adapter.rs b/src/bridge/effect_adapter.rs index 32f6fac5..70ea33e1 100644 --- a/src/bridge/effect_adapter.rs +++ b/src/bridge/effect_adapter.rs @@ -25,6 +25,12 @@ use crate::safety::SafetyLayer; use crate::tools::rate_limiter::RateLimiter; use crate::tools::{ApprovalRequirement, ToolRegistry}; +/// Callback invoked when a credential is missing and the user needs to authenticate. +/// Parameters: (credential_name, action_name). +/// The router sets this to emit SSE events; mission threads may have a no-op. +pub type AuthRequiredCallback = + Box; + /// Wraps the existing tool pipeline to implement the engine's `EffectExecutor`. /// /// Enforces all v1 security controls at the adapter boundary: @@ -41,6 +47,8 @@ pub struct EffectBridgeAdapter { rate_limiter: RateLimiter, /// Mission manager for handling mission_* function calls. mission_manager: RwLock>>, + /// Optional callback for when a credential is missing (emits AuthRequired SSE). + auth_required_callback: RwLock>>, } impl EffectBridgeAdapter { @@ -57,6 +65,19 @@ impl EffectBridgeAdapter { call_count: std::sync::atomic::AtomicU32::new(0), rate_limiter: RateLimiter::new(), mission_manager: RwLock::new(None), + auth_required_callback: RwLock::new(None), + } + } + + /// Set the callback invoked when a credential is missing. + pub async fn set_auth_required_callback(&self, cb: Arc) { + *self.auth_required_callback.write().await = Some(cb); + } + + /// Emit an auth_required signal (best-effort, non-blocking). + async fn emit_auth_required(&self, credential_name: &str, action_name: &str) { + if let Some(cb) = self.auth_required_callback.read().await.as_ref() { + cb(credential_name, action_name); } } @@ -415,6 +436,24 @@ impl EffectExecutor for EffectBridgeAdapter { } Err(e) => { let error_msg = format!("Tool '{}' failed: {}", lookup_name, e); + + // Detect authentication_required errors from the HTTP tool. + // Emit an AuthRequired SSE event as a side effect (for connected + // frontends) but return the error normally — the LLM sees it and + // tells the user. This avoids blocking mission/sub-threads that + // have no channel context. + if error_msg.contains("authentication_required") { + if let Some(cred_name) = extract_credential_name(&error_msg) { + tracing::warn!( + credential = %cred_name, + tool = %lookup_name, + user = %context.user_id, + "Credential missing — emitting auth_required event" + ); + self.emit_auth_required(&cred_name, action_name).await; + } + } + let sanitized = self.safety.sanitize_tool_output(lookup_name, &error_msg); Ok(ActionResult { @@ -502,9 +541,24 @@ fn parse_cadence(s: &str) -> ironclaw_engine::types::mission::MissionCadence { } } -/// Tools that depend on v1 runtime components (RoutineEngine, Scheduler, -/// ContainerJobManager) and cannot work in engine v2's minimal JobContext. -/// Note: routine_* tools are NOT blocked — they map to mission operations. +/// Extract credential name from an authentication_required error message. +/// +/// The HTTP tool returns errors like: +/// `{"error":"authentication_required","credential_name":"github_token",...}` +fn extract_credential_name(error_msg: &str) -> Option { + // The error is JSON-encoded inside the tool error string. + // Find the JSON portion and parse credential_name from it. + if let Some(json_start) = error_msg.find('{') { + if let Ok(parsed) = serde_json::from_str::(&error_msg[json_start..]) { + return parsed + .get("credential_name") + .and_then(|v| v.as_str()) + .map(String::from); + } + } + None +} + fn is_v1_only_tool(name: &str) -> bool { matches!( name, @@ -579,4 +633,91 @@ mod tests { adapter.auto_approve_tool("shell").await; assert!(adapter.auto_approved.read().await.contains("shell")); } + + // ── extract_credential_name tests ────────────────────────── + + #[test] + fn extract_credential_from_auth_required_error() { + let msg = r#"Tool 'http' failed: execution failed: {"error":"authentication_required","credential_name":"github_token","message":"Credential 'github_token' is not configured."}"#; + assert_eq!( + extract_credential_name(msg), + Some("github_token".to_string()) + ); + } + + #[test] + fn extract_credential_from_nested_json() { + let msg = r#"Tool 'http' failed: {"error":"authentication_required","credential_name":"linear_api_key","message":"Use auth_setup"}"#; + assert_eq!( + extract_credential_name(msg), + Some("linear_api_key".to_string()) + ); + } + + #[test] + fn extract_credential_returns_none_for_non_auth_error() { + let msg = "Tool 'http' failed: connection timeout"; + assert_eq!(extract_credential_name(msg), None); + } + + #[test] + fn extract_credential_returns_none_for_json_without_credential() { + let msg = r#"Tool 'http' failed: {"error":"not_found","message":"404"}"#; + assert_eq!(extract_credential_name(msg), None); + } + + // ── auth_required_callback tests ─────────────────────────── + + #[tokio::test] + async fn auth_callback_fires_on_missing_credential() { + let adapter = make_adapter(); + + let fired = Arc::new(std::sync::Mutex::new(Vec::<(String, String)>::new())); + let fired_clone = Arc::clone(&fired); + + adapter + .set_auth_required_callback(Arc::new(Box::new(move |cred, action| { + fired_clone + .lock() + .unwrap() + .push((cred.to_string(), action.to_string())); + }))) + .await; + + adapter.emit_auth_required("github_token", "http").await; + + let calls = fired.lock().unwrap(); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].0, "github_token"); + assert_eq!(calls[0].1, "http"); + } + + #[tokio::test] + async fn auth_callback_not_set_is_noop() { + let adapter = make_adapter(); + // No callback set — should not panic + adapter.emit_auth_required("some_token", "http").await; + } + + // ── is_v1_only_tool tests ────────────────────────────────── + + #[test] + fn routine_tools_are_v1_only() { + assert!(is_v1_only_tool("routine_create")); + assert!(is_v1_only_tool("routine_list")); + assert!(is_v1_only_tool("routine_fire")); + assert!(is_v1_only_tool("routine_delete")); + assert!(is_v1_only_tool("routine_pause")); + assert!(is_v1_only_tool("routine_resume")); + assert!(is_v1_only_tool("routine_update")); + } + + #[test] + fn mission_tools_are_not_v1_only() { + assert!(!is_v1_only_tool("mission_create")); + assert!(!is_v1_only_tool("mission_list")); + assert!(!is_v1_only_tool("mission_fire")); + assert!(!is_v1_only_tool("http")); + assert!(!is_v1_only_tool("web_search")); + } } diff --git a/src/bridge/router.rs b/src/bridge/router.rs index 711384a4..72bd3e0d 100644 --- a/src/bridge/router.rs +++ b/src/bridge/router.rs @@ -114,6 +114,26 @@ pub async fn init_engine(agent: &Agent) -> Result<(), Error> { agent.hooks().clone(), )); + // Wire auth_required callback: emits SSE event when a credential is missing. + // Best-effort — if no frontend is connected, the event is silently dropped. + if let Some(sse) = agent.deps.sse_tx.clone() { + let sse_for_auth = sse; + effect_adapter + .set_auth_required_callback(Arc::new(Box::new(move |credential_name, action_name| { + let event = ironclaw_common::AppEvent::AuthRequired { + extension_name: credential_name.to_string(), + instructions: Some(format!( + "Tool '{}' needs the '{}' credential. Please authenticate to continue.", + action_name, credential_name + )), + auth_url: None, + setup_url: None, + }; + sse_for_auth.broadcast(event); + }))) + .await; + } + let store = Arc::new(HybridStore::new(agent.workspace().cloned())); store.load_state_from_workspace().await; @@ -329,6 +349,9 @@ pub async fn init_engine(agent: &Agent) -> Result<(), Error> { .set_mission_manager(Arc::clone(&mission_manager)) .await; + // Wire mission manager into agent for /expected command + agent.set_mission_manager(Arc::clone(&mission_manager)).await; + *guard = Some(EngineState { thread_manager, conversation_manager, @@ -976,6 +999,11 @@ async fn await_thread_outcome( let sse = state.sse.as_ref(); let tid_str = thread_id.to_string(); + // Safety timeout: if the thread doesn't finish within 5 minutes, + // break out to avoid hanging the user session forever (e.g. after + // a denied approval where the thread fails to resume). + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(300); + loop { tokio::select! { event = event_rx.recv() => { @@ -996,6 +1024,13 @@ async fn await_thread_outcome( if !state.thread_manager.is_running(thread_id).await { break; } + if tokio::time::Instant::now() >= deadline { + tracing::warn!( + thread_id = %thread_id, + "await_thread_outcome timed out after 5 minutes — breaking to avoid hang" + ); + break; + } } } } @@ -1103,6 +1138,15 @@ async fn await_thread_outcome( action_name ))) } + ThreadOutcome::NeedAuthentication { credential_name, .. } => { + // This shouldn't reach here in the non-blocking design (the error + // flows through the LLM as a normal action result), but handle + // gracefully in case it does. + Ok(Some(format!( + "Authentication required for '{}'. Please set up the credential and try again.", + credential_name + ))) + } } }