fix: routines — make cadence mandatory and expose guardrails

[gagdiez]

Agents could not create reactive missions (e.g. "log every telegram message") because of three shortcomings:

1. Cadence (i.e. when to trigger the mission) was an optional parameter that defaulted to "manual", even worst, malformed values (e.g. if the llm made a mistake and passed "every 5 min on telegram") were interpreted as: "this is a routine that needs manual triggering"
2. After fixing that, I discovered that reactive missions (event/webhook) had a hardcoded 300s cooldown, and there was no way to override it, so my event would only "react" every 5 minutes
3. mission_update and mission_delete existed in code but were not documented in the CodeAct preamble or the Tier 0 ActionDef schemas, so the LLM never called them

Changes:

• "cadence" now is a mandatory field, and it needs to be well-formed, otherwise we return an error so the LLM can correct the call
• mission_create and mission_update now expose guardrail params (cooldown_secs, max_concurrent, dedup_window_secs, max_threads_per_day) so llms can define them
• mission_list output now includes cadence and guardrails so misconfigured missions are visible
• mission_update and mission_delete are exposed
• Regression tests for malformed cadence rejection

Change Type

• Bug fix
• New feature
• Refactor
• Documentation
• CI/Infrastructure
• Security
• Dependencies

Linked Issue

Closes #2524

Validation

• cargo fmt --all -- --check
• cargo clippy --all --benches --tests --examples --all-features -- -D warnings
• cargo build
• Relevant tests pass: cargo test -p ironclaw --lib effect_adapter
• cargo test --features integration if database-backed or integration behavior changed
• Manual testing: deployed the compiled binary to my server, made sure that now ironclaw knows how to create "a routine to log all telegram messages locally"
• If a coding agent was used and supports it, review-pr or pr-shepherd --fix was run before requesting review

Security Impact

None

Database Impact

None

Blast Radius

routines

[gemini-code-assist]

Code Review

This pull request enhances the mission management system by making the cadence parameter mandatory and introducing new guardrail parameters such as cooldown_secs, max_concurrent, and max_threads_per_day. The parse_cadence function was refactored to return a Result for improved error reporting, and the mission_list output now includes these additional configuration fields. Review feedback highlights that notify_channels is missing from the preamble documentation, success_criteria is not being extracted or defined in the mission_create schema, and the mission_list output should use a more user-friendly string representation for the cadence field instead of Rust's Debug format.

[Copilot]

Pull request overview

This PR fixes mission/routine triggering configuration so agents can reliably create reactive missions by making cadence validation strict, exposing guardrail knobs via the mission tools, and documenting the full mission lifecycle actions for the LLM.

Changes:

• Make cadence required for mission_create and reject unrecognized cadence strings instead of silently defaulting to manual.
• Allow specifying guardrails (cooldown_secs, max_concurrent, dedup_window_secs, max_threads_per_day) via mission_create/mission_update, and include cadence/guardrails in mission_list output.
• Update the CodeAct preamble to document mission_update / mission_delete and the updated mission_create signature.

Reviewed changes

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

File	Description
src/bridge/router.rs	Updates mission ActionDef schemas (required cadence; guardrail params for create/update).
src/bridge/effect_adapter.rs	Enforces cadence presence/validation, plumbs guardrail params through post-create update, and expands mission_list output; adds regression tests.
crates/ironclaw_engine/prompts/codeact_preamble.md	Documents updated mission tool signatures and adds mission_update/mission_delete to the preamble.

---

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

[Copilot]

Pull request overview

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

---

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

[Copilot]

Pull request overview

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

---

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

[Copilot]

Pull request overview

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

---

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

[Copilot]

Pull request overview

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

---

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

[Copilot]

Pull request overview

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

---

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

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

src/bridge/router.rs:1161

• mission_delete is documented here as “mark completed”, but the schema still describes the id as “ID … to delete”, and the implementation currently returns { "status": "deleted" } while calling complete_mission. This mismatch can confuse tool/LLM callers; consider aligning terminology and output (e.g., change the status string to completed and update the parameter description), or revert the docs if the intent is true deletion.

                description: "Mark a mission or routine as completed (sets status to completed).".into(),
                parameters_schema: serde_json::json!({
                    "type": "object",
                    "properties": {
                        "id": {"type": "string", "description": "Mission/routine ID to delete"}
                    },

---

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

[henrypark133]

Review: request changes

This closes most of the earlier bot comments and the stricter cadence/guardrail validation is the right direction, but I found two remaining contract bugs plus a coverage gap in the new regression tests.

Positives:

• cadence now fails loudly instead of silently defaulting to Manual.
• Guardrail integers are validated before create/update, which avoids the earlier ghost-mission path.

Concerning: mission_list is still not round-trippable for system missions

File: src/bridge/effect_adapter.rs:382, src/bridge/effect_adapter.rs:1562
mission_list now advertises cadence as something callers/LLMs can reuse, but cadence_to_round_trip_string() emits system_event:<source>/<event_type> for OnSystemEvent missions while parse_cadence() still rejects that syntax. MissionManager::list_missions() includes the built-in learning missions created by ensure_learning_missions(), so the model can list one of those missions and then immediately fail to feed the returned cadence back into mission_update.

Suggested fix: either keep mission_list structured for non-user-editable cadences, or teach parse_cadence() / the update contract how to round-trip OnSystemEvent.

Concerning: mission_delete still reports a delete that never happened

File: src/bridge/effect_adapter.rs:462
The preamble and ActionDef now correctly say mission_delete marks a mission completed, but the action result still returns { "status": "deleted" }. That leaves the model/client with conflicting signals about whether the mission was removed or just transitioned to Completed.

Suggested fix: return a status like "completed" or "marked_completed" so the runtime result matches the documented semantics.

Concerning: the new regression coverage stops at helpers instead of the mission action call path

File: src/bridge/effect_adapter.rs:2473
The added tests only exercise extract_guardrails() / parse_cadence() directly. The behavior change actually lands in handle_mission_call() where JSON extraction, defaults, and create_mission/update_mission wiring happen, and .claude/rules/testing.md explicitly requires caller-level coverage for helper-gated side effects. Without an execute_action("mission_create"/"mission_update", ...) regression, this exact layer can still regress even if the helper tests stay green.

Suggested fix: add one execute_action("mission_create", ...) regression for missing/malformed cadence and one execute_action("mission_update", ...) regression for wrong-type guardrails.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/bridge/router.rs:1161

• mission_delete is described as marking a mission completed, but the parameter schema still says the id is the mission/routine ID "to delete". This is inconsistent/misleading for callers now that the action is exposed; update the parameter description (and/or action name) so the contract matches the implemented semantics (complete vs delete).

                description: "Mark a mission or routine as completed (sets status to completed).".into(),
                parameters_schema: serde_json::json!({
                    "type": "object",
                    "properties": {
                        "id": {"type": "string", "description": "Mission/routine ID to delete"}
                    },

---

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

[gagdiez]

addressed all issues raised by @henrypark133 in f45d8a0