fix(triggers): add unattended scheduled-run protocol - #7495
serrrfirat wants to merge 1 commit into
Conversation
github-actions
Bot
added
scope: docs
🧭 IronLoop Run · ReviewThis comment updates in place as the Run moves through its stages. 🟩 Final result · Completed
Automatic trigger · attempt 1 of 3 · completed in 14m 54s IronLoop completed the review and posted it to GitHub. 🔗 Result |
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesScheduled-trigger prompt behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/app/ironclaw_composition/src/root/default_system_prompt.rs`:
- Around line 558-604: The test
scheduled_trigger_origin_appends_unattended_protocol_only_to_triggered_runs
currently covers only Inbound as the non-scheduled case. Add a
TurnOriginKind::WebUi context, resolve its prompt content through the same
helper, and assert it does not contain the unattended scheduled-run instructions
while preserving the existing Inbound and ScheduledTrigger assertions.
In `@crates/loop/ironclaw_loop_host/prompts/scheduled_trigger_mode.md`:
- Around line 5-6: Update the scheduled trigger instructions near “When details
are ambiguous” to define the unsafe-ambiguity path: stop execution, report the
specific missing input, and record a self-contained result without asking a
question or inventing a value. Preserve the existing requirement to make bounded
assumptions when they are safe.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5a44ea44-f1ba-46e2-bd93-8d842ca4fb3f
📒 Files selected for processing (7)
crates/app/ironclaw_composition/src/root/default_system_prompt.rscrates/loop/ironclaw_loop_host/AGENTS.mdcrates/loop/ironclaw_loop_host/prompts/scheduled_trigger_mode.mdcrates/loop/ironclaw_loop_host/src/lib.rscrates/loop/ironclaw_loop_host/src/system_prompt_assets.rsdocs/reborn/contracts/triggers.mddocs/reborn/target-architecture/families/loop.md
| #[tokio::test] | ||
| async fn scheduled_trigger_origin_appends_unattended_protocol_only_to_triggered_runs() { | ||
| let root = tempfile::tempdir().expect("tempdir"); | ||
| let storage_root = root.path().canonicalize().expect("canonical root"); | ||
| let prompt_path = storage_root.join("system/prompts/default-system.md"); | ||
| seed_default_system_prompt(&storage_root, &prompt_path).expect("prompt seeds"); | ||
| let source = | ||
| DefaultSystemPromptIdentitySource::try_new(storage_root, prompt_path, false, false) | ||
| .expect("prompt loads"); | ||
| let interactive_context = run_context_with_origin(TurnOriginKind::Inbound).await; | ||
| let scheduled_context = run_context_with_origin(TurnOriginKind::ScheduledTrigger).await; | ||
|
|
||
| async fn resolve_content( | ||
| source: &DefaultSystemPromptIdentitySource, | ||
| context: &LoopRunContext, | ||
| ) -> String { | ||
| let candidates = source | ||
| .load_identity_candidates(context, PromptMode::TextOnly) | ||
| .await | ||
| .expect("candidates load"); | ||
| source | ||
| .resolve_identity_message_content( | ||
| context, | ||
| candidates[0] | ||
| .message_ref | ||
| .as_ref() | ||
| .expect("trusted identity has ref"), | ||
| ) | ||
| .await | ||
| .expect("resolve content") | ||
| .expect("content exists") | ||
| .content | ||
| } | ||
|
|
||
| let interactive_content = resolve_content(&source, &interactive_context).await; | ||
| let scheduled_content = resolve_content(&source, &scheduled_context).await; | ||
|
|
||
| assert!( | ||
| !interactive_content.contains("Unattended Scheduled Run"), | ||
| "interactive runs must retain the ordinary ask-the-user escape valve" | ||
| ); | ||
| assert!(scheduled_content.contains("Unattended Scheduled Run")); | ||
| assert!(scheduled_content.contains("There is no human present")); | ||
| assert!(scheduled_content.contains("Never end the run with a question")); | ||
| assert!(scheduled_content.contains("final reply is the run's recorded output")); | ||
| } | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Cover the TurnOriginKind::WebUi negative path.
The PR promises unchanged WebUI prompt behavior, but this test checks only TurnOriginKind::Inbound. Add TurnOriginKind::WebUi to the same assertion matrix so all non-scheduled product origins are covered.
The PR objective states that interactive, inbound-channel, and WebUI prompt behavior remains unchanged.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/app/ironclaw_composition/src/root/default_system_prompt.rs` around
lines 558 - 604, The test
scheduled_trigger_origin_appends_unattended_protocol_only_to_triggered_runs
currently covers only Inbound as the non-scheduled case. Add a
TurnOriginKind::WebUi context, resolve its prompt content through the same
helper, and assert it does not contain the unattended scheduled-run instructions
while preserving the existing Inbound and ScheduledTrigger assertions.
| Never end the run with a question, a menu of options, or a description of what you could do next. Use the capabilities available to perform the task now. When details are ambiguous, make reasonable, bounded assumptions that stay within the stored request and state material assumptions briefly in the final reply. | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Define the failure path when no bounded assumption is safe.
Line 5 requires a bounded assumption for ambiguity, but it does not define what to do when no safe assumption exists. Add a rule to stop, report the missing input, and avoid a question or fabricated value.
The trigger contract requires bounded assumptions and a self-contained recorded result.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/loop/ironclaw_loop_host/prompts/scheduled_trigger_mode.md` around
lines 5 - 6, Update the scheduled trigger instructions near “When details are
ambiguous” to define the unsafe-ambiguity path: stop execution, report the
specific missing input, and record a self-contained result without asking a
question or inventing a value. Preserve the existing requirement to make bounded
assumptions when they are safe.
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 IronLoop review
One medium-severity regression found in scheduled prompt assembly.
Findings: 🟠 Medium 1
🟠 Medium · Keep the scheduled directive within the identity budget
Inline on crates/app/ironclaw_composition/src/root/default_system_prompt.rs:100. See the inline comment for details.
Validation
- ✅ Scheduled-origin prompt unit test — The added scheduled-trigger prompt selection test passed.
- ✅ Identity-budget unit test — The loop-host test confirming that an over-budget trusted identity candidate is omitted passed.
Review details
- Run:
60abf5ab-4529-42c0-8222-380ce433e1ed - Workflow: Review
- Attempts: 1
| .map(|context| context.origin), | ||
| Some(TurnOriginKind::ScheduledTrigger) | ||
| ) { | ||
| append_section(&mut content, SCHEDULED_TRIGGER_MODE_PROTOCOL_PROMPT); |
There was a problem hiding this comment.
🔍 IronLoop review · Inline finding
🟠 Medium · Keep the scheduled directive within the identity budget
This adds the protocol to the same single identity candidate as the user-editable `SYSTEM.md`. The normal identity budget is 8,000 tokens, and an over-budget first candidate is omitted entirely. A valid custom prompt is allowed to be up to 64 KiB; even a roughly 31.5 KiB prompt that previously fit can be pushed over the limit by this new section, causing scheduled runs to receive no system prompt at all—including the unattended directive. Reserve/admit this protocol independently (or enforce a compatible prompt-size limit) and add a boundary regression test.
|
Superseded by #7497, which uses the upstream nearai/ironclaw branch as the PR head. |
This branch was successfully deployed
1 participant
Summary
ScheduledTriggerChange Type
Linked Issue
Related #6879
Validation
cargo fmt --all -- --checkcargo clippy --all --benches --tests --examples --all-features -- -D warnings— targeted owning-crate clippy was run insteadcargo build— covered by the targeted test and clippy builds belowcargo test -p <owning-crate> --features integration— Not applicable: no database-backed owning-crate integration feature changedreview-prorpr-shepherd --fixwas run before requesting review — not runTest Strategy
User behavior: Scheduled routines are explicitly told that no human is present, must perform the stored task instead of ending with a question, and must produce a self-contained recorded result. Interactive runs retain their existing ask-the-user behavior.
Risk areas:
Tests added or updated:
scheduled_trigger_origin_appends_unattended_protocol_only_to_triggered_runs; extended prompt-asset guards for the new Markdown asset.reborn_integration_triggered_submitsuite passes (18 tests). The harness intentionally wiresEmptyIdentityContextSource, so the new prompt assertion lives at the realHostIdentityContextSourcecaller seam instead of being vacuous in this suite.What the tests prove: A trusted
ScheduledTriggercontext receives the unattended protocol, an explicitInboundcontext does not, the asset remains valid and distinct, existing triggered submission behavior remains green, and prompt text stays outside the composition crate.Commands run:
cargo fmt --all -- --checkcargo test -p ironclaw_composition scheduled_trigger_origin_appends_unattended_protocol_only_to_triggered_runs --libcargo test -p ironclaw_loop_host system_prompt_assets --libcargo test -p ironclaw_integration_tests --test reborn_integration_triggered_submitcargo test -p ironclaw_architecture_tests --test reborn_composition_boundaries composition_root_embeds_no_prompt_contentcargo clippy -p ironclaw_loop_host -p ironclaw_composition --lib --tests -- -D warningspython3 scripts/ci/docs_publication_boundary.pySecurity Impact
None. This does not change authorization, approvals, authentication, permissions, secrets, tool execution, or delivery routing. The protocol explicitly preserves existing host gates and forbids guessing credentials, permissions, or destinations.
Reborn Trust-Boundary Checklist
serde(default)fields fail closed or have migration tests: Not applicable; no serialized fields changed.ScheduledTriggerremains the existing trusted-origin discriminator; no runtime lane names changed.Database Impact
None. No schema, migration, query, or backend behavior changes.
Blast Radius
Limited to default system-prompt assembly for runs whose trusted product origin is
ScheduledTrigger, plus the loop-host prompt asset registry and trigger contract documentation. Interactive, inbound-channel, and WebUI turns do not receive the new protocol.Rollback Plan
Revert this commit. There is no migration or persisted-state compatibility concern; removing the conditional append restores the previous prompt behavior immediately.
Review Follow-Through
A
[SILENT]-style suppression sentinel is intentionally not included in this slice because no deterministic delivery-layer consumer exists yet; prompt-only support could deliver the literal marker. That should land with explicit suppression semantics in a follow-up.Review track: B