Skip to content

fix(telegram): dedupe direct chat context against session history#82194

Closed
lidge-jun wants to merge 6 commits into
openclaw:mainfrom
lidge-jun:codex/82040-telegram-context-dedupe
Closed

fix(telegram): dedupe direct chat context against session history#82194
lidge-jun wants to merge 6 commits into
openclaw:mainfrom
lidge-jun:codex/82040-telegram-context-dedupe

Conversation

@lidge-jun

@lidge-jun lidge-jun commented May 15, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Problem: Telegram direct-message prompts can re-ingest messages that already belong to the active session history, causing duplicated transcript context and lower quality responses.
  • Why it matters: direct chats are the most sensitive path for repeated assistant/user turns; duplicate context wastes prompt budget and can cause the model to answer stale turns.
  • What changed: Telegram cached messages can now remember session keys, outbound Telegram replies are marked after successful dispatch, and direct-chat prompt context skips ordinary same-session messages while preserving reply targets.
  • What did NOT change (scope boundary): group/supergroup ambient context is preserved, reply-chain targets remain selectable, core SDK/plugin contracts were not changed, and no changelog entry was edited.
  • Contributor context: while working on an Oracle zip workflow, prompt/runtime overhead stood out as a practical performance issue; this PR is a small Telegram-side cleanup intended to help OpenClaw stay lean upstream.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

Real behavior proof (required for external PRs)

  • Behavior or issue addressed: a same-session Telegram message is recorded once in durable cache metadata and excluded from later direct-chat ambient context, unless it is needed as a reply target.
  • Real environment tested: macOS local workstation, Node v24.14.1, pnpm v11.1.0, OpenClaw built from source with the Telegram extension runtime/cache harness.
  • Exact steps or command run after this patch:
    • 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.ts
    • pnpm build
    • git diff --check origin/main..HEAD
  • Evidence after fix (screenshot, recording, terminal capture, console output, redacted runtime log, linked artifact, or copied live output):
    • Copied live terminal output from the local Telegram runtime/cache harness command:
$ 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.ts
RUN  v4.1.6 /Users/jun/Developer/new/700_projects/cli-jaw/devlog/_plan/260515_codex_rs_openclaw_hermes_runtime/openclaw-pr-b-82040

Test Files  5 passed (5)
Tests  100 passed (100)
Duration  13.98s
  • The persisted-cache regression scenario records a message, calls markSessionBound, resets/reloads the persisted cache bucket, and verifies the reloaded same-session message is deduped.
  • The runtime private-chat scenario sends two private-chat messages in the same chat and verifies the second prompt no longer includes duplicate UntrustedStructuredContext.
  • Build after final rebase: pnpm build completed successfully through write-cli-compat.
  • Whitespace check after final rebase: git diff --check origin/main..HEAD produced no output.
  • Observed result after fix: direct-chat prompt context excludes ordinary same-session cached messages after reload, while reply-target selection remains intact; the copied runtime/cache harness output shows 5 Telegram files and 100 scenarios passing with the new session-bound dedupe behavior.
  • What was not tested: a live credentialed Telegram bot run against Telegram servers.
  • Before evidence (optional but encouraged): [Bug] Telegram conversation context duplicates session history, causing context pollution and degraded response quality #82040 describes duplicated Telegram session history causing context pollution and degraded response quality.

Root Cause (if applicable)

  • Root cause: Telegram cached messages did not carry session-bound metadata, so prompt context selection could not tell whether a cached Telegram message had already been represented in the active session transcript.
  • Missing detection / guardrail: existing tests covered reply-chain and context selection, but not persisted same-session dedupe after outbound Telegram dispatch.
  • Contributing context (if known): direct-message chats reuse the same chat window, so previous assistant/user messages are easy to pick up again as ambient context unless the active session boundary is explicit.

Regression Test Plan (if applicable)

  • Coverage level that should have caught this:
    • Unit test
    • Seam / integration test
    • End-to-end test
    • Existing coverage already sufficient
  • Target test or file:
    • extensions/telegram/src/message-cache.test.ts
    • extensions/telegram/src/bot.test.ts
  • Scenario the test should lock in:
    • persisted sessionKeys survive reload and dedupe direct-chat ambient context
    • same-session reply targets are preserved when selected through a reply chain
    • a second private-chat prompt does not include duplicate same-session context
  • Why this is the smallest reliable guardrail: the bug is inside Telegram-local cache/context selection, so focused cache and runtime handler tests cover the contract without requiring live Telegram credentials.
  • Existing test that already covers this (if any): existing reply-chain tests covered target selection but not session-bound dedupe or persisted session keys.
  • If no new test is added, why not: N/A, tests were added.

