Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
1727b74
fix(workspace): collapse reindex delete+insert into one transaction t…
ilblackdragon Apr 9, 2026
9b90335
fix(auth): resolve display name + extension target from action when s…
ilblackdragon Apr 9, 2026
8b85176
fix(mcp): canonicalize MCP tool identifiers to snake_case at registra…
ilblackdragon Apr 9, 2026
d060aa7
fix(llm): flatten top-level schema unions for OpenAI + symmetric Pyth…
ilblackdragon Apr 9, 2026
b269c99
test(live): add Drive auth-gate round-trip live test + supporting har…
ilblackdragon Apr 9, 2026
eba5e1a
review(llm): log on python_json_to_action_calls deserialize failure
ilblackdragon Apr 9, 2026
7613eec
review(test): generate random master key per Config::for_testing call
ilblackdragon Apr 9, 2026
3db6bde
review(mcp): normalize all non-identifier characters in mcp_tool_id
ilblackdragon Apr 9, 2026
dccf707
review(llm): make flatten_top_level hint keyword-aware
ilblackdragon Apr 9, 2026
defc3d8
review(workspace): serialize postgres replace_chunks via FOR UPDATE o…
ilblackdragon Apr 10, 2026
d9175e0
review(llm): merge top-level union variants into flatten_top_level en…
ilblackdragon Apr 10, 2026
02cc0dd
review(router): extract resolve_extension_for_action helper for 3 dup…
ilblackdragon Apr 10, 2026
8633cb9
review(mcp): warn on post-normalization tool name collisions in creat…
ilblackdragon Apr 10, 2026
64fa506
review(engine): drop entries from action_calls_to_python_json on fail…
ilblackdragon Apr 10, 2026
5b4bd64
review(engine): summarize action_calls in warn log to avoid leaking PII
ilblackdragon Apr 10, 2026
791ec4c
incorporate #2227: factory server_name normalization, registry bidire…
ilblackdragon Apr 10, 2026
3bc68be
review(manager): normalize server name prefix in starts_with tool-lis…
ilblackdragon Apr 10, 2026
4f1e26b
review(llm): accept array type containing "object" in needs_top_level…
ilblackdragon Apr 10, 2026
31a4019
review(mcp): update seen_ids on collision so 3rd collision reports ag…
ilblackdragon Apr 10, 2026
25b5ac0
review(wasm): fix CI type mismatch + add string-matching fallback for…
ilblackdragon Apr 10, 2026
daa1811
review: tighten unreachable trap match, cap schema serialization, doc…
ilblackdragon Apr 10, 2026
a02066d
fix(llm): ensure array items is a JSON Schema object for OpenAI stric…
ilblackdragon Apr 10, 2026
a8536bc
fix(llm): add post-normalization validation to catch schema rules the…
ilblackdragon Apr 10, 2026
e913567
fix(llm): normalize merged properties on flatten path + silence null …
ilblackdragon Apr 10, 2026
42add86
review(llm): replace node-counting DoS guard with size-capped serializer
ilblackdragon Apr 10, 2026
311be2b
fix(engine): serialize bootstrap context action_calls through PythonA…
ilblackdragon Apr 10, 2026
0844b5d
test(engine): add bootstrap-path round-trip test to guard against fut…
ilblackdragon Apr 10, 2026
7612c2f
fix(llm): skip strict-mode post-validator on flatten path to eliminat…
ilblackdragon Apr 10, 2026
a7d696a
test: close 5 coverage gaps across schema normalization, action_calls…
ilblackdragon Apr 10, 2026
d897cb1
review: address all 6 review items — UTF-8 safety, FOR UPDATE row che…
ilblackdragon Apr 10, 2026
6d3ebce
review(workspace): narrow reindex concurrency check error handling
ilblackdragon Apr 10, 2026
a2cc460
fix(tools): canonicalize paths in file_history to fix macOS symlink m…
ilblackdragon Apr 10, 2026
c2869c9
fix(ci): update e2e_live_personas to match refactored test harness APIs
henrypark133 Apr 10, 2026
a92ce09
fix(tools): deduplicate path canonicalization in file_history::snapshot
henrypark133 Apr 10, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
579 changes: 566 additions & 13 deletions crates/ironclaw_engine/src/executor/orchestrator.rs

Large diffs are not rendered by default.

