Skip to content

Log Telegram outbound delivery success#83202

Closed
jrwrest wants to merge 3 commits into
openclaw:mainfrom
jrwrest:review/83196-telegram-outbound-success-log
Closed

Log Telegram outbound delivery success#83202
jrwrest wants to merge 3 commits into
openclaw:mainfrom
jrwrest:review/83196-telegram-outbound-success-log

Conversation

@jrwrest

@jrwrest jrwrest commented May 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • add an info-level Telegram outbound send success log with account/chat/message identifiers
  • log successful text sends after chunk delivery and media sends after Telegram returns the media id
  • cover text/media success logging and assert body/caption/media locations are not logged

Fixes #83196

Verification

  • pnpm exec vitest run extensions/telegram/src/send.test.ts
  • pnpm tsgo:extensions:test
  • pnpm build
  • git diff --check

Real behavior proof

  • Behavior or issue addressed: Telegram outbound delivery success now emits a journal-facing telegram outbound send ok line with account id, chat id, Telegram message id, operation, thread id, reply-to id, silent flag, and chunk count.
  • Real environment tested: Local OpenClaw checkout at /Users/jameswrest/tmp/openclaw-83196-review, Node v22.22.0, macOS, after building this branch.
  • Exact steps or command run after this patch: Ran a Node/tsx command against extensions/telegram/src/send.ts using the real sendMessageTelegram() module path and a redacted Telegram-shaped API adapter to avoid sending a live Telegram message from CI credentials.
  • Evidence after fix: Redacted runtime log excerpt from the local OpenClaw logger file:
{"message":"telegram outbound send ok accountId=default chatId=-1001234567890 messageId=98765 operation=sendMessage deliveryKind=text threadId=271 replyToMessageId=123 silent=true chunkCount=1"}
  • Observed result after fix: The runtime log contains the new success line with operational identifiers and no outbound body text. The body used for the command was redacted proof message body, and that string does not appear in the emitted log line.
  • What was not tested: Live Telegram API delivery was not exercised in this PR because no production bot token/chat was used from this environment; the send-level success behavior was verified at the OpenClaw Telegram send boundary.

@openclaw-barnacle openclaw-barnacle Bot added channel: telegram Channel integration: telegram size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 17, 2026
@clawsweeper

clawsweeper Bot commented May 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge.

Workflow note: Future ClawSweeper reviews update this same comment in place.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by maintainer comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors can comment @clawsweeper re-review or @clawsweeper re-run on their own open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Summary
The branch adds Telegram outbound success logging/tests and a scoped Codex auth overlay fix with regression coverage.

Reproducibility: yes. Source inspection shows the PR branch can retry without message_thread_id after Telegram reports a missing thread, while the new success log still reads the original topic params.

Real behavior proof
Sufficient (logs): The PR body includes redacted runtime log proof for the Telegram send boundary, and a follow-up comment includes BSM smoke output for the Codex auth overlay path; the remaining fallback defect is not covered by that proof.

Next step before merge
Automerge is opted in and the remaining blocker is a narrow source-reproducible Telegram logging defect that an automated repair can address on the PR branch.

Security
Cleared: The diff adds logging/tests and a scoped Codex auth overlay fix without new dependencies, workflows, or broader secret exposure.

Review findings

  • [P2] Log the effective Telegram thread after fallback — extensions/telegram/src/send.ts:786
Review details

Best possible solution:

Keep the success log in the Telegram send boundary, but carry the effective accepted Telegram request params out of the fallback helper so the journal only reports the topic Telegram actually accepted.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection shows the PR branch can retry without message_thread_id after Telegram reports a missing thread, while the new success log still reads the original topic params.

Is this the best way to solve the issue?

No, not yet. Adding the log at extensions/telegram/src/send.ts is the right boundary, but the implementation needs to log the effective accepted params after fallback.

Full review comments:

  • [P2] Log the effective Telegram thread after fallback — extensions/telegram/src/send.ts:786
    When Telegram returns message thread not found, withTelegramThreadFallback() retries without message_thread_id and can then succeed. This success log still reports the original threadId, and the media log has the same pattern, so the journal can claim a topic delivery even though Telegram accepted a threadless root-chat send. Please log the effective accepted params and cover that fallback case.
    Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.9

Acceptance criteria:

  • node scripts/run-vitest.mjs extensions/telegram/src/send.test.ts
  • pnpm tsgo:extensions:test
  • git diff --check

What I checked:

  • Current main lacks the requested success log: Current main has Telegram thread handling but no telegram outbound send ok line in the modern sendMessageTelegram() path. (extensions/telegram/src/send.ts:393, fb53c2d61092)
  • Thread fallback strips topic params: withTelegramThreadFallback() retries message thread not found sends with removeMessageThreadIdParam(params), so a successful retry may be accepted without the original message_thread_id. (extensions/telegram/src/send.ts:605, 2425c1e8aa94)
  • Text success log uses original params: The new text success log reads threadParams.message_thread_id after the send returns instead of logging the effective params accepted after fallback. (extensions/telegram/src/send.ts:786, 2425c1e8aa94)
  • Media success log has the same mismatch: The media success log also reads threadParams.message_thread_id after sendMedia() succeeds, so media fallback can journal a topic that Telegram did not accept. (extensions/telegram/src/send.ts:1031, 2425c1e8aa94)
  • Maintainer Telegram note supports accurate topic handling: The Telegram maintainer note says wrong-surface topic success should not be hidden, which supports logging only the effective accepted send surface. (.agents/maintainer-notes/telegram.md:18)
  • Codex auth overlay change is narrow: The PR switches forced Codex plugin-harness startup to load ensureAuthProfileStore(... externalCliProviderIds: ["openai-codex"]) and keeps non-Codex plugin harnesses on the lightweight store in tests. (src/agents/pi-embedded-runner/run.ts:698, 2425c1e8aa94)

