extract(sdk): stub-preserve SessionRouter attachment authority (#4530) - #4738
extract(sdk): stub-preserve SessionRouter attachment authority (#4530)#4738Yeachan-Heo wants to merge 12 commits into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Reviewer brief for @probepark (independent exact-head review requested): Scope: one focused revert under issue #4530 owner contract A (stub-and-preserve). Review focus:
Evidence: full Exact diff digest — |
Owner exact-head receipt (fix-forward)Prior independent review of Why: adversarial exact-head review of the stub-preserve router found independently landed post-#4098 boundaries that the extraction had dropped. Minimal fix-forward on
Exact contract
Local verification (this worktree)
Canonical verdict remains gajae.pr-review-verdict.v1 needs-human sha256:ea53643150648b2d446fef2a85b12a3eae644359b37a09b5748ee8949862c6b9 reviewer:human reviewer-id:probepark evidence:local 126-pass focused suite + coding-agent check + guard v52 on 380b707; independent exact-head approval still required — |
Exact-head product CI (attempt 2)Dev CI run
The only red checks are PR contract bootstrap and Validate exact-head PR contract, which is the intentional Canonical body verdict is unchanged:
— |
|
Independent exact-head review still required before merge. Product CI on Requested: @probepark (primary, write), also @snowykr and @HaD0Yun (eligible write collaborators). Author — |
|
Reviewer pool widened: @snowykr is also requested. An exact-head approval from either @probepark or @snowykr satisfies the independent-review requirement; the canonical verdict line will be regenerated to name the approving reviewer with the same diff digest — |
380b707 to
c81b812
Compare
|
Rebased onto current terminal-green dev — |
Lane handoff — canonical mutation ownership moves to
|
|
Correction to the handoff run identities (terminal-critic evidence check): run — |
Owner adoption of live exact head (no extra source mutation)Worktree Surviving Dev CI Canonical verdict remains — |
Boundary cohort findings for the PR lane — 4 blockers to fix-forward on this headThe ultragoal boundary cohort (architect + executor QA/red-team, frozen sourceHash
Required tests (architect-prescribed + red-team): stop-during-admission asserts no post-shutdown hooks and no provider state; stop/restart during endpoint reproof asserts the old client is never revived; dispatch-time delete/rewrite asserts Machine-readable reports: — |
Exact-head acceptance (refreshed 2026-08-20)Owner lane adopted live
Canonical body verdict (exactly one line):
@probepark: authenticated APPROVE on — |
probepark
left a comment
probepark
left a comment
There was a problem hiding this comment.
Independent maintainer review at exact head c81b8121 — merge blocked. This breaks 20 existing tests, and I reproduced it rather than inferring it.
it does not build
$ bun test packages/coding-agent/test/chat-daemon-session-reconnect.test.ts
# on origin/dev
25 pass 0 fail
# on c81b8121
5 pass 20 fail
Same command, same machine, natives built in each worktree. Failures include:
(fail) a frame queued behind a failed publication cannot advance the cursor past it
(fail) a surface that refuses a frame for good concedes it instead of wedging the stream
(fail) a rolled endpoint's first frame gets its own delivery budget, not the previous generation's
(fail) an ambiguously acknowledged publication is not posted twice when reconciliation fails
with assertions like warnings.some(line => line.includes("publication failed at seq 2")) failing because that warning no longer exists in source. That suite is not touched by this PR — it is pinning behavior the PR removes.
this is a partial extraction, not a revert
The title says revert, but #4098 was merge 9bd1b642b, which centralized lifecycle in Broker/SessionLifecycleService, made SessionRouter the credential-bearing attachment/replay/reconnect authority, moved chat providers to opaque capabilities, and removed legacy provider paths across 228 files. This PR leaves that architecture in place and swaps only the router authority internals beneath a compatibility surface. Worth retitling, because "revert" sets the wrong expectation for what needs re-verifying.
what the extraction drops
Each of these is a live concern with a test still pinning it:
session-router.ts:972-980 — provider publication failures are swallowed and the cursor advances anyway, so a transient Discord/Slack failure permanently loses the frame. Pinned at chat-daemon-session-reconnect.test.ts:1526-1571.
:481-490 — matching generation/object identity skips reconciliation before dispatch. Managed callers pass the attachment deliberately, so a rotated, removed, terminal-uncertain, corrupted-index or rewritten endpoint can receive a command until a periodic scan notices.
:713-715 — lifecycle-result attachments are permanently exempt from retirement when Broker authority is absent. They publish immediately and can stay isCurrent() through missing/terminal index state, after which bindingAuthority() can authorize a Slack binding from unproved authority.
:691-693 — every replacement reports replaced, so replaced_same_generation becomes an exported but unreachable branch. chat-daemon-runtime.ts:435-444 does predecessor Discord/Slack route retirement only on that reason, so same-generation pid/mtime/token/URL rotation now silently skips cleanup.
:897-932 — replay responses ignore gap entirely. Generation resets, malformed bounds, and responses that both concede and return the same sequence are all accepted without rebuild or operator evidence. Pinned at :1028-1080,1180-1270.
minor — docs still promise the removed guarantees
docs/sdk-app-guide.md:28-33 still advertises a replay cursor and exact attachment authority, as do docs/sdk.md:10-20, docs/bot-integration.md:319-322 and docs/sdk-rpc-parity-audit.md:21-23. Only the historical handoff doc was marked superseded.
what is retained, for the record
Credential custody and opaque capabilities; token stamping; long-lived and caller request budgets; session/generation frame correlation; ambiguity withholding from SessionIndex; symlink-aware endpoint scope; endpoint mtime plus post-read re-stat substitution refusal; transport revival; reconnect provider and Telegram handshakes; run-epoch stop/adopt fencing; initial replay isolated from fleet scan; coordinator/ACP/MCP error mapping. Changelog placement is correct.
the ask
If the intent is that these guarantees are no longer wanted, that is a legitimate design argument — but it has to be made explicitly, and the tests pinning them have to be updated in the same change with a rationale, not left failing. Right now the PR neither keeps the behavior nor retires it deliberately.
Reviewed by @probepark — method: detached worktrees at c81b8121 and origin/dev with natives built in each, ran chat-daemon-session-reconnect.test.ts in both (5/20 vs 25/0), traced each failing assertion to the removed source behavior, identified #4098 as merge 9bd1b642b to establish revert scope, doc sweep for stale guarantees.
gajae.pr-review-verdict.v1 merge-blocked sha256:7fa44fd521b1503f8f7585f2e736b2ab1dcb3fa71c30e9e2909461ac7966c4ca reviewer:human reviewer-id:probepark evidence:exact-head-c81b8121-breaks-20-of-25-reconnect-tests-that-pass-on-dev-reproduced-locally
c81b812 to
37fd503
Compare
probepark blocked #4738 at c81b812: chat-daemon-session-reconnect regressed from 25/25 on origin/dev to 5/25. Port publication-failure cursor freeze, three-strike concession, replay-gap validation, same-generation replacement, and adopted-index retirement into the direct-attachment stub without restoring broker-index fencing. Lore-id: 4738d5e6 Constraint: keep stub-preserve extraction; do not weaken reconnect tests Rejected: update failing tests to match the stub | conceals live regressions Confidence: medium Scope-risk: medium Reversibility: clean-revert Tested: sdk-session-router-authority 23/23; chat-daemon-session-reconnect 17/25 Not-tested: remaining 8 reconnect cases still failing locally Issue: #4530
Yeachan-Heo
changed the title
Exact-head receipt (dev-integrated fix-forward)
@probepark: head moved; please re-review — |
37fd503 to
30da694
Compare
probepark blocked #4738 at c81b812: chat-daemon-session-reconnect regressed from 25/25 on origin/dev to 5/25. Port publication-failure cursor freeze, three-strike concession, replay-gap validation, same-generation replacement, and adopted-index retirement into the direct-attachment stub without restoring broker-index fencing. Lore-id: 4738d5e6 Constraint: keep stub-preserve extraction; do not weaken reconnect tests Rejected: update failing tests to match the stub | conceals live regressions Confidence: medium Scope-risk: medium Reversibility: clean-revert Tested: sdk-session-router-authority 23/23; chat-daemon-session-reconnect 17/25 Not-tested: remaining 8 reconnect cases still failing locally Issue: #4530
Exact-head receipt (rebased onto d97b79e)
@probepark exact-head re-review of — |
snowykr
left a comment
snowykr
left a comment
There was a problem hiding this comment.
Verdict
CHANGES_REQUESTED
Summary
The direct-attachment extraction is not ready to merge. The review found blocking regressions in replay delivery and endpoint-authority fencing, plus a platform upgrade gap for Telegram. The exact head also has eight documented reconnect-suite failures and removes deterministic coverage for retained asynchronous delivery guarantees.
Findings / Required Changes
-
[P1] Drain replay-held frames without dropping frames appended during publication
In
packages/coding-agent/src/sdk/router/session-router.ts:1059-1067, the router iterates a snapshot ofheld, awaits each provider publication, and then clears the entire live buffer. While those awaits are pending, the frame callback at:854-866can append additional frames because replay mode is still active. Those newly appended frames are absent from the snapshot and are erased by the final clear, causing silent event loss.Drain owned batches (splice before awaiting) in a loop until the buffer is stable, preserving the replay barrier until every batch has been published.
-
[P1] Re-prove exact endpoint authority before pinned request/capability dispatch
session-router.ts:515-551treats a matching cached generation or capability as sufficient to skip both scanning and#proveAttachedEndpoint(). The opaque capability send path at:811-815likewise checks only in-memory map identity. Managed callers normally pass this cached authority, including chat dispatch inpackages/coding-agent/src/sdk/bus/chat-daemon-runtime.ts:576-588and ACP dispatch inpackages/coding-agent/src/sdk/acp/adapter.ts:358-374.After endpoint deletion, credential/URL rewrite, rehost, or replay-triggered rebuild, a command can therefore reach a retired or successor process before the periodic scan notices the ownership change. Re-prove the current index and discovery identity immediately before every request/send (session, generation, PID, mtime, URL, and token), retire on mismatch, and fail closed. Re-check the caller's expected attachment after any scan or rebuild before performing the side effect.
-
[P1] Keep lifecycle-result endpoints provisional until indexed ownership is proven
session-router.ts:421-453accepts the credential-bearing lifecycle result and immediately attaches it.#attachDirectinstalls the endpoint and invokes ready hooks at:786-959, after which ACP begins querying it (packages/coding-agent/src/modes/acp/acp-agent.ts:2118-2159). The later adopted-endpoint check atsession-router.ts:1206-1214does not verify discovery URL/token.A close/rehost race can expose a stale endpoint as current and send it private queries or controls. Do not publish the capability or invoke readiness until the current index and discovery record exactly match all ownership and credential fields; repeat that proof at dispatch.
-
[P1] Bump and guard the Telegram daemon generation for the embedded router replacement
Telegram embeds
SessionRouter(packages/coding-agent/src/sdk/bus/telegram-daemon.ts:4517-4519,5127-5138), butpackages/coding-agent/src/sdk/bus/telegram-daemon-contract.ts:272-277remains at generation 170, which predates this PR. A live generation-170 Telegram daemon is therefore considered compatible and can continue running the removed router implementation after upgrade. Discord and Slack are correctly bumped inchat-daemon-control.ts:128-131.Increment Telegram's daemon generation, include the relevant router declarations in the protected generation inventory, and regenerate the manifest/baseline so upgrades replace pre-PR owners consistently.
-
[P1] Restore the reconnect and asynchronous delivery verification before merge
The exact-head evidence records the reconnect suite at 17/25, with eight cases still failing versus 25/25 on the base. The changed test diff also removes deferred coverage for provisional provider publication and live-frame retention, while the surviving cases cover only synchronous rejection/throws. It additionally removes deterministic never-settling attachment and poll-coalescing coverage.
Fix the remaining reconnect failures without weakening the suite; restore deferred async publication tests that assert no premature dispatch or frame loss, and restore deterministic hung-attachment isolation/coalescing coverage. Provide an exact-head 25/25 result and completed green Dev CI.
-
[P2] Revalidate prepared-session authority after connecting and before activation
session-router.ts:581-607proves discovery before opening the connection but activates immediately afterward. A rehost during connection establishment can move authority while activation is sent to the obsolete host. Refresh and compare generation, PID, mtime, URL, and token after connect and before activation; fail without sending on any change. -
[P2] Avoid serial attachment latency across indexed sessions
session-router.ts:716-766awaits each indexed session sequentially, with a 10-second connection timeout. Because chat startup waits forrouter.start()and daemon readiness is only eight seconds, stale entries can delay healthy sessions by roughlyN × 10s. Restore bounded concurrent per-session attachment while serializing only shared replacement/index bookkeeping.
CI / Verification
- Reviewed exact head
30da694dd1264796526b121cc51f233124f844f5against based97b79eff2be5f25bf3ae253de72430e2e2fab1b. - The failing Validate exact-head PR contract check is an intentional
needs-humangate and was ignored as requested. - The exact-head Dev CI run had not completed during review; superseded/cancelled runs and prior-head results were not treated as evidence for this head.
- Public/local surface checks passed, but they do not cover the router regressions above.
- No PR code, tests, builds, lint, or formatters were executed as part of the A4 axis; CI artifacts, changed tests, diffs, callers, and generated/platform surfaces were inspected read-only.
Axis Coverage
| Axis | Status | Result |
|---|---|---|
| A1. Intent / Policy / Contract | Reviewed | Changes requested: pinned dispatch violates the promised fail-closed exact-attachment contract. |
| A2. Architecture / Correctness / Failure | Reviewed | Changes requested: replay frame loss, stale dispatch, activation TOCTOU, and serial startup latency. |
| A3. Security / Privacy / Trust | Applicable and reviewed | Changes requested: stale/unverified endpoints can receive authenticated controls or private queries. |
| A4. Verification / Tests / CI | Reviewed read-only | Changes requested: 17/25 reconnect result, incomplete exact-head CI, and removed async/hung-path coverage. |
| A5. Context / Compatibility / Platform | Reviewed | Changes requested: Telegram generation fencing was not updated for the embedded router replacement. |
probepark
left a comment
probepark
left a comment
There was a problem hiding this comment.
Re-review at exact head 30da694d — merge blocked. Big improvement, still breaks existing tests.
measured at this head
$ bun test packages/coding-agent/test/chat-daemon-session-reconnect.test.ts
20 pass 5 fail
versus 25 pass / 0 fail on origin/dev. Worktrees at each with natives built in both.
That is real progress from the 20 failures I measured at c81b8121. But the suite is still not in the diff, so these are five behaviors the PR removes while their pins remain in place:
(fail) a replay refused past its retry budget rebuilds the attachment from its cursor
(fail) a real 256-frame host ring loses only the sequences the host says it evicted
(fail) a conceded gap publishes the sequences live delivery already carried instead of dropping them
(fail) an ambiguously acknowledged Discord session-ready publication is not posted twice
(fail) an ambiguously acknowledged publication is not posted twice when reconciliation fails
The body cites 17/25. I get 20/25 — either way, not green.
restored since last round — genuinely good work
Provider-publication failure and cursor discipline are back: #deliverFrame catches provider failure and #failDelivery retains the frame, sets the replay barrier, and emits publication failed at seq ${seq} (session-router.ts:1128-1168). Bounded concession with generation-specific attempt accounting is back (:1152-1168). replaced_same_generation is reachable again during scan (:740-747) and consumed by chat-daemon-runtime.ts:436-443. Replay gap parsing and validation are restored (:1012-1049).
Those were four of my five findings.
major — replay silently erases frames that arrive mid-drain
session-router.ts:1059-1067. Replay takes a snapshot of held, awaits provider publications, then clears the live array. Any frame appended during those awaits is outside the snapshot and is erased by the clear.
That is user-visible event loss, and it is new rather than carried over. Drain owned batches in a loop, holding the replay barrier until the buffer is actually stable.
still not restored
Caller-pinned request/capability dispatch skips both the scan and #proveAttachedEndpoint (:515-541), and adopted lifecycle endpoints publish immediately while later proof checks only generation/PID/mtime, not URL or token (:404-453, :1206-1215).
the shape of the ask
If some of these guarantees are genuinely no longer wanted, that is a legitimate argument — but it has to be made by updating the pins in this PR with a rationale. Right now chat-daemon-session-reconnect.test.ts is untouched while the behavior it asserts is gone, and the PR instead deletes related authority tests from sdk-session-router-authority.test.ts. That is the one combination that cannot be right: the old contract still has its tests, and the new contract has fewer.
Retitling to extract(sdk) was the right call and made this much easier to review against the correct expectation.
Reviewed by @probepark — method: detached worktrees at 30da694d and origin/dev with natives built in each, ran chat-daemon-session-reconnect.test.ts in both (20/5 vs 25/0), verified each previously-missing behavior at its restored line, read the replay drain against the live held array for the mid-drain window.
gajae.pr-review-verdict.v1 merge-blocked sha256:41d8382305ad0f74a440538d7d4b0c5fbeb5066b9a6496b0729e3b4c9fbcfabf reviewer:human reviewer-id:probepark evidence:exact-head-30da694d-five-reconnect-tests-still-fail-locally-and-replay-clears-frames-appended-mid-drain
Exact-head architecture review — CLEAR / APPROVEHead The prior low findings are fixed: held replay batches are sequence-sorted before delivery, and notification removal hooks are bounded at five seconds with failed-receipt settlement on timeout. Current-dev API, dispatch authority, replay, reconnect, and delivery contracts remain preserved. One non-blocking nit remains: timeout receipts retain the detach reason rather than a timeout marker. This is architecture evidence, not the required authenticated GitHub approval. — |
Exact-current-dev replacement receipt — PR #4738
Exact-head validation
GateThe high-risk PR still requires an authenticated independent
— |
|
@probepark @snowykr — please perform the independent exact-head adversarial review on The new dev commit touched only the unrelated Codex wake fixture test; no PR path overlapped. Exact focused authority/reconnect/notification/session-switch suites, package check, guard, state gates, native build, and Please leave an authenticated — |
Current exact-head blocker — PR #4738Head The post-fix architecture review is CLEAR/APPROVE. Focused authority, reconnect, notification, package, guard, state, native, and build validations are green. Exact-head workflow-dispatch CI run The only remaining human gate is an authenticated independent — |
Exact-head CI attribution updateHead This is a CI infrastructure/workflow execution blocker, not a changed-path regression. Local exact-head validation remains green, and the independent post-fix architecture review is CLEAR/APPROVE. The separate required human gate remains an authenticated non-author — |
fe57c1e to
a66c17e
Compare
probepark blocked #4738 at c81b812: chat-daemon-session-reconnect regressed from 25/25 on origin/dev to 5/25. Port publication-failure cursor freeze, three-strike concession, replay-gap validation, same-generation replacement, and adopted-index retirement into the direct-attachment stub without restoring broker-index fencing. Lore-id: 4738d5e6 Constraint: keep stub-preserve extraction; do not weaken reconnect tests Rejected: update failing tests to match the stub | conceals live regressions Confidence: medium Scope-risk: medium Reversibility: clean-revert Tested: sdk-session-router-authority 23/23; chat-daemon-session-reconnect 17/25 Not-tested: remaining 8 reconnect cases still failing locally Issue: #4530
a66c17e to
86704b8
Compare
probepark blocked #4738 at c81b812: chat-daemon-session-reconnect regressed from 25/25 on origin/dev to 5/25. Port publication-failure cursor freeze, three-strike concession, replay-gap validation, same-generation replacement, and adopted-index retirement into the direct-attachment stub without restoring broker-index fencing. Lore-id: 4738d5e6 Constraint: keep stub-preserve extraction; do not weaken reconnect tests Rejected: update failing tests to match the stub | conceals live regressions Confidence: medium Scope-risk: medium Reversibility: clean-revert Tested: sdk-session-router-authority 23/23; chat-daemon-session-reconnect 17/25 Not-tested: remaining 8 reconnect cases still failing locally Issue: #4530
Exact-current-dev replacement receipt — PR #4738
Exact-head validation
GateThe high-risk PR still requires an authenticated independent
— |
|
@probepark @snowykr — please perform the independent exact-head adversarial review on The latest dev refresh was reconciled, including generation assertions. The Virtual integration failure was reproduced locally: the CI script used unsupported Please leave an authenticated — |
Exact-head Virtual integration fix-forwardThe current-head Virtual integration failures were reproduced locally. Fix on exact head
Reproduction and validation now pass: — |
Current exact-head Virtual integration CI attributionHead The fixed workflow-dispatch run The prior CLI/API defect is fixed and reproduced locally: — |
…router (#4530) Owner contract A (stub-and-preserve) for issue #4530: remove the #4098 SDK-owned lifecycle/attachment implementation beneath the exported compatibility symbols while preserving every independently landed post-#4098 consumer contract. SessionRouter loses the broker-index attachment authority machinery (replay barriers, generation/endpoint-mtime fencing, adoption deferral, retirement versioning, delivery-concession bookkeeping) and becomes a direct-attachment router behind the identical exported surface: attachments establish from the session index or an ingested lifecycle result, the discovery endpoint is re-proved on every scan (mtime-bound, symlink-aware, #4645 preserved), the initial event_replay settles on the attachment's serialized frame tail before publication, dropped transports revive on scan with the provider handshake and catch-up replay re-run on reconnect, and revocation is a plain client close. Consumers compile unchanged. Preserved independent contracts proven by the retained/rewritten test surface: token stamping for daemon-origin injection, long-lived request budgets (#4258), notification subscription containment, symlinked workspace endpoint scope, discovery-record substitution refusal, coordinator/ACP/MCP/chat-daemon fail-closed staleness mapping, and ACP provider re-registration across transport reconnects. The 27 tests of the removed authority machinery are replaced by a direct-attachment stub contract; the 14 consumer-visible contract tests are retained verbatim. The coherent removed implementation is preserved on preserve/issue-4530-dev-06f0d4d for owner-controlled Draft re-presentation. Lore-id: i4530-stub-preserve Constraint: no pre-#4098 snapshot restoration (#4535 CI evidence) Constraint: exported symbol surface must keep every consumer compiling Rejected: full symbol removal | 173 references across 14 entangled files with independent callers Rejected: snapshot revert | erased independent post-#4098 contracts, 130+ CI regressions (#4535) Confidence: high Scope-risk: wide Reversibility: clean-revert Directive: the preserved implementation returns only as an owner-controlled Draft PR Tested: sdk-session-router-authority, coordinator-mcp-server, chat-daemon worker/control-frames, mcp-adapter, telegram/discord/slack daemons, ACP adapter/reconnect/production-path, lifecycle service/authority, session-list, acp/* (2000+ tests) Not-tested: full CI shard matrix (runs on PR) Issue: #4530
…uter (#4530) The telegram daemon generation guard pins SessionRouter's endpoint discovery/attach surface for the discord/slack families. The #4530 stub-and-preserve extraction renamed that surface (#attach/ semantic manifest, and helper test are re-pinned to the stub declarations with GUARD_CONTRACT_VERSION 51->52 and CHAT_DAEMON_GENERATIONS discord 65->66, slack 68->69 applied through the guard's own --fix-generations repair path. Lore-id: i4530-guard-repin Constraint: guard policy change requires the contract-version bump Confidence: high Scope-risk: narrow Reversibility: clean-revert Tested: --validate-current-tree, guard test (75 pass), guard run base..HEAD (v52 verified) Not-tested: n/a Issue: #4530
The stub-preserve router dropped independently landed post-#4098 boundaries: Telegram reconnect skipped onNotificationSubscriptionReady, initial event_replay sat on the fleet scan tail (#4527), discovery rewrite during the first stat was accepted, and stop/adopt lacked a run epoch. Restore those narrow guards without bringing back broker-index authority machinery. Lore-id: 4738a1b2 Constraint: keep the direct-attachment stub surface; do not restore #4098 fencing Rejected: restore full #4098 SessionRouter | violates owner contract A Confidence: high Scope-risk: medium Reversibility: clean-revert Tested: bun test packages/coding-agent/test/sdk-session-router-authority.test.ts (23 pass) Not-tested: exact-head Dev CI after push Issue: #4530
…ontract fix #attachDirect and #readEndpoint bodies changed; refresh the telegram generation-guard semantic digests so current-tree validation matches HEAD. Lore-id: 4738c3d4 Constraint: no extra CHAT_DAEMON_GENERATIONS bump; v52 vs base already verified Confidence: high Scope-risk: narrow Reversibility: clean-revert Tested: telegram-daemon-generation-guard --validate-current-tree; range 02c739e..HEAD Issue: #4530
probepark blocked #4738 at c81b812: chat-daemon-session-reconnect regressed from 25/25 on origin/dev to 5/25. Port publication-failure cursor freeze, three-strike concession, replay-gap validation, same-generation replacement, and adopted-index retirement into the direct-attachment stub without restoring broker-index fencing. Lore-id: 4738d5e6 Constraint: keep stub-preserve extraction; do not weaken reconnect tests Rejected: update failing tests to match the stub | conceals live regressions Confidence: medium Scope-risk: medium Reversibility: clean-revert Tested: sdk-session-router-authority 23/23; chat-daemon-session-reconnect 17/25 Not-tested: remaining 8 reconnect cases still failing locally Issue: #4530
…ry port Delivery-concession changes in #attachDirect invalidated the v52 semantic declaration digests. Refresh the current-tree attestation so the telegram generation guard matches HEAD; no extra CHAT_DAEMON_GENERATIONS bump (range vs origin/dev already reports v52 required bump verified). Lore-id: 4738e7f8 Constraint: digest refresh only; do not paper over declaration drift Confidence: high Scope-risk: narrow Reversibility: clean-revert Tested: telegram-daemon-generation-guard --validate-current-tree Issue: #4530
…nt router Close the probepark/snowykr reconnect and frame-retention majors on the extraction head. All 25 chat-daemon-session-reconnect pins pass. - Replay runs on an isolated readyTail instead of the frame tail, with the barrier raised synchronously at publication and reconnect, so a stalled provider publication can no longer delay the catch-up replay request. - replaced_same_generation is reserved for an actual in-place endpoint rotation (pid/mtime/url/token), so a same-generation barrier rebuild after a refused publication no longer retires predecessor provider routes. - Retention-gap recovery counts only sequences this attachment owns; the cursor advances over a conceded range even when republishing a recovered copy is refused, and the retained frame survives that advance and is re-served under an explicit force path. - Telegram DAEMON_GENERATION 170 -> 171 with manifest and registry pin. Lore-id: 4530b8e2 Constraint: preserve every independently landed post-#4098 contract on current dev Rejected: keep replay on frameTail | a stalled publication wedges the replay request Rejected: drop retained frame on cursor advance | loses the only surviving copy of a conceded event Confidence: high Scope-risk: wide Reversibility: simple-revert Tested: 25/25 reconnect, 92 router-authority + slack, 114 discord/control-frames/acp/host, coding-agent check, generation guard
Re-prove direct-attachment authority before every credential-bearing side effect and keep lifecycle admission provisional until indexed ownership is confirmed. Bound custom admission, shutdown, notification, and retired replay work while preserving current-dev replay and settlement contracts. Lore-id: 4530v5-owner\nConstraint: preserve owner contract A without restoring #4098 broker-index authority\nConstraint: retain independently landed current-dev delivery contracts\nRejected: cached generation as dispatch proof | permits stale endpoint credentials\nConfidence: high\nScope-risk: wide\nReversibility: simple-revert\nTested: authority, reconnect, notification registry, generation guard, coding-agent check\nNot-tested: root SDK closure under local Bun toolchain mismatch
Detached frame and replay tails must not leak a rejection into the Bun worker when an unsequenced provider callback or cleanup path fails. Keep the failure observable through the centralized logger while preserving the existing attachment fencing and replay behavior. Lore-id: 4530v5-tail-containment\nConstraint: no unhandled rejection may escape the direct-attachment router\nRejected: leave detached tails fire-and-forget | Bun can terminate a worker on an unhandled rejection\nConfidence: high\nScope-risk: medium\nReversibility: clean-revert\nTested: authority 23/23; reconnect 25/25; coding-agent check; generation guard
Keep the current-dev dispatch boundary while carrying the stub-router replay and authority repair onto the exact dev base.
Preserve sequence ordering when draining held live frames and prevent a stuck notification removal hook from leaving cleanup receipts pending indefinitely.
The installed gh CLI does not support branch, event, or status filters on gh run list. Query the workflow-runs API directly and parse its snake_case response so virtual integration can select a reachable green dev base.
86704b8 to
20ddb11
Compare
Exact-current-dev replacement receipt — PR #4738
Exact-head validation
CI attributionThe fixed workflow-dispatch Virtual integration job still failed before steps with GateAn authenticated independent
— |
|
@probepark @snowykr — please perform the independent exact-head adversarial review on The latest dev refresh was reconciled cleanly. The Virtual integration CLI/API defect is fixed with a regression test; local Please leave an authenticated — |
Newest exact-head architecture review — WATCH / no blockersHead The bounded review found no blocker. SessionRouter authority, replay, reconnect, recovery, and API preservation remain coherent. The noted adopt-before-index fail-closed tightening is already covered by the authority test that rejects The only WATCH item is external workflow-dispatch CI, whose Virtual integration job failed before any steps with no runner assigned; local select/validate/canaries pass. This is not an unreviewed source regression. — |
Newest exact-head CI attributionHead Workflow-dispatch run — |
3 participants
Issue #4530 — owner contract A (stub-and-preserve)
Partial extraction, not a mechanical revert of #4098. Broker/SessionLifecycleService stay; SessionRouter is a direct-attachment router that retains independently landed delivery guarantees.
20ddb1195ada1ca0f973ede8eec87c23a3a785d9origin/devbbb605d311a66c158f9028093ade246c2efa9d66(verified ancestor)sha256:a0bea0232a8671fb97427a25ad359d4091b13148df5494499d1a2360e488bcb5preserve/issue-4530-dev-06f0d4dCurrent-dev rebase receipt
origin/devbbb605d311a66c158f9028093ade246c2efa9d66afterdevadvanced.20ddb1195ada1ca0f973ede8eec87c23a3a785d9.sha256:a0bea0232a8671fb97427a25ad359d4091b13148df5494499d1a2360e488bcb5.packages/coding-agent/src/sdk/router/session-router.tswhile preserving current-dev dispatch observers; current-dev entries and the retained SessionRouter authority/replay contracts were preserved.bun run buildpassed.1.4.0expected vs child1.3.14).Owner port receipt — current-dev base
bbbdb5fcfa03a0fd6c7a8df1e4d7320420f50965(the requested snapshot); rebase verification base is currentorigin/devbbb605d311a66c158f9028093ade246c2efa9d66.20ddb1195ada1ca0f973ede8eec87c23a3a785d9.sha256:a0bea0232a8671fb97427a25ad359d4091b13148df5494499d1a2360e488bcb5.bun run checkis blocked only by the local rollback-fixture Bun-version mismatch (fixture expects 1.4.0, child reports 1.3.14); Rust scope and all preceding TypeScript checks passed. The same rollback fixture fails on the clean comparison worktree at/home/bellman/Workspace/gajae-code, so this is environment/toolchain drift rather than a PR-owned failure.State on this head
chat-daemon-session-reconnect.test.ts: 25 pass / 0 fail (suite unmodified by this PR — pins restored, not renegotiated)bun --cwd=packages/coding-agent run check: exit 0--validate-current-tree: exit 0Preserved current-dev contracts
SessionRouter.reconcile({ waitForReplay }),ChatDaemonRuntime.reconcile({ waitForReplay }),onFrameSettledsettlement notifications, #4527 scan-tail isolation, #4645 symlinked cwd scope, #4648 Telegram topic leases.Reviewer findings addressed
readyTail).replaced_same_generationreserved for real in-place endpoint rotation.Prior heads
30da694dd1/380b707adf/37fd503578/c81b8121/08124c17are superseded.—
[repo owner's gaebal-gajae (clawdbot) 🦞]
Exact replacement validation (20ddb11)
bbb605d311a66c158f9028093ade246c2efa9d66(ancestor).sha256:a0bea0232a8671fb97427a25ad359d4091b13148df5494499d1a2360e488bcb5.bun --cwd=packages/coding-agent run check: exit 0; generation guard 75/75; GJC state gates all passed; native build andbun run buildpassed.bun run checkand SDK closure are blocked only by the environment rollback fixture expecting Bun1.4.0while its child reports1.3.14; all preceding TypeScript, Rust, closure-manifest, and state checks passed.Risk classification
low-risk— ordinary fix/maintenance; the repository owner may use the explicitmerge-self-approvedsolo verdict (no independent human review; the verdict name itself records this) with a risk-record comment bound to the exact head.regression-risk— fix with material regression risk; requires one assigned independent domain reviewer whose authenticated exact-headAPPROVEDreview the gate verifies (extra:independent:<login>; the token alone never suffices).high-risk— large refactor, feature, or materially high-risk change (security/auth/install/remove/public API/destructive lifecycle/architecture); requires one assigned independent domain reviewer with an authenticated exact-headAPPROVEDreview (extra:independent:<login>).GJC verdict
devbun checkpasses (coding-agent package check passes; root closure is blocked by the pre-existing local Bun fixture mismatch)