138 changes: 114 additions & 24 deletions src/bridge/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,56 @@ fn gate_display_parameters(pending: &PendingGate) -> serde_json::Value {
.unwrap_or_else(|| pending.parameters.clone())
}

async fn send_pending_gate_status(agent: &Agent, message: &IncomingMessage, pending: &PendingGate) {
/// Resolve the owning extension name for a tool action, falling back to a
/// credential name when the action isn't extension-backed. This is the
/// shared core of the auth-gate display + submit routing logic — the same
/// `provider_extension_for_tool + unwrap_or_else(credential_name)` pattern
/// fired in three different sites in this file before, each one with the
/// same fallback rationale: the engine's `ResumeKind::Authentication` only
/// carries `credential_name` (e.g. `google_oauth_token`), which is opaque
/// to users AND fails when fed back into `submit_auth_token` for
/// WASM-tool-backed credentials, while the owning extension name (e.g.
/// `google-drive-tool`) is what both the user-facing UI and
/// `submit_auth_token` actually want. For built-in tools, HTTP, and skill
/// credentials there's no owning extension and the fallback is the right
/// thing.
async fn resolve_extension_for_action(
tools: &crate::tools::ToolRegistry,
action_name: &str,
credential_fallback: &str,
) -> String {
tools
.provider_extension_for_tool(action_name)
.await
.unwrap_or_else(|| credential_fallback.to_string())
}

/// Resolve the user-facing name to use when surfacing an authentication
/// gate to a channel. Thin wrapper around `resolve_extension_for_action`
/// that handles the non-Authentication ResumeKind variants by falling back
/// to the action name (since they don't have a credential name to use).
async fn resolve_auth_gate_display_name(
tools: &crate::tools::ToolRegistry,
pending: &PendingGate,
) -> String {
if let ironclaw_engine::ResumeKind::Authentication {
credential_name, ..
} = &pending.resume_kind
{
resolve_extension_for_action(tools, &pending.action_name, credential_name).await
} else {
// Non-authentication gates don't use this string; return
// something innocuous.
pending.action_name.clone()
}
}

async fn send_pending_gate_status(
agent: &Agent,
message: &IncomingMessage,
pending: &PendingGate,
auth_display_name: &str,
) {
let display_parameters = gate_display_parameters(pending);

match &pending.resume_kind {
Expand All @@ -72,16 +121,16 @@ async fn send_pending_gate_status(agent: &Agent, message: &IncomingMessage, pend
.await;
}
ironclaw_engine::ResumeKind::Authentication {
credential_name,
instructions,
auth_url,
..
} => {
let _ = agent
.channels
.send_status(
&message.channel,
StatusUpdate::AuthRequired {
extension_name: credential_name.clone(),
extension_name: auth_display_name.to_string(),
instructions: Some(instructions.clone()),
auth_url: auth_url.clone(),
setup_url: None,
Expand All @@ -94,17 +143,15 @@ async fn send_pending_gate_status(agent: &Agent, message: &IncomingMessage, pend
}
}

fn pending_gate_prompt_message(pending: &PendingGate) -> Option<String> {
fn pending_gate_prompt_message(pending: &PendingGate, auth_display_name: &str) -> Option<String> {
match &pending.resume_kind {
ironclaw_engine::ResumeKind::Approval { .. } => Some(format!(
"Tool '{}' requires approval. Reply 'yes' to approve, 'no' to deny.",
pending.action_name
)),
ironclaw_engine::ResumeKind::Authentication {
credential_name, ..
} => Some(format!(
ironclaw_engine::ResumeKind::Authentication { .. } => Some(format!(
"Authentication required for '{}'. Paste your token below (or type 'cancel'):",
credential_name
auth_display_name
)),
ironclaw_engine::ResumeKind::External { .. } => Some(format!(
"Waiting for external confirmation (gate: {})...",
Expand Down Expand Up @@ -244,10 +291,12 @@ fn parse_credential_name(text: &str) -> Option<String> {
async fn notify_pending_gate(
agent: &Agent,
sse: Option<Arc<SseManager>>,
tools: &crate::tools::ToolRegistry,
message: &IncomingMessage,
pending: &PendingGate,
) -> Result<Option<String>, Error> {
let display_parameters = gate_display_parameters(pending);
let auth_display_name = resolve_auth_gate_display_name(tools, pending).await;

if let Some(sse) = sse {
sse.broadcast_for_user(
Expand Down Expand Up @@ -276,8 +325,8 @@ async fn notify_pending_gate(
);
}

send_pending_gate_status(agent, message, pending).await;
Ok(pending_gate_prompt_message(pending))
send_pending_gate_status(agent, message, pending, &auth_display_name).await;
Ok(pending_gate_prompt_message(pending, &auth_display_name))
}

async fn insert_and_notify_pending_gate(
Expand All @@ -292,7 +341,14 @@ async fn insert_and_notify_pending_gate(
.await
.map_err(|e| engine_err("pending gate insert", e))?;

notify_pending_gate(agent, state.sse.clone(), message, &pending).await
notify_pending_gate(
agent,
state.sse.clone(),
state.effect_adapter.tools(),
message,
&pending,
)
.await
}

async fn execute_pending_gate_action(
Expand Down Expand Up @@ -1566,6 +1622,21 @@ pub async fn resolve_gate(
..
} = pending.resume_kind
{
// `submit_auth_token` expects an *extension name* as
// its first argument and uses `configure_token` to walk
// the extension's capabilities file for the actual
// secret name. The engine's `ResumeKind::Authentication`
// only carries `credential_name`, which fails closed
// when fed there for WASM-tool-backed credentials. See
// `resolve_extension_for_action` for the full rationale.
let submit_target = resolve_extension_for_action(
state.effect_adapter.tools(),
&pending.action_name,
credential_name,
)
.await;
let display_name = submit_target.clone();

if let Some(ref sse) = state.sse {
sse.broadcast_for_user(
&message.user_id,
Expand All @@ -1584,7 +1655,7 @@ pub async fn resolve_gate(
}
if let Some(ref auth_manager) = state.auth_manager {
match auth_manager
.submit_auth_token(credential_name, &token, &message.user_id)
.submit_auth_token(&submit_target, &token, &message.user_id)
.await
{
Ok(result) if result.activated => {
Expand All @@ -1593,7 +1664,7 @@ pub async fn resolve_gate(
.send_status(
&message.channel,
StatusUpdate::AuthCompleted {
extension_name: credential_name.clone(),
extension_name: display_name.clone(),
success: true,
message: format!("{}. Resuming...", result.message),
},
Expand All @@ -1607,7 +1678,7 @@ pub async fn resolve_gate(
.send_status(
&message.channel,
StatusUpdate::AuthRequired {
extension_name: credential_name.clone(),
extension_name: display_name.clone(),
instructions: Some(result.message.clone()),
auth_url: result.auth_url.clone(),
setup_url: None,
Expand All @@ -1623,7 +1694,7 @@ pub async fn resolve_gate(
.send_status(
&message.channel,
StatusUpdate::AuthRequired {
extension_name: credential_name.clone(),
extension_name: display_name.clone(),
instructions: Some(msg.clone()),
auth_url: None,
setup_url: None,
Expand All @@ -1640,7 +1711,7 @@ pub async fn resolve_gate(
.send_status(
&message.channel,
StatusUpdate::AuthCompleted {
extension_name: credential_name.clone(),
extension_name: display_name.clone(),
success: false,
message: msg.clone(),
},
Expand Down Expand Up @@ -2226,14 +2297,19 @@ async fn handle_with_engine_inner(
) =>
{
let pending = gate.clone();
// Clone the SSE arc out of state, then drop the engine read
// guard before awaiting on broadcast + channel I/O. The auth
// branch above does the same, and `notify_pending_gate` is
// signed to accept an owned Option<Arc<SseManager>> precisely
// so this terminal-return branch can release the lock.
// Clone the SSE arc and the tools registry out of state,
// then drop the engine read guard before awaiting on
// broadcast + channel I/O. The auth branch above does the
// same, and `notify_pending_gate` is signed to accept an
// owned Option<Arc<SseManager>> precisely so this
// terminal-return branch can release the lock. The tools
// registry handle is needed by `notify_pending_gate` to
// resolve the auth-gate display name without holding the
// engine state lock.
let sse = state.sse.clone();
let tools = Arc::clone(state.effect_adapter.tools());
drop(guard);
return notify_pending_gate(agent, sse, message, &pending).await;
return notify_pending_gate(agent, sse, tools.as_ref(), message, &pending).await;
}
PendingGateResolution::Ambiguous => {
return Ok(Some(
Expand Down Expand Up @@ -2715,12 +2791,26 @@ async fn await_thread_outcome(
instructions,
auth_url,
} => {
// Channel UIs render `extension_name` as "Authentication
// required for 'X'", and `credential_name` (e.g.
// `google_oauth_token`) is opaque to users, while the
// owning extension name (e.g. `google-drive-tool`) is
// the integration they recognise. See
// `resolve_extension_for_action` for the full rationale
// and the fallback semantics for non-WASM credentials.
let extension_for_display = resolve_extension_for_action(
state.effect_adapter.tools(),
&action_name,
credential_name,
)
.await;

let _ = agent
.channels
.send_status(
&message.channel,
StatusUpdate::AuthRequired {
extension_name: credential_name.clone(),
extension_name: extension_for_display.clone(),
instructions: Some(instructions.clone()),
auth_url: auth_url.clone(),
setup_url: None,
Expand All @@ -2731,7 +2821,7 @@ async fn await_thread_outcome(

Ok(Some(format!(
"Authentication required for '{}'. Paste your token below (or type 'cancel'):",
credential_name
extension_for_display
)))
}
ironclaw_engine::ResumeKind::External { callback_id } => {
Expand Down
6 changes: 4 additions & 2 deletions src/channels/wasm/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ impl WasmChannelLoader {
}

let name = match path.file_stem().and_then(|s| s.to_str()) {
Some(n) => n.to_string(),
Some(n) => n.replace('-', "_"),
None => {
results.errors.push((
path.clone(),
Expand All @@ -233,6 +233,8 @@ impl WasmChannelLoader {
}
};

// Look up capabilities using the original filename (before
// hyphen normalization) so existing sidecar files are found.
let cap_path = path.with_extension("capabilities.json");
let has_cap = cap_path.exists();
channel_entries.push((name, path, if has_cap { Some(cap_path) } else { None }));
Expand Down Expand Up @@ -382,7 +384,7 @@ pub async fn discover_channels(
}

let name = match path.file_stem().and_then(|s| s.to_str()) {
Some(n) => n.to_string(),
Some(n) => n.replace('-', "_"),
None => continue,
};

Expand Down
40 changes: 39 additions & 1 deletion src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,27 @@ pub struct Config {
pub relay: Option<RelayConfig>,
}

/// Generate a fresh random AES-256-GCM master key for `Config::for_testing`.
///
/// Returns a hex-encoded 32-byte key (64 hex chars), satisfying the length
/// check in `SecretsConfig::resolve`. Each call returns a different value —
/// tests don't need cross-process determinism (each test builds a fresh
/// secrets store on top of a fresh temp DB), and committing a constant
/// master key into the source tree would mean every developer who built
/// with `--features libsql` had a publicly-known key in their process.
#[cfg(feature = "libsql")]
Comment thread
henrypark133 marked this conversation as resolved.
fn generate_test_master_key() -> secrecy::SecretString {
use rand::RngCore;
let mut bytes = [0u8; 32];
rand::thread_rng().fill_bytes(&mut bytes);
let mut hex = String::with_capacity(64);
for b in bytes {
use std::fmt::Write;
let _ = write!(hex, "{:02x}", b);
}
secrecy::SecretString::from(hex)
}

impl Config {
/// Create a full Config for integration tests without reading env vars.
///
Expand Down Expand Up @@ -176,7 +197,24 @@ impl Config {
enabled: false,
..WasmConfig::default()
},
secrets: SecretsConfig::default(),
// Test config gets a freshly-generated random master key so
// the secrets store is wired up out of the box. Without this,
// every replay-mode test that touches credentials would have
// to either build its own SecretsStore or skip the secrets
// path entirely. The key is generated per call (NOT a
// hardcoded constant) — `Config::for_testing` is `pub` so
// anything in the crate or downstream tests can call it, and
// committing a known master key into the source tree would
// mean every developer who built with `--features libsql`
// had a publicly-known AES-256-GCM key sitting in their
// process. Tests don't need cross-process determinism here:
// each test creates its own temp DB, so the secrets store
// is born fresh on every call anyway.
secrets: SecretsConfig {
master_key: Some(generate_test_master_key()),
enabled: true,
source: crate::settings::KeySource::Env,
},
builder: BuilderModeConfig {
enabled: false,
..BuilderModeConfig::default()
Expand Down
Loading
Loading