fix(bridge): coerce engine action params per schema (#3132)

[ilblackdragon]

Summary

• mission_create and mission_update rejected cooldown_secs="120" with 'cooldown_secs' must be an integer, got "120" when the LLM passed integer guardrails as JSON strings (issue Mission creation fails - error': '\'cooldown_secs\' must be an integer, got "120"' #3132).
• Root cause was a pipeline split: host tools coerce stringified scalars against their JSON Schema in ToolDispatcher::dispatch → prepare_tool_params, but engine actions (mission_*, routine_* aliases, tool_info) are intercepted in effect_adapter::execute_action_internal before that coercion runs.
• Lift schema-guided coercion to the engine-action dispatch boundary so both pipelines share the same input pre-amble. Schema sources, in order: orchestrator-populated available_actions_snapshot, bridge-known mission action defs (mission_capability_actions()), host tool registry. Idempotent — host-tool path sees unchanged shape since prepare_tool_params runs again downstream.
• extract_guardrails / strict_u64 stay strict as defense in depth: if a future code path bypasses the new pre-amble, it still fails loudly rather than silently dropping the value (the bug shape from before Unlock REPL stdin when auth is required #2630).

Why this approach over making engine actions into Tools

Engine actions are engine-runtime operations on MissionManager / Store / LeaseManager. They take ThreadExecutionContext (step_id, current_call_id, available_actions_snapshot, user_timezone, thread_goal), not JobContext; they are gated by CapabilityLease+EffectType rather than requires_approval/rate-limit; and they belong to the engine crate, which has no host dependency. Engine v2's design direction is the opposite — Capability/ActionDef replacing Tool. Forcing engine actions through Tool would re-introduce a circular dependency, force the wrong context type, and double up on gating semantics. The right fix is to share the parts of the pipeline that should be shared (coercion, validation, redaction, sanitization), not to merge the abstractions.

Test plan

• cargo fmt --check
• cargo clippy --all --benches --tests --examples --all-features — zero warnings
• cargo test --lib — 5598 passed, 0 failed
• mission_create_string_guardrails_coerced_via_execute_action — drives execute_action with all four guardrail params (cooldown_secs, max_concurrent, dedup_window_secs, max_threads_per_day) as strings; asserts the values persist correctly to the mission row in the backing store.
• mission_update_string_guardrails_coerced_via_execute_action — same coverage for the update path.
• mission_create_non_coercible_string_guardrail_returns_error — cooldown_secs="abc" still surfaces a clean error.
• Existing extract_guardrails_rejects_string_typed_integers continues to pass; doc comment updated to reflect its defense-in-depth role.

Resolves #3132.

[gemini-code-assist]

Code Review

This pull request implements schema-guided parameter coercion for engine actions and host tools, allowing the bridge to correctly type stringified scalars from LLMs. The changes involve resolving schemas from multiple sources and applying coercion via a newly exported utility. Review feedback suggests refactoring the coercion logic in effect_adapter.rs to improve readability and avoid variable shadowing by using an if/else if structure instead of nested matches.

[gemini-code-assist]

The logic for finding the action schema and coercing parameters is a bit complex and could be simplified for better readability. The current implementation uses shadowing and a match statement that could be expressed more clearly. Consider refactoring this block to use a single if/else if chain. This would make the fallback logic for finding the schema more explicit and easier to follow, separating the checks for distinct conditions. This avoids shadowing action_schema and consolidates the coercion logic.

        let action_schema = context
            .available_actions_snapshot
            .as_ref()
            .and_then(|snapshot| {
                ActionDiscovery::resolve(snapshot.as_ref(), canonical_action_name)
                    .map(|action| action.parameters_schema.clone())
            })
            .or_else(|| engine_action_schema(canonical_action_name));

        let parameters = if let Some(schema) = action_schema {
            crate::tools::prepare_params_for_schema(&parameters, &schema)
        } else if let Some(tool) = self.tools.get(&lookup_name).await {
            let schema = tool.parameters_schema();
            crate::tools::prepare_params_for_schema(&parameters, &schema)
        } else {
            parameters
        };

References

1. Separate checks for distinct conditions to improve code clarity and robustness.

[ilblackdragon]

Applied in 023a4c5 — collapsed the double-match into the if/else if chain you suggested. Used discovery_schema() on both branches to match prepare_tool_params (also addresses the sibling Copilot comment).

[Copilot]

Pull request overview

This PR fixes engine-v2 action dispatch so schema-guided parameter coercion happens before bridge-handled engine actions execute, aligning mission/routine action handling more closely with the existing host-tool normalization pipeline. In the broader codebase, this targets the bridge layer where engine-native actions bypass the standard tool dispatcher.

Changes:

• Exported shared schema-coercion helper so bridge code can normalize params directly from a JSON Schema.
• Added pre-dispatch schema-based coercion in EffectBridgeAdapter::execute_action_internal, sourcing schemas from action snapshots, engine mission action defs, or the tool registry.
• Reworked regression tests to assert stringified mission guardrail integers are now persisted correctly and that non-coercible strings still fail cleanly.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/tools/mod.rs	Re-exports the lower-level schema coercion helper for bridge use.
src/bridge/effect_adapter.rs	Adds early action parameter coercion, helper schema lookup, updated comments, and mission guardrail regression tests.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ilblackdragon]

Already addressed in commit e492f59 — both the snapshot/engine_action and host-tool branches now use discovery_schema() (the structural follow-up in 023a4c5 keeps that). The intent matches prepare_tool_params's historical behavior, including for tools where discovery_schema() is richer than parameters_schema() (compacted WASM schemas, routine compat aliases).

[ilblackdragon]

Doc rewritten in 023a4c5 — only mission_* is actually absent from the host registry. routine_* (legacy v1 host Tools, intercepted by the alias path before they execute) and tool_info (a v1/v2 host tool) are present and reach execute_action_internal's registry branch directly. The new doc explains the asymmetry honestly and clarifies that this helper is a defense-in-depth fallback for the only category that doesn't have a host Tool.

[ilblackdragon]

Pushed e492f598 to actually lift the old usage rather than just adding a new one alongside it:

• Use discovery_schema() consistently in the upstream coercion (matches prepare_tool_params's behavior for tools where discovery_schema is the richer one).
• Drop the explicit prepare_tool_params call in the sandbox branch — parameters is already coerced once upstream, so both the validator and maybe_intercept see the same shape without a duplicate pass.

The prepare_tool_params call still lives inside execute_tool_with_safety (tools/execute.rs:40); for engine v2 callers it's now idempotent, and v1 callers like execute_chat_tool_standalone still rely on it. Pulling that one out is a v1-side refactor outside #3132's scope.

Verification: cargo fmt --check ✓, cargo clippy --all-features ✓ (zero warnings), cargo test --lib ✓ (5598 passed).

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.