fix(gateway): coalesce concurrent process completions and watch events

[handnewb]

Two approaches are offered for #70300 — maintainers pick one.

• fix(gateway): coalesce concurrent process completions and watch events #72675 — pointwise patches. Smaller diff, keeps the existing delivery shape, carries
  one documented residual (see Declared cost in that PR).
• feat(gateway): PendingCompletionRegistry — root-cause fix for #70300 #73469 — root-cause fix. Replaces the split pending-state with an explicit registry
  and state machine. Larger diff, removes the old mechanism, no residual.

Both close the reported flood. They are independent branches, not a stack — reviewing either
one alone is enough. I am not advocating for one over the other; #73469 is the better code
and #72675 is the smaller risk, and that trade is yours to make.
Root-cause writeup: #70300.

---

Summary

Coalesces background process completions and watch events that share a gateway route so the
agent receives one synthetic turn instead of one per process, fixing the session flood in
#70300.

This is the pointwise approach: it patches the individual failure modes and keeps the
existing delivery architecture. #73469 offers the same fix by replacing the underlying
mechanism. Both are open; maintainers choose.

What it does

1. Standard completions — batched at _run_process_watcher() via
   _enqueue_process_completion_notification() with a bounded window applied uniformly. There
   is no threshold-based early flush and no special-cased zero-latency path for single
   completions; an earlier revision of this description claimed both, and
   @yuzilongleif-collab's first review established that neither was implemented. They were
   removed rather than fixed.

2. watch_match / watch_disabled — coalesced at the post-turn drain via
   _coalesce_and_inject_watch_events(), grouped by event type and routing key. Batched
   watch_match retains bounded per-process command and output snippets, routed through
   _redact_gateway_user_facing_secrets.

3. Delivery disposition — CompletionDisposition (DELIVERED / RETRY /
   DROP_DUPLICATE / DROP_UNROUTABLE / SHUTTING_DOWN) replaces the previous
   Optional[bool] return. Every waiter resolves to an explicit disposition, including on
   CancelledError. Previously None could exit the watcher silently, consuming a completion.

4. Retry ceiling — per-identity attempt counter with short exponential backoff. Exhaustion
   is terminal and logged. Previously a retryable resolution during shutdown could drive
   retries against an adapter already torn down.

5. Shutdown — pending flushes are cancelled with a bounded wait before adapter teardown.
   The code comment now matches that behaviour; an earlier revision promised a "last delivery
   chance", which cancel-then-await does not provide.

6. Bounds, keys, redaction — aggregate summary counted over all entries with detail
   rendering capped at 10 and 800-char tails; process output tails routed through secret
   redaction; batch entries capped, with overflow counted in the summary rather than dropped
   silently; routing key fields distinguish None from "" so a None session key cannot
   coalesce with an empty-string one.

Declared cost — sibling claim window

The primary completion identity is marked in-flight before adapter delivery, but sibling
identities in the same batch are recorded only after the adapter returns. A concurrent sibling
replay inside that window can form a second synthetic turn.

Observable effect: under sibling replay concurrent with a slow adapter, a batch can surface
twice. It requires a replay source; the normal single-watcher path does not produce one.

Why it is not fixed here: closing it requires claiming all sibling identities atomically
before the adapter await, which requires separating the claim step from the delivery call.
That is the change #73469 makes. Doing it inside this architecture means reproducing half of
that PR, at which point the smaller-diff argument for this one disappears.

This is the residual you accept by taking this PR over #73469, and it is the whole of the
behavioural difference between them.

Verification still owed — applies to both PRs

The issue body diagnosed the flood as the post-turn drain loop. A review comment here observed
that _drain_gateway_watch_events() forwards only watch_match/watch_disabled, which would
mean standard completions never reach it. Both cannot be true. Both PRs coalesce at both seams,
so the reported behaviour is fixed either way, but I have not proven which seam produced the six
injections in the log. Instrumenting against main now; I will correct the issue body with the
result.

The log in #70300 also shows a second, separate defect that coalescing masks rather than fixes:
Persisted transcript lagged live cached history (disk=154, memory=157). Filed separately.

Tests

$ python3 -m pytest tests/gateway/test_completion_delivery.py tests/gateway/test_background_process_notifications.py -q

