fix(sdk): broadcast positioned session events live to attached subscribers - #4570
Conversation
|
Maintainer review/fix-forward ownership is active on the exact submitted head.
Review is independently reproducing the live/replay divergence with the real endpoint and public SDK client, then checking capability/authorization parity, sequence and generation ordering, replay/live boundary duplication, correlated requester delivery, multi-subscriber isolation, cleanup, and event-envelope validation. Valuable findings will be fix-forwarded on this PR rather than closed. The lane remains responsible through exact-head approval, green CI, — |
|
Exact-head CI failure classified as PR-induced and assigned to the active fix-forward lane.
This PR changes a Telegram topic-admission protected declaration (and the tool-activity capability surface in — |
Independent maintainer review — signed evidence (supersedes prior run)Verdict: APPROVE with fix-forward applied. Head under review: Reproduction (real endpoint + public
|
|
@probepark @HaD0Yun — independent exact-head review requested for merge.
What the PR does: live delivery of ring-positioned session events to attached direct SDK subscribers (#4569) — the notification-adapter endpoint previously retained positioned events for replay only. Maintainer review evidence already posted (signed, in comments): red→green reproduction at exact base/head using the real endpoint + public Repair commit Local validation at Required from you: an APPROVED review at exactly gaebal-gajae |
065e794 to
5ce4b30
Compare
Signed supersession evidence — rebase onto current dev (replaces prior-head evidence)Prior evidence at head Current exact head: Re-validated at
|
|
@probepark @HaD0Yun — formal review request (you are now the PR's requested reviewers; the list was empty, review ownership was missing).
What you are approving: contributor fix Please review at exactly gaebal-gajae |
OWNER_CONFIRMATION_REQUIRED — human-blocked terminal status (signed)
CI state at exact head — terminalDev CI run Review ownership
What is required to proceed (human-only)An independent exact-head APPROVED review at All autonomous work is complete and verified: red→green reproduction at both bases, ten scrutiny-axis probes against the real endpoint + public gaebal-gajae |
probepark
left a comment
There was a problem hiding this comment.
Approve at 5ce4b30e5
fix(sdk): broadcast positioned session events live to attached subscribers, head commit "bump
Telegram daemon generation for live positioned-event broadcasts". 7 files, +120.
Differential
# base 96e718a2b, with this head's test files applied
163 pass 2 fail
# head 5ce4b30e5
165 pass 0 fail
$ bun --cwd=packages/coding-agent run check -> exit 0
The generation bump is the right call, and I checked it is present
Changing what attached subscribers receive is a contract change for daemon owners already attached
across the upgrade. Bumping the Telegram daemon generation fences pre-upgrade owners rather than
leaving them attached to a stream whose shape changed underneath them - the same discipline
#4563 applied after #4542 changed the attachment contract.
Doing the bump in the same PR as the behaviour change is better than #4563's split, since there is
no window where the new broadcast ships without the fence.
merge-approved.
Reviewed by @probepark - method: fresh-worktree run and package typecheck at the exact head, separate clean base worktree with the head's test files to prove the differential, generation-bump presence confirmed in the diff.
Freshness dependency hold — current dev is redThis PR remains open at exact head Current dev is not a valid green rebase target: push run Disposition: item-specific hold remains active behind #4574. After #4574 merges and dev CI is green, this PR must be reconciled to that exact new dev head, with refreshed digest, CI, and review before any merge-ready claim. — |
Merge authorization revoked pending current-dev freshnessThe authenticated @probepark approval remains real for exact head The PR body verdict has therefore been lowered from — |
grantjayy
left a comment
There was a problem hiding this comment.
Revalidate the exact-head PR contract after the merge-approved verdict update. This comment does not replace probepark’s exact-head approval.
Fail-closed: stale-base approval cannot authorize mergeThe restored A dedicated maintainer lane now owns rebase onto the first green current-dev head, focused conflict verification, exact-head CI, and a fresh independent approval. Existing old-base approval and contract runs are retained as historical evidence only and are not merge authorization. — |
5ce4b30 to
9ecaaff
Compare
Signed supersession evidence — current-dev reconstructionAll prior approval and CI evidence for
The complete contributor delta was reconstructed as commits Current local evidence at this exact head:
@probepark @HaD0Yun — fresh independent exact-head review is requested. Please review Current gaebal-gajae |
9ecaaff to
630e9ea
Compare
Signed supersession evidence — rebased after #4577The Current sole candidate authority:
Reconstruction preserves all three required commits: positioned live delivery, Telegram generation acknowledgment, and bounded directed-writer backlog. The only rebase conflict was Fresh exact-head local evidence:
@probepark @HaD0Yun — fresh independent authorized review is requested for exact head PR #4575 remains the isolated browser repair. If it merges and advances gaebal-gajae |
probepark
left a comment
There was a problem hiding this comment.
Approve at 630e9eae4
Head moved since my approval of 5ce4b30e5 and grew from +120 to +458; the new head commit is
"bound live positioned-event delivery backlog", so the broadcast now carries a backlog bound.
Differential
# base 2e3ccb589, with this head's test files applied
(fail) publishes exact durable authority generation 169 at serving epoch 87
(fail) live positioned tool events use the same negotiated capability gate as replay [4442.96ms]
error: timeout waiting for capability-gated live positioned tool events
(fail) SDK host replays file attachment data as base64 while passing raw bytes to N-API [5001.22ms]
(fail) SDK host preserves positioned live order and replay parity for every attached direct subscriber [5001.75ms]
error: Timed out waiting for live positioned file attachment
error: Timed out waiting for live positioned terminal events
172 pass 4 fail
# head 630e9eae4
176 pass 0 fail
$ bun --cwd=packages/coding-agent run check -> exit 0
Three of the four base failures are multi-second timeouts - live events never arriving - which is
the correct signature for "broadcast does not reach attached subscribers". The fourth is the
generation pin.
The two assertions I care about
live positioned tool events use the same negotiated capability gate as replay - a live path that
bypasses the capability gate the replay path honours would leak events to subscribers that never
negotiated for them. Pinning both paths to one gate is the right invariant.
preserves positioned live order and replay parity for every attached direct subscriber - parity
between live and replay is the whole contract here. A live stream that is merely fast but ordered
differently from replay would make the two views disagree about the same session.
Bounding the backlog is the necessary companion: an unbounded live queue turns a slow subscriber
into unbounded memory growth on the host. Adding the bound in the same PR as the broadcast avoids
shipping the growth path first.
The generation bump is present, so pre-upgrade daemon owners are fenced rather than left attached
across a contract change - same discipline as #4563 after #4542, and again done in-PR rather than
as a follow-up.
merge-approved at 630e9eae4.
Reviewed by @probepark - method: fresh-worktree run and package typecheck at the exact head, separate clean base worktree with the head's test files to prove the differential.
…ubscribers On endpoints hosted by the notification-adapter transport, events appended to the resumable ring were never broadcast as their positioned envelopes: the native broadcast channel round-trips a closed frame enum that reduces non-native kinds (terminal agent lifecycle included) to empty unknown frames, and correlated lifecycle went raw to only the submitting connection. An already-attached direct SDK subscriber could therefore observe a later positioned event, including a turn's terminal lifecycle, only by issuing another replay. Every ring append now also fans the positioned envelope out per connection over the validated directed leg, to exactly the attached-subscriber set, gated by the same capability rule replay applies, so live and replay delivery are one truth per connection. Lore-id: c7699448 Constraint: no polling, status refresh, replay-on-demand, or second terminal path Constraint: preserve ring persistence, event positions, replay ordering, deduplication, endpoint authorization, and socket cleanup Rejected: adding an event variant to the native broadcast enum | wire-protocol change and native rebuild where the validated directed leg already carries positioned envelopes Rejected: broadcasting only terminal lifecycle kinds | special-cases one event class and leaves every other ring event replay-only Confidence: high Scope-risk: narrow Reversibility: trivial Tested: red-to-green live-delivery regression on the real endpoint and SDK client; 346 tests across host wiring, host runtime, transport lifecycle, surface parity, import graph, router authority, client, broker transport, broker integration, provider reconnect, daemon control frames, control dispatch, daemon session reconnect, and notification orchestration; coding-agent biome and type checks Not-tested: live multi-daemon fleet under real chat providers Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…roadcasts The live positioned-event delivery added for Yeachan-Heo#4569 edits two Telegram-generation-fenced declarations: createNotificationsExtension (topic-admission contract) and the shared TOOL_ACTIVITY_CAPABILITY constant (tool-activity contract). Both are protected because the notification bus owns session admission identity and capability-gated delivery for the Telegram daemon; live event transport changes what attached SDK subscribers observe, so the guard correctly requires an acknowledged generation bump. Applied via the documented repair path (guard --fix-generations against exact base 64c1528): DAEMON_GENERATION 168->169, topic-registry durable-authority pin synced 168->169, semantic manifest regenerated. Guard, --validate-current-tree, --check-authority, guard tests, topic-registry tests, sdk-host-wiring (105), and package check all pass. Lore-id: c77a0142 Constraint: repair must use the guard's base-aware path, never hand-edited digests Constraint: no behavior change beyond the generation acknowledgment Confidence: high Scope-risk: narrow Reversibility: trivial Tested: guard full/base-aware/authority; guard unit tests 75; topic-registry 60; sdk-host-wiring 105; probe suites 333 Not-tested: live multi-daemon fleet under real chat providers Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Live positioned events now use the directed connection writer, so a stalled subscriber could otherwise accumulate an unbounded queue independently of its own request rate. Bound queued host-directed frames to the replay-ring capacity and reject overflow for authoritative replay recovery.\n\nStrengthen integration coverage for multi-subscriber order, replay parity, capability gating, and attachment races on the real endpoint. Lore-id: pr4570-live-backpressure\nConfidence: high\nScope-risk: narrow\nTested: gjc-sdk crate, SDK host and notification suites, Telegram baseline, session caches, tools, coding-agent check, Rust check, binary build
630e9ea to
586201a
Compare
Signed supersession evidence — rebased after #4579The Current sole candidate authority:
All three required commits rebased without conflict. Fresh current-head evidence:
@probepark @HaD0Yun — fresh independent exact-head review is requested for PR #4575 remains isolated and open. If it merges and advances gaebal-gajae |
|
MERGE_READY Exact head
No stale-head approval or CI is used. No release or tag. gaebal-gajae |
Terminal receipt — merged and lane retiredPR #4570 was squash-merged into
The dedicated reconstruction lane is retired. gaebal-gajae |
Fixes #4569
Summary
eventenvelope used byevent_replay.Root cause
The notification-adapter endpoint appended events to the replay ring but did not broadcast their positioned envelopes.
Its native broadcast channel round-trips a closed frame enum. Event kinds outside that enum become
unknownframes. Correlated lifecycle events also reached only the submitting connection as raw frames.An attached direct SDK subscriber therefore needed another replay to observe a later retained event.
Reconstruction
The contributor's complete two-commit delta and required bounded-backlog maintainer repair are reconstructed onto current
devf024ded61408ee77c1fbcb20539146081a28e08d. The merged #4577 model-profile work and #4579 iTerm pet geometry repair are preserved without scope crossover; range-diff reports all three PR commits patch-equivalent to the prior reconstruction.Current reconstructed head:
586201a68ff23adbbc92ca720b944058dad05a2c.PR #4575 remains the isolated #4574 browser repair. If it merges and
devadvances, this head and its evidence become stale and the PR must rebase, re-digest, re-test, and receive fresh exact-head approval again before authorization.Verification
cargo test -p gjc-sdk— 149 passed.bun --cwd=packages/coding-agent run check— clean.bun run check:rs— clean.git range-diffreports all three commits patch-equivalent across the fix(tui): adapt iTerm pet to terminal geometry #4579 base advance.git diff --check— passed.Scope
No dependency, package version, public schema, polling path, replay workaround, event-specific callback, browser repair, pet repair, release, or tag was added.
gajae.pr-review-verdict.v1 merge-approved sha256:10088661a605c24e2625b0e06a289535fa829c50e5beb124dee20df68d70c98e reviewer:human reviewer-id:probepark evidence:#4570 (review)