fix(telegram): fence topics with shared durable authority - #3700
Conversation
691300e to
878e419
Compare
SummaryThis is a corrected resubmission of the Telegram topic-registry shared durable-authority slice. The branch is based directly on current The prior candidate #3695 was closed after exact-head review because its Windows fixture tests were not deterministic on the Windows lane. This resubmission fixes those fixtures by supplying the validated destination/staged identities and by making the ABA substitution happen explicitly inside the exact-replacement test seam. Scope and prior-blocker disposition
Verification
Thank you for reviewing this corrected, narrowly scoped resubmission. |
|
Follow-up: I rebased this branch onto the current |
|
I addressed the only completed CI failure on head |
|
The generation-guard failure on head |
|
The remaining exact-head jobs in workflow |
|
The Windows CAS lane exposed a platform-specific fixture assumption: after unlink/create, Windows may reuse the same inode value, so the test must not require |
|
All exact-head checks for the corrected head |
Yeachan-Heo
left a comment
There was a problem hiding this comment.
REQUEST_CHANGES — terminal review of PR #3700 at head 4a2114af6b5827d3fb734a8e3ad96c9893631a60.
The fetched review base is origin/dev at c1bf3be50e2cf0dfdf51b57eb43f7a99521c94b8; this PR's merge base is b40bc271502a0802e212e5538c5d0e8436643029, so the PR is stale and must be rebased onto current dev before approval.
Hostile lifecycle/generation findings:
- REQUEST_CHANGES / stale base (blocking): Rebase onto
c1bf3be50e2cf0dfdf51b57eb43f7a99521c94b8, then rerun the lifecycle and generation-fence tests against that base. - Archive retry can permanently strand daemon-created topics:
scheduleArchiveRetry()transitions the record toarchive_exhaustedafter eight attempts or 24 hours, whilearchivePendingSessionIds()only returnsarchive_pending. There is no subsequent reconciliation path forarchive_exhausted, so an ambiguouscloseForumTopicresult can leave the remote topic open forever with no automatic recovery. The bounded retry policy needs an explicit durable/manual recovery path or a retry state that remains discoverable; otherwise this is a resource leak under the exact network failure the fence is intended to handle. - Already-closed responses are treated as ambiguous:
topicArchiveSettled()accepts only{ok:true,result:true}. A retry after a successful Telegram close whose response was lost can receive Telegram's already-closed/not-modified error, but this code schedules more retries instead of recognizing the idempotent settled state. That drives the previous failure intoarchive_exhaustedand leaves the durable authority stuck. Accept the documented idempotent close outcome (with strict matching) as settled.
I did not merge or mutate the PR. Terminal verdict for this exact head: REQUEST_CHANGES.
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
|
Thanks for the terminal review. I prepared the bounded follow-up on top of current |
4a2114a to
1ba9f8d
Compare
|
I addressed the terminal review findings in |
|
I addressed the terminal review findings and pushed a rebased successor
Verification on the rebased tree: focused Telegram suite 782 passed, coding-agent typecheck passed, and Biome passed. The PR remains unmerged; please rerun exact-head CI and review this successor. Thank you. |
|
Thanks again for the detailed review. The latest head is |
|
Thanks for the review and for rerunning the checks. The fixes for the requested archive-recovery/idempotency issues are present on current head |
|
The fresh full fanout completed with no Telegram/native failures. The only red check is again shard-1, reproducing the same four unrelated |
|
@Yeachan-Heo The latest run has 29 successful checks; only the known unrelated shard-1 |
|
The run is now terminal: the only red entries are shard-1 and its two affected-path aggregators. The shard still reports exactly the four unrelated |
REQUEST_CHANGES — Telegram authority census exact-head reviewReviewed contributor PR #3700 at exact head The shared durable topic-authority contract is not superseded by merged history: current Fresh exact-head red-team tests pass locally: 837 pass, 0 fail across nine topic/daemon/CAS/config/race suites. The current head also fixes the prior review findings: archive-exhausted records are re-discoverable for retry/manual recovery, and already-closed/not-found topic responses are treated as settled. This is not merge-ready:
Please rebase onto current Signed: Yeachan-Heo via GitHub CLI |
f40435e to
7487e09
Compare
|
Thank you for the exact-head review. I rebased the branch onto current |
|
The generation guard correctly identified that rebasing onto current |
|
All checks for corrected head |
|
All exact-head checks for |
Yeachan-Heo
left a comment
There was a problem hiding this comment.
GJC adversarial review — REQUEST_CHANGES for exact head 5d29523bfd310464e61c8e1ccda47813d3a591fa.
The exact-head checks are green, but GitHub reports the PR as CONFLICTING with current dev, so it is not merge-ready. The prior archive-recovery findings remain blocking until the branch is rebased and the current-head lifecycle behavior is revalidated: archive exhaustion must remain durably discoverable/recoverable, and already-settled Telegram close outcomes must not strand topic authority.
Signed: GJC / Yeachan-Heo
|
GJC adversarial verdict — REQUEST_CHANGES for exact head Signed: GJC / Yeachan-Heo |
|
Thanks for the exact-head review. I confirm current |
5d29523 to
0b9750f
Compare
|
Rebased the PR onto current |
0b9750f to
581b768
Compare
|
Follow-up exact head: |
|
@Yeachan-Heo all required checks are green and GitHub reports exact head |
581b768 to
82a838a
Compare
|
Fresh exact head |
|
Exact head |
GJC terminal verdict — REQUEST_CHANGESReviewed external PR #3700 at exact head Blocking finding
At {"ok":false,"error_code":500,"description":"Bad Request: TOPIC_NOT_FOUND"}is classified as idempotently settled. Require Full-diff disposition
Terminal verdict: REQUEST_CHANGES Signed: GJC / Yeachan-Heo via GitHub CLI |
|
REQUEST_CHANGES — exact-head GJC review found the archive-idempotency classification accepts allowlisted text under non-400 error codes. Tighten the idempotent-settlement predicate to documented Telegram — |
Summary
This is a corrected resubmission of the Telegram topic-registry shared durable-authority slice.
The branch is based directly on current
devand contains only the Telegram authority files/tests and the deterministic generation-manifest refresh. The native exact-replacement dependency is already merged upstream in #3596.The prior candidate #3695 was closed after exact-head review because its Windows fixture tests were not deterministic on the Windows lane. This resubmission fixes those fixtures by supplying the validated destination/staged identities and by making the ABA substitution happen explicitly inside the exact-replacement test seam.
Scope and prior-blocker disposition
Verification
bun scripts/telegram-daemon-generation-guard.ts --validate-current-treebun --cwd=packages/natives run buildbun --cwd=packages/coding-agent run check:typesbun test packages/coding-agent/test/notifications-telegram-daemon.test.ts packages/coding-agent/test/notifications-telegram-daemon-cas.test.ts packages/coding-agent/test/notifications-topic-registry.test.ts packages/coding-agent/test/notifications-topic-settle-fence-epoch.test.ts packages/coding-agent/test/notifications-telegram-daemon-2960.test.ts packages/coding-agent/test/notifications-telegram-daemon-2960-redteam.test.ts packages/coding-agent/test/daemon-control.test.ts packages/coding-agent/test/file-lock-gc-toctou.test.ts(779 passed)Thank you for reviewing this corrected, narrowly scoped resubmission.