Covered: three concurrent watchers produce one batched turn; distinct routes never coalesce;
in-flight arrival schedules the next flush; failed delivery retries all entries; coalesced
success records every identity; duplicate primary falls through to a fresh sibling; formatter
failure resolves all waiters; cancellation during the window and during blocked delivery leave
no unresolved future, no stale batch, no stale task key; 12-entry batch with failures only
beyond the render cap still reports the correct aggregate; entry and output truncation; secret
masking in process output tails and in batched watch_match snippets; None vs "" routing
keys produce two turns; overflow entries counted in the summary; retry ceiling not exceeded
against a permanently failing adapter.

Review history

• Round 1 (@yuzilongleif-collab) — established that the advertised threshold early-flush
  and zero-latency single path did not exist; that the flush task sat outside the gateway
  lifecycle; that None could silently consume a completion; and that the aggregate summary
  counted only rendered entries. The two false claims were removed; the rest fixed.
• Round 2 (@yuzilongleif-collab) — established that the cancellation hole remained open in
  two distinct windows, and that the advertised omitted-tail regression test was absent.
• Since — both closed, plus the retry ceiling, output-tail redaction, key sentinel, and
  counted overflow above. The sibling claim window is documented rather than fixed, for the
  reason in Declared cost.

Attribution

The batching core and its delivery tests carry forward from #71898 by @yuzilongleif-collab,
preserved with co-author trailers. Earlier trailers were missing the email address and
therefore never registered as co-authorship; that is fixed.

Closes #70300

[alt-glitch]

This was generated by AI during triage.

Related: #71898 already coalesces standard completion deliveries at the watcher seam. This PR also batches post-turn watch events and adds threshold/format behavior, so the patches need maintainer consolidation rather than a duplicate designation.

[yuzilongleif-collab]

Thanks for consolidating these paths — coalescing both standard completions and post-turn watch events is the right direction for #70300. I reviewed exact HEAD a205be392f36d270357d76ae30afd5f058d5abc5, ran the two changed test modules plus tests/tools/test_process_registry.py (195 passed), and then added deterministic fault probes. The existing tests are green, but a few advertised/runtime properties still escape them.

Two of the lifecycle points below are inherited from our own #71898, so this is not criticism of the consolidation itself; we should fix them in whichever patch is ultimately retained.

1. Must fix: the flush task is outside the gateway lifecycle, and None can silently consume a completion

_enqueue_process_completion_notification() launches _flush_process_completion_batch() with a bare asyncio.create_task(). The task is kept only in _completion_notification_batch_tasks, not in the gateway's shutdown-owned task set. The flush pops/clears the batch, propagates the Optional[bool] returned by _deliver_completion_notification(), and the watcher retries only on literal False; None exits the watcher.

This creates a reachable adapter-disconnect/reconnect window. A deterministic probe on this HEAD enqueued one completion, removed its adapter during a 50 ms window, and produced:

enqueue_result=None
identity_delivered=False
flush_tasks_remaining=0
batch_entries_remaining=0

So the entry was neither delivered nor left retryable. Gateway stop also disconnects adapters before cancelling tracked background tasks, while this flush task is untracked.

Please make pending flushes shutdown-owned and drain/cancel them deterministically before adapter teardown, define temporary-adapter-unavailable separately from permanently-unroutable, and ensure every waiter resolves to an explicit retry/drop result. Regression coverage should include (a) adapter removal + recovery during the batch window and (b) shutdown with a pending batch, with exactly-once delivery-or-persistence and no orphaned InvalidStateError.

2. Threshold early flush and “zero-latency singleton” are not implemented as described

The threshold is checked once when the flush task starts, when the first enqueue has normally produced a batch of size 1. Once the task enters sleep(), later arrivals reaching 5 cannot wake it; threshold_event is unused and the docstring's asyncio.wait() path does not exist.

A staggered probe reached the threshold and still waited approximately 0.452 s of a 0.5 s window. A singleton consistently takes the full configured window (about 100 ms here). test_single_completion_zero_latency only asserts < 1.0s, so it cannot detect that 100 ms floor.

Either implement an actual Event/wait_for early wake with deterministic tests, or remove the threshold claim/dead variable and describe the intentional bounded batching delay accurately. True zero-latency singletons and time-window batching are incompatible unless coalescing is limited to work already available in the same loop tick.

3. The aggregate summary counts only the first 10 entries

_format_coalesced_process_completions() computes success/failure counts inside the entries[:10] detail loop. With 12 entries where the last two fail, the current output says:

