[auto] #67 phase 1: stabilize fresh prompt-step startup on brand-new choir runs - #71
Merged
Conversation
Prevent fresh prompt steps from falsely completing before the agent has consumed the prompt. The startup grace period (default 30s) gates completion detection until real working activity is observed. Key changes: - wait_for_agent_idle accepts a startup_grace Duration parameter - AgentStatus::Working counts as activity even without pane hash change, requiring 2+ consecutive polls to avoid flicker false positives - First pane capture no longer counts as a hash "change" - Completion signals before any activity are held until grace expires - "Unravelling" added to claude-code working patterns - startup_grace_secs field threaded through config and automation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a configurable startup grace window for Prompt steps, propagated through config, automation resolution/execution, health idle-wait logic, CLI send/run usage, and runtime status detection (adds "Unravelling" to Working patterns). Includes changelog entry and a new unit test. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (run/send)
participant Config as Config
participant Automation as Automation/Executor
participant Health as Health checker
participant Runtime as Runtime adapter
CLI->>Config: load workflow (may include startup_grace_secs)
CLI->>Automation: start execution (resolved steps include startup_grace_secs)
Automation->>Health: wait_for_agent_idle(startup_grace)
Health->>Runtime: poll status & hashes
Runtime-->>Health: status (Working/Idle), content like "Unravelling"
Health-->>Automation: idle/detected activity (honors startup_grace & consecutive-working)
Automation->>CLI: step completion / continue
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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
🧹 Nitpick comments (2)
src/automation/mod.rs (1)
455-469: Expose prompt wait settings in dry-run output too.
ResolvedStep::Promptnow carries behavior-affecting wait metadata, buttt run --dry-run/--jsonstill strips prompt wait settings insrc/cli/run.rsvia..matches. That makes a non-defaultstartup_grace_secsoverride impossible to inspect before execution. Please thread these fields through the plan serializers while this model change is fresh.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/automation/mod.rs` around lines 455 - 469, The dry-run/json serializer is dropping the new prompt wait metadata from ResolvedStep::Prompt (wait_for_idle, wait_timeout_secs, startup_grace_secs) via a `..` match when converting resolved steps to the plan/JSON representation; update the plan/serializer code that transforms ResolvedStep::Prompt into the serializable step type (the mapping function that handles Prompt variants and the target serializable Prompt struct) to explicitly include and forward wait_for_idle, wait_timeout_secs, and startup_grace_secs (add those fields to the serializable struct if missing) so the dry-run and --json output reflect the runtime wait overrides.src/health/mod.rs (1)
253-265: Consider resettingconsecutive_working_pollswhen hash changes.The counter increments when
runtime_is_working && !changedand resets when!runtime_is_working, but it's not reset whenchangedis true. This means if the hash changes while working, the counter retains its previous value.While this doesn't affect immediate correctness (since
changedalready triggers activity), it could lead to subtle issues: after a hash change, the counter may already be at 1, meaning only one more Working poll without hash change would cross the threshold instead of requiring two consecutive polls.♻️ Suggested fix to reset counter on hash change
if runtime_is_working && !changed { consecutive_working_polls += 1; } else if !runtime_is_working { consecutive_working_polls = 0; + } else if changed { + consecutive_working_polls = 0; }Or more concisely:
if runtime_is_working && !changed { consecutive_working_polls += 1; } else { consecutive_working_polls = 0; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/health/mod.rs` around lines 253 - 265, The consecutive_working_polls counter should be reset when the hash changes to avoid carrying prior counts across a change; modify the logic around runtime_is_working/changed so that when changed is true you set consecutive_working_polls = 0, otherwise keep the existing behavior (increment when runtime_is_working && !changed and reset to 0 when !runtime_is_working). Update the block that currently checks runtime_is_working and adjusts consecutive_working_polls (the variables runtime_is_working, changed, consecutive_working_polls and the constant WORKING_STATUS_CONSECUTIVE_THRESHOLD) so changed always clears the counter before any increment logic.
🤖 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/config/mod.rs`:
- Around line 156-160: Add a validation check in the prompt/step config
validator (where the struct with fields wait_for_idle, wait_timeout_secs,
startup_grace_secs is validated) to reject configurations that set
wait_timeout_secs or startup_grace_secs when wait_for_idle is false or absent;
return a user-facing error that instructs to "set `wait_for_idle = true` or
remove the wait settings." Locate the validator (e.g., impl validate/try_from
for that config struct in src/config/mod.rs or the function that currently
checks wait_timeout_secs) and add an if condition that errors when
(wait_for_idle == Some(false) || wait_for_idle.is_none()) &&
(wait_timeout_secs.is_some() || startup_grace_secs.is_some()), using the
existing error type/flow so the message is surfaced to users.
---
Nitpick comments:
In `@src/automation/mod.rs`:
- Around line 455-469: The dry-run/json serializer is dropping the new prompt
wait metadata from ResolvedStep::Prompt (wait_for_idle, wait_timeout_secs,
startup_grace_secs) via a `..` match when converting resolved steps to the
plan/JSON representation; update the plan/serializer code that transforms
ResolvedStep::Prompt into the serializable step type (the mapping function that
handles Prompt variants and the target serializable Prompt struct) to explicitly
include and forward wait_for_idle, wait_timeout_secs, and startup_grace_secs
(add those fields to the serializable struct if missing) so the dry-run and
--json output reflect the runtime wait overrides.
In `@src/health/mod.rs`:
- Around line 253-265: The consecutive_working_polls counter should be reset
when the hash changes to avoid carrying prior counts across a change; modify the
logic around runtime_is_working/changed so that when changed is true you set
consecutive_working_polls = 0, otherwise keep the existing behavior (increment
when runtime_is_working && !changed and reset to 0 when !runtime_is_working).
Update the block that currently checks runtime_is_working and adjusts
consecutive_working_polls (the variables runtime_is_working, changed,
consecutive_working_polls and the constant WORKING_STATUS_CONSECUTIVE_THRESHOLD)
so changed always clears the counter before any increment logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 786ef039-f64a-42cd-9979-ff42a2007ab5
📒 Files selected for processing (8)
CHANGELOG.mdsrc/automation/mod.rssrc/cli/run.rssrc/cli/send.rssrc/config/mod.rssrc/health/mod.rssrc/runtime/claude_code.rssrc/runtime/mod.rs
- Reduce DEFAULT_STARTUP_GRACE_SECS from 30 to 10 so the completion-before-activity path fires before typical wait timeouts - Validate that wait_timeout_secs/startup_grace_secs are only set when wait_for_idle is true, failing fast with actionable guidance Addresses CodeRabbit feedback on PR #71. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/cli/send.rs (1)
13-13: Consider centralizing startup-grace defaults to prevent behavior drift.Line 13 defines a
send-specific default (10s), while automation uses a different default (30s insrc/automation/mod.rs), so prompt-wait behavior diverges by entrypoint. Consider moving this default to a shared constant/config source and reusing it in both paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/send.rs` at line 13, The send-specific constant DEFAULT_STARTUP_GRACE_SECS (used in send) causes divergent prompt-wait behavior vs the automation path (which uses its own 30s default); extract this value into a shared constant or configuration (e.g., STARTUP_GRACE_SECS in a new or existing config/module) and replace usages of DEFAULT_STARTUP_GRACE_SECS in send and the hard-coded 30s in src/automation/mod.rs to reference that single shared symbol so both entrypoints use the same startup grace default.src/config/mod.rs (1)
665-673: Add a targeted regression test for the new wait-setting guard.This new branch should be pinned with a unit test so config validation behavior doesn’t regress in future refactors.
Proposed test addition
#[cfg(test)] mod tests { use super::*; + + #[test] + fn validate_prompt_wait_settings_require_wait_for_idle() { + let toml_str = r#" +[workspace] +name = "test" + +[[agent]] +name = "backend" +runtime = "claude-code" + +[[workflow]] +name = "verify" + +[[workflow.step]] +type = "prompt" +agent = "backend" +text = "check" +wait_timeout_secs = 30 +"#; + let config: TuttiConfig = toml::from_str(toml_str).unwrap(); + let err = config.validate().unwrap_err(); + assert!(err.to_string().contains("set wait_for_idle = true or remove the wait settings")); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config/mod.rs` around lines 665 - 673, Add a unit test in the config validation tests (e.g., inside the tests module in src/config/mod.rs) that constructs a workflow/step with wait_for_idle = false while setting wait_timeout_secs and/or startup_grace_secs, calls the configuration validation function (the code path that can return TuttiError::ConfigValidation) and asserts it returns Err(TuttiError::ConfigValidation) and the error message mentions the workflow name and the offending step index; this pins the new guard branch that checks wait_for_idle, wait_timeout_secs, and startup_grace_secs so future changes will fail the test if the validation regresses.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/cli/send.rs`:
- Line 13: The send-specific constant DEFAULT_STARTUP_GRACE_SECS (used in send)
causes divergent prompt-wait behavior vs the automation path (which uses its own
30s default); extract this value into a shared constant or configuration (e.g.,
STARTUP_GRACE_SECS in a new or existing config/module) and replace usages of
DEFAULT_STARTUP_GRACE_SECS in send and the hard-coded 30s in
src/automation/mod.rs to reference that single shared symbol so both entrypoints
use the same startup grace default.
In `@src/config/mod.rs`:
- Around line 665-673: Add a unit test in the config validation tests (e.g.,
inside the tests module in src/config/mod.rs) that constructs a workflow/step
with wait_for_idle = false while setting wait_timeout_secs and/or
startup_grace_secs, calls the configuration validation function (the code path
that can return TuttiError::ConfigValidation) and asserts it returns
Err(TuttiError::ConfigValidation) and the error message mentions the workflow
name and the offending step index; this pins the new guard branch that checks
wait_for_idle, wait_timeout_secs, and startup_grace_secs so future changes will
fail the test if the validation regresses.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c6d137a7-3e2b-4353-969a-ca711fc83189
📒 Files selected for processing (2)
src/cli/send.rssrc/config/mod.rs
This was referenced Mar 19, 2026
nutt-adam
added a commit
that referenced
this pull request
Mar 19, 2026
Take main's idle detection (startup_grace, consecutive working polls) from merged PR #71. Restore expanded Claude Code completion/idle patterns (don't ask on, shift+tab to cycle) and working patterns (Searching for, (thinking)) needed for reliable prompt-bar detection. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
nutt-adam
added a commit
that referenced
this pull request
Mar 19, 2026
Take main's idle detection (startup_grace, consecutive working polls) from merged PR #71. Restore expanded Claude Code completion/idle patterns (don't ask on, shift+tab to cycle) and working patterns (Searching for, (thinking)) needed for reliable prompt-bar detection. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Mar 19, 2026
This was referenced May 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated SDLC cycle for #67.
Summary by CodeRabbit
New Features
Improvements