Skip to content

fix(agents): keep an unread stale completion actionable for the parent (#1092) - #1574

Open
BGamboa13 wants to merge 1 commit into
Gentleman-Programming:mainfrom
BGamboa13:fix/unread-stale-completion-signal
Open

BGamboa13 wants to merge 1 commit into
Gentleman-Programming:mainfrom
BGamboa13:fix/unread-stale-completion-signal

Conversation

@BGamboa13

@BGamboa13 BGamboa13 commented Sep 30, 2026 •

Copy link
Copy Markdown

Linked Issue

Closes #1092

(The issue does not carry status:approved yet; happy to adjust scope if a maintainer prefers a different design.)

PR Type

  • Bug fix

Summary

  • An unread background completion that ages past 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 call subagent_result / subagent_status. The report itself is never replayed.
  • Every fix(agents): bound background completion delivery to the current turn #913 invariant is preserved: consumed results stay fully suppressed (consume() runs before the flush), the session-ownership gate is unchanged, delivery stays at-most-once (no later replay or second notice at agent_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 long bash, a blocking capture, waiting on an ask_user_choice answer — is enough to push an unread completion past 90 s, at which point deliverStale() only called pi.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-result entries with ageSeconds between 140 and 427, zero corresponding gentle-agents.result messages for those tasks, and in every case the completion settled while the parent was inside one tool call (long bash, ask_user_choice, a review capture). The parent only learned about each result when it happened to call subagent_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

File Change
extensions/gentle-agents.ts AGENTS_STALE_NOTICE_TYPE; deliverStale() keeps appendEntry and adds one compact steer notice (task id, agent, label, status, age — no report), behind the same session gate
lib/agents-completion-delivery.ts Comment only: corrected rationale for STALE_COMPLETION_MS
tests/gentle-agents.test.ts The #913 stale test now also asserts exactly one notice (steer + triggerTurn, display: false, names the task, points to subagent_result, does not contain the report) and no further sends at agent_end / agent_settled; new tests: consumed-then-stale sends nothing and appends nothing, stale owned by another session sends nothing

Test 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 test

Observed:

  • RED first: with the new tests and the pre-change sources, exactly one test fails — a completion held past the stale window becomes transcript-only content and never re-enters the conversation → expected: 1, actual: 0 on the notice count. That failure is the defect.
  • GREEN: focused tests 138 / pass 138 / fail 0.
  • pnpm run typecheck: exit 0, 187 recorded diagnostics, no regressions — identical to a clean main checkout.
  • pnpm run check:runtime-modules: exit 0, all 8 generated modules match their sources.
  • Unit tests, file by file (all 232 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 clean main checkout (WSL2 environment; they touch the native review binary and Windows-only paths). None of them involves the changed files.

Notes for Reviewers

Summary by CodeRabbit

  • New Features
    • When an unread task completion arrives after it has become stale, you’ll receive a brief notice with the task outcome and instructions for retrieving the full result. The full report is not included in the notice.
  • Behavior Changes
    • Stale reports are no longer replayed after 90 seconds. Completions you’ve already consumed, or that belong to a different session, do not generate a stale notice.

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
@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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 configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: d08e18fc-f41d-4bd3-9597-14f45a977c81

📥 Commits

Reviewing files that changed from the base of the PR and between 4b6b148 and 1929700.

📒 Files selected for processing (3)
  • extensions/gentle-agents.ts
  • lib/agents-completion-delivery.ts
  • tests/gentle-agents.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

Unread 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 subagent_result or subagent_status without including the report.

Changes

Stale Completion Delivery

Layer / File(s) Summary
Stale notice delivery and validation
extensions/gentle-agents.ts, lib/agents-completion-delivery.ts, tests/gentle-agents.test.ts
deliverStale sends a hidden, turn-triggering notice for an unread stale completion in the active session while retaining the transcript entry. The notice omits the report and directs retrieval through subagent_result or subagent_status. Tests check notice content, duplicate suppression, consumed results, and session ownership. Documentation clarifies the 90-second freshness behavior.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: alan-thegentleman

Merge Risk: ⚪ Minimal · up to 19297

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 Review

Security architecture risk: 🔵 Low · up to 19297

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
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated exposure change is stale task metadata entering the owning parent's model context and requesting another turn. The inspected path does not distribute reports to another session or add tool authority; broader effects depend on the surrounding runtime's steering semantics.

Security Findings and Attack Paths

  • inferred — Launch-derived labels now reach model context even after the stale threshold, but a newly effective privilege-escalation or report-injection path was not established. The report is excluded, labels are normalized and bounded, and equivalent metadata already reaches the same channel through fresh completions. Terminal-text normalization is not evidence of semantic prompt-injection protection.

Trust Boundaries and Controls

  • observed — The stale handler retains the active-session equality check before transcript append and notice forwarding. Regression assertions cover foreign-session suppression, consumed-result suppression, report exclusion, and absence of duplicate notices at later boundaries; these are mocked-runtime assertions, not proof of host behavior.

Resilience and Maintainability Implications

  • observed — Consumption and delivered-ID bookkeeping suppress repetition before forwarding. Session start and shutdown clear pending delivery state, and shutdown cancels remaining work. These existing lifecycle controls continue to contain replay into a replacement session.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: keeping unread stale agent completions actionable for the parent.
Linked Issues check ✅ Passed Issue #1092 coding requirements are met. For an unread stale completion, deliverStale keeps the durable appendEntry record and sends one hidden gentle-agents.stale-notice steer with `triggerTurn…
Out of Scope Changes check ✅ Passed The changed files stay within issue #1092. The new stale-notice type and delivery logic implement the required model-facing wake path. The rationale comment documents the stale-window behavior. The up…
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@MarsSall

MarsSall commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Thanks for this fix. We independently reviewed head 19297002215df8bf8e25613a20c474a8775b7090 against base 4b6b148b7dbd741b3097c00d5888f0209a567f94 and found no blocking candidate-caused defects in this scope.

Component verification used exact-SHA source reads and production queue/delivery snippets in memory, with a mocked host and clock:

  • Base reproduced the missing model-facing notice at 90,000, 90,001 and 102,000 ms; the PR produced exactly one hidden steer notice with triggerTurn: true, without report/error replay.
  • Fresh delivery below the threshold remained unchanged. Consumed results and foreign-session completions remained suppressed; repeated flushes and duplicate enqueueing produced no duplicate delivery.

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.ts

138 passed, 0 failed, 0 skipped; exit 0. An earlier sandbox attempt had nine Git-fixture failures because /dev/null was read-only; after validating private sandbox devices and git init, the final run passed all 138. All temporary dependencies, caches and source fixtures were removed; the original checkout and live installation were unchanged.

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.

@BGamboa13

Copy link
Copy Markdown
Author

Thanks for the careful independent run, @MarsSall. Here are the two things you asked for.

Repository CI

Neither workflow has run on 19297002 yet. Both runs (CI 36670380566 and Windows Hidden Internal Processes 36670380728) are sitting in action_required, because this is a fork PR and needs a maintainer to approve the workflow run.

Until someone approves it, I ran the Linux CI job steps myself on the exact head. I used Node 24.14.1, the same major as the workflow, with pnpm install --frozen-lockfile. I compared against the PR's merge-base, 664bfdd. The 4b6b148 base in your note is current main, which has moved on since this branch was cut, so a diff against it picks up unrelated files.

step PR head 19297002 merge-base 664bfdd
pnpm run typecheck pass —
pnpm run check:runtime-modules pass —
node scripts/verify-package-files.mjs pass —
pnpm run test:packed-package pass pass
Darwin transport subset (includes tests/gentle-agents.test.ts) 179/179 pass —
pnpm test 62 failing 63 failing
  • The PR adds no failures. The 62 failing tests on the head are a strict subset of the 63 failing on the merge-base.
  • They are environmental. They are all in review-*, gentle-ai, Herdr permission-lifecycle and package-manifest suites, which this PR does not touch, and they fail the same way on the untouched merge-base. The machine is WSL2, so I read them as host-specific rather than something the official runner will hit.
  • Node 26 run. On Node 26.10 the packed-package step also failed, identically on both sides. It passes on 24.

Real-host cases (Pi 0.99.1)

Setup. Pi 0.99.1 with gentle-pi 3.7.0. Its lib/agents-completion-delivery.ts is byte-identical to this PR's base before the patch, and to this PR's head after it. The session transcript gives exact timestamps: when each task settled, when it was flushed, which parent tool call was running, and when the parent first read the result.

Before the patch: 9 cases

Each background completion settled while the parent was inside one long tool call:

  • bash running a build or test suite, 5–20 min
  • ask_user_choice waiting on the human, 3.5–7 min
  • a long codemode call

The result was flushed at the next turn_end, 140–1148 s old. All 9 were recorded only as gentle-agents.stale-result, with no model-facing message at all. None had been read, and none had any fresh delivery before the flush.

What happened to them:

  • Never read (3). The three that flushed together at 186, 152 and 140 s were never read by the parent.
  • Read late (6). The rest were read 18 s to 60 min later, only when the conversation happened to come back to them.

After the patch: 3 cases

Each produced exactly one hidden gentle-agents.stale-notice, and the parent called subagent_result for that task within 5–13 s:

  • 126 s: a deliberate probe. I launched a task and then held the parent in a 151 s bash.
  • 487 s: organic. The parent was in a 646 s bash waiting on external CI.
  • 101 s: organic, with no tool in flight. The task settled while the parent's model was generating a single 133 s turn. A slow model turn alone can cross the 90 s threshold, not just a long tool call.

One caveat on my host

My install also carries a separate local workaround for the idle-wake problem (earendil-works/pi#5581 / #1528). It wraps sendMessage so that an idle parent gets nextTurn plus a prompt, and my backport of this PR's notice goes through that wrapper.

  • 126 s and 487 s cases: these flushed mid-run, where the wrapper delegates to exactly the sendMessage(..., { deliverAs: "steer", triggerTurn: true }) this PR makes.
  • 101 s case: the flush landed at the same moment the run ended, and a user message arrived. The notice then surfaced about 10 minutes later, at the following turn boundary. That delay comes from my wrapper's idle path, not from this PR.

To be clear about scope, this is the same as yours: evidence for the scoped fix, not a merge-readiness claim.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(agents): unread background completion becomes transcript-only during a long parent tool call

2 participants