feat(messaging): telegram channels-status health probe#6887
Conversation
Signed-off-by: Hung Le <hple@nvidia.com>
Signed-off-by: Hung Le <hple@nvidia.com>
…channels-status-health
Signed-off-by: Hung Le <hple@nvidia.com>
…channels-status-health
…state Signed-off-by: Hung Le <hple@nvidia.com>
…channels-status-health
Signed-off-by: Hung Le <hple@nvidia.com>
…channels-status-health
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
📝 WalkthroughWalkthroughTelegram diagnostics now use gateway log breadcrumbs and bounded sandbox probes to produce structured runtime verdicts for OpenClaw. Summary views avoid live probing, Hermes falls back to configuration reporting, and detailed status supports Telegram-specific health results. ChangesTelegram health diagnostics
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant ChannelStatus
participant Sandbox
participant TelegramDiagnostics
Operator->>ChannelStatus: Request detailed Telegram status
ChannelStatus->>Sandbox: Execute bounded gateway log/process probe
Sandbox-->>ChannelStatus: Return sentinel results and matched signals
ChannelStatus->>TelegramDiagnostics: Evaluate probe input
TelegramDiagnostics-->>ChannelStatus: Return verdict and diagnostic signals
ChannelStatus-->>Operator: Render Telegram health report
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the branch. TypeScript / code-coverage/cliThe overall coverage in the branch is 80%. The coverage in the branch is 79%. Show a code coverage summary of the most impacted files.
Updated |
|
🌿 Preview your docs: https://nvidia-preview-pr-6887.docs.buildwithfern.com/nemoclaw |
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 1 optional E2E recommendation
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Hung Le <hple@nvidia.com>
…t-side Signed-off-by: Hung Le <hple@nvidia.com>
…channels-status-health
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/lib/actions/sandbox/telegram-probe.test.ts (1)
33-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a behavior-oriented suite title.
buildTelegramProbeInputis an implementation name rather than test behavior.Suggested change
-describe("buildTelegramProbeInput", () => { +describe("when collecting Telegram runtime probe evidence", () => {As per coding guidelines, “Use behavior-oriented test titles.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/actions/sandbox/telegram-probe.test.ts` at line 33, Rename the test suite described by the describe block around buildTelegramProbeInput to a behavior-oriented title that states what the Telegram probe input-building behavior does, rather than naming the implementation function.Source: Coding guidelines
src/lib/sandbox/telegram-diagnostics.ts (1)
260-267: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
pickVerdictre-derives config/policy state via signal-label string matching instead of readinginputdirectly.
input.channelEnabledInRegistryandinput.presetInRegistry/presetOnGatewayare already booleans onTelegramProbeInput; matching againstsignalsbylabel === "Channel registration"couplespickVerdictto the exact label strings used inconfigCoverageSignal/policyCoverageSignal, so a future label rename silently breaks verdict classification with no type error.♻️ Suggested simplification
function pickVerdict(signals: DiagnosticSignal[], input: TelegramProbeInput): TelegramVerdict { if (!input.probeReachable) return "probe_failed"; - if (signals.some((s) => s.label === "Channel registration" && s.severity === "fail")) { - return "config_gap"; - } - if (signals.some((s) => s.label === "Policy coverage" && s.severity === "fail")) { - return "policy_gap"; - } + if (!input.channelEnabledInRegistry) return "config_gap"; + if (!input.presetInRegistry) return "policy_gap"; ...🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/sandbox/telegram-diagnostics.ts` around lines 260 - 267, Update pickVerdict to classify config_gap and policy_gap from TelegramProbeInput’s boolean fields directly: use channelEnabledInRegistry for configuration status and the presetInRegistry/presetOnGateway values for policy coverage. Remove the signal label/severity matching from verdict selection while preserving the existing probe_failed precedence and remaining verdict behavior.src/lib/actions/sandbox/telegram-probe.ts (1)
94-100: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the redundant
try/catcharounddeps.getGatewayPresets()
getGatewayPresets()already returnsnullfor gateway failures, so this wrapper only adds another path to reason about. Handle thenullcase directly unless an injected dependency is expected to throw.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/actions/sandbox/telegram-probe.ts` around lines 94 - 100, Remove the try/catch surrounding deps.getGatewayPresets in the gateway preset lookup, and assign presetOnGateway directly from its null-aware result. Preserve null when getGatewayPresets returns null and check for "telegram" only for non-null preset lists.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/actions/sandbox/channel-status.ts`:
- Around line 542-551: Thread a paused-channel indicator through
buildBasicChannelReport and its paused-channel call site so the Runtime health
signal reflects the actual request context. For paused single-channel detail
requests, avoid saying “not checked in summary view” and do not suggest
rerunning the same --channel command; preserve the existing summary-view message
and hint for non-paused summary reports.
In `@src/lib/sandbox/telegram-diagnostics.ts`:
- Around line 206-213: Update the parser’s startupHttpError handling alongside
providerReady and tokenRejected so it participates in the same latest-evidence
winner block rather than being assigned unconditionally. Ensure later
provider-ready/inbound evidence clears or supersedes an earlier HTTP error,
allowing reachabilitySignal to report the current healthy state; add a
regression test covering stale HTTP 502/503 evidence followed by provider-ready
evidence.
---
Nitpick comments:
In `@src/lib/actions/sandbox/telegram-probe.test.ts`:
- Line 33: Rename the test suite described by the describe block around
buildTelegramProbeInput to a behavior-oriented title that states what the
Telegram probe input-building behavior does, rather than naming the
implementation function.
In `@src/lib/actions/sandbox/telegram-probe.ts`:
- Around line 94-100: Remove the try/catch surrounding deps.getGatewayPresets in
the gateway preset lookup, and assign presetOnGateway directly from its
null-aware result. Preserve null when getGatewayPresets returns null and check
for "telegram" only for non-null preset lists.
In `@src/lib/sandbox/telegram-diagnostics.ts`:
- Around line 260-267: Update pickVerdict to classify config_gap and policy_gap
from TelegramProbeInput’s boolean fields directly: use channelEnabledInRegistry
for configuration status and the presetInRegistry/presetOnGateway values for
policy coverage. Remove the signal label/severity matching from verdict
selection while preserving the existing probe_failed precedence and remaining
verdict behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c1d86ef7-c4b4-4513-91de-a964bdf15a60
📒 Files selected for processing (16)
docs/reference/commands.mdxsrc/lib/actions/sandbox/channel-status-config-core.test.tssrc/lib/actions/sandbox/channel-status-summary.test.tssrc/lib/actions/sandbox/channel-status-telegram-policy.test.tssrc/lib/actions/sandbox/channel-status.test-helpers.tssrc/lib/actions/sandbox/channel-status.tssrc/lib/actions/sandbox/telegram-probe.test.tssrc/lib/actions/sandbox/telegram-probe.tssrc/lib/messaging/channels/telegram/manifest.tssrc/lib/messaging/diagnostics.test.tssrc/lib/messaging/diagnostics.tssrc/lib/messaging/manifest/types.tssrc/lib/sandbox/diagnostic-signal.tssrc/lib/sandbox/telegram-diagnostics.test.tssrc/lib/sandbox/telegram-diagnostics.tssrc/lib/sandbox/whatsapp-diagnostics.ts
| if (bc.startupHttpError !== null) { | ||
| return { | ||
| label: "Bot API reachability", | ||
| severity: "warn", | ||
| detail: `Telegram startup probe returned HTTP ${bc.startupHttpError}`, | ||
| hint: "check `nemoclaw <sandbox> logs --follow`", | ||
| }; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Stale startupHttpError breaks the documented "latest evidence wins" guarantee.
The parser's own comment (lines 360-369) promises the most recent breadcrumb evidence wins, but startupHttpError (line 381) is set unconditionally on every match rather than through the winner-take-all block (391-415) that governs providerReady/tokenRejected/etc. Result: a stale HTTP-error line followed by a later provider ready + inbound line yields verdict "healthy" (correct, since pickVerdict never reads startupHttpError), but reachabilitySignal (206) still reports a warn signal with the stale HTTP code because it checks startupHttpError !== null before providerReady. The report's own verdict and signal list then contradict each other — exactly the stale/misleading-diagnostic failure mode this PR is meant to eliminate.
🐛 Proposed fix: track startupHttpError with the same latest-evidence semantics
let lastReached = -1;
let lastNetworkFail = -1;
let lastBridgeNotStarted = -1;
let lastTokenRejected = -1;
let lastCredentialUnresolved = -1;
+ let lastHttpError = -1;
+ let lastHttpErrorCode: number | null = null;
telegramLines.forEach((line, index) => {
const httpErr = /startup probe returned HTTP\s+(\d{3})/i.exec(line);
- if (httpErr) bc.startupHttpError = Number(httpErr[1]);
+ if (httpErr) {
+ lastHttpError = index;
+ lastHttpErrorCode = Number(httpErr[1]);
+ }
if (/inbound update received|inbound message telegram/i.test(line)) bc.inboundReceived = true;
if (REACHED.test(line)) lastReached = index;
if (NETWORK_FAIL.test(line)) lastNetworkFail = index;
if (/bridge did not start within/i.test(line)) lastBridgeNotStarted = index;
if (/rejected startup probe with HTTP\s+(401|404)/i.test(line)) lastTokenRejected = index;
if (/credential placeholder.*(missing|mismatch|unresolved)/i.test(line)) {
lastCredentialUnresolved = index;
}
});
const lastCause = Math.max(lastTokenRejected, lastCredentialUnresolved, lastNetworkFail);
- const lastEvidence = Math.max(lastReached, lastCause, lastBridgeNotStarted);
+ const lastEvidence = Math.max(lastReached, lastCause, lastBridgeNotStarted, lastHttpError);
if (lastReached !== -1 && lastReached >= lastEvidence) {
bc.providerReady = true;
} else if (...) {
...
} else if (lastNetworkFail !== -1) {
bc.startupFailedNetwork = true;
} else if (lastBridgeNotStarted !== -1) {
bc.bridgeNotStarted = true;
+ } else if (lastHttpError !== -1) {
+ bc.startupHttpError = lastHttpErrorCode;
}Worth adding a regression test mirroring the existing #6887 stale-401 tests but for a stale HTTP 502/503 followed by a later provider ready.
Also applies to: 379-415
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/sandbox/telegram-diagnostics.ts` around lines 206 - 213, Update the
parser’s startupHttpError handling alongside providerReady and tokenRejected so
it participates in the same latest-evidence winner block rather than being
assigned unconditionally. Ensure later provider-ready/inbound evidence clears or
supersedes an earlier HTTP error, allowing reachabilitySignal to report the
current healthy state; add a regression test covering stale HTTP 502/503
evidence followed by provider-ready evidence.
Signed-off-by: Hung Le <hple@nvidia.com>
Signed-off-by: Hung Le <hple@nvidia.com>
…channels-status-health
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/sandbox/telegram-diagnostics.ts (1)
403-424: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHonor a later HTTP failure over earlier failure breadcrumbs.
lastHttpErroris included inlastEvidence, but the token/network branches never compare their timestamps to it. A401at index 0 followed by an HTTP502at index 1 still setstokenRejected, hiding the latest non-auth reachability failure.Proposed fix
if (lastReached !== -1 && lastReached >= lastEvidence) { bc.providerReady = true; + } else if (lastHttpError === lastEvidence) { + bc.startupHttpError = lastHttpErrorCode; } else if ( Math.max(lastTokenRejected, lastCredentialUnresolved) !== -1 && Math.max(lastTokenRejected, lastCredentialUnresolved) >= lastNetworkFail @@ } else if (lastBridgeNotStarted !== -1) { bc.bridgeNotStarted = true; - } else if (lastHttpError !== -1) { - bc.startupHttpError = lastHttpErrorCode; }Add a regression case for rejected-token evidence followed by HTTP 502.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/sandbox/telegram-diagnostics.ts` around lines 403 - 424, Update the verdict selection around lastEvidence so a later non-auth HTTP failure takes precedence over earlier token, credential, or network failure breadcrumbs. Ensure the token/network branches only set their flags when their evidence is at least as recent as lastHttpError, while preserving reached and bridge precedence; add a regression case covering rejected-token evidence followed by HTTP 502.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/lib/sandbox/telegram-diagnostics.ts`:
- Around line 403-424: Update the verdict selection around lastEvidence so a
later non-auth HTTP failure takes precedence over earlier token, credential, or
network failure breadcrumbs. Ensure the token/network branches only set their
flags when their evidence is at least as recent as lastHttpError, while
preserving reached and bridge precedence; add a regression case covering
rejected-token evidence followed by HTTP 502.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4a9f685e-f681-468d-a959-404146f4aa3f
📒 Files selected for processing (4)
src/lib/actions/sandbox/channel-status.test.tssrc/lib/actions/sandbox/channel-status.tssrc/lib/sandbox/telegram-diagnostics.test.tssrc/lib/sandbox/telegram-diagnostics.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/sandbox/telegram-diagnostics.test.ts
- src/lib/actions/sandbox/channel-status.ts
|
add new hooks type -> channel status health check -> call getMeReachability. |
Summary
Adds a live health probe for the Telegram messaging channel to
nemoclaw <sandbox> channels status --channel telegram, mirroring the existing WhatsApp probe: it classifies the channel into a verdict (healthy/idle/unreachable/token_rejected/not_started) by reading the OpenClaw gateway's own log breadcrumbs, so a Telegram bridge that cannot reach Telegram or has a rejected token no longer renders as all-[ok]. The defaultchannels statussummary now prints an honestRuntime health: not checkedpointer for probe-capable channels instead of a silent all-green, and--channel telegrammerges the existing non-secret config comparison with the health signals.Related Issue
Closes #6888
Addresses the "Telegram configuration state and health diagnostics are misleading" item of the DGX Spark/Station VDR checklist #6743 (Parts of #6743).
Changes
src/lib/sandbox/telegram-diagnostics.ts(verdict + hints, fixture-testable, no I/O) and host-side probesrc/lib/actions/sandbox/telegram-probe.ts(tails/tmp/gateway.log+pgrep); the probe never issues its own getMe, so the resolved bot token stays inside the gateway.deepProbemarker from"in-sandbox-qr"to"in-sandbox-qr" | "log-tail", derived from a new declarative manifest fieldChannelManifest.diagnosticsProbe. Current consumer: Telegram only (declaresdiagnosticsProbe: "log-tail"); a hardcoded channel check is insufficient because Slack/Discord/WeChat also ship bridge-health hooks and must not be mis-routed to the Telegram evaluator. Dispatch uses aLOG_TAIL_EVALUATORSchannel→evaluator map, gated to OpenClaw (the only agent with the breadcrumb producer). Protected bymessaging/diagnostics.test.tsand thechannel-statusdispatch/Hermes-fallback tests.DiagnosticSeverity/DiagnosticSignalintosrc/lib/sandbox/diagnostic-signal.tsso the WhatsApp and Telegram report union uses one type (WhatsApp re-exports for existing importers).--channel telegrammerges the existing config-value comparison (group policy / mention mode / allowed IDs) with the health signals and exits non-zero when the channel is unhealthy (was previously always exit 0).healthy; one blocked again reportsunreachable). Recognizes OpenClaw's ownInbound message telegram:/isolated polling ingress startedandNetwork request … failed/UND_ERR_SOCKETlines, since the preload does not reliably emit its positive breadcrumbs.docs/reference/commands.mdxchannels statussection documents the Telegram probe, verdicts, exit code, and summary pointer.Type of Change
Quality Gates
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Hung Le hple@nvidia.com
Summary by CodeRabbit
--jsonexit behavior when verdicts are not healthy/unknown.