Measure Telegram QA reply RTT#70550
Conversation
Greptile SummaryThis PR adds per-scenario Telegram QA reply RTT (driver send → observed SUT reply) to Markdown reports and JSON summaries, and caches supergroup forum-flag lookups in a module-level Confidence Score: 5/5Safe to merge; all remaining findings are minor P2 suggestions that do not affect correctness. No P0/P1 issues found. The two P2 comments cover an unbounded cache (rare operational edge case) and a minor timing granularity point in the RTT measurement. Both are non-blocking quality suggestions. No files require special attention beyond the P2 notes on helpers.ts and telegram-live.runtime.ts. Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/telegram/src/bot/helpers.ts
Line: 43
Comment:
**Unbounded module-level cache with no expiry**
`telegramForumFlagByChatId` accumulates entries for the lifetime of the process and never evicts stale data. If a supergroup's forum status changes (upgraded or downgraded), the cached boolean will silently remain wrong until the process restarts. A `WeakRef`-based expiry or a simple TTL wrapper would make the cache resilient to that operational change — or at minimum, a comment noting the intent to cache for the process lifetime would help maintainers understand the scope.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: extensions/qa-lab/src/live-transports/telegram/telegram-live.runtime.ts
Line: 616
Comment:
**`observedAtMs` captures loop-iteration time, not message-receipt time**
`Date.now()` is called after iterating over the `updates` batch and finding the matching message. If the batch contains several messages before the matching one, the timestamp reflects when the loop body reached that entry, not when the HTTP response arrived. For more accurate RTT, capture `Date.now()` once immediately after `callTelegramApi` returns (before the inner loop) and use that as the observation timestamp for all messages in that batch.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "perf(telegram): cache forum metadata loo..." | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6750a17620
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
da7c550 to
ea267b9
Compare
🔒 Aisle Security AnalysisWe found 1 potential security issue(s) in this PR:
1. 🟡 OPENCLAW_QA_REDACT_PUBLIC_METADATA does not redact per-scenario timing and timestamps in Telegram QA artifacts
DescriptionThe Telegram QA live runtime adds per-scenario timing fields ( Even when
This can cause information disclosure by leaking precise operational timestamps and per-reply latency that can be used to correlate runs, infer internal performance/availability characteristics, and potentially correlate chat activity patterns even when other metadata is redacted. Vulnerable code (example for canary scenario): scenarioResults.push({
...
details: redactPublicMetadata
? `reply matched in ${canaryTiming.rttMs}ms`
: `reply message ${canaryTiming.responseMessageId} matched in ${canaryTiming.rttMs}ms`,
rttMs: canaryTiming.rttMs,
requestStartedAt: canaryTiming.requestStartedAt,
responseObservedAt: canaryTiming.responseObservedAt,
sentMessageId: redactPublicMetadata ? undefined : canaryTiming.sentMessageId,
responseMessageId: redactPublicMetadata ? undefined : canaryTiming.responseMessageId,
});RecommendationWhen Options:
Example fix (full redaction): const timingFields = redactPublicMetadata
? {}
: {
rttMs: canaryTiming.rttMs,
requestStartedAt: canaryTiming.requestStartedAt,
responseObservedAt: canaryTiming.responseObservedAt,
};
scenarioResults.push({
id: "telegram-canary",
title: "Telegram canary",
status: "pass",
details: redactPublicMetadata ? "reply matched" : `reply message ${canaryTiming.responseMessageId} matched`,
...timingFields,
sentMessageId: redactPublicMetadata ? undefined : canaryTiming.sentMessageId,
responseMessageId: redactPublicMetadata ? undefined : canaryTiming.responseMessageId,
});Also ensure Analyzed PR: #70550 at commit Last updated on: 2026-04-23T09:11:11Z |
Summary
Validation