User-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)

Before:
outbound Telegram message -> cached with no session marker
next direct prompt -> ambient context can include same-session cached message again

After:
outbound Telegram message -> cached with sessionKeys[]
next direct prompt -> ordinary same-session cached message skipped
reply target -> still included when explicitly selected

Security Impact (required)

  • New permissions/capabilities? (Yes/No): No
  • Secrets/tokens handling changed? (Yes/No): No
  • New/changed network calls? (Yes/No): No
  • Command/tool execution surface changed? (Yes/No): No
  • Data access scope changed? (Yes/No): No
  • If any Yes, explain risk + mitigation: N/A

Repro + Verification

Environment

  • OS: macOS
  • Runtime/container: local Node v24.14.1, pnpm v11.1.0
  • Model/provider: N/A
  • Integration/channel (if any): Telegram extension
  • Relevant config (redacted): no live Telegram secrets used for local verification

Steps

  1. Build OpenClaw from source with pnpm build.
  2. Run the focused Telegram cache/context/runtime test suite.
  3. Confirm the diff is whitespace-clean with git diff --check origin/main..HEAD.

Expected

  • Same-session direct-chat cached messages are deduped.
  • Reply targets and reply-chain context are preserved.
  • Group/supergroup context behavior remains unchanged.
  • Tests and build pass.

Actual

  • Telegram focused suite passed: 5 files / 100 tests.
  • pnpm build passed.
  • git diff --check origin/main..HEAD produced no output.

Evidence

Attach at least one:

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

  • Verified scenarios:
    • persisted session keys survive cache reload
    • direct-chat same-session ambient context is deduped
    • reply targets are not dropped by dedupe
    • group/supergroup context keeps existing reply/ambient behavior
  • Edge cases checked:
    • channel_post/supergroup path remains outside direct-chat dedupe
    • Telegram-local callback wiring stays confined to extensions/telegram/src
    • no src/plugin-sdk or core SDK files changed
  • What you did not verify:
    • live Telegram bot behavior against Telegram servers

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

  • Backward compatible? (Yes/No): Yes
  • Config/env changes? (Yes/No): No
  • Migration needed? (Yes/No): No
  • If yes, exact upgrade steps: N/A

Risks and Mitigations

  • Risk: overly broad dedupe could hide a message the user explicitly replied to.
    • Mitigation: same-session nodes are skipped only for ordinary ambient context; reply-target nodes are preserved and covered by tests.
  • Risk: persisted cache files may contain older entries without sessionKeys.
    • Mitigation: parser treats the field as optional and defaults to normal existing behavior for older entries.

@openclaw-barnacle openclaw-barnacle Bot added channel: telegram Channel integration: telegram size: M triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels May 15, 2026
@clawsweeper

clawsweeper Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge.

Summary
Adds Telegram cached-message session markers and direct-chat prompt-context dedupe, with focused Telegram tests plus small channels-add and cron store changes.

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
Needs real behavior proof before merge: Only copied focused test/build output is supplied, and the PR says no live Telegram bot run was tested. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, ask a maintainer to comment @clawsweeper re-review.

Next step before merge
Needs contributor-side cleanup of the cron validation regression plus live or equivalent Telegram proof before normal maintainer review can proceed.

Security
Cleared: No concrete security or supply-chain concern found; the diff does not add dependencies, CI execution, permissions, secret handling, or new network surfaces.

Review findings

  • [P2] Keep blank agentTurn jobs invalid — src/cron/persisted-shape.ts:67
Review details

Best 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:

  • [P2] Keep blank agentTurn jobs invalid — src/cron/persisted-shape.ts:67
    This drops the trim().length check when loading persisted cron jobs, so a whitespace-only agentTurn message now loads as valid instead of being skipped as invalid-payload. The load path warns and skips invalid persisted jobs today, while later runtime paths trim blank messages to an empty prompt, so this unrelated change can revive corrupt jobs rather than repair or reject them.
    Confidence: 0.87

Overall correctness: patch is incorrect
Overall confidence: 0.84

Acceptance criteria:

  • 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.ts
  • node scripts/run-vitest.mjs src/cron/service/store.test.ts
  • pnpm build
  • live or equivalent Telegram proof for direct-chat reply-context behavior

What I checked:

Likely related people:

  • @obviyus: Prior ClawSweeper discussion maps the local chat-context window feature to this handle and commit, which is the behavior this PR changes. (role: feature-history owner; confidence: medium; commits: 4cdf19eabe61; files: extensions/telegram/src/bot-handlers.runtime.ts, extensions/telegram/src/bot-message-context.ts, extensions/telegram/src/message-cache.ts)
  • Shakker: Current checkout blame for the central Telegram context/cache files and cron persisted-shape file points to the recent runtime-loading refactor commit. (role: recent area contributor; confidence: medium; commits: c90e42aaa766; files: extensions/telegram/src/bot-handlers.runtime.ts, extensions/telegram/src/message-cache.ts, src/cron/persisted-shape.ts)
  • @steipete: The related timeline references a separate recently merged Telegram PR by this author; that work is distinct but in the same Telegram runtime area. (role: adjacent Telegram contributor; confidence: low; commits: 7270bb95b70e; files: extensions/telegram/src)

Remaining risk / open question:

  • No live or equivalent Telegram proof has been supplied for the reply-context path.
  • The branch still carries an unrelated cron validation change that can revive whitespace-only persisted agentTurn jobs.

Codex review notes: model gpt-5.5, reasoning high; reviewed against d887eb8dc2cc.

@lidge-jun

Copy link
Copy Markdown
Contributor Author

The current ClawSweeper review did not complete because Codex timed out (spawnSync codex ETIMEDOUT), so it did not assess the change or proof.

Current state from the PR checks:

  • latest Real behavior proof check passed
  • focused Telegram tests and build evidence are already in the PR body
  • the remaining review comment is a reviewer execution timeout, not a code finding

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@lidge-jun
lidge-jun force-pushed the codex/82040-telegram-context-dedupe branch from 583c2d0 to 346d8ef Compare May 16, 2026 04:35
@lidge-jun
lidge-jun force-pushed the codex/82040-telegram-context-dedupe branch from a6c397e to 890c893 Compare May 17, 2026 01:53
@openclaw-barnacle openclaw-barnacle Bot added channel: whatsapp-web Channel integration: whatsapp-web extensions: memory-core Extension: memory-core labels May 17, 2026
@clawsweeper clawsweeper Bot added the P2 Normal backlog priority with limited blast radius. label May 17, 2026
@lidge-jun
lidge-jun force-pushed the codex/82040-telegram-context-dedupe branch from 890c893 to c12ef91 Compare May 17, 2026 02:36
@openclaw-barnacle openclaw-barnacle Bot added commands Command implementations size: L and removed size: M labels May 17, 2026
@clawsweeper clawsweeper Bot added the impact:session-state Session, memory, transcript, context, or agent state can drift or corrupt. label May 17, 2026
@lidge-jun
lidge-jun force-pushed the codex/82040-telegram-context-dedupe branch from c12ef91 to fc16e15 Compare May 17, 2026 02:47
@openclaw-barnacle openclaw-barnacle Bot removed channel: whatsapp-web Channel integration: whatsapp-web extensions: memory-core Extension: memory-core size: L labels May 17, 2026
@amknight

Copy link
Copy Markdown
Member

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 main baseline this PR was written against no longer exists. Commit 3cf806d172 ("fix(telegram): cache outbound replies for context", 2026-05-23) converted the entire TelegramMessageCache surface (record, get, recentBefore, around) and buildTelegramConversationContext itself to async. This PR's hunks edit the sync forms; rebase will be substantial, and the new markSessionBound would need to become async too (with the persistence calls re-checked, since appendPersistedMessage may have shifted as well). The CI greens in the PR body are against a snapshot that no longer applies.

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 extensions/telegram/src/outbound-message-context.ts so they dedupe by message_id. This PR's session-key tagging design doesn't coordinate with that work (no calls to recordOutboundMessageForPromptContext). Shipping both risks one becoming dead code. A maintainer needs to decide if they're complementary or one should replace the other.

3. Scope creep — unrelated changes mixed in. src/commands/channels/add.ts, src/cron/persisted-shape.ts, and src/cron/service/store.test.ts have zero relationship to Telegram dedupe. In particular, persisted-shape.ts silently relaxes cron agentTurn payload validation to allow empty/whitespace message strings — a behavioral change with no test and no justification. This needs to be split out (and probably reverted from this PR's intent regardless).

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 outbound-message-context.ts instead of duplicating dedupe, (c) drops the cron/channels changes, (d) bounds sessionKeys[] growth per node, (e) replaces the MessageSid/SessionKey typeof === "string" casts with a typed envelope shape, and (f) tightens the chat-type gate from !== "group" && !== "supergroup" to a positive === "private" allow-list would be a much stronger basis.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: telegram Channel integration: telegram commands Command implementations impact:session-state Session, memory, transcript, context, or agent state can drift or corrupt. P2 Normal backlog priority with limited blast radius. proof: supplied External PR includes structured after-fix real behavior proof. size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Telegram conversation context duplicates session history, causing context pollution and degraded response quality

2 participants