fix(telegram): dedupe direct chat context against session history#82194
fix(telegram): dedupe direct chat context against session history#82194lidge-jun wants to merge 6 commits into
Conversation
|
Codex review: needs real behavior proof before merge. Summary Reproducibility: yes. at source level: current main builds Telegram Conversation context from cached recent/reply messages without any active-session marker. I did not establish a live Telegram reproduction in this read-only review. Real behavior proof Next step before merge Security Review findings
Review detailsBest possible solution: Keep the Telegram-local session-key dedupe approach, remove or split the cron persisted-shape change, and verify the direct-chat reply-context path with live or equivalent Telegram proof before merge. Do we have a high-confidence way to reproduce the issue? Yes, at source level: current main builds Telegram Conversation context from cached recent/reply messages without any active-session marker. I did not establish a live Telegram reproduction in this read-only review. Is this the best way to solve the issue? Mostly yes for the Telegram direction: session-bound cache metadata with direct-chat-only exclusion is a narrow plugin-local fix that preserves reply targets. No for the PR as submitted, because the cron validation relaxation should not merge with this Telegram fix and real Telegram proof is still required. Full review comments:
Overall correctness: patch is incorrect Acceptance criteria:
What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against d887eb8dc2cc. |
|
The current ClawSweeper review did not complete because Codex timed out ( Current state from the PR checks:
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
583c2d0 to
346d8ef
Compare
a6c397e to
890c893
Compare
890c893 to
c12ef91
Compare
c12ef91 to
fc16e15
Compare
|
Closing this PR — the design idea is reasonable and the persistence-reload test is genuinely good, but it can't merge as-is and the world around it has changed. Concrete reasons: 1. The 2. A parallel competing approach has already landed. Steinberger's commit takes a different path for the same class of bug — cache outbound messages first-class via 3. Scope creep — unrelated changes mixed in. 4. The user-visible problem is not yet fully fixed. #87566 (filed 2026-05-28) reports the same family of bug after the outbound-cache landing — so neither the merged work nor this PR alone resolves what users are seeing. If the session-key-tagging approach is still worth pursuing, a fresh PR that (a) rebases onto the current async cache surface, (b) coordinates with Closing in favor of that re-plan + ongoing work on #82040 and #87566. Thanks for the design exploration — the persistence-reload test in particular is worth carrying forward. |
Summary
Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Real behavior proof (required for external PRs)
node scripts/run-vitest.mjs extensions/telegram/src/message-cache.test.ts extensions/telegram/src/bot-message-context.dm-session.test.ts extensions/telegram/src/bot-message-context.thread-binding.test.ts extensions/telegram/src/bot-handlers.runtime.test.ts extensions/telegram/src/bot.test.tspnpm buildgit diff --check origin/main..HEADmarkSessionBound, resets/reloads the persisted cache bucket, and verifies the reloaded same-session message is deduped.UntrustedStructuredContext.pnpm buildcompleted successfully throughwrite-cli-compat.git diff --check origin/main..HEADproduced no output.5Telegram files and100scenarios passing with the new session-bound dedupe behavior.Root Cause (if applicable)
Regression Test Plan (if applicable)
extensions/telegram/src/message-cache.test.tsextensions/telegram/src/bot.test.tssessionKeyssurvive reload and dedupe direct-chat ambient contextUser-visible / Behavior Changes
Telegram direct chats should include less duplicate conversation context. Group/supergroup context and explicit reply targets should keep their previous behavior.
Diagram (if applicable)
Security Impact (required)
Yes/No): NoYes/No): NoYes/No): NoYes/No): NoYes/No): NoYes, explain risk + mitigation: N/ARepro + Verification
Environment
Steps
pnpm build.git diff --check origin/main..HEAD.Expected
Actual
5 files / 100 tests.pnpm buildpassed.git diff --check origin/main..HEADproduced no output.Evidence
Attach at least one:
Human Verification (required)
What you personally verified (not just CI), and how:
channel_post/supergroup path remains outside direct-chat dedupeextensions/telegram/srcsrc/plugin-sdkor core SDK files changedReview Conversations
Compatibility / Migration
Yes/No): YesYes/No): NoYes/No): NoRisks and Mitigations
sessionKeys.