Likely related people:

  • Peter Steinberger: The available current-main blame and path history for both central files point to commit 543518b as the boundary commit that introduced the checked-out file contents. (role: recent area contributor; confidence: medium; commits: 543518bd43d7; files: extensions/telegram/src/send.ts, src/agents/pi-embedded-runner/run.ts)
  • Takhoffman: PR history shows this person requested @clawsweeper automerge, so they are a likely maintainer-side routing contact for this review result. (role: review requester; confidence: medium; files: extensions/telegram/src/send.ts, src/agents/pi-embedded-runner/run.ts)

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

@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 17, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. P2 Normal backlog priority with limited blast radius. labels May 17, 2026
@Takhoffman

Takhoffman commented May 17, 2026

Copy link
Copy Markdown
Contributor

@clawsweeper automerge

@clawsweeper clawsweeper Bot added the clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge label May 17, 2026
@clawsweeper

clawsweeper Bot commented May 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🔧
ClawSweeper picked up the repair feedback.

Source: clawsweeper[bot]
Feedback: structured ClawSweeper marker: fix-required (finding=review-feedback sha=2425c1e8aa94df5cd7c5dd444d31bbfc78d72770)
Action: repair worker queued. Run: https://github.com/openclaw/clawsweeper/actions/runs/26000421821
Model: gpt-5.5

I will update this PR branch, or open a safe credited replacement, if the repair worker finds a narrow fix.

Automerge progress:

  • 2026-05-17 19:25:20 UTC review requested repair 2425c1e8aa94 (structured ClawSweeper marker: fix-required (finding=review-feedback sha=2425c1...)

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling and removed size: S labels May 17, 2026
@jrwrest

jrwrest commented May 17, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up hotfix added after BSM deploy testing:

  • Fixed scoped-agent Codex runs so openai-codex loads the external Codex CLI OAuth overlay instead of the lightweight no-external auth store. This prevents scoped agents like thor from falling through to the generic API-key missing path.
  • Added regression coverage in src/agents/pi-embedded-runner/run.overflow-compaction.test.ts proving forced Codex runtime loads externalCliProviderIds: ["openai-codex"] and auto-selects openai-codex:default from the overlay.

Verification:

  • node scripts/test-projects.mjs src/agents/pi-embedded-runner/run.overflow-compaction.test.ts -> 27 passed
  • pnpm exec vitest run src/agents/model-auth.test.ts -> 98 passed
  • pnpm build -> passed
  • Deployed hotfix tarball to BSM host and restarted gateway.
  • BSM smoke: openclaw agent --agent thor --message "Reply with exactly: OK" --timeout 180 --json returned status: ok, provider openai-codex, model gpt-5.5, authMode: auth-profile, final text OK.
  • Post-deploy journal window has no No API key found / Missing API key entries.

@jrwrest

jrwrest commented May 17, 2026

Copy link
Copy Markdown
Contributor Author

QA follow-up added in 2425c1e8aa:

  • Added a non-Codex plugin harness regression test proving those paths stay on the lightweight/no-external auth store and skip generic API-key bootstrap.
  • Added a Telegram chunked text regression test proving no outbound success log is emitted if a later chunk fails, and the message body is not logged.
  • Fixed the test harness mock type so check:test-types passes.

Local verification after QA changes:

  • pnpm check:test-types -> passed
  • node scripts/test-projects.mjs src/agents/pi-embedded-runner/run.overflow-compaction.test.ts -> 28 passed
  • node scripts/test-projects.mjs extensions/telegram/src/send.test.ts -> 102 passed
  • git diff --check -> passed

@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 17, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 17, 2026
@clawsweeper clawsweeper Bot mentioned this pull request May 17, 2026
Merged
@clawsweeper

clawsweeper Bot commented May 17, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper 🐠 reef update

Thanks for the work on this. ClawSweeper could not push to this branch with the permissions available, so it opened a narrow replacement PR to keep the fix swimming forward without losing the contributor trail. not your fault, just GitHub branch-permission tides.

Why replacement: ClawSweeper could not update the source PR branch directly; GitHub did not grant sufficient push rights to the bot for that branch.
Replacement PR: #83247
Why close: this run explicitly closes the superseded source PR after the credited replacement PR is open, so review continues in one place.
Closing this one because the run was configured to close superseded source PRs after opening the replacement.
Contributor credit is copied into the replacement PR notes and changelog path.
Co-author credit kept:

fish notes: model gpt-5.5, reasoning high; reviewed against b860487.

@clawsweeper clawsweeper Bot closed this May 17, 2026
@clawsweeper clawsweeper Bot mentioned this pull request May 18, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

agents Agent runtime and tooling channel: telegram Channel integration: telegram clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. size: M

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Telegram durable replies should log outbound delivery success

2 participants

@jrwrest @Takhoffman