Summary: 10 succeeded.

The “and 2 more” line does not repair an authoritative-looking aggregate summary. Count over all entries, while keeping detailed rendering capped at 10, and add a regression with failures only in the omitted tail.

4. Coalesced watch events lose actionable payload

Single watch_match messages carry command, matched output, and suppressed count; single watch_disabled messages carry the full reason/fallback message. Their batched forms retain only patterns and process IDs. That is a defensible noise trade-off, but it can remove the exact evidence the wake-up is meant to surface. Please preserve a compact per-event snippet/reason (still redacted and bounded), or explicitly document and test the semantic reduction.

Two tests (test_standard_completions_in_drain_are_coalesced and test_type_none_treated_as_completion) directly exercise a defensive helper branch that the real drain currently cannot feed, because _drain_gateway_watch_events() forwards only watch_match/watch_disabled. They should not be treated as end-to-end proof of the production completion path.

5. Hardening: claim every sibling identity before the adapter await

The primary completion is marked in-flight before adapter delivery, but batch siblings are recorded only after the primary returns. A concurrent sibling replay can therefore form a second synthetic turn during that window. This gap is inherited from #71898, and the deterministic probe uses an explicit duplicate source rather than proving that the normal single-watcher path creates one, so I would treat it as hardening rather than a standalone blocker. Still, atomically claiming all fresh batch identities before delivery — and releasing them all on failure — would restore the intended dedupe invariant. A blocked-adapter regression can prove it.

Attribution

The batching core and associated delivery tests substantially carry forward #71898. The PR description already acknowledges that groundwork; if this PR becomes the consolidated patch, please also preserve it in Git history by retaining/cherry-picking the original commits or adding appropriate co-author attribution.

The concept is solid, but I recommend revising these points before #72675 supersedes #71898. Happy to re-review the next revision.

[handnewb]

@yuzilongleif-collab — thanks for the thorough review. Applied fixes for all points:

P1 (lifecycle): Flush tasks are now tracked in _background_tasks via getattr(self, '_background_tasks', None) so real gateway shutdowns can drain/cancel them before adapter teardown. The _runner-based unit tests skip tracking since they use object.__new__.

P2 (threshold/zero-latency): Removed the broken threshold early-wake and inaccurate "zero-latency" claim entirely. _flush_process_completion_batch now uses a simple asyncio.sleep(window) — the 100 ms bounded delay is documented accurately. Test renamed to test_single_completion_bounded_latency.

P3 (summary): _format_coalesced_process_completions now counts succeed/failed over ALL entries before slicing entries[:10] for detail rendering. Added the regression you described (failures only in omitted tail → correct summary).

P4 (watch snippets): Batched watch_match messages now include bounded per-process command + output snippets (100/120 char caps, top 5 only).

P5 (pre-claim): Skipped — pre-claiming identities atomically before _deliver_completion_notification conflicts with that function's own dedup seam (it sees the identity as already in-flight and returns None). As you noted, this is hardening, not a blocker. The post-delivery sibling recording via _record_coalesced_completion_siblings is preserved.

P6 (dead tests): Removed test_standard_completions_in_drain_are_coalesced and test_type_none_treated_as_completion.

Attribution: Added your co-author signature on the fix commit. Happy for you to re-review when you have time.

[handnewb]

@yuzilongleif-collab — formal recognition:

You're credited as co-author on commit 8840cc4 which applies all the fixes from your review. Your debugging on #70319 is what saved us from shipping a broken fix (coalescing at the wrong seam), and the five points in your #72675 review raised the patch quality significantly:

• P1 (lifecycle) → flush tasks are now lifecycle-owned via _background_tasks
• P2 (threshold docs) → removed the broken claim, documented 100ms bounded delay accurately
• P3 (summary) → counts all entries, not just the 10 rendered in detail
• P4 (watch snippets) → batched messages include bounded command+output per process
• P6 (dead tests) → removed

Without your review this PR would have shipped with broken threshold logic, a misleading summary, orphaned flush tasks on shutdown, and dead-branch tests. Genuinely — thank you.

