feat: Add worktree support for scheduled agents with validation#90
feat: Add worktree support for scheduled agents with validation#902witstudios merged 4 commits intomainfrom
Conversation
Scheduled agents previously always spawned as root agents (hardcoded root: true). This adds --root and --name flags to `pu schedule create` so schedules can spawn into isolated worktrees, matching `pu spawn` semantics. Default is worktree mode (--name required); --root opts into the previous root-agent behavior for read-only/cross-project tasks. Additionally adds ScheduleDef::validate() that enforces consistency between root and agent_name fields. Validation is called on save and on load (scan_dir/find_in_dir), rejecting malformed schedules upfront. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe pull request extends the schedule creation system with two new parameters: Changes
Sequence DiagramsequenceDiagram
participant User as User/CLI
participant CLI as CLI Parser<br/>(main.rs)
participant Cmd as Schedule Command<br/>(schedule.rs)
participant Proto as Protocol<br/>(protocol.rs)
participant Engine as Engine<br/>(engine.rs)
participant Def as Schedule Definition<br/>(schedule_def.rs)
User->>CLI: schedule create --name agent1 --root
CLI->>Cmd: run_create(root=true, agent_name=Some("agent1"))
Cmd->>Def: validate(root, agent_name)
Def-->>Cmd: Ok() - root=true, agent_name ignored
Cmd->>Proto: SaveSchedule {root: true, agent_name: None, ...}
Proto->>Engine: Handle SaveSchedule request
Engine->>Engine: fire_schedule() with root context
Engine->>Proto: Request::Spawn {root: true, agent_name: Some("agent1"), ...}
Engine-->>User: Schedule saved & executed at project root
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Resolve conflict in fire_schedule: keep main's updated template rendering logic while preserving project_path rename to avoid shadowing the schedule.root field. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use is_none_or instead of !is_some_and for clearer boolean logic in ScheduleDef::validate(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/pu-engine/src/engine.rs (1)
2861-2868:⚠️ Potential issue | 🟠 MajorRecurring worktree schedules will collide after the first run.
When
rootisfalse, every fire forwards the sameschedule.agent_namewithworktree: None, sohandle_spawn()creates a brand-new worktree on the same branch slug each time. The first run leaves that branch/worktree in place, and the next recurrence has no reuse or cleanup path here, so it fails instead of spawning. Please either reject recurring non-root schedules, reuse a stable worktree, or derive a unique per-fire name.Also applies to: 2887-2894
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/pu-engine/src/engine.rs` around lines 2861 - 2868, The spawn call always forwards schedule.agent_name with worktree: None when schedule.root is false, causing recurring fires to reuse the same branch slug and collide; update the spawn plumbing in the block that constructs Request::Spawn (and the equivalent block around the other occurrence) to either (a) reject recurring non-root schedules early (return Err/skip when schedule.is_recurring && !schedule.root), (b) reuse a stable worktree by passing worktree: Some(stable_name) where stable_name is derived from schedule.id/agent_name (so subsequent fires target the same worktree), or (c) derive a unique per-fire worktree name (e.g., format!("{agent_name}-{schedule_id}-{fire_timestamp}") and pass it as worktree: Some(unique_name)) — choose one strategy and implement it in the code paths that build Request::Spawn so handle_spawn receives a non-colliding, explicit worktree value (or rejects the schedule).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/pu-cli/assets/SKILL.md`:
- Around line 89-101: Update the schedule examples that hard-code the date
"2026-03-07" so they won’t become stale: replace the fixed --start-at value in
the examples (the commands starting with "pu schedule create overnight-build"
and "pu schedule create morning-status") with a clear placeholder like
"<FUTURE_ISO_TIMESTAMP>" or add an inline note telling readers to replace the
timestamp with a future ISO datetime; ensure both the worktree example
(overnight-build) and the root example (morning-status) use the placeholder or
instruction and keep the rest of the flags (--name, --root, --trigger,
--trigger-prompt) unchanged.
In `@crates/pu-cli/src/commands/schedule.rs`:
- Around line 40-45: Currently the validation only errors when !root &&
agent_name.is_none(), but doesn't reject the invalid combination of supplying
both --root and --name; add a guard that returns Err(CliError::Other("cannot use
--name with --root".into())) when root && agent_name.is_some(). Update the same
validation block (where variables root and agent_name are checked) to first
check for the conflicting case (root && agent_name.is_some()) and then keep the
existing missing-name check so the CLI rejects the invalid combination locally
before IPC.
In `@crates/pu-core/src/schedule_def.rs`:
- Around line 77-91: The validate() logic in schedule_def::validate currently
only rejects non-empty agent_name when root is true, allowing Some("") to pass;
update the root branch so any presence of agent_name is rejected (i.e. treat
Some("") the same as Some("foo")). Specifically, in the validate() function
adjust the check on self.root/self.agent_name so that when root is true you
return an InvalidInput error if self.agent_name.is_some() (or equivalently
is_some_and(|_| true)), leaving the existing non-root check (which already
rejects empty strings) unchanged; referencing validate(), the agent_name field,
and Request::Spawn/handle_spawn to locate the related behavior.
---
Outside diff comments:
In `@crates/pu-engine/src/engine.rs`:
- Around line 2861-2868: The spawn call always forwards schedule.agent_name with
worktree: None when schedule.root is false, causing recurring fires to reuse the
same branch slug and collide; update the spawn plumbing in the block that
constructs Request::Spawn (and the equivalent block around the other occurrence)
to either (a) reject recurring non-root schedules early (return Err/skip when
schedule.is_recurring && !schedule.root), (b) reuse a stable worktree by passing
worktree: Some(stable_name) where stable_name is derived from
schedule.id/agent_name (so subsequent fires target the same worktree), or (c)
derive a unique per-fire worktree name (e.g.,
format!("{agent_name}-{schedule_id}-{fire_timestamp}") and pass it as worktree:
Some(unique_name)) — choose one strategy and implement it in the code paths that
build Request::Spawn so handle_spawn receives a non-colliding, explicit worktree
value (or rejects the schedule).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 12afb861-9b9c-4b64-b192-eec92055e4c7
📒 Files selected for processing (7)
crates/pu-cli/assets/SKILL.mdcrates/pu-cli/src/commands/schedule.rscrates/pu-cli/src/main.rscrates/pu-cli/src/output.rscrates/pu-core/src/protocol.rscrates/pu-core/src/schedule_def.rscrates/pu-engine/src/engine.rs
| # Worktree spawn (default) — needs --name | ||
| pu schedule create overnight-build \ | ||
| --start-at "2026-03-07T22:30:00" \ | ||
| --name overnight-build \ | ||
| --trigger inline-prompt \ | ||
| --trigger-prompt "build a feature" # spawns in worktree pu/overnight-build | ||
|
|
||
| # Root spawn — for read-only/cross-project tasks | ||
| pu schedule create morning-status \ | ||
| --start-at "2026-03-07T08:00:00" \ | ||
| --root \ | ||
| --trigger inline-prompt \ | ||
| --trigger-prompt "do the thing" # one-shot scheduled agent | ||
| pu schedule create <name> \ | ||
| --start-at "2026-03-07T14:00:00Z" \ | ||
| --trigger-prompt "scan commits" # spawns in project root |
There was a problem hiding this comment.
These schedule examples will go stale immediately.
The one-shot examples hard-code 2026-03-07. After March 7, 2026, copy-pasting them creates schedules whose start_at is already in the past, so they never fire. Please switch these to placeholders or explicitly tell readers to replace them with a future timestamp.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/pu-cli/assets/SKILL.md` around lines 89 - 101, Update the schedule
examples that hard-code the date "2026-03-07" so they won’t become stale:
replace the fixed --start-at value in the examples (the commands starting with
"pu schedule create overnight-build" and "pu schedule create morning-status")
with a clear placeholder like "<FUTURE_ISO_TIMESTAMP>" or add an inline note
telling readers to replace the timestamp with a future ISO datetime; ensure both
the worktree example (overnight-build) and the root example (morning-status) use
the placeholder or instruction and keep the rest of the flags (--name, --root,
--trigger, --trigger-prompt) unchanged.
There was a problem hiding this comment.
These examples use specific dates as documentation illustrations, following the existing pattern in the file (the original examples also used hard-coded dates like 2026-03-07T14:00:00Z). Since this is a skill reference doc showing command syntax, not a runnable script, the exact date is secondary to demonstrating the flag combinations. Happy to use relative placeholders like <datetime> if you feel strongly about it.
There was a problem hiding this comment.
@2witstudios, that's a fair point — a skill reference doc is primarily showing flag syntax and patterns, not serving as a copy-pasteable script. The existing file already follows this convention, so the approach is consistent. No changes needed here.
- Reject --root + --name as mutually exclusive in CLI (schedule.rs)
- Reject any Some(_) agent_name when root=true in validate(), not just
non-empty strings, preventing Some("") from persisting
- Add test for root=true + empty agent_name rejection
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
Re-lands PR #84 (reverted in #89) with the additional validation fix from code review:
rootandagent_namefields toScheduleDef, protocol (SaveSchedule,ScheduleInfo,ScheduleDetail), withdefault: truefor backward compatibility--rootflag and--name <worktree>arg topu schedule create— default is worktree mode (requires--name),--rootopts into root-agent spawningfire_schedule()to passschedule.rootandschedule.agent_nametoSpawninstead of hardcodingroot: trueScheduleDef::validate()— enforces "if root is true, agent_name must be None; if root is false, agent_name must be Some(non-empty)". Called fromsave_schedule_def(),scan_dir(), andfind_in_dir()to reject malformed schedules at both save and load timeTest plan
cargo test -p pu-core -- schedule— all schedule tests pass (34/34, excluding 2 pre-existing environmental failures)cargo test -p pu-cli -- schedule— all 7 CLI schedule tests passcargo test -p pu-core -- protocol::tests::given_schedule— 2/2 pass--name test-wt, verifypu schedule showdisplaysroot: false--root, confirm backward-compatible root spawning🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
--rootand--nameCLI options to schedule creation, enabling agents to spawn at the project root or in worktrees with custom naming.