fix(automation): poll artifact_glob after wait_for_idle settles - #124
Conversation
The post-step artifact capture in the wait_for_idle = true branch was a single-shot check after a 2s sleep. wait_for_agent_idle can return as soon as the runtime adapter detects a completion signal (Claude Code emits this at end-of-turn), which can fire before the agent's last file write is flushed to disk — or before the agent has even started writing the file referenced in its final response. Replace the sleep+check with a bounded polling loop (1s cadence, window = max(wait_timeout_secs / 30, 5s)..60s). The artifact-polling mode branch (active when wait_for_idle = false) already uses this pattern at lines 1080-1255; this aligns the wait_for_idle = true path with that proven approach. Fixes #120
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe pull request replaces fixed-sleep artifact capture with bounded polling in ChangesArtifact Capture Polling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Possibly related PRs
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. ✨ 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. Review rate limit: 0/1 reviews remaining, refill in 45 minutes and 50 seconds.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
🧹 Nitpick comments (1)
src/automation/mod.rs (1)
2887-2908: ⚡ Quick winMake the poll-expiry error actionable.
When the deadline expires, this helper returns the last
capture_artifact(...)error verbatim, so callers only surface a generic “matched no new files” message. Wrapping that case with the poll window and a hint to verifyartifact_globor increase the timeout would make these failures much easier to diagnose.Suggested change
fn capture_artifact_with_poll( pre_snapshot: &HashSet<PathBuf>, pattern: &str, artifact_name: &str, poll_secs: u64, poll_interval: Duration, ) -> Result<PathBuf> { let poll_deadline = Duration::from_secs(poll_secs); let poll_start = std::time::Instant::now(); loop { match capture_artifact(pre_snapshot, pattern, artifact_name) { Ok(path) => break Ok(path), - Err(err) => { + Err(_) => { if poll_start.elapsed() >= poll_deadline { - break Err(err); + break Err(TuttiError::ConfigValidation(format!( + "artifact '{}' did not appear within {}s after completion; verify artifact_glob '{}' or increase wait_timeout_secs", + artifact_name, poll_secs, pattern + ))); } std::thread::sleep(poll_interval); } } } }As per coding guidelines, "User-facing errors should include actionable guidance".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/automation/mod.rs` around lines 2887 - 2908, In capture_artifact_with_poll wrap the Err returned when poll_start.elapsed() >= poll_deadline into a new, more actionable error that mentions the poll window expired and suggests verifying the artifact_glob/pattern and increasing poll_secs; preserve or attach the original capture_artifact error details (from capture_artifact(...)) so callers still see the underlying cause. Update the return at the timeout branch to construct that wrapped error instead of returning err verbatim, keeping the Ok path unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/automation/mod.rs`:
- Around line 1565-1627: The current code bails out of prompt-step success paths
with early continues so artifact capture (artifact_pre_snapshot/artifact_name ->
capture_artifact_with_poll -> store_artifact_output) is skipped for some exits;
refactor so every successful prompt-step funnels through a single post-step
artifact-capture routine instead of returning/continuing early. Concretely:
remove or replace the early continue/return in the prompt-step completion
branches and call a shared helper (e.g., implement a new function like
finalize_prompt_step_artifacts that takes &self, run_id, step_index, started,
artifact_pre_snapshot, artifact_name, post_idle_poll_secs, output_files,
outputs, step_results, failed_steps) which runs capture_artifact_with_poll and
store_artifact_output and pushes to output_files/outputs or records failures
into step_results/failed_steps; invoke that helper from all success exit points
(including where implement_code previously returned) so artifact capture always
runs once per successful prompt-step.
---
Nitpick comments:
In `@src/automation/mod.rs`:
- Around line 2887-2908: In capture_artifact_with_poll wrap the Err returned
when poll_start.elapsed() >= poll_deadline into a new, more actionable error
that mentions the poll window expired and suggests verifying the
artifact_glob/pattern and increasing poll_secs; preserve or attach the original
capture_artifact error details (from capture_artifact(...)) so callers still see
the underlying cause. Update the return at the timeout branch to construct that
wrapped error instead of returning err verbatim, keeping the Ok path unchanged.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6f6fe5e9-771f-4cf6-8ec6-6442de5f569e
📒 Files selected for processing (1)
src/automation/mod.rs
2 participants
Supersedes #121 from Brian's fork so the required repository CI can attach normally after the branch protection updates. This preserves Brian's original fix plus the two CodeRabbit-requested follow-ups that were already reviewed on #121.
Fixes #120.
Versioning
Validation