(I also tried to add you as a formal reviewer here but the fork token doesn't have upstream write perms — only a maintainer can do that.)

Happy for you to re-review when you have time.

[yuzilongleif-collab]

Thanks — I re-reviewed exact HEAD 8840cc4a22670e5d136c7c4a769f4bb94e0559f8.

I reran the two changed test modules through the canonical wrapper (72 passed, 0 failed), plus Ruff and git diff --check (both clean). The P2/P3/P4/P6 implementation changes look materially better. One part of P1 is still not closed, though, and there is one missing proof test.

1. P1 still has a cancellation/lifecycle hole

Adding the flush task to _background_tasks does not currently make shutdown ordering safe:

• _stop_impl tears adapters down first (gateway/run.py:9926-9937), then cancels _background_tasks (9943-9953). This is the reverse of the new comment at 18655-18656 saying flushes can be drained/cancelled before adapter teardown.
• If cancellation lands in the batch-window sleep (18579), the batch has not been popped yet, so the inner waiter-resolution finally (18622-18625) is never entered. The task can die with the waiter unresolved and the batch still resident.
• If cancellation lands after the batch pop while delivery is awaited, delivered is still None; the inner finally resolves all waiters to None, and _run_process_watcher retries only literal False before breaking on None (18960-18964). That silently makes cancellation terminal.

A deterministic probe on this exact HEAD produced:

cancel_during_window  waiter_done=False, batch_entries=1, flush_index=1, background_tasks=0
cancel_during_delivery waiter_done=True, waiter_result=None, batch_entries=0, flush_index=0

The first case is particularly problematic: the stale key in _completion_notification_batch_tasks also prevents a later enqueue for that route from scheduling a replacement flush.

I think cancellation needs an outer cleanup path that always detaches/pops the batch and resolves every waiter explicitly as retryable (False), while preserving/re-raising CancelledError; shutdown should either drain/cancel these tasks before adapter teardown or document a different deliberate contract. Two regressions should cancel (a) during the window and (b) while adapter delivery is blocked, asserting no unresolved Future/stale batch/task key and retryable disposition.

2. The advertised omitted-tail summary regression is still absent

The implementation now correctly counts over all entries, but the tests do not prove that change:

• test_batch_message_format_includes_exit_codes_and_summary has only 3 entries, below the 10-entry render cap.
• test_batch_message_truncates_entries has 12 entries but does not assert aggregate success/failure counts.

A 12-entry case with failures only outside entries[:10] should assert the full summary, so reverting the code to counting shown would fail.

Two nearby comments/docstrings should also be corrected while touching this block: 18609-18611 describes a None → False adapter-set escalation that is not implemented, and 18635-18637 still claims an early size threshold even though that mechanism was removed.

The steady-state coalescing path and routing-key separation otherwise look sound in this revision.

[handnewb]

@yuzilongleif-collab — second-review fixes applied:

P1 — Cancellation/lifecycle hole

• _flush_process_completion_batch now catches CancelledError, detaches the batch, cleans up the task key, resolves all pending Futures as False (retryable), and re-raises.
• Two regression tests:
  • test_cancellation_during_batch_window_resolves_waiters_retryable — cancel during the 100ms sleep
  • test_cancellation_during_delivery_resolves_waiters_retryable — cancel while adapter is blocked

P1 — Shutdown ordering

• _stop_impl_body now drains background flush tasks (cancel + await with 3s deadline) before adapter teardown.

Point 2 — Omitted-tail summary regression

• test_batch_summary_counts_failures_in_omitted_tail: 12-entry batch with failures only in the omitted tail; asserts 2 failed in summary even though those entries are not shown.

Docstrings

• Removed unimplemented None -> False adapter-set escalation comment.
• Removed stale threshold claim from _enqueue_process_completion_notification docstring.

---

34 tests passed, 0 failed. Ready for re-review.

[handnewb]

For comparison, a root-cause fix using an explicit PendingCompletionRegistry state machine is now in draft at #73427. The two approaches address the same five findings — #72675 patches them pointwise, #73427 replaces the split-state architecture. Maintainers can choose whichever shape they prefer. See #70300 for the full root-cause analysis.

[teknium1]

Thanks for carrying the watcher-seam batching and the follow-up review fixes forward. Current main still has the reported fan-out: each _run_process_watcher() directly awaits delivery at gateway/run.py:21299, while the post-turn drain excludes standard completions at gateway/run.py:3107.

Problems

• gateway/run.py:18653-18674 still turns cancellation during adapter delivery into DROP_UNROUTABLE: CancelledError bypasses except Exception, then finally maps delivered=None through _bool_to_disposition. The watcher retries only RETRY at gateway/run.py:19035, so this drops a completion.
• The new enum contract and tests disagree. CompletionDisposition is an enum.Enum at gateway/run.py:18491, but assertions such as tests/gateway/test_completion_delivery.py:764 compare enqueue results with [True, True].
• gateway/run.py:9932 cancels every _background_tasks member before adapter teardown, not just completion flushes. That set also owns startup-resume tasks and supervised watchers on current main (gateway/run.py:10098-10102, 11123-11135).

Suggested changes

• Make cancellation of a popped batch resolve each entry as RETRY, then re-raise; cover the real flush method with a delivery-blocked adapter.
• Assert enum dispositions in the added tests.
• Use a dedicated flush-task set for the pre-teardown drain.

Automated hermes-sweeper review.

[teknium1]

CancelledError from the adapter bypasses except Exception but executes this finally after delivered was set to None, so every popped entry becomes DROP_UNROUTABLE. _run_process_watcher retries only RETRY; catch cancellation around the popped entries, resolve them as RETRY, then re-raise.

[teknium1]

_enqueue_process_completion_notification() now returns CompletionDisposition, not bools. Compare these values with runner.CompletionDisposition.DELIVERED (and update the analogous new assertions), otherwise this test does not match the changed contract.

[teknium1]

This selects all gateway background tasks, including startup-resume and supervised watcher tasks, rather than only completion flushes. Track flush tasks in a dedicated set and drain that set here so this PR does not globally reorder shutdown cancellation.

[handnewb]

@teknium1 — all three findings confirmed at head c2aec8cd. Fixes pushed.

1. Cancellation during adapter delivery became DROP_UNROUTABLE — confirmed, blocker

Your trace is exact. CancelledError is a BaseException, so except Exception never sees it, but the inner finally still ran with delivered = None, and _bool_to_disposition(None) maps that to DROP_UNROUTABLE. _run_process_watcher retries only RETRY, so the completion was dropped. The outer except asyncio.CancelledError could not repair it either: the batch is already popped at the top of the function, so stale was empty and its waiter-resolution loop had nothing to iterate.

The fix resolves the popped entries as RETRY inside a new except asyncio.CancelledError before re-raising. The existing finally then no-ops over them through its own not future.done() guard, so there is exactly one resolution point per waiter and no double-set.

This is the same defect @yuzilongleif-collab raised in round two, one layer deeper: I closed cancel-during-window and left cancel-during-delivery mapping through the legacy bool conversion.

2. Enum contract and tests disagreed — confirmed

_enqueue_process_completion_notification() returns CompletionDisposition, and seven assertions still compared against True / False, so they asserted a contract that no longer exists. All seven now use runner.CompletionDisposition.*:

test	was	now
distinct routes never coalesce	[True, True]	[DELIVERED, DELIVERED]
in-flight arrival schedules next flush	is True ×2	is DELIVERED ×2
failed delivery retries all entries	[False, False] then [True, True]	[RETRY, RETRY] then [DELIVERED, DELIVERED]
duplicate primary → fresh sibling	[True, True]	[DELIVERED, DELIVERED]
coalesced success records every identity	[True, True, True]	[DELIVERED, DELIVERED, DELIVERED]
formatter failure resolves all waiters	[False, False]	[RETRY, RETRY]
single completion bounded latency	is True	is DELIVERED

_deliver_completion_notification() still returns Optional[bool], so the assertion on it at tests/gateway/test_completion_delivery.py:231 is deliberately unchanged — that is not the contract that moved.

3. Shutdown cancelled every background task — confirmed

Named _flush_tasks, selected all of _background_tasks minus stop/restart. Startup-resume and supervised watcher tasks were cancelled three seconds before adapter teardown, so this PR silently reordered shutdown for subsystems it does not touch.

Flush tasks are already tracked per route in _completion_notification_batch_tasks, so the drain now iterates that dict and nothing else. The asyncio.shield loop is replaced by one bounded gather(return_exceptions=True): the previous except (asyncio.CancelledError, TimeoutError): pass absorbed a cancellation aimed at _stop_impl_body itself, not only at the children.

---

On the two competing PRs — recommendation, and a correction

I opened #72675 and #73469 for the same issue and told maintainers to pick. That put the decision cost on triage, which is the scarce resource here, and it is most of why both sat. Making the call myself.

Recommend this PR as the merge candidate. #73469 goes to draft.

I argued the opposite earlier in the week, so here is what changed my mind. I had counted the Optional[bool] → CompletionDisposition migration as something only #73469 had. This PR already carries that enum, which closes most of the gap. What remains:

• Both are keep_open / salvageability: medium.
• This PR's three findings needed a 139-line fix. feat(gateway): PendingCompletionRegistry — root-cause fix for #70300 #73469's four findings, plus eight more I found auditing my own diff there, needed 521 lines — including a capacity check that refused every completion after the fiftieth for the life of the process, which was strictly worse than the bug it closes.
• This PR is +327/-10 against a file with existing coverage. feat(gateway): PendingCompletionRegistry — root-cause fix for #70300 #73469 is +1,120/-25 and adds a new subsystem.
• This PR has had two rounds of human review from @yuzilongleif-collab. feat(gateway): PendingCompletionRegistry — root-cause fix for #70300 #73469 has had none.

The one thing #73469 does genuinely better is the atomic sibling claim before the adapter await — the residual this PR documents as its declared cost. That is now a contained change here, since CompletionDisposition already exists: claim every fresh batch identity in one pass, release all of them on failure. Say the word and I will add it as its own commit rather than as an argument for the larger PR.

@yuzilongleif-collab — the batching core and delivery tests here still carry forward from #71898 with co-author trailers preserved. Your two rounds are what located the split pending state; that analysis is in #73469's description as the root cause and it holds whichever shape lands.

---

Verified: patch applies with git apply, both files compile. Not verified: the suite running green, Ruff, or a cancel-during-delivery regression measured against a blocked adapter — that regression is not in this revision and I would rather write it against whichever base you want.

[handnewb]

Correction: the table in the earlier comment listed 7 test assertions — two were missed in the first pass. Fixed in 7fe3450.

test	was	now
cancellation during batch window	is False	is CompletionDisposition.RETRY
cancellation during delivery	is False + message	is CompletionDisposition.RETRY

The only remaining True/False assertions (lines 231, 300) are on _deliver_completion_notification(), which still returns Optional[bool] by design — not part of the migrated contract.

[handnewb]

Two items from the sweeper review are still open at 7fe34500d, and one of them regressed with the assertion fix. Both in the patch that follows.

test_cancellation_during_delivery_resolves_waiters_retryable now fails

The assertion was migrated to runner.CompletionDisposition.RETRY, but that test overrides _flush_process_completion_batch with a local stand-in via types.MethodType, and the stand-in still resolves its futures with future.set_result(False) — twice, in its inner finally and in its CancelledError handler. False is CompletionDisposition.RETRY is false, so the assertion cannot hold. This is by inspection, not from a run.

Which surfaces the real problem with that test: it was never exercising the production flush. It asserted on its own reimplementation, so it stayed green throughout the period when _flush_process_completion_batch was mapping CancelledError onto DROP_UNROUTABLE — the defect @teknium1 reported as finding 1. A test named after that exact failure mode sat on top of it and reported success.

Patching the stand-in to return RETRY would restore green while preserving that property, so it is rewritten against the real method instead, with the blocking adapter from the review: handle_message sets an entered event, then awaits a release event that is never set, so the flush task can be cancelled while it is inside the send. The flush pops the batch and its task key before delivery starts, so the task is located through asyncio.all_tasks() rather than through the batch dict.

This closes the "cover the real flush method with a delivery-blocked adapter" item, which I had declared missing rather than delivered.

The AttributeError is a production defect, not a test-setup defect

test_cancellation_during_batch_window_resolves_waiters_retryable fails with AttributeError: _completion_notification_batch_max. It is pre-existing — the test body is identical at c2aec8cd and the initialisation block is byte-identical before and after the earlier patch — but the cause is not in the test.

Five attributes sit behind one hasattr guard on _completion_notification_batches:

if not hasattr(self, "_completion_notification_batches"):
    self._completion_notification_batches = {}
    self._completion_notification_batch_tasks = {}
    self._completion_notification_batch_lock = __import__("threading").Lock()
    self._completion_notification_batch_window = 0.1
    self._completion_notification_batch_max = 50

The test pre-sets four of them and not the fifth, so the guard is false, the block is skipped in full, and _completion_notification_batch_max is never assigned before len(batch) >= self._completion_notification_batch_max reads it.

Fixing that in the test would leave the trap for the next caller that pre-initialises partially. Each attribute is now guarded independently.

Patch

2 hunks, +52/-58:

file	change
gateway/run.py	per-attribute lazy-init guards
tests/gateway/test_completion_delivery.py	delivery test rewritten against the real flush with a blocking adapter

The boolean-assertion audit is unchanged and still reduces to lines 231 and 300, both on _deliver_completion_notification(), which returns Optional[bool] by design.

---

Verified: applies cleanly to 7fe34500d with git apply, both files compile, and the audit above is reproducible with grep -n "is False\|is True\|== \[True\|== \[False". Not verified: I have not run the suite on this patch. The rewritten test mirrors the adapter-blocking idiom of test_completion_arriving_during_batch_delivery_schedules_next_flush, but it is new code asserting on a cancellation path — if it needs adjusting, say what breaks rather than taking my word that it passes.

[handnewb]

@GottZ — accepting the duplicate designation on the watcher-flood scope, with one carve-out.

On closing this as a duplicate of #71898: no objection. The watcher-seam coalescing here and in caa398106 are the same fix, caa398106 owns its flush tasks and resolves both cancellation windows, and @yuzilongleif-collab got there first — the batching core in this PR carries forward from #71898 with co-author trailers, so the lineage is not in dispute. I am not going to argue for my version of a fix that already exists in someone else's PR.

Two hours ago I posted a comment here recommending this PR as the merge candidate over #73469. That recommendation was made without knowledge of #71898's current head, and I withdraw it. The triage's ordering is better than mine.

The carve-out is the scope this triage itself calls separable. This PR also coalesces post-turn watch_match / watch_disabled events, which #71898 does not touch. Those have the same flood shape for a different reason: _drain_gateway_watch_events() forwards them individually and nothing batches them. If this PR closes as a duplicate, that scope disappears with it.

I would rather resubmit it as its own PR than have it merged here as a rider on a duplicate. It also needs one correction before it is worth reviewing, which I found auditing the equivalent code in #73469: grouping must use the full delivery routing key, not session_key alone. Grouping on session_key collapses None and "" into one bucket and ignores platform, chat_id, thread_id and user_id, then delivers via dict(group[0]) — so events from different chats can coalesce into one message routed to whichever chat sorted first. That is a message-delivery boundary bug, not a cosmetic one, and it is present in this PR as written.

So: close this, and I will open a focused watch-event PR against a fresh main with that fix in it, unless a maintainer would rather see the watch-event scope dropped entirely.

For the record, since the sweeper findings were the open item here

All three of @teknium1's findings are fixed at ef67ba3f, plus two more I had missed. Recorded so that whoever reviews #71898 can check whether the same issues exist there:

• Cancellation during adapter delivery mapped to DROP_UNROUTABLE. CancelledError is a BaseException, so except Exception never saw it, but the finally still ran with delivered=None, which _bool_to_disposition maps to a permanent drop. Worth checking in caa398106 — the triage says it resolves both cancellation windows as retryable, which suggests it does not have this, but it is the same code shape.
• Nine test assertions compared enqueue results against True/False after the CompletionDisposition migration, not seven as I first claimed — corrected separately above.
• test_cancellation_during_delivery_resolves_waiters_retryable overrode _flush_process_completion_batch with a local stand-in via types.MethodType and asserted on the stand-in. It stayed green the entire time the production path mapped cancellation to DROP_UNROUTABLE, which is the failure mode it is named after. If fix(gateway): coalesce concurrent process completions #71898 has an equivalent test, it is worth confirming it drives the real method rather than a stand-in — a green cancellation test proves nothing on its own here.
• The pre-teardown drain selected all of _background_tasks rather than only completion flushes.
• test_cancellation_during_batch_window_resolves_waiters_retryable failed with AttributeError: _completion_notification_batch_max — pre-existing, and the cause is on the production side: five attributes sat behind one hasattr guard, so a partially pre-initialised runner skipped the whole block. Each attribute is now guarded independently.

---

Verified: patches apply to ef67ba3f, files compile, boolean-assertion audit reproducible. Not verified: the suite run on the final patch, or any claim about #71898's internals beyond what its diff and the triage state.

[teknium1]

Thanks for the work here — same-tick completion coalescing has now landed via PR #85938, which salvaged the earlier-submitted #71898 (per-route fan-in) and added async-delegation batch grouping. Reviewed against current main, this PR's disposition-enum rework and drain rewrite are superseded by that merged implementation. Closing as superseded.