fix(agents): preserve prompt lifecycle for idle child delivery - #1631
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughParent delivery now routes according to the parent’s run state. Idle parents receive stored messages and a coalesced wake; active parents receive steering messages. Lifecycle hooks manage deferred flushes, bounded wake retries, and session-state resets. ChangesParent delivery lifecycle
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Bug fix · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant ChildCompletion
participant gentle-agents
participant pi.sendMessage
participant pi.sendUserMessage
ChildCompletion->>gentle-agents: deliver child content
gentle-agents->>pi.sendMessage: store content without triggering a turn when parent is idle
gentle-agents->>pi.sendUserMessage: send a coalesced system wake when parent is idle
gentle-agents->>pi.sendMessage: steer content when parent has an active run
Suggested reviewers: Merge Risk: ⚪ Minimal · up to Idle parents now receive stored child results along with a single wake that goes through the normal prompt lifecycle. Active runs still use steering. The open questions about compaction timing and session-reset races were checked and did not reveal defects. Two items remain the author's to resolve before release: remote CI is still pending, and the live bridge reproduction was not performed. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change preserves session ownership and separates child output from automated wake instructions. No introduced permission bypass or cross-session exposure was established. Remaining uncertainty concerns live integration behavior and recovery after failed delivery. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
…he stale-completion branch
…outer (Gentleman-Programming#1092) Gentleman-Programming#1631 moved child content onto an idle/run/hold router so an idle parent is woken through the prompt lifecycle (Gentleman-Programming#1528). The stale-completion notice still called sendMessage(steer, triggerTurn), which on an idle parent starts a direct turn that skips before_agent_start. It now goes through sendToParent with the route flushCompletions already computes: an idle parent stores it with triggerTurn:false plus one coalesced wake, a running parent gets it steered into the current turn, and a parent busy without a run keeps it held until the next boundary flush. Tests (red before this commit): idle parent gets one wake and never a direct triggerTurn message; a held notice is routed at the boundary; a stale and a fresh completion in one idle flush share one wake. A run-route guard stays green.
Linked issue
Closes #1528
PR type
Summary
Changes
Test plan
Size exception
Explicitly authorized by the maintainer. Keep this coherent delivery-state fix and its regression tests together; do not remove coverage or split artificially. Source/test changes are 523 authored diff lines. Inherited float-chrome changes are excluded from this PR base.
Notes
Source and tests have not changed after native review; the final commit only records approval in the passive task document. No shell scripts or skills changed, so shellcheck and skill-loading checks are not applicable. A slow pre-run race and missing-compaction-event delay remain documented limitations.
Summary by CodeRabbit