Conversation
A background completion is flushed at the parent's next turn boundary, i.e. after the parent's current tool call returns. One tool call that outlasts STALE_COMPLETION_MS (a long bash, a blocking capture, waiting on ask_user_choice) was enough to turn an unread completion stale, and deliverStale() only appended a transcript entry - excluded from LLM context - so the parent model was never told the task finished. A stale completion still never replays its report into the conversation, but it now also sends one compact steer notice (display: false) naming the task and pointing to subagent_result / subagent_status. Consumed results stay fully suppressed by consume(), the session gate and at-most-once delivery are unchanged, and fresh completions are delivered as before. Closes Gentleman-Programming#1092
|
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 (3)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughUnread stale completions for the active session now retain a durable transcript entry and trigger a compact steer notice. The notice identifies the task outcome and age, and directs retrieval through ChangesStale Completion Delivery
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to Unread stale completions receive a retrieval notice without replaying their reports. No concrete merge-blocking issue is established; merge after normal checks pass. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change makes previously silent completions actionable without automatically replaying their reports. Existing ownership and duplicate-suppression checks remain. Remaining uncertainty concerns how the surrounding runtime handles hidden notices and delivery failures. Retained concerns Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
Thanks for this fix. We independently reviewed head Component verification used exact-SHA source reads and production queue/delivery snippets in memory, with a mocked host and clock:
Update: independently executed the focused suite on the same pinned head. With Node 24.14.1 and pnpm 11.1.1, frozen-lockfile installation with lifecycle scripts disabled succeeded. In an isolated temporary environment with network disabled during tests, this command passed: node --experimental-strip-types --test tests/agents-completion-delivery.test.ts tests/gentle-agents.test.ts138 passed, 0 failed, 0 skipped; exit 0. An earlier sandbox attempt had nine Git-fixture failures because This verifies the focused suite, not the full suite, typecheck, packaging, official CI or live Pi integration. Hidden-message semantics were checked statically against Pi 0.99.1, not exercised in a live host. Could you confirm the repository CI results and a real-host case where an unread completion ages past 90 seconds during a long parent tool call? This evidence supports the scoped fix, but is not a formal approval or a merge-readiness claim. |
|
Thanks for the careful independent run, @MarsSall. Here are the two things you asked for. Repository CINeither workflow has run on Until someone approves it, I ran the Linux
Real-host cases (Pi 0.99.1)Setup. Pi 0.99.1 with gentle-pi 3.7.0. Its Before the patch: 9 casesEach background completion settled while the parent was inside one long tool call:
The result was flushed at the next What happened to them:
After the patch: 3 casesEach produced exactly one hidden
One caveat on my hostMy install also carries a separate local workaround for the idle-wake problem (earendil-works/pi#5581 / #1528). It wraps
To be clear about scope, this is the same as yours: evidence for the scoped fix, not a merge-readiness claim. |
Linked Issue
Closes #1092
(The issue does not carry
status:approvedyet; happy to adjust scope if a maintainer prefers a different design.)PR Type
Summary
STALE_COMPLETION_MS(90 s) before the parent's next turn boundary no longer disappears from the model's view. It keeps the existing transcript-only stale entry for the human and now sends one compact, model-facing notice (gentle-agents.stale-notice,deliverAs: "steer",triggerTurn: true,display: false) naming the task and telling the parent to callsubagent_result/subagent_status. The report itself is never replayed.consume()runs before the flush), the session-ownership gate is unchanged, delivery stays at-most-once (no later replay or second notice atagent_end/agent_settled), and fresh completions (< 90 s) are delivered exactly as before.Why
Completions settle into the queue and are flushed at
turn_end/agent_end, i.e. only after the parent's current tool call returns. A single tool call that outlasts the window — a longbash, a blocking capture, waiting on anask_user_choiceanswer — is enough to push an unread completion past 90 s, at which pointdeliverStale()only calledpi.appendEntry. Custom entries are excluded from LLM context, so the parent model was never told the task finished and typically sat waiting for it.Field evidence from one real orchestrator session (pi 0.99.1, gentle-pi 3.7.0, claude-bridge provider): 8
gentle-agents.stale-resultentries withageSecondsbetween 140 and 427, zero correspondinggentle-agents.resultmessages for those tasks, and in every case the completion settled while the parent was inside one tool call (longbash,ask_user_choice, a review capture). The parent only learned about each result when it happened to callsubagent_status/subagent_list_tasks, or when the human asked.The 90 s rationale ("the parent had many turns to pull the result") does not hold for one long tool call; the protection it was meant to provide — never replaying an already-used result — is already enforced by
consume(). The comments in both files are updated accordingly.Changes
extensions/gentle-agents.tsAGENTS_STALE_NOTICE_TYPE;deliverStale()keepsappendEntryand adds one compact steer notice (task id, agent, label, status, age — no report), behind the same session gatelib/agents-completion-delivery.tsSTALE_COMPLETION_MStests/gentle-agents.test.tsdisplay: false, names the task, points tosubagent_result, does not contain the report) and no further sends atagent_end/agent_settled; new tests: consumed-then-stale sends nothing and appends nothing, stale owned by another session sends nothingTest Plan
node --experimental-strip-types --test tests/agents-completion-delivery.test.ts tests/gentle-agents.test.ts pnpm run typecheck pnpm run check:runtime-modules pnpm testObserved:
a completion held past the stale window becomes transcript-only content and never re-enters the conversation→expected: 1, actual: 0on the notice count. That failure is the defect.tests 138 / pass 138 / fail 0.pnpm run typecheck: exit 0, 187 recorded diagnostics, no regressions — identical to a cleanmaincheckout.pnpm run check:runtime-modules: exit 0, all 8 generated modules match their sources.tests/*.test.ts, each with a timeout, because the combined run exceeds my environment's tool watchdog): every file completes; 11 files fail (gentle-ai,package-manifest,review-authority,review-consent-latch,review-controller,review-gate,review-object-store,review-repository,review-snapshot,review-transaction,windows-hidden-processes) with exactly the same failure count per file on a cleanmaincheckout (WSL2 environment; they touch the native review binary and Windows-only paths). None of them involves the changed files.Notes for Reviewers
display: false: the human already sees the stale entry rendered in the transcript; the notice exists only so the model can act.workingwith steering queued), which is a different symptom.Summary by CodeRabbit