refactor(scheduled_action): split agent executor into automations crate#4167
refactor(scheduled_action): split agent executor into automations crate#4167ehayes2000 wants to merge 1 commit into
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughA new Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 1
🤖 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 `@rust/cloud-storage/automations/src/lib.rs`:
- Line 1: Add the `#![deny(missing_docs)]` attribute to the crate root in lib.rs
to enforce documentation requirements on all public items. This attribute should
be placed at the top of the file alongside the existing `#![recursion_limit =
"256"]` directive. This is a workspace coding guideline for new crates that
ensures all public items have proper documentation.
🪄 Autofix (Beta)
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: CHILL
Plan: Pro
Run ID: 430c784d-ca48-4e6f-868a-4ab0f95705d1
⛔ Files ignored due to path filters (1)
rust/cloud-storage/Cargo.lockis excluded by!**/*.lock,!**/Cargo.lock
📒 Files selected for processing (14)
.github/services-config.json.github/workspace-dep-closures.jsonflake.nixinfra/stacks/agent-schedule-service/index.tsrust/cloud-storage/Cargo.tomlrust/cloud-storage/automations/Cargo.tomlrust/cloud-storage/automations/src/bins/service.rsrust/cloud-storage/automations/src/inprocess_executor/agent_task.rsrust/cloud-storage/automations/src/inprocess_executor/mod.rsrust/cloud-storage/automations/src/inprocess_executor/notify.rsrust/cloud-storage/automations/src/lib.rsrust/cloud-storage/scheduled_action/Cargo.tomlrust/cloud-storage/scheduled_action/src/outbound.rsrust/cloud-storage/workspace-hack/Cargo.toml
💤 Files with no reviewable changes (2)
- rust/cloud-storage/scheduled_action/src/outbound.rs
- rust/cloud-storage/scheduled_action/Cargo.toml
| @@ -0,0 +1,18 @@ | |||
| #![recursion_limit = "256"] | |||
There was a problem hiding this comment.
Add missing docs enforcement in the new crate root.
Line 1 is missing #![deny(missing_docs)], which is required for new lib.rs files in this workspace.
Suggested patch
#![recursion_limit = "256"]
+#![deny(missing_docs)]As per coding guidelines, “Add #![deny(missing_docs)] to lib.rs in new crates to enforce documentation on all public items.”
🤖 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 `@rust/cloud-storage/automations/src/lib.rs` at line 1, Add the
`#![deny(missing_docs)]` attribute to the crate root in lib.rs to enforce
documentation requirements on all public items. This attribute should be placed
at the top of the file alongside the existing `#![recursion_limit = "256"]`
directive. This is a workspace coding guideline for new crates that ensures all
public items have proper documentation.
Source: Coding guidelines
ehayes2000
force-pushed
the
eric.hayes/split-automations-crate
branch
from
9c7b914 to
87b1278
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@rust/cloud-storage/scheduled_action/src/outbound/inprocess_executor.rs`:
- Around line 60-65: The action claimed via self.repo.claim_action(&id) on line
60 is not released if the subsequent self.runner.create_resource(&action) call
fails on line 64, since the question mark operator causes an early return
without cleanup. Wrap the create_resource call in error handling or use a
pattern that ensures self.repo.release_action(&id) is called to release the
claim whenever create_resource fails, preventing the action from being blocked
indefinitely until the claim times out.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 13c573ac-e2b2-44b7-a733-1f8bfe850236
⛔ Files ignored due to path filters (1)
rust/cloud-storage/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
.github/services-config.json.github/workspace-dep-closures.jsonflake.nixinfra/stacks/agent-schedule-service/index.tsrust/cloud-storage/Cargo.tomlrust/cloud-storage/automations/Cargo.tomlrust/cloud-storage/automations/src/agent_task.rsrust/cloud-storage/automations/src/bins/service.rsrust/cloud-storage/automations/src/lib.rsrust/cloud-storage/automations/src/notify.rsrust/cloud-storage/automations/src/runner.rsrust/cloud-storage/scheduled_action/Cargo.tomlrust/cloud-storage/scheduled_action/src/domain/ports.rsrust/cloud-storage/scheduled_action/src/outbound/inprocess_executor.rsrust/cloud-storage/workspace-hack/Cargo.toml
💤 Files with no reviewable changes (1)
- rust/cloud-storage/scheduled_action/Cargo.toml
| self.repo.claim_action(&id).await?; | ||
|
|
||
| // Create the chat up front so the caller gets a chat_id synchronously | ||
| // and the eventual execution record can link back to it. | ||
| let chat_id = match action.kind { | ||
| ActionKind::Agent => agent_task::create_run_chat(&self.db, &action).await?, | ||
| }; | ||
| // Create the run resource up front so the caller gets a resource id | ||
| // synchronously and the eventual execution record can link back to it. | ||
| let resource_id = self.runner.create_resource(&action).await?; | ||
|
|
There was a problem hiding this comment.
Release the action claim if resource creation fails.
On Line 60 the action is claimed, but on Line 64 a create_resource failure returns early and skips release_action, leaving the action blocked until claim timeout.
Suggested patch
- let resource_id = self.runner.create_resource(&action).await?;
+ let resource_id = match self.runner.create_resource(&action).await {
+ Ok(resource_id) => resource_id,
+ Err(e) => {
+ if let Err(release_err) = self.repo.release_action(&id).await {
+ tracing::error!(
+ error=?release_err,
+ action_id=?id,
+ "failed to release action claim after resource creation failure",
+ );
+ }
+ return Err(e);
+ }
+ };🤖 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 `@rust/cloud-storage/scheduled_action/src/outbound/inprocess_executor.rs`
around lines 60 - 65, The action claimed via self.repo.claim_action(&id) on line
60 is not released if the subsequent self.runner.create_resource(&action) call
fails on line 64, since the question mark operator causes an early return
without cleanup. Wrap the create_resource call in error handling or use a
pattern that ensures self.repo.release_action(&id) is called to release the
claim whenever create_resource fails, preventing the action from being blocked
indefinitely until the claim times out.
cac7d80 to
3e745c2
Compare
ehayes2000
force-pushed
the
eric.hayes/split-automations-crate
branch
from
3e745c2 to
2532598
Compare
scheduled_action becomes a pure scheduling library (cron schedule, repo, polling/tokio dispatchers, HTTP router, live updates). The concrete agent executor and the deployable service binary move to a new automations crate that depends on scheduled_action + memory + ai_tools. This breaks the scheduled_action -> memory edge out of the core library so memory can depend on scheduled_action (for upcoming composed-memory work) without a cargo cycle. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ehayes2000
force-pushed
the
eric.hayes/split-automations-crate
branch
from
2532598 to
f2ff1a8
Compare
1 participant
No description provided.