feat(cli): add telegram channel adapter, scoped bridge config, and diagnostics#110
Closed
cdenneen wants to merge 28 commits intohappier-dev:devfrom
Closed
feat(cli): add telegram channel adapter, scoped bridge config, and diagnostics#110cdenneen wants to merge 28 commits intohappier-dev:devfrom
cdenneen wants to merge 28 commits intohappier-dev:devfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a Channel Bridge subsystem: provider-agnostic bridge worker, Telegram adapter (polling/webhook), server-side KV schema and optimistic persistence, multi-level config resolution and CLI, daemon integration, doctor validations, extensive tests, and documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant Telegram as Telegram (API/Webhook)
participant Adapter as Telegram Adapter
participant Worker as Bridge Worker
participant Store as Binding Store
participant Session as Happier Session
participant Agent as AI Agent
rect rgba(100, 150, 200, 0.5)
note over Telegram,Agent: Inbound → Forwarding
Telegram->>Adapter: Update (chat/topic)
Adapter->>Adapter: Parse & filter (allowedChatIds, topics)
Adapter->>Worker: pullInboundMessages()
Worker->>Store: list/getBinding(provider,conversation,thread)
Store-->>Worker: ChannelSessionBinding
Worker->>Session: sendUserMessageToSession(sessionId, content)
Session->>Agent: process user input
Agent-->>Session: agent reply
end
rect rgba(150, 200, 100, 0.5)
note over Worker,Adapter: Agent → Outbound
Worker->>Session: fetchAgentMessagesAfterSeq(sessionId, seq)
Session-->>Worker: agent messages
Worker->>Store: updateLastForwardedSeq(bindingKey, newSeq)
Worker->>Adapter: sendMessage(conversation, thread, text)
Adapter->>Telegram: API sendMessage
Telegram-->>Adapter: send response
end
sequenceDiagram
participant CLI as CLI / Env
participant Resolver as Config Resolver
participant Settings as Local Settings
participant KV as Server KV
participant Runtime as Runtime Config
rect rgba(200, 150, 100, 0.5)
note over CLI,Runtime: Config resolution & overlay
Resolver->>CLI: read env vars (HAPPIER_TELEGRAM_*)
Resolver->>Settings: read scoped settings.json
Resolver->>KV: fetch server KV record (if server/account)
KV-->>Resolver: server record (non-secret)
Resolver->>Resolver: merge overlay (server → settings, secrets from local)
Resolver->>Runtime: produce final runtime config (env > account > server > global)
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
cdenneen
changed the title
Contributor
Greptile SummaryThis PR adds Telegram channel bridge support on top of the provider-agnostic bridge core, including polling/webhook adapter, account-scoped CLI config management, server KV persistence, doctor diagnostics, and comprehensive test coverage. The implementation is well-structured and a large number of issues raised in prior review rounds have been thoroughly addressed. Two issues from the previous review thread remain open with no developer acknowledgement:
Confidence Score: 3/5
|
| Filename | Overview |
|---|---|
| apps/cli/src/channels/telegram/telegramAdapter.ts | Telegram adapter implementation with polling and webhook modes. All previously flagged issues (long-polling, queue drain race, updateOffset docs) have been addressed. One remaining issue: senderId is computed in parseInboundFromUpdate for bot self-filtering but is not included in the returned ChannelBridgeInboundMessage, causing actor.senderId to always be null in the worker. |
| apps/cli/src/channels/channelBindingStore.server.ts | Server-backed KV binding store with optimistic write and LRU cache. Previously flagged issues (spurious writes, conflict decode, primary load hardening, retryCurrent optimization) have been addressed. One remaining issue: when readChannelBridgeBindingsFromKv throws a recoverable decode error, the actual server version from the HTTP response is lost, causing all subsequent writes to fail permanently with wrong-version 409s that cannot be recovered. |
| apps/cli/src/channels/startChannelBridgeWorker.ts | Bridge worker startup with Telegram adapter wiring, webhook relay, LRU session cache, and paginated session listing. All previously reviewed issues (webhook fallback, unbounded cache, positional adapter replacement, session fetch caching) are addressed cleanly. |
| apps/cli/src/ui/doctor.ts | Doctor diagnostics with channel bridge health checks, secret masking, and critical failure aggregation. All previously reviewed issues (duplicate reads, false-positive critical failures, runtime defaults for validation, dead code) are addressed. Logic is consistent and correct. |
| apps/cli/src/cli/commands/bridge.ts | Bridge CLI commands (list, telegram set, telegram clear). All previously reviewed issues (KV-before-local ordering, allowUnsupportedSchema, webhook-secret character validation) are addressed. Command flow is clean and correct. |
| apps/cli/src/channels/channelBridgeServerKv.ts | KV transport layer for Telegram config and bindings. All previously reviewed issues (schema blocking, conflict-hint optimization, clear retry without re-read) are addressed. Encoding, parsing, optimistic write, and error types are correct. |
| apps/cli/src/channels/telegram/telegramWebhookRelay.ts | Fastify webhook relay with constant-time header token validation, path-token character-set enforcement, and relay-stop error isolation. All previously flagged issues are addressed. Implementation is clean. |
| apps/cli/src/daemon/startDaemon.ts | Daemon startup with bounded channel bridge stop timeout and allowUnsupportedSchema: true in KV reads. Previously flagged issues are resolved. Integration is correct. |
| apps/cli/src/channels/channelBridgeAccountConfig.ts | Account-scoped bridge config helpers with secret/shared field splitting. Previously flagged dead-code and silent-secret-loss issues are resolved. Logic for splitting updates and upserting scoped settings is correct. |
| apps/cli/src/channels/channelBridgeConfig.ts | Runtime config resolution applying env > server KV > local settings > defaults. Default values for webhookHost (127.0.0.1) and webhookPort (8787) are always applied, preventing null returns. Clean and correct. |
Sequence Diagram
sequenceDiagram
participant TG as Telegram API
participant Relay as TelegramWebhookRelay (Fastify)
participant Adapter as TelegramAdapter
participant Worker as ChannelBridgeWorker
participant KV as Server KV
participant Session as Happier Session
Note over TG,Session: Polling mode (default)
Worker->>Adapter: pullInboundMessages()
Adapter->>TG: POST getUpdates(offset, timeout=25s)
TG-->>Adapter: updates[]
Adapter-->>Worker: ChannelBridgeInboundMessage[]
Note over TG,Session: Webhook mode (optional)
TG->>Relay: POST /telegram/webhook/{secretPathToken}
Relay->>Relay: validate X-Telegram-Bot-Api-Secret-Token (timingSafeEqual)
Relay->>Adapter: enqueueWebhookUpdate(update)
Worker->>Adapter: pullInboundMessages()
Adapter-->>Worker: ChannelBridgeInboundMessage[]
Note over Worker,Session: Inbound message routing
Worker->>KV: getBinding(conversationRef)
KV-->>Worker: ChannelSessionBinding (sessionId, lastForwardedSeq)
Worker->>Session: sendUserMessageToSession(sessionId, text)
Note over Worker,Session: Outbound forwarding
Worker->>Session: fetchAgentMessagesAfterSeq(sessionId, afterSeq)
Session-->>Worker: AgentMessage[]
Worker->>KV: updateLastForwardedSeq(ref, seq)
Worker->>Adapter: sendMessage(conversationId, threadId, text)
Adapter->>TG: POST sendMessage(chat_id, text)
Last reviewed commit: bfd28ab
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Nitpick comments (6)
apps/cli/src/ui/doctor.test.ts (1)
97-109: Avoid exact copy-pinning for webhook issue assertions.This test currently hard-codes full user-facing messages. Prefer asserting stable substrings/field keys (e.g.,
webhook.secret,required when webhook.enabled=true) to reduce brittle failures on wording changes.Based on learnings, "Avoid brittle 'content policing' tests that pin ... exact ... user-facing copy ... Prefer stable identifiers ... and key substrings."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/ui/doctor.test.ts` around lines 97 - 109, The test in apps/cli/src/ui/doctor.test.ts pins full user-facing messages from collectMissingRequiredWebhookFields; change it to assert stable identifiers/substrings instead of exact copy-pinned strings — for example, replace the toEqual assertion with assertions that the returned array includes entries containing 'webhook.secret', 'webhook.host', 'webhook.port' and the phrase 'required when webhook.enabled=true' (use expect.arrayContaining combined with expect.stringContaining or map the result to keys and assert those keys exist) so wording changes won't break the test.apps/cli/src/channels/telegram/telegramWebhookRelay.test.ts (1)
6-35: Add negative-path coverage for startup validation.Current tests only validate the happy path. Please add cases for invalid secret path token and out-of-range port so relay startup validation is locked in.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/channels/telegram/telegramWebhookRelay.test.ts` around lines 6 - 35, Add two new tests in telegramWebhookRelay.test.ts that assert startup validation errors from startTelegramWebhookRelay: one that calls startTelegramWebhookRelay with an invalid secretPathToken (e.g., an empty string or a token containing a slash like '/bad-token') and expects the promise to reject/throw with a validation error, and another that calls startTelegramWebhookRelay with an out-of-range port (e.g., -1 or 70000) and expects rejection/throw; use async/await with expect(...).rejects or try/catch to assert the error and ensure you only call relay.stop() if the relay was successfully created. Reference startTelegramWebhookRelay, relay.port and relay.path when structuring the tests.apps/cli/src/cli/commands/bridge.test.ts (1)
46-70: Consolidate duplicatebeforeEachhooks into a single hook.The static analysis correctly identifies two separate
beforeEachhooks. While both will execute before each test, consolidating them improves readability and maintainability.♻️ Proposed consolidation
- beforeEach(() => { + beforeEach(async () => { vi.resetModules(); vi.clearAllMocks(); readCredentialsMock.mockResolvedValue({ token: 'token.jwt' }); decodeJwtPayloadMock.mockReturnValue({ sub: 'acct_123' }); readSettingsMock.mockResolvedValue({}); checkDaemonMock.mockResolvedValue(false); updateSettingsMock.mockImplementation(async (updater: (current: unknown) => Promise<unknown> | unknown) => { await updater({}); }); createKvClientMock.mockReturnValue({ get: vi.fn(), mutate: vi.fn() }); upsertKvConfigMock.mockResolvedValue(undefined); clearKvConfigMock.mockResolvedValue(undefined); - }); - beforeEach(async () => { homeDir = await mkdtemp(join(tmpdir(), 'happier-bridge-cmd-')); process.env.HAPPIER_HOME_DIR = homeDir; process.env.HAPPIER_SERVER_URL = 'http://127.0.0.1:3005'; process.env.HAPPIER_WEBAPP_URL = 'http://127.0.0.1:3006'; reloadConfiguration(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/cli/commands/bridge.test.ts` around lines 46 - 70, There are two beforeEach hooks; consolidate them into a single beforeEach that runs all setup steps: keep the mock resets and mockResolvedValue/mockReturnValue/setup implementations (readCredentialsMock, decodeJwtPayloadMock, readSettingsMock, checkDaemonMock, updateSettingsMock, createKvClientMock, upsertKvConfigMock, clearKvConfigMock) and also the async temp-dir setup (mkdtemp -> homeDir), environment variable assignments (process.env.HAPPIER_HOME_DIR, HAPPIER_SERVER_URL, HAPPIER_WEBAPP_URL) and reloadConfiguration() in the same hook so both synchronous mocks and async filesystem/env setup run together before each test; update any async handling so the combined beforeEach is declared async and awaits mkdtemp and any mock async initializers.apps/cli/src/channels/core/channelBridgeWorker.ts (1)
1-2: Add a file-level JSDoc header for worker lifecycle/ownership boundariesGiven this file contains core runtime orchestration, a top-level contract note will make maintenance and future adapters safer.
As per coding guidelines, "Include comprehensive JSDoc comments as file header comments explaining file responsibilities".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/channels/core/channelBridgeWorker.ts` around lines 1 - 2, Add a file-level JSDoc header to channelBridgeWorker.ts above the imports that documents the file’s responsibility (core runtime orchestration for channel bridge workers), lifecycle boundaries (when and how startSingleFlightIntervalLoop is started/stopped), public API/handles (SingleFlightIntervalLoopHandle and any exported functions), ownership/maintainers, and key invariants or adapter constraints callers must respect; keep it concise but specific so future maintainers know contract expectations and to update the header when behavior or ownership changes.apps/cli/src/channels/channelBridgeAccountConfig.ts (1)
1-1: Add a file-level JSDoc header describing scoped config responsibilitiesThis file is a central config mutator/normalizer and would benefit from a concise top-level contract note.
As per coding guidelines, "Include comprehensive JSDoc comments as file header comments explaining file responsibilities".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/channels/channelBridgeAccountConfig.ts` at line 1, Add a concise file-level JSDoc header at the top of this module describing that this file is the scoped config mutator/normalizer for channel bridge account settings: state its responsibilities (normalizing, validating, and merging scoped account config), the expected input shape (e.g., RecordLike), key exported utilities or functions provided by the module (named exports you see in this file), any side-effects (mutations, I/O, or global config changes), and the contract for callers (what guarantees normalized config will have). Keep it brief (2–4 sentences) and place it above the existing declarations so future readers understand the module's purpose and usage.apps/cli/src/channels/channelBridgeServerKv.ts (1)
1-6: Add a file-level JSDoc header for this KV moduleThis module defines cross-cutting persistence contracts and should have a top-level responsibility header for maintainability.
As per coding guidelines, "Include comprehensive JSDoc comments as file header comments explaining file responsibilities".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/channels/channelBridgeServerKv.ts` around lines 1 - 6, Add a top-level JSDoc header to this KV module describing its responsibility as the cross-cutting persistence contract for channel bridge state, list the main exports/types it provides (e.g., ScopedTelegramBridgeUpdate or other KV interfaces), describe intended usage patterns (what data is stored, retention/consistency expectations, and who should call these APIs), and note any invariants or side-effects (e.g., synchronous vs async, error handling expectations). Place the comment at the very top of channelBridgeServerKv.ts so future readers immediately see the module purpose, public surface, and operational guidance.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/cli/src/channels/channelBindingStore.server.ts`:
- Around line 101-130: The helper withOptimisticWrite currently always attempts
a KV write even when the operation produced no changes; modify it to compare
op.nextBindings with the loaded current.bindings (use deep equality or a
deterministic canonical comparison of ChannelSessionBinding arrays) and, if
identical, skip writeChannelBridgeBindingsToKv and simply return op.result (also
avoid updating version/cache), so no-op calls from updateLastForwardedSeq and
removeBinding won't bump the KV version; apply the same no-op-skip logic to the
other similar usage at the 169-195 region.
In `@apps/cli/src/channels/channelBridgeAccountConfig.ts`:
- Around line 127-192: readScopedTelegramBridgeConfig currently omits tickMs
from the returned normalized config causing persistence/round-trip bugs; update
the function to read tickMs from the account scope (e.g., accountScope?.tickMs
or telegram.tickMs depending on where it's stored) and, if it's a finite number,
set normalized.tickMs (use Math.trunc or Number.isFinite to normalize to an
integer) before returning normalized so the persisted tickMs is preserved by the
scoped read API.
- Around line 215-217: Assigning params.update.allowedChatIds to
telegram.allowedChatIds keeps a reference to caller-owned state; instead set
telegram.allowedChatIds to a shallow copy (e.g., Array.from(...) or spread copy)
when Array.isArray(params.update.allowedChatIds) is true so external mutations
won't affect the stored config; update the assignment in
channelBridgeAccountConfig.ts where telegram.allowedChatIds is set from
params.update.allowedChatIds.
In `@apps/cli/src/channels/channelBridgeConfig.test.ts`:
- Line 79: The test "applies env overrides over settings values" mixes valid env
overrides with an intentionally invalid port string '8_877'
(HAPPIER_TELEGRAM_WEBHOOK_PORT), which parseStrictInteger rejects so webhookPort
falls back to settings; either replace '8_877' with a valid numeric string like
'8877' to assert an actual environment override of webhookPort, or split/rename
the test into two: one that verifies env overrides (use '8877') and a separate
one that verifies fallback behavior when parseStrictInteger returns null (use
'8_877' and assert settings value is used). Ensure references to
HAPPIER_TELEGRAM_WEBHOOK_PORT, parseStrictInteger, and webhookPort are updated
accordingly.
In `@apps/cli/src/channels/channelBridgeServerKv.ts`:
- Around line 341-351: The catch block currently returns record: null but
preserves row.version, which lets later upserts merge from an empty baseline;
instead, when decodeBase64ToJson/parseTelegramConfigRecord fails, do not keep
the existing version—either rethrow the error or return a sentinel that prevents
merge (e.g. return { record: null, version: undefined }); update the catch in
the parse block around
parseTelegramConfigRecord(decodeBase64ToJson(row.valueBase64)) and apply the
same change to the similar catch at lines 426-460 so invalid payloads cannot be
used as a valid baseline for upsert/merge.
- Around line 279-289: The function decodeChannelBridgeBindingsDocFromBase64
currently swallows decode/parsing failures and returns an empty {
schemaVersion:1, bindings:[] }, which can hide corrupted payloads; change it to
fail fast: when decodeBase64ToJson throws or parseBindingsDocument returns
null/undefined, throw a descriptive Error (include the caught error message when
available and mention the offending base64 value or its truncated prefix) so
callers can detect and handle invalid payloads instead of silently treating it
as empty; update references to decodeChannelBridgeBindingsDocFromBase64 and
callers if they expect the previous empty fallback to handle errors.
In `@apps/cli/src/channels/core/channelBridgeWorker.ts`:
- Around line 67-69: The bindingKey function is collision-prone because it
conflates null threadId with the literal '-' and can be broken by IDs containing
the '::' delimiter; update bindingKey (and any consumers expecting its format)
to produce an unambiguous key—for example serialize the
ChannelBridgeConversationRef into a compact, reversible form such as
JSON.stringify or a deterministic base64 encoding of the concatenated parts,
ensure threadId null is represented explicitly (e.g. null vs empty string) and
avoid raw delimiter concatenation so providerId, conversationId, and threadId
cannot collide. Use the function name bindingKey and the type
ChannelBridgeConversationRef to locate and change the implementation and any
corresponding parsing logic.
- Around line 242-290: The current loop over params.adapters calls
adapter.pullInboundMessages() and processes each event without isolating
failures, so any thrown error aborts the entire tick; wrap the call to
pullInboundMessages() in a try/catch (log via params.deps.onWarning) and
continue to the next adapter on error, and also wrap the per-event processing
(including parseSlashCommand/handleCommand, params.store.getBinding,
params.deps.sendUserMessageToSession, and replyToConversation calls) in its own
try/catch so a failure for one event logs a warning and continues processing
remaining events and adapters; ensure you still await operations but never let
an exception bubble out of the adapter or event loops.
In `@apps/cli/src/channels/startChannelBridgeWorker.ts`:
- Around line 213-215: The debug log in startChannelBridgeWorker.ts currently
prints the full webhook URL including relayHandle.path (which contains the
secretPathToken); remove the secret from logs by logging only non-sensitive
parts (e.g., host and relayHandle.port) or by masking the token before logging.
Update the logger.debug call that references relayHandle.path so it omits or
redacts secretPathToken (or replace relayHandle.path with a maskedPath) while
keeping the rest of the message intact.
- Around line 254-261: The current stop() implementation may skip
relayHandle.stop() if await worker.stop() throws; change stop() (the exported
object's stop method that returns trigger and stop) to always attempt relay
cleanup by invoking relayHandle.stop() in a finally block (or equivalent
try/catch/finally) after calling worker.stop() (or if worker.stop() fails),
ensure relayHandle.stop() is awaited, and if both worker.stop() and
relayHandle.stop() error, surface the primary error or aggregate both so
resources aren't leaked while still propogating the failure.
In `@apps/cli/src/channels/telegram/telegramAdapter.ts`:
- Line 165: queuedWebhookUpdates is an unbounded array that can grow
indefinitely; fix by introducing a max capacity constant (e.g.,
MAX_QUEUED_WEBHOOKS) and enforce it wherever items are added to
queuedWebhookUpdates (drop oldest via shift() or reject new pushes and log/warn)
so the array never exceeds that capacity; update all places that push to
queuedWebhookUpdates (refer to the queuedWebhookUpdates symbol at its
declaration and the add/push sites around lines ~165 and ~190-192) to apply the
cap and record when items are dropped to aid monitoring.
- Around line 194-209: The code clears queuedWebhookUpdates and advances
updateOffset before calling parseUpdates, which can drop updates on transient
failures; fix by making local copies (e.g., const updates = webhookMode ?
queuedWebhookUpdates.slice() : await api.getUpdates(...)) and only mutate state
after parseUpdates succeeds: call parseUpdates(updates) first, and only if it
resolves, then clear queuedWebhookUpdates (queuedWebhookUpdates.length = 0) when
webhookMode is true and set updateOffset = maxUpdateId + 1 when
parseHighestUpdateOffset(updates) returns a value; ensure you still compute
maxUpdateId from the local updates variable so state changes happen
post-success.
In `@apps/cli/src/channels/telegram/telegramWebhookRelay.ts`:
- Around line 15-23: The webhook token and port need stricter validation: ensure
secretPathToken is URL-safe by encoding it (use encodeURIComponent on
secretPathToken when building path instead of interpolating raw) and keep the
existing empty-check; also validate requestedPort by rejecting values > 65535
(throw an Error if params.port is outside 0–65535) rather than silently
clamping, and continue to use the validated host and port when creating the path
and calling listen; update references to secretPathToken, path, requestedPort
(and params.port) so the encoded token and validated port are used everywhere.
In `@apps/cli/src/daemon/startDaemon.ts`:
- Around line 1382-1384: Make stopping the channel bridge worker best-effort by
wrapping the await channelBridgeWorker.stop() call in a try/catch so errors do
not abort the rest of the daemon shutdown/cleanup. Specifically, around the
channelBridgeWorker.stop() invocation (referencing the channelBridgeWorker
object and its stop() method), catch any thrown error, log the failure with the
existing daemon logger (e.g., processLogger or logger) including the error
details, and then continue with the remaining cleanup sequence.
- Around line 1197-1235: The three logger.warn calls in this startup block
(after readSettings.catch, inside the catch around
readChannelBridgeTelegramConfigFromKv, and the .catch on
startChannelBridgeFromEnv) currently log raw error objects which may contain
sensitive transport/token metadata; replace each logger.warn(..., error) with a
sanitized message that only includes a non-sensitive string (e.g. derive a
safeErrorMessage via (error && typeof error.message === 'string') ?
error.message : String(error) or error?.name + ': ' + String(error) and do not
include stack/headers/objects), keeping the original descriptive context
strings; update the warnings in the readSettings.catch, the readChannelBridge...
catch, and the startChannelBridgeFromEnv.catch accordingly (functions
referenced: readSettings, readChannelBridgeTelegramConfigFromKv,
startChannelBridgeFromEnv, createAxiosChannelBridgeKvClient,
overlayServerKvTelegramConfigInSettings).
In `@apps/cli/src/ui/doctor.ts`:
- Around line 316-320: The current webhookPortRaw parsing silently accepts
malformed numeric strings (e.g. "8080abc"); update the string branch to enforce
a digits-only check before parsing: for webhookPortRaw keep the number branch
using Math.trunc(webhookConfig.port) but for the string branch first test
webhookConfig.port against /^\d+$/ and only then call
Number.parseInt(webhookConfig.port, 10); otherwise set null. Apply the same
strict-digit validation change to the other similar port parsing block
referenced (lines 374–378) so both places only accept pure digit strings before
parsing.
- Around line 394-396: The catch block that currently swallows exceptions during
the channel-bridge diagnostics should mark the run as failed and surface a
user-facing error: inside the catch for the channel-bridge check (in the same
function where hasCriticalFailures is used) set hasCriticalFailures = true and
emit a user-visible error message using the same logger used elsewhere in this
function (e.g., the function's logger/processLogger or UI error helper) so
failures are not silently ignored and match existing error-handling behavior.
- Around line 279-366: The diagnostics block in apps/cli/src/ui/doctor.ts uses
many unsafe casts (channelBridgeRoot, byServerId, serverScope, byAccountId,
accountScope, providers, telegram, providerEntries) and silences errors with a
bare catch; replace those ad-hoc (as any) casts with a small typed
extractor/validator function that defines interfaces for ChannelBridgeSettings,
ProviderConfig, WebhookConfig, Secrets etc., uses runtime guards (typeof checks,
Array.isArray) to narrow unknown → typed objects, and returns typed values used
by the existing logic (token, webhookConfig, allowedChatIds, webhookPortRaw,
webhookIssues via collectMissingRequiredWebhookFields, and maskValue). Also
remove the silent catch and replace it with explicit error handling that logs
the error (use the same logger/console pattern used nearby) so failures in
extraction are visible and set hasCriticalFailures accordingly. Ensure
providerEntries is derived from the validated typed providers object rather than
from any casts.
In `@docs/channel-bridge-uat.md`:
- Line 85: Update the test/verification step that currently reads "If using
webhook mode, verify secret mismatch returns unauthorized." to reflect actual
relay behavior under path-token routing: change the expectation to "verify
secret mismatch results in a non-200 response (typically 404) or otherwise does
not forward the request" so the doc matches runtime behavior; locate the
sentence string "If using webhook mode, verify secret mismatch returns
unauthorized." in docs/channel-bridge-uat.md and replace it with the suggested
wording.
---
Nitpick comments:
In `@apps/cli/src/channels/channelBridgeAccountConfig.ts`:
- Line 1: Add a concise file-level JSDoc header at the top of this module
describing that this file is the scoped config mutator/normalizer for channel
bridge account settings: state its responsibilities (normalizing, validating,
and merging scoped account config), the expected input shape (e.g., RecordLike),
key exported utilities or functions provided by the module (named exports you
see in this file), any side-effects (mutations, I/O, or global config changes),
and the contract for callers (what guarantees normalized config will have). Keep
it brief (2–4 sentences) and place it above the existing declarations so future
readers understand the module's purpose and usage.
In `@apps/cli/src/channels/channelBridgeServerKv.ts`:
- Around line 1-6: Add a top-level JSDoc header to this KV module describing its
responsibility as the cross-cutting persistence contract for channel bridge
state, list the main exports/types it provides (e.g., ScopedTelegramBridgeUpdate
or other KV interfaces), describe intended usage patterns (what data is stored,
retention/consistency expectations, and who should call these APIs), and note
any invariants or side-effects (e.g., synchronous vs async, error handling
expectations). Place the comment at the very top of channelBridgeServerKv.ts so
future readers immediately see the module purpose, public surface, and
operational guidance.
In `@apps/cli/src/channels/core/channelBridgeWorker.ts`:
- Around line 1-2: Add a file-level JSDoc header to channelBridgeWorker.ts above
the imports that documents the file’s responsibility (core runtime orchestration
for channel bridge workers), lifecycle boundaries (when and how
startSingleFlightIntervalLoop is started/stopped), public API/handles
(SingleFlightIntervalLoopHandle and any exported functions),
ownership/maintainers, and key invariants or adapter constraints callers must
respect; keep it concise but specific so future maintainers know contract
expectations and to update the header when behavior or ownership changes.
In `@apps/cli/src/channels/telegram/telegramWebhookRelay.test.ts`:
- Around line 6-35: Add two new tests in telegramWebhookRelay.test.ts that
assert startup validation errors from startTelegramWebhookRelay: one that calls
startTelegramWebhookRelay with an invalid secretPathToken (e.g., an empty string
or a token containing a slash like '/bad-token') and expects the promise to
reject/throw with a validation error, and another that calls
startTelegramWebhookRelay with an out-of-range port (e.g., -1 or 70000) and
expects rejection/throw; use async/await with expect(...).rejects or try/catch
to assert the error and ensure you only call relay.stop() if the relay was
successfully created. Reference startTelegramWebhookRelay, relay.port and
relay.path when structuring the tests.
In `@apps/cli/src/cli/commands/bridge.test.ts`:
- Around line 46-70: There are two beforeEach hooks; consolidate them into a
single beforeEach that runs all setup steps: keep the mock resets and
mockResolvedValue/mockReturnValue/setup implementations (readCredentialsMock,
decodeJwtPayloadMock, readSettingsMock, checkDaemonMock, updateSettingsMock,
createKvClientMock, upsertKvConfigMock, clearKvConfigMock) and also the async
temp-dir setup (mkdtemp -> homeDir), environment variable assignments
(process.env.HAPPIER_HOME_DIR, HAPPIER_SERVER_URL, HAPPIER_WEBAPP_URL) and
reloadConfiguration() in the same hook so both synchronous mocks and async
filesystem/env setup run together before each test; update any async handling so
the combined beforeEach is declared async and awaits mkdtemp and any mock async
initializers.
In `@apps/cli/src/ui/doctor.test.ts`:
- Around line 97-109: The test in apps/cli/src/ui/doctor.test.ts pins full
user-facing messages from collectMissingRequiredWebhookFields; change it to
assert stable identifiers/substrings instead of exact copy-pinned strings — for
example, replace the toEqual assertion with assertions that the returned array
includes entries containing 'webhook.secret', 'webhook.host', 'webhook.port' and
the phrase 'required when webhook.enabled=true' (use expect.arrayContaining
combined with expect.stringContaining or map the result to keys and assert those
keys exist) so wording changes won't break the test.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 17f0243c-9dd6-4b8d-b630-09d4bd5561eb
⛔ Files ignored due to path filters (3)
docs/assets/doctor-critical-summary.pngis excluded by!**/*.pngdocs/assets/doctor-missing-host-port-summary.pngis excluded by!**/*.pngyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (29)
README.mdapps/cli/src/channels/channelBindingStore.server.test.tsapps/cli/src/channels/channelBindingStore.server.tsapps/cli/src/channels/channelBridgeAccountConfig.test.tsapps/cli/src/channels/channelBridgeAccountConfig.tsapps/cli/src/channels/channelBridgeConfig.test.tsapps/cli/src/channels/channelBridgeConfig.tsapps/cli/src/channels/channelBridgeServerConfigOverlay.test.tsapps/cli/src/channels/channelBridgeServerConfigOverlay.tsapps/cli/src/channels/channelBridgeServerKv.test.tsapps/cli/src/channels/channelBridgeServerKv.tsapps/cli/src/channels/core/channelBridgeWorker.test.tsapps/cli/src/channels/core/channelBridgeWorker.tsapps/cli/src/channels/startChannelBridgeWorker.test.tsapps/cli/src/channels/startChannelBridgeWorker.tsapps/cli/src/channels/telegram/telegramAdapter.test.tsapps/cli/src/channels/telegram/telegramAdapter.tsapps/cli/src/channels/telegram/telegramWebhookRelay.test.tsapps/cli/src/channels/telegram/telegramWebhookRelay.tsapps/cli/src/cli/commandRegistry.tsapps/cli/src/cli/commands/bridge.test.tsapps/cli/src/cli/commands/bridge.tsapps/cli/src/cli/dispatch.tsapps/cli/src/daemon/startDaemon.tsapps/cli/src/persistence.tsapps/cli/src/ui/doctor.test.tsapps/cli/src/ui/doctor.tsdocs/channel-bridge-uat.mddocs/telegram-channel-bridge.md
cdenneen
force-pushed
the
feat/telegram-channel-adapter
branch
from
8c60a3a to
9eb06ac
Compare
Author
|
@coderabbitai please re-review this PR after latest force-push (review-thread fixes and stack rebase onto #109 head). @greptile-apps please run a fresh pass too. |
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
apps/cli/src/channels/startChannelBridgeWorker.ts (1)
281-289: Ensure deps disposal still runs if relay shutdown fails.
defaultDepsHandle?.dispose()is currently skipped whenrelayHandle.stop()throws, leaving cache/state cleanup incomplete.Based on learnings: "Implement proper process lifecycle and cleanup handling".♻️ Cleanup ordering fix
return { trigger: worker.trigger, stop: async () => { + let primaryError: unknown = null; try { await worker.stop(); + } catch (error) { + primaryError = error; } finally { - if (relayHandle) { - await relayHandle.stop(); + try { + if (relayHandle) { + await relayHandle.stop(); + } + } catch (error) { + if (primaryError === null) { + primaryError = error; + } + } finally { + defaultDepsHandle?.dispose(); } - defaultDepsHandle?.dispose(); + } + if (primaryError !== null) { + throw primaryError; } }, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/channels/startChannelBridgeWorker.ts` around lines 281 - 289, The stop handler currently awaits relayHandle.stop() inside a finally which can throw and prevent defaultDepsHandle?.dispose() from running; update the stop implementation (the async stop closure that calls worker.stop()) so that cleanup always runs: call await worker.stop() in its try, then in the finally ensure relayHandle.stop() is invoked inside its own try/catch (or swallow/handle errors) so exceptions from relayHandle.stop() do not abort subsequent cleanup, and always call defaultDepsHandle?.dispose() after relay shutdown regardless of relay errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/cli/src/channels/channelBindingStore.server.test.ts`:
- Around line 6-120: The tests use in-process test doubles
createInMemoryKvClient and createCountingKvClient instead of exercising the real
KV API; replace those helpers with the project's integration test fixture that
performs real API-backed KV calls (e.g., the shared fixture that returns an
actual ChannelBridgeKvClient connected to the test backend), update tests that
import or call createInMemoryKvClient/createCountingKvClient to instead obtain
the real client from the integration fixture, remove the in-memory helper
functions and any mutateCallCount logic, and ensure tests run against the real
backend client (using existing setup/teardown from the integration fixtures) so
they no longer rely on mock-driven behavior.
In `@apps/cli/src/channels/core/channelBridgeWorker.ts`:
- Around line 568-573: The shutdown currently uses Promise.all over
params.adapters which can reject early; replace that with Promise.allSettled (or
manually await each stop inside a loop) so every adapter.stop() is attempted
even if some reject, then collect any errors from the settled results and either
log them or throw an aggregated error; update the code where
params.adapters.map(async (adapter) => { if (typeof adapter.stop !== 'function')
return; await adapter.stop(); }) is used (in channelBridgeWorker.ts) to use
Promise.allSettled and handle rejected outcomes deterministically.
In `@apps/cli/src/channels/startChannelBridgeWorker.ts`:
- Around line 171-176: The onWarning handler currently logs raw error objects
(logger.warn in the onWarning callback), which can leak secrets; update the
handler used at lines around the onWarning definition to sanitize/normalize the
error before logging — e.g., implement or call a shared sanitizeError utility to
extract only safe fields (name, message, truncated stack) and redact known
sensitive keys (token, auth, cookie, session, apiKey) or stringify with a
redaction policy, then pass that sanitized object/string to logger.warn; apply
the same change to the other warning handler occurrences mentioned (around the
240-242 area) so all warning paths log only sanitized payloads.
In `@apps/cli/src/channels/telegram/telegramAdapter.test.ts`:
- Around line 7-31: The test currently uses mocked API stubs (the local "api"
object with getMe, getUpdates, sendMessage implemented via vi.fn) which violates
the guideline to use real Telegram integration fixtures; replace these vi.fn
mocks in telegramAdapter.test.ts (the "api" fixture and the repeated mocks at
the other occurrences) with real integration fixtures that perform actual API
calls or recorded wire fixtures provided by the project test helpers, ensuring
getMe(), getUpdates(), and sendMessage() use the real transport/fixture helpers
instead of vi.fn so the suite exercises real API behavior and avoids drift.
In `@apps/cli/src/channels/telegram/telegramWebhookRelay.ts`:
- Around line 14-15: The webhook handler is currently invoking the onUpdate
callback fire-and-forget which returns { ok: true } before onUpdate completes;
change the handler to await the onUpdate(update) call (wrap in try/catch) and
only send the success response after it completes, logging or returning an error
response when onUpdate throws so failed async handlers aren’t masked; look for
the onUpdate parameter and the HTTP request handler that sends the JSON response
(in the Telegram webhook relay implementation) and replace the fire-and-forget
invocation with an awaited call and proper error handling.
---
Nitpick comments:
In `@apps/cli/src/channels/startChannelBridgeWorker.ts`:
- Around line 281-289: The stop handler currently awaits relayHandle.stop()
inside a finally which can throw and prevent defaultDepsHandle?.dispose() from
running; update the stop implementation (the async stop closure that calls
worker.stop()) so that cleanup always runs: call await worker.stop() in its try,
then in the finally ensure relayHandle.stop() is invoked inside its own
try/catch (or swallow/handle errors) so exceptions from relayHandle.stop() do
not abort subsequent cleanup, and always call defaultDepsHandle?.dispose() after
relay shutdown regardless of relay errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 21104746-2621-42e1-bb00-3c5bdca7dc0c
📒 Files selected for processing (18)
apps/cli/src/channels/channelBindingStore.server.test.tsapps/cli/src/channels/channelBindingStore.server.tsapps/cli/src/channels/channelBridgeAccountConfig.test.tsapps/cli/src/channels/channelBridgeAccountConfig.tsapps/cli/src/channels/channelBridgeConfig.test.tsapps/cli/src/channels/channelBridgeServerKv.test.tsapps/cli/src/channels/channelBridgeServerKv.tsapps/cli/src/channels/core/channelBridgeWorker.test.tsapps/cli/src/channels/core/channelBridgeWorker.tsapps/cli/src/channels/startChannelBridgeWorker.tsapps/cli/src/channels/telegram/telegramAdapter.test.tsapps/cli/src/channels/telegram/telegramAdapter.tsapps/cli/src/channels/telegram/telegramWebhookRelay.test.tsapps/cli/src/channels/telegram/telegramWebhookRelay.tsapps/cli/src/daemon/startDaemon.tsapps/cli/src/ui/doctor.tsdocs/channel-bridge-uat.mddocs/telegram-channel-bridge.md
🚧 Files skipped from review as they are similar to previous changes (6)
- docs/telegram-channel-bridge.md
- apps/cli/src/channels/channelBridgeConfig.test.ts
- apps/cli/src/channels/channelBridgeAccountConfig.test.ts
- apps/cli/src/channels/telegram/telegramWebhookRelay.test.ts
- apps/cli/src/channels/channelBridgeServerKv.test.ts
- docs/channel-bridge-uat.md
Author
|
Follow-up clarity pass — exact fix locations for resolved bot items after latest stack rebase ( Core carryover from #109 (
#110-specific hardening (
This is a reviewer navigation map so each resolved thread can be validated directly against the updated code paths. |
Contributor
Additional Comments (1)
When the settings dump is printed at line 267, these plaintext secrets are visible in the JSON output of the A dedicated bridge section (starting line 292) correctly uses function redactSettingsForDisplay(settings: SettingsForDisplay): SettingsForDisplay {
const redacted = JSON.parse(JSON.stringify(settings ?? {})) as SettingsForDisplay;
const redactedRecord = redacted as unknown as Record<string, unknown>;
if (Object.prototype.hasOwnProperty.call(redactedRecord, 'localEnvironmentVariables')) {
delete redactedRecord.localEnvironmentVariables;
}
// Redact channel bridge secrets so that botToken / webhookSecret
// are never emitted in plain text by the doctor command.
const cb = redactedRecord.channelBridge;
if (cb && typeof cb === 'object' && !Array.isArray(cb)) {
redactedRecord.channelBridge = '<redacted – use `happier bridge list` to inspect>';
}
return redacted;
} |
cdenneen
force-pushed
the
feat/telegram-channel-adapter
branch
from
9eb06ac to
562bee5
Compare
Author
|
@coderabbitai @greptile-apps follow-up fixes were pushed (commits 562bee5 and 1eead1d), and #110 remains stacked directly on #109 head; please re-review. |
|
The PR description images show:
🧠 Learnings used✅ Actions performedFull review triggered. |
Contributor
Additional Comments (1)
Hoisting a single shared let settings: Awaited<ReturnType<typeof readSettings>> | null = null;
try {
settings = await readSettings();
} catch {
// ...
}
// then use `settings` in both the display block and the channel-bridge section |
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (3)
apps/cli/src/channels/core/channelBridgeWorker.test.ts (1)
573-575: Replace fixed sleeps with condition-based waits to reduce flakiness.These assertions currently depend on timing (
setTimeout(25/50ms)), which can intermittently fail on slower CI runners. ReusewaitFor(...)on observable conditions instead of fixed delays.Based on learnings: Tests must be deterministic and isolated with no shared global state and no ordering reliance.
Also applies to: 613-619
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/channels/core/channelBridgeWorker.test.ts` around lines 573 - 575, Replace the fixed setTimeout-based waits with condition-based waits: instead of awaiting new Promise(resolve => setTimeout(...)) then asserting sentToSession length, use a waitFor helper to poll until sentToSession has the expected length (e.g., waitFor(() => expect(sentToSession).toHaveLength(1))). Update both occurrences referenced (the block around sentToSession assertion at lines ~573–575 and the similar block around ~613–619) to import/use the test runner’s waitFor utility (from `@testing-library/react` or vitest) and wait on the observable condition rather than sleeping, ensuring tests become deterministic and resilient to CI slowness.apps/cli/src/channels/channelBridgeAccountConfig.ts (2)
1-1: Add a file-level JSDoc header describing module responsibility.This file exposes multiple public config APIs and normalization rules; a short header doc would make scope/precedence intent much easier to maintain.
As per coding guidelines: "Include comprehensive JSDoc comments as file header comments explaining file responsibilities".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/channels/channelBridgeAccountConfig.ts` at line 1, Add a file-level JSDoc header at the top of this module that explains the file's responsibility (public config APIs and normalization rules), scope/precedence intent, and any important usage notes or side effects; place it above the existing type alias RecordLike and reference the primary exports (e.g., the exported config API functions and normalization helpers) so readers know what to expect and how to use the module.
258-264: Prune emptytelegram.webhookafter secret migration.When moving
webhook.secretintosecrets.webhookSecret, an emptytelegram.webhookobject can remain if it only containedsecret.Proposed cleanup
if (typeof params.update.webhookSecret === 'string') { secrets.webhookSecret = params.update.webhookSecret; const webhook = asRecord(telegram.webhook); - if (webhook && 'secret' in webhook) { - delete webhook.secret; + if (webhook) { + if ('secret' in webhook) { + delete webhook.secret; + } + if (isEmptyRecord(webhook)) { + delete telegram.webhook; + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/channels/channelBridgeAccountConfig.ts` around lines 258 - 264, After migrating params.update.webhookSecret into secrets.webhookSecret and deleting webhook.secret on the local webhook object (in the block referencing params.update.webhookSecret, secrets.webhookSecret, telegram.webhook, asRecord and delete webhook.secret), add a check that if the resulting webhook object has no own enumerable keys (is empty) then remove telegram.webhook entirely (e.g., delete telegram.webhook or set it to undefined) so an empty object isn't left behind; ensure this runs immediately after the delete webhook.secret operation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/cli/src/channels/channelBridgeAccountConfig.ts`:
- Around line 287-304: The removal path in removeScopedTelegramBridgeConfig only
deletes providers.telegram but leaves accountScope.tickMs behind, which can
resurrect old polling intervals; update the removal logic (the block referencing
providers.telegram, accountScope, byAccountId, serverScope, byServerId) to also
delete accountScope.tickMs when removing the Telegram provider (e.g., add
"delete accountScope.tickMs" after "delete providers.telegram"), and keep the
existing empty-record cleanups (isEmptyRecord(accountScope) / byAccountId /
serverScope) so tickMs is removed whenever the Telegram config for that account
is deleted.
In `@apps/cli/src/channels/channelBridgeServerKv.test.ts`:
- Line 93: The test currently uses a broad cast "(config.record as any)" which
defeats type-safety; replace it by narrowing the type for config.record (or
casting to a targeted Partial type) and assert via the concrete property path
(config.record.telegram?.botToken) so the assertion keeps type checking.
Specifically, update the assertion around config.record and telegram.botToken in
channelBridgeServerKv.test.ts to cast config.record to a precise type (e.g.,
Partial<YourRecordType> or the existing ChannelRecord type) or use optional
chaining, rather than "as any", so the test remains type-safe while still
checking that telegram.botToken is undefined.
In `@apps/cli/src/channels/channelBridgeServerKv.ts`:
- Around line 175-202: The loop that decodes rawBindings in
channelBridgeServerKv.ts (variables rawBindings, asRecord, and bindings)
currently silently skips malformed rows which can cause partial documents and
data loss; instead, validate each row and fail fast by throwing a descriptive
error when asRecord(row) returns falsy or when required fields providerId,
conversationId, or sessionId are missing/empty or numeric fields
(lastForwardedSeq, createdAtMs, updatedAtMs) are invalid, including the
offending row data in the error message so the caller can surface and fix the
corrupt input rather than continuing and producing a partial bindings array.
In `@apps/cli/src/channels/startChannelBridgeWorker.ts`:
- Around line 218-247: The adapter is created with webhookMode: true before
startTelegramWebhookRelay, so if relay startup fails inbound delivery stops;
change the catch block to fall back to a polling adapter: either toggle the
existing telegramAdapter into polling mode (if it exposes a method/property) or
create a new adapter via createTelegramChannelAdapter({ botToken,
allowedChatIds, requireTopics, webhookMode: false }) and replace the entry in
adapters (and ensure enqueueWebhookUpdate binding is preserved if needed), then
log the fallback; reference createTelegramChannelAdapter, telegramAdapter,
startTelegramWebhookRelay, webhookMode, enqueueWebhookUpdate and adapters when
making the change.
In `@apps/cli/src/cli/commands/bridge.test.ts`:
- Around line 46-70: Combine the two beforeEach blocks into a single async
beforeEach so setup ordering is explicit: first call vi.resetModules() and
vi.clearAllMocks(), then configure all mocks (readCredentialsMock,
decodeJwtPayloadMock, readSettingsMock, checkDaemonMock, updateSettingsMock
implementation, createKvClientMock, upsertKvConfigMock, clearKvConfigMock), then
create the temp homeDir via mkdtemp, set process.env.HAPPIER_HOME_DIR /
HAPPIER_SERVER_URL / HAPPIER_WEBAPP_URL, and finally call reloadConfiguration();
ensure the updateSettingsMock implementation and other mock setups remain
unchanged and that the async mkdtemp/await happens after mocks are configured.
In `@apps/cli/src/cli/commands/bridge.ts`:
- Around line 78-79: Update the CLI usage strings in
apps/cli/src/cli/commands/bridge.ts so the help lines include the supported
webhook flags parsed by cmdTelegramSet (e.g., webhook-related flags like
--webhook-url, --webhook-cert, or whatever webhook flags cmdTelegramSet reads);
locate the usage declarations near the top examples (the line with "happier
bridge telegram set --bot-token ...") and the repeated block around the later
example (the area noted as also applies to 198-201) and append the webhook flag
options to those usage lines so the displayed help matches the flags parsed by
cmdTelegramSet.
- Around line 257-264: The code uses a broad "as any" to silence types inside
the settings mutation—remove the cast and make the mutation return the correct
Settings type by aligning upsertScopedTelegramBridgeConfig's return/type
signatures or by mapping its result to the expected Settings shape before
returning; update the call site in updateSettings (the lambda passed to
updateSettings that currently calls upsertScopedTelegramBridgeConfig with
serverId/accountId and split.localUpdate) to return a properly typed Settings
object (or cast a narrow, justified type) instead of "as any"; apply the same
fix for the other similar updateSettings/upsertScopedTelegramBridgeConfig usage
in this file so both mutation paths preserve compile-time type safety.
In `@apps/cli/src/daemon/startDaemon.ts`:
- Around line 1197-1245: The channel bridge startup is currently gated on
machine registration and therefore never runs if registration fails; refactor so
the bridge startup (the block that reads settings via readSettings(), optionally
fetches KV using createAxiosChannelBridgeKvClient() and
readChannelBridgeTelegramConfigFromKv(), computes channelBridgeRuntimeSettings
with overlayServerKvTelegramConfigInSettings(), and finally calls
startChannelBridgeFromEnv()) runs independently as a best-effort operation
regardless of machine-registration outcome: move or duplicate this block out of
the registration-success-only path, ensure it only attempts server KV fetch when
serverId and accountId are available (and silently continues on any errors using
existing logger.warn + serializeAxiosErrorForLog behavior), and always call
startChannelBridgeFromEnv() (catching/logging errors and returning null) so
bridge startup is attempted even if registration failed.
In `@apps/cli/src/ui/doctor.ts`:
- Around line 292-399: The doctor currently inspects account-scoped values
derived from readSettings() (see settingsRecord -> accountScope -> providers ->
telegram) but must instead use the runtime-resolved channel-bridge configuration
(respecting env and server KV overlays) when evaluating Telegram and other
providers; replace the usage of providers/telegram derived from accountScope
with the runtime-resolved providers for the active server/account (obtain this
by calling the existing runtime settings resolver or by merging precedence: env
> server KV > local settings based on configuration.activeServerId and
readSettings()), then feed that resolved providers object into the same checks
that use parseStrictWebhookPort, collectMissingRequiredWebhookFields and
maskValue so hasCriticalFailures reflects actual runtime behavior rather than
raw local scoped settings.
In `@README.md`:
- Around line 196-200: Replace the ambiguous section header "Channel
Integrations (A/1)" with a clear title like "Channel Integrations" and change
the code-formatted doc path to a proper Markdown link by updating the bullet so
the Telegram doc is linked as [Telegram bi-directional bridge
setup](docs/telegram-channel-bridge.md); ensure the header and the link text
match (e.g., "Telegram bi-directional bridge setup (BotFather, topics/DM
mapping, optional webhook relay)") and that the link points to
docs/telegram-channel-bridge.md so it renders clickable in the README.
---
Nitpick comments:
In `@apps/cli/src/channels/channelBridgeAccountConfig.ts`:
- Line 1: Add a file-level JSDoc header at the top of this module that explains
the file's responsibility (public config APIs and normalization rules),
scope/precedence intent, and any important usage notes or side effects; place it
above the existing type alias RecordLike and reference the primary exports
(e.g., the exported config API functions and normalization helpers) so readers
know what to expect and how to use the module.
- Around line 258-264: After migrating params.update.webhookSecret into
secrets.webhookSecret and deleting webhook.secret on the local webhook object
(in the block referencing params.update.webhookSecret, secrets.webhookSecret,
telegram.webhook, asRecord and delete webhook.secret), add a check that if the
resulting webhook object has no own enumerable keys (is empty) then remove
telegram.webhook entirely (e.g., delete telegram.webhook or set it to undefined)
so an empty object isn't left behind; ensure this runs immediately after the
delete webhook.secret operation.
In `@apps/cli/src/channels/core/channelBridgeWorker.test.ts`:
- Around line 573-575: Replace the fixed setTimeout-based waits with
condition-based waits: instead of awaiting new Promise(resolve =>
setTimeout(...)) then asserting sentToSession length, use a waitFor helper to
poll until sentToSession has the expected length (e.g., waitFor(() =>
expect(sentToSession).toHaveLength(1))). Update both occurrences referenced (the
block around sentToSession assertion at lines ~573–575 and the similar block
around ~613–619) to import/use the test runner’s waitFor utility (from
`@testing-library/react` or vitest) and wait on the observable condition rather
than sleeping, ensuring tests become deterministic and resilient to CI slowness.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b402c91c-6319-4398-ac54-dadc223aec8d
⛔ Files ignored due to path filters (3)
docs/assets/doctor-critical-summary.pngis excluded by!**/*.pngdocs/assets/doctor-missing-host-port-summary.pngis excluded by!**/*.pngyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (29)
README.mdapps/cli/src/channels/channelBindingStore.server.test.tsapps/cli/src/channels/channelBindingStore.server.tsapps/cli/src/channels/channelBridgeAccountConfig.test.tsapps/cli/src/channels/channelBridgeAccountConfig.tsapps/cli/src/channels/channelBridgeConfig.test.tsapps/cli/src/channels/channelBridgeConfig.tsapps/cli/src/channels/channelBridgeServerConfigOverlay.test.tsapps/cli/src/channels/channelBridgeServerConfigOverlay.tsapps/cli/src/channels/channelBridgeServerKv.test.tsapps/cli/src/channels/channelBridgeServerKv.tsapps/cli/src/channels/core/channelBridgeWorker.test.tsapps/cli/src/channels/core/channelBridgeWorker.tsapps/cli/src/channels/startChannelBridgeWorker.test.tsapps/cli/src/channels/startChannelBridgeWorker.tsapps/cli/src/channels/telegram/telegramAdapter.test.tsapps/cli/src/channels/telegram/telegramAdapter.tsapps/cli/src/channels/telegram/telegramWebhookRelay.test.tsapps/cli/src/channels/telegram/telegramWebhookRelay.tsapps/cli/src/cli/commandRegistry.tsapps/cli/src/cli/commands/bridge.test.tsapps/cli/src/cli/commands/bridge.tsapps/cli/src/cli/dispatch.tsapps/cli/src/daemon/startDaemon.tsapps/cli/src/persistence.tsapps/cli/src/ui/doctor.test.tsapps/cli/src/ui/doctor.tsdocs/channel-bridge-uat.mddocs/telegram-channel-bridge.md
🚧 Files skipped from review as they are similar to previous changes (11)
- apps/cli/src/channels/channelBridgeAccountConfig.test.ts
- apps/cli/src/channels/channelBridgeServerConfigOverlay.test.ts
- docs/telegram-channel-bridge.md
- apps/cli/src/channels/channelBindingStore.server.ts
- apps/cli/src/ui/doctor.test.ts
- apps/cli/src/channels/channelBindingStore.server.test.ts
- apps/cli/src/persistence.ts
- apps/cli/src/channels/startChannelBridgeWorker.test.ts
- apps/cli/src/channels/channelBridgeConfig.test.ts
- docs/channel-bridge-uat.md
- apps/cli/src/channels/telegram/telegramWebhookRelay.test.ts
cdenneen
force-pushed
the
feat/telegram-channel-adapter
branch
from
1eead1d to
0aceec6
Compare
cdenneen
added a commit
to cdenneen/happier-dev
that referenced
this pull request
Mar 4, 2026
Author
|
@coderabbitai review\n@greptile-ai review\n\nFollow-up fixes are pushed in f448629 and f2879a4, and all previously open review threads have been addressed and resolved. |
|
The PR description images show:
🧠 Learnings used✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/cli/src/daemon/startDaemon.ts (1)
1153-1245:⚠️ Potential issue | 🟠 MajorDecouple channel bridge startup from machine registration success.
startChannelBridgeFromEnv(...)is still inside the success path afterensureMachineRegistered(...). If registration fails (temporary backend outage, timeout), the daemon keeps running but bridge startup is skipped entirely until restart.Suggested refactor direction
+const startChannelBridgeBestEffort = async (): Promise<ChannelBridgeRuntimeHandle | null> => { + const bridgeSettings = await readSettings().catch(() => null); + // optional KV overlay when serverId/accountId are available + return await startChannelBridgeFromEnv({ credentials, settings: bridgeSettings, ... }); +}; + +channelBridgeWorker = await startChannelBridgeBestEffort().catch(() => null); + void (async () => { try { const ensured = await ensureMachineRegistered(...); // machine sync wiring only - channelBridgeWorker = await startChannelBridgeFromEnv(...); } catch (error) { logger.warn('... continuing without machine sync ...'); } })();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/daemon/startDaemon.ts` around lines 1153 - 1245, The channel bridge startup (startChannelBridgeFromEnv) is tied to the successful return of ensureMachineRegistered so if registration fails the bridge is never attempted; move the channel-bridge initialization out of the post-registration success path so it runs independently (e.g., after the registration block or in a finally/independent async task), but still use existing credentials/token and optional server/account ids derived via decodeJwtPayload and readSettings; update references to channelBridgeRuntimeSettings/channelBridgeServerId/channelBridgeAccountId and startChannelBridgeFromEnv so they are computed and invoked even when ensureMachineRegistered throws or returns a fallback, ensuring failures are caught and logged as they currently are (use the same serializeAxiosErrorForLog and warning behavior).
🧹 Nitpick comments (1)
apps/cli/src/ui/doctor.ts (1)
46-77: Add JSDoc for new exported webhook diagnostics helpers.
isMissingRequiredTelegramWebhookSecretandcollectMissingRequiredWebhookFieldsare public exports with core validation behavior; brief JSDoc would improve maintainability.As per coding guidelines "Add JSDoc comments for public APIs and complex logic".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/ui/doctor.ts` around lines 46 - 77, Add brief JSDoc comments for the two exported functions isMissingRequiredTelegramWebhookSecret and collectMissingRequiredWebhookFields: for isMissingRequiredTelegramWebhookSecret document the purpose (returns true when webhook.enabled is true and webhookSecret is empty/whitespace), describe the params object (webhookEnabled:boolean, webhookSecret:string) and the boolean return; for collectMissingRequiredWebhookFields document that it returns an array of human-readable issue strings when webhookEnabled is true, describe the params object (webhookEnabled:boolean, webhookSecret:string, webhookHost:string, webhookPort:number|null) and explain the validation rules it applies (empty secret/host, and port must be finite integer in 1..65535), and include `@param` and `@returns` tags so these public helpers are self-describing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/cli/src/channels/startChannelBridgeWorker.ts`:
- Around line 204-205: relayHandle and defaultDepsHandle are set up before
calling startChannelBridgeWorker(...) so if that call throws the created
resources aren't cleaned up; wrap the worker creation and any subsequent
initialization in a try/catch (or try/finally) so you only assign to
relayHandle/defaultDepsHandle after successful creation or ensure you
close/cleanup them in the catch, specifically around the call to
startChannelBridgeWorker and the code that constructs adapters (refer to
relayHandle, defaultDepsHandle, startChannelBridgeWorker, and params.adapters),
and on error call the appropriate shutdown/close methods for those handles to
guarantee no leaked resources.
- Around line 40-63: The extractAssistantText function uses broad "as any" casts
which bypass TS safety; replace them by defining discriminated interfaces for
the content shapes (e.g., TextContent { type: 'text'; text: string }, AcpContent
{ type: 'acp'; data: AcpData }, OutputContent { type: 'output'; data: OutputData
} and AcpData variants like MessageData { type: 'message' | 'reasoning';
message: string }) and implement type-guard functions (isTextContent(content),
isAcpContent(content), isOutputContent(content), isMessageData(data)) to narrow
the union; then access content.type, content.text, data.type and data.message
with proper typings instead of casting, keeping the existing empty/null checks
(and Array.isArray guards) but using the narrowed types for safe property access
inside extractAssistantText.
In `@apps/cli/src/ui/doctor.ts`:
- Around line 318-320: The runtimeConfigSource detection is using the wrapper
object returned by readChannelBridgeTelegramConfigFromKv (assigned to
serverRecord) which is always truthy; update the conditional to check the actual
record field instead (use if (serverRecord.record != null)) so
runtimeConfigSource is set to 'server KV overlay + local settings' only when an
actual KV record exists; change the check surrounding runtimeConfigSource
assignment (referencing variables runtimeConfigSource and serverRecord)
accordingly.
---
Duplicate comments:
In `@apps/cli/src/daemon/startDaemon.ts`:
- Around line 1153-1245: The channel bridge startup (startChannelBridgeFromEnv)
is tied to the successful return of ensureMachineRegistered so if registration
fails the bridge is never attempted; move the channel-bridge initialization out
of the post-registration success path so it runs independently (e.g., after the
registration block or in a finally/independent async task), but still use
existing credentials/token and optional server/account ids derived via
decodeJwtPayload and readSettings; update references to
channelBridgeRuntimeSettings/channelBridgeServerId/channelBridgeAccountId and
startChannelBridgeFromEnv so they are computed and invoked even when
ensureMachineRegistered throws or returns a fallback, ensuring failures are
caught and logged as they currently are (use the same serializeAxiosErrorForLog
and warning behavior).
---
Nitpick comments:
In `@apps/cli/src/ui/doctor.ts`:
- Around line 46-77: Add brief JSDoc comments for the two exported functions
isMissingRequiredTelegramWebhookSecret and collectMissingRequiredWebhookFields:
for isMissingRequiredTelegramWebhookSecret document the purpose (returns true
when webhook.enabled is true and webhookSecret is empty/whitespace), describe
the params object (webhookEnabled:boolean, webhookSecret:string) and the boolean
return; for collectMissingRequiredWebhookFields document that it returns an
array of human-readable issue strings when webhookEnabled is true, describe the
params object (webhookEnabled:boolean, webhookSecret:string, webhookHost:string,
webhookPort:number|null) and explain the validation rules it applies (empty
secret/host, and port must be finite integer in 1..65535), and include `@param`
and `@returns` tags so these public helpers are self-describing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 90fb4f4d-a41b-4083-976f-d39b4d55d96c
⛔ Files ignored due to path filters (3)
docs/assets/doctor-critical-summary.pngis excluded by!**/*.pngdocs/assets/doctor-missing-host-port-summary.pngis excluded by!**/*.pngyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (29)
README.mdapps/cli/src/channels/channelBindingStore.server.test.tsapps/cli/src/channels/channelBindingStore.server.tsapps/cli/src/channels/channelBridgeAccountConfig.test.tsapps/cli/src/channels/channelBridgeAccountConfig.tsapps/cli/src/channels/channelBridgeConfig.test.tsapps/cli/src/channels/channelBridgeConfig.tsapps/cli/src/channels/channelBridgeServerConfigOverlay.test.tsapps/cli/src/channels/channelBridgeServerConfigOverlay.tsapps/cli/src/channels/channelBridgeServerKv.test.tsapps/cli/src/channels/channelBridgeServerKv.tsapps/cli/src/channels/core/channelBridgeWorker.test.tsapps/cli/src/channels/core/channelBridgeWorker.tsapps/cli/src/channels/startChannelBridgeWorker.test.tsapps/cli/src/channels/startChannelBridgeWorker.tsapps/cli/src/channels/telegram/telegramAdapter.test.tsapps/cli/src/channels/telegram/telegramAdapter.tsapps/cli/src/channels/telegram/telegramWebhookRelay.test.tsapps/cli/src/channels/telegram/telegramWebhookRelay.tsapps/cli/src/cli/commandRegistry.tsapps/cli/src/cli/commands/bridge.test.tsapps/cli/src/cli/commands/bridge.tsapps/cli/src/cli/dispatch.tsapps/cli/src/daemon/startDaemon.tsapps/cli/src/persistence.tsapps/cli/src/ui/doctor.test.tsapps/cli/src/ui/doctor.tsdocs/channel-bridge-uat.mddocs/telegram-channel-bridge.md
✅ Files skipped from review due to trivial changes (1)
- docs/telegram-channel-bridge.md
🚧 Files skipped from review as they are similar to previous changes (11)
- apps/cli/src/channels/telegram/telegramWebhookRelay.test.ts
- apps/cli/src/ui/doctor.test.ts
- apps/cli/src/channels/channelBridgeAccountConfig.test.ts
- apps/cli/src/channels/channelBridgeServerKv.test.ts
- apps/cli/src/persistence.ts
- apps/cli/src/channels/telegram/telegramAdapter.ts
- README.md
- docs/channel-bridge-uat.md
- apps/cli/src/channels/startChannelBridgeWorker.test.ts
- apps/cli/src/cli/commandRegistry.ts
- apps/cli/src/cli/dispatch.ts
cdenneen
added a commit
to cdenneen/happier-dev
that referenced
this pull request
Mar 4, 2026
Author
|
@coderabbitai review\n@greptile-ai review\n\nAddressed all newly opened #110 comments (including nitpick/duplicate handling), pushed in f730cb2, and resolved the corresponding threads with per-thread notes. |
|
The PR description images show:
🧠 Learnings used✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
apps/cli/src/daemon/startDaemon.ts (1)
1197-1245:⚠️ Potential issue | 🟠 MajorStart channel bridge independently of machine registration.
At Line 1197 this startup path is still nested under successful
ensureMachineRegistered(...). If registration fails, daemon continues but channel bridge never starts for the process lifetime.🔧 Suggested refactor direction
+ const startChannelBridgeBestEffort = async (): Promise<ChannelBridgeRuntimeHandle | null> => { + const bridgeSettings = await readSettings().catch(() => null); + // optional KV overlay... + return await startChannelBridgeFromEnv({ credentials, ...(bridgeSettings ? { settings: bridgeSettings } : {}) }) + .catch(() => null); + }; + + channelBridgeWorker = await startChannelBridgeBestEffort(); + void (async () => { try { const ensured = await ensureMachineRegistered({ ... }); ... - const bridgeSettings = await readSettings()... - channelBridgeWorker = await startChannelBridgeFromEnv(...) } catch (error) { ... } })();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/daemon/startDaemon.ts` around lines 1197 - 1245, The channel bridge startup (call to startChannelBridgeFromEnv using channelBridgeRuntimeSettings, channelBridgeServerId, channelBridgeAccountId) is currently executed only inside the successful ensureMachineRegistered flow, so a failed ensureMachineRegistered prevents the bridge from ever starting; refactor by extracting the bridge-start logic (the readSettings -> overlayServerKvTelegramConfigInSettings -> startChannelBridgeFromEnv sequence) into a separate path that runs even if ensureMachineRegistered throws or returns failure, ensuring credentials/tokenPayload are available, preserving existing error handling/log messages (logger.warn and serializeAxiosErrorForLog), and keeping the createAxiosChannelBridgeKvClient/readChannelBridgeTelegramConfigFromKv fetch only guarded by presence of channelBridgeServerId and channelBridgeAccountId.
🧹 Nitpick comments (2)
apps/cli/src/ui/doctor.ts (1)
67-75: Minor: Redundant null check in port validation.
Number.isFinite(null)returnsfalse, so the|| params.webhookPort === nullcheck on line 69 is redundant—the first condition already handlesnull. This doesn't affect correctness but could be simplified.♻️ Optional simplification
if ( !Number.isFinite(params.webhookPort) - || params.webhookPort === null || !Number.isInteger(params.webhookPort) || params.webhookPort <= 0 || params.webhookPort > 65_535 ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/ui/doctor.ts` around lines 67 - 75, The null check is redundant in the webhook port validation: remove the `params.webhookPort === null` branch and rely on Number.isFinite(params.webhookPort) and the other integer/range checks to validate the port; update the conditional in the block that references params.webhookPort (the if that pushes 'webhook.port: <empty/invalid>...') to drop the null check so the logic is simpler but functionally identical.apps/cli/src/channels/core/channelBridgeWorker.test.ts (1)
643-646: Replace fixed sleeps with explicit synchronization to reduce flaky tests.Using
setTimeout(50/25)for correctness sequencing can intermittently fail under CI load. Prefer deterministic gates (e.g., pull-call counters and deferred “tick entered” promises).Proposed refactor
function createAdapterHarness(providerId: string = 'telegram'): { adapter: ChannelBridgeAdapter; pushInbound: (event: ChannelBridgeInboundMessage) => void; sent: SentConversationMessage[]; failPullOnce: (error: Error) => void; stopCalls: () => number; + pullCalls: () => number; } { @@ let stopCallCount = 0; + let pullCallCount = 0; @@ pullInboundMessages: async () => { + pullCallCount += 1; if (pullError) { @@ stopCalls: () => stopCallCount, + pullCalls: () => pullCallCount, }; } @@ - worker.trigger(); - - await new Promise((resolve) => setTimeout(resolve, 50)); + worker.trigger(); + await waitFor(() => harness.pullCalls() >= 2); expect(sentToSession).toHaveLength(1); @@ - const gate = createDeferredPromise<void>(); + const gate = createDeferredPromise<void>(); + const tickEntered = createDeferredPromise<void>(); const { deps } = createDepsHarness({ sendUserMessageToSession: async () => { + tickEntered.resolve(); await gate.promise; }, }); @@ - await new Promise((resolve) => setTimeout(resolve, 25)); + await tickEntered.promise; @@ - await new Promise((resolve) => setTimeout(resolve, 25)); + await Promise.resolve(); expect(harness.stopCalls()).toBe(0);Also applies to: 685-691
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/channels/core/channelBridgeWorker.test.ts` around lines 643 - 646, Replace the fragile setTimeout-based waits after calling worker.trigger() with an explicit synchronization primitive: create a deferred promise (e.g., tickEntered) that is resolved from the code path that indicates the worker tick has started or completed (for example inside the same place that will eventually call the session send used in this test), then await that promise in the test instead of setTimeout; specifically, change the test that calls worker.trigger() to await the new tickEntered promise (or await until the sentToSession spy/handler is invoked) before asserting expect(sentToSession).toHaveLength(1), and apply the same pattern for the other occurrence around lines 685-691 so the test deterministically waits for worker tick completion rather than sleeping.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/cli/src/channels/channelBridgeServerKv.ts`:
- Around line 175-208: The code is currently coercing non-string threadId values
to null which can merge distinct bindings; update the parsing loop that
processes rawBindings (inside the function that builds
ChannelBridgeServerBindingRecord using asRecord) to validate threadId the same
way as providerId/conversationId/sessionId: if item.threadId is defined but not
a string, throw an Error indicating an invalid threadId at the current index;
otherwise set threadId to the trimmed string (or undefined if you prefer
consistent typing) instead of silently assigning null; ensure consumers
expecting threadId (and the creation of the binding object where threadId and
lastForwardedSeq are set) use the validated value.
In `@apps/cli/src/channels/startChannelBridgeWorker.ts`:
- Around line 316-324: The current stop implementation's finally block calls
relayHandle.stop() directly so any exception there prevents
defaultDepsHandle?.dispose() from running; update the finally block in the stop
function to ensure defaultDepsHandle?.dispose() always runs by isolating
relayHandle.stop() in its own try/catch (or using an inner try/finally) and
swallowing or logging relay stop errors, then calling
defaultDepsHandle?.dispose() in the outer finally; refer to the stop function
and the symbols worker.stop(), relayHandle.stop(), and
defaultDepsHandle?.dispose() when making the change.
In `@apps/cli/src/channels/telegram/telegramWebhookRelay.test.ts`:
- Around line 41-67: The test name says it covers both missing and invalid
header tokens but only checks the missing-header case; add a second POST to the
same relay URL that sends the header 'x-telegram-bot-api-secret-token' (or the
header key your relay expects) with an incorrect value (e.g., 'invalid-token')
using the same validateStatus: () => true, then assert response.status === 401
and that received remains empty; keep using startTelegramWebhookRelay
(secretHeaderToken: 'header-token-abc'), relay.port/relay.path, the received
array populated by onUpdate, and ensure relay.stop() runs in the finally block.
In `@apps/cli/src/cli/commands/bridge.ts`:
- Around line 239-241: The CLI currently allows persisting webhook port 0;
update the validation at the assignment to update.webhookPort so
parseIntegerInput('--webhook-port') enforces the valid TCP port range 1..65535
by changing the minimum from 0 to 1 (i.e., call
parseIntegerInput(webhookPortRaw, '--webhook-port', 1, 65535)); ensure the same
1..65535 constraint is applied wherever user-specified bridge webhook ports are
validated so a saved config can never be 0.
---
Duplicate comments:
In `@apps/cli/src/daemon/startDaemon.ts`:
- Around line 1197-1245: The channel bridge startup (call to
startChannelBridgeFromEnv using channelBridgeRuntimeSettings,
channelBridgeServerId, channelBridgeAccountId) is currently executed only inside
the successful ensureMachineRegistered flow, so a failed ensureMachineRegistered
prevents the bridge from ever starting; refactor by extracting the bridge-start
logic (the readSettings -> overlayServerKvTelegramConfigInSettings ->
startChannelBridgeFromEnv sequence) into a separate path that runs even if
ensureMachineRegistered throws or returns failure, ensuring
credentials/tokenPayload are available, preserving existing error handling/log
messages (logger.warn and serializeAxiosErrorForLog), and keeping the
createAxiosChannelBridgeKvClient/readChannelBridgeTelegramConfigFromKv fetch
only guarded by presence of channelBridgeServerId and channelBridgeAccountId.
---
Nitpick comments:
In `@apps/cli/src/channels/core/channelBridgeWorker.test.ts`:
- Around line 643-646: Replace the fragile setTimeout-based waits after calling
worker.trigger() with an explicit synchronization primitive: create a deferred
promise (e.g., tickEntered) that is resolved from the code path that indicates
the worker tick has started or completed (for example inside the same place that
will eventually call the session send used in this test), then await that
promise in the test instead of setTimeout; specifically, change the test that
calls worker.trigger() to await the new tickEntered promise (or await until the
sentToSession spy/handler is invoked) before asserting
expect(sentToSession).toHaveLength(1), and apply the same pattern for the other
occurrence around lines 685-691 so the test deterministically waits for worker
tick completion rather than sleeping.
In `@apps/cli/src/ui/doctor.ts`:
- Around line 67-75: The null check is redundant in the webhook port validation:
remove the `params.webhookPort === null` branch and rely on
Number.isFinite(params.webhookPort) and the other integer/range checks to
validate the port; update the conditional in the block that references
params.webhookPort (the if that pushes 'webhook.port: <empty/invalid>...') to
drop the null check so the logic is simpler but functionally identical.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bb788a04-beed-491b-b6cb-99ad9c48857a
⛔ Files ignored due to path filters (3)
docs/assets/doctor-critical-summary.pngis excluded by!**/*.pngdocs/assets/doctor-missing-host-port-summary.pngis excluded by!**/*.pngyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (30)
README.mdapps/cli/src/channels/channelBindingStore.server.test.tsapps/cli/src/channels/channelBindingStore.server.tsapps/cli/src/channels/channelBridgeAccountConfig.test.tsapps/cli/src/channels/channelBridgeAccountConfig.tsapps/cli/src/channels/channelBridgeConfig.test.tsapps/cli/src/channels/channelBridgeConfig.tsapps/cli/src/channels/channelBridgeServerConfigOverlay.test.tsapps/cli/src/channels/channelBridgeServerConfigOverlay.tsapps/cli/src/channels/channelBridgeServerKv.test.tsapps/cli/src/channels/channelBridgeServerKv.tsapps/cli/src/channels/core/channelBridgeWorker.test.tsapps/cli/src/channels/core/channelBridgeWorker.tsapps/cli/src/channels/startChannelBridgeWorker.startupFailure.test.tsapps/cli/src/channels/startChannelBridgeWorker.test.tsapps/cli/src/channels/startChannelBridgeWorker.tsapps/cli/src/channels/telegram/telegramAdapter.test.tsapps/cli/src/channels/telegram/telegramAdapter.tsapps/cli/src/channels/telegram/telegramWebhookRelay.test.tsapps/cli/src/channels/telegram/telegramWebhookRelay.tsapps/cli/src/cli/commandRegistry.tsapps/cli/src/cli/commands/bridge.test.tsapps/cli/src/cli/commands/bridge.tsapps/cli/src/cli/dispatch.tsapps/cli/src/daemon/startDaemon.tsapps/cli/src/persistence.tsapps/cli/src/ui/doctor.test.tsapps/cli/src/ui/doctor.tsdocs/channel-bridge-uat.mddocs/telegram-channel-bridge.md
✅ Files skipped from review due to trivial changes (1)
- apps/cli/src/channels/channelBridgeConfig.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- apps/cli/src/ui/doctor.test.ts
- apps/cli/src/cli/commands/bridge.test.ts
- apps/cli/src/channels/channelBridgeAccountConfig.test.ts
- apps/cli/src/channels/startChannelBridgeWorker.test.ts
- apps/cli/src/channels/channelBridgeConfig.test.ts
- apps/cli/src/channels/channelBindingStore.server.ts
- apps/cli/src/persistence.ts
- apps/cli/src/cli/dispatch.ts
- apps/cli/src/channels/telegram/telegramAdapter.test.ts
- docs/channel-bridge-uat.md
- apps/cli/src/channels/channelBridgeServerKv.test.ts
- apps/cli/src/channels/channelBindingStore.server.test.ts
Author
|
@greptile review |
cdenneen
added a commit
to cdenneen/happier-dev
that referenced
this pull request
Mar 4, 2026
cdenneen
force-pushed
the
feat/telegram-channel-adapter
branch
from
52227ea to
bfd28ab
Compare
Author
|
@coderabbitai review |
Author
|
@greptile review |
✅ Actions performedReview triggered.
|
Author
|
@greptile review |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/cli/src/daemon/startDaemon.automation.integration.test.ts (1)
265-267:⚠️ Potential issue | 🟡 MinorClear hoisted mock state between tests.
harness.startChannelBridgeFromEnvis created once invi.hoisted, butafterEachonly restores spies. Its call history survives across tests, so the Line 328toHaveBeenCalledTimes(1)assertion becomes order-dependent if the first test also boots the bridge.Suggested fix
afterEach(() => { + vi.clearAllMocks(); vi.restoreAllMocks(); });Based on learnings "Tests must be deterministic and isolated with no shared global state and no ordering reliance."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/daemon/startDaemon.automation.integration.test.ts` around lines 265 - 267, The hoisted mock harness.startChannelBridgeFromEnv (created via vi.hoisted) retains its call history across tests because afterEach only calls vi.restoreAllMocks(); add vi.clearAllMocks() (or vi.resetAllMocks() if you also want to reset implementations) to the afterEach so the harness.startChannelBridgeFromEnv call counts are cleared between tests and the toHaveBeenCalledTimes assertion becomes deterministic.
🧹 Nitpick comments (5)
docs/telegram-channel-bridge.md (1)
11-11: Make the architecture doc references clickable.Both
docs/channel-bridge.mdmentions render as plain code, not links. Converting them to Markdown links will make the guide much easier to navigate.As per coding guidelines "
**/*.md: Review documentation for clarity and accuracy, proper formatting, working links, code example correctness."Also applies to: 223-223
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/telegram-channel-bridge.md` at line 11, Replace the plain code-style reference `docs/channel-bridge.md` in docs/telegram-channel-bridge.md with a proper Markdown link so it becomes clickable (e.g., [docs/channel-bridge.md](docs/channel-bridge.md)); search for the exact text "For core bridge architecture, ingress mode matrix, and relay deployment model, see `docs/channel-bridge.md`" and update that backticked filename to a Markdown link, and check the other occurrence around line 223 for the same replacement.apps/cli/src/persistence.ts (1)
99-102: GivechannelBridgea typed settings shape.Adding this field as
unknownthrows away type checking on a config surface that is now read in multiple places. Even a minimalChannelBridgeSettingsinterface here would catch schema drift before runtime.As per coding guidelines "No untyped code in TypeScript - use strict typing with explicit parameter and return types" and "Prefer
interfaceovertypefor defining object shapes in TypeScript".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/persistence.ts` around lines 99 - 102, Replace the untyped channelBridge property with a concrete interface: define an exported interface ChannelBridgeSettings (with the minimal expected fields used elsewhere—e.g., enabled: boolean, url?: string, token?: string—or whatever properties the code reads) and change the channelBridge declaration from "channelBridge?: unknown" to "channelBridge?: ChannelBridgeSettings"; update any places that access channelBridge to use the typed properties and add explicit types to functions/methods that accept or return ChannelBridgeSettings (reference symbol: channelBridge and new ChannelBridgeSettings interface).apps/cli/src/channels/channelBindingStore.server.ts (1)
1-11: Align this new CLI module with repo conventions.The file is missing the required file-level JSDoc header, and the internal source imports are still relative. Please add the header and switch source imports to
@/…so the new module matches the rest ofapps/cli.As per coding guidelines "Include comprehensive JSDoc comments as file header comments explaining file responsibilities" and "Use
@/alias for src imports (e.g., import { logger } from '@/ui/logger')".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/channels/channelBindingStore.server.ts` around lines 1 - 11, Add a file-level JSDoc header describing this module's responsibility (managing channel binding storage for the CLI) and switch the relative internal imports to use the "@/…" source alias so the module matches repo conventions; specifically change imports referencing './core/channelBridgeWorker' and './channelBridgeServerKv' to '@/channels/core/channelBridgeWorker' and '@/channels/channelBridgeServerKv' (or the correct repo src path) and keep the existing import of logger from '@/ui/logger'; ensure the JSDoc includes brief summary, author/maintainer and expected exports such as ChannelBindingStore, ChannelBridgeConversationRef, ChannelSessionBinding, decodeChannelBridgeBindingsDocFromBase64, readChannelBridgeBindingsFromKv, writeChannelBridgeBindingsToKv, ChannelBridgeKvVersionMismatchError and ChannelBridgeBadPayloadError.apps/cli/src/cli/commands/bridge.ts (1)
225-226: Consider rejecting empty--bot-tokenvalues.When
--bot-token ""is passed,botTokenbecomes an empty string after trim, andif (botToken)evaluates to false, so the empty value is silently ignored. This is probably fine, but if the user explicitly passes an empty token, it might indicate user error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/cli/commands/bridge.ts` around lines 225 - 226, The current check silently ignores an explicitly passed empty --bot-token (botToken becomes empty and if (botToken) is false); change the logic around the botToken variable so that if the flag was provided but the trimmed value is empty you reject it (throw an error or print a usage error) instead of silently ignoring it: detect that botToken is not undefined (the flag was passed), then if botToken.trim() === "" return/throw a clear error, otherwise set update.botToken = botToken; keep references to the existing botToken variable and update.botToken assignment in the bridge command handler.apps/cli/src/ui/doctor.ts (1)
84-95: Port parsing allows negative integers but validation rejects them later.
parseStrictWebhookPortaccepts negative strings (regex^[-]?\d+$) and returns their parsed value. Later validation incollectMissingRequiredWebhookFieldsrejects ports<= 0, so this works correctly end-to-end. However, returning a negative port from a function named "strict" is misleading and could cause confusion if this helper is reused elsewhere.Consider rejecting negatives at parse time for consistency:
♻️ Suggested refinement
function parseStrictWebhookPort(value: unknown): number | null { if (typeof value === 'number' && Number.isFinite(value)) { - return Math.trunc(value); + const truncated = Math.trunc(value); + return truncated > 0 && truncated <= 65_535 ? truncated : null; } if (typeof value === 'string') { const trimmed = value.trim(); - if (!/^[-]?\d+$/.test(trimmed)) return null; + if (!/^\d+$/.test(trimmed)) return null; const parsed = Number.parseInt(trimmed, 10); - return Number.isFinite(parsed) ? parsed : null; + return Number.isFinite(parsed) && parsed > 0 && parsed <= 65_535 ? parsed : null; } return null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/ui/doctor.ts` around lines 84 - 95, parseStrictWebhookPort currently accepts negative integer strings (regex allows '-' and returns negative numbers), which conflicts with later validation in collectMissingRequiredWebhookFields that rejects ports <= 0; update parseStrictWebhookPort to reject negative values at parse time by adjusting the validation to only accept non-negative integer strings and return null for negative results so the helper's "strict" behavior aligns with collectMissingRequiredWebhookFields and avoids reuse surprises.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/cli/src/channels/channelBindingStore.server.ts`:
- Around line 94-119: In load(), when readChannelBridgeBindingsFromKv throws a
recoverable decode error (checked by isRecoverableBindingsReadError), do not
replace the cache with an empty bindings array; instead if cache exists return
the last known-good cache (cache) (optionally updating fetchedAtMs) and only if
cache is undefined rethrow the error so the caller surfaces the failure—remove
the logic that sets bindings: [] and ensure the logger still records the decode
problem; this keeps the previous valid state instead of silently failing closed.
In `@apps/cli/src/channels/telegram/telegramWebhookRelay.ts`:
- Around line 38-42: The current code accepts params.host verbatim (host) which
allows non-loopback addresses like 0.0.0.0; update the validation to reject any
non-loopback bind address for this daemon-local relay by checking the resolved
host value (host / params.host) and throwing an error unless it is a loopback
address (e.g. '127.0.0.1', any '127.*.*.*' IPv4, '::1', or 'localhost');
implement this using Node's net.isIP(host) to distinguish IPv4/IPv6 and for IPv4
treat any address starting with '127.' as loopback, otherwise throw the same
kind of Error so the listener cannot be bound to 0.0.0.0 or other non-local
interfaces.
---
Outside diff comments:
In `@apps/cli/src/daemon/startDaemon.automation.integration.test.ts`:
- Around line 265-267: The hoisted mock harness.startChannelBridgeFromEnv
(created via vi.hoisted) retains its call history across tests because afterEach
only calls vi.restoreAllMocks(); add vi.clearAllMocks() (or vi.resetAllMocks()
if you also want to reset implementations) to the afterEach so the
harness.startChannelBridgeFromEnv call counts are cleared between tests and the
toHaveBeenCalledTimes assertion becomes deterministic.
---
Nitpick comments:
In `@apps/cli/src/channels/channelBindingStore.server.ts`:
- Around line 1-11: Add a file-level JSDoc header describing this module's
responsibility (managing channel binding storage for the CLI) and switch the
relative internal imports to use the "@/…" source alias so the module matches
repo conventions; specifically change imports referencing
'./core/channelBridgeWorker' and './channelBridgeServerKv' to
'@/channels/core/channelBridgeWorker' and '@/channels/channelBridgeServerKv' (or
the correct repo src path) and keep the existing import of logger from
'@/ui/logger'; ensure the JSDoc includes brief summary, author/maintainer and
expected exports such as ChannelBindingStore, ChannelBridgeConversationRef,
ChannelSessionBinding, decodeChannelBridgeBindingsDocFromBase64,
readChannelBridgeBindingsFromKv, writeChannelBridgeBindingsToKv,
ChannelBridgeKvVersionMismatchError and ChannelBridgeBadPayloadError.
In `@apps/cli/src/cli/commands/bridge.ts`:
- Around line 225-226: The current check silently ignores an explicitly passed
empty --bot-token (botToken becomes empty and if (botToken) is false); change
the logic around the botToken variable so that if the flag was provided but the
trimmed value is empty you reject it (throw an error or print a usage error)
instead of silently ignoring it: detect that botToken is not undefined (the flag
was passed), then if botToken.trim() === "" return/throw a clear error,
otherwise set update.botToken = botToken; keep references to the existing
botToken variable and update.botToken assignment in the bridge command handler.
In `@apps/cli/src/persistence.ts`:
- Around line 99-102: Replace the untyped channelBridge property with a concrete
interface: define an exported interface ChannelBridgeSettings (with the minimal
expected fields used elsewhere—e.g., enabled: boolean, url?: string, token?:
string—or whatever properties the code reads) and change the channelBridge
declaration from "channelBridge?: unknown" to "channelBridge?:
ChannelBridgeSettings"; update any places that access channelBridge to use the
typed properties and add explicit types to functions/methods that accept or
return ChannelBridgeSettings (reference symbol: channelBridge and new
ChannelBridgeSettings interface).
In `@apps/cli/src/ui/doctor.ts`:
- Around line 84-95: parseStrictWebhookPort currently accepts negative integer
strings (regex allows '-' and returns negative numbers), which conflicts with
later validation in collectMissingRequiredWebhookFields that rejects ports <= 0;
update parseStrictWebhookPort to reject negative values at parse time by
adjusting the validation to only accept non-negative integer strings and return
null for negative results so the helper's "strict" behavior aligns with
collectMissingRequiredWebhookFields and avoids reuse surprises.
In `@docs/telegram-channel-bridge.md`:
- Line 11: Replace the plain code-style reference `docs/channel-bridge.md` in
docs/telegram-channel-bridge.md with a proper Markdown link so it becomes
clickable (e.g., [docs/channel-bridge.md](docs/channel-bridge.md)); search for
the exact text "For core bridge architecture, ingress mode matrix, and relay
deployment model, see `docs/channel-bridge.md`" and update that backticked
filename to a Markdown link, and check the other occurrence around line 223 for
the same replacement.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 165dbcee-20d7-40f8-bb5b-1a15f199e1c1
⛔ Files ignored due to path filters (3)
docs/assets/doctor-critical-summary.pngis excluded by!**/*.pngdocs/assets/doctor-missing-host-port-summary.pngis excluded by!**/*.pngyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (32)
README.mdapps/cli/src/channels/channelBindingStore.server.test.tsapps/cli/src/channels/channelBindingStore.server.tsapps/cli/src/channels/channelBridgeAccountConfig.test.tsapps/cli/src/channels/channelBridgeAccountConfig.tsapps/cli/src/channels/channelBridgeConfig.test.tsapps/cli/src/channels/channelBridgeConfig.tsapps/cli/src/channels/channelBridgeServerConfigOverlay.test.tsapps/cli/src/channels/channelBridgeServerConfigOverlay.tsapps/cli/src/channels/channelBridgeServerKv.test.tsapps/cli/src/channels/channelBridgeServerKv.tsapps/cli/src/channels/core/channelBridgeWorker.test.tsapps/cli/src/channels/core/channelBridgeWorker.tsapps/cli/src/channels/startChannelBridgeWorker.startupFailure.test.tsapps/cli/src/channels/startChannelBridgeWorker.test.tsapps/cli/src/channels/startChannelBridgeWorker.tsapps/cli/src/channels/telegram/telegramAdapter.test.tsapps/cli/src/channels/telegram/telegramAdapter.tsapps/cli/src/channels/telegram/telegramWebhookRelay.test.tsapps/cli/src/channels/telegram/telegramWebhookRelay.tsapps/cli/src/cli/commandRegistry.tsapps/cli/src/cli/commands/bridge.test.tsapps/cli/src/cli/commands/bridge.tsapps/cli/src/cli/dispatch.tsapps/cli/src/daemon/startDaemon.automation.integration.test.tsapps/cli/src/daemon/startDaemon.tsapps/cli/src/persistence.tsapps/cli/src/ui/doctor.test.tsapps/cli/src/ui/doctor.tsdocs/README.mddocs/channel-bridge-uat.mddocs/telegram-channel-bridge.md
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/cli/src/channels/channelBridgeServerKv.test.ts
- docs/channel-bridge-uat.md
- apps/cli/src/channels/channelBridgeServerConfigOverlay.test.ts
- apps/cli/src/channels/startChannelBridgeWorker.ts
- apps/cli/src/cli/commandRegistry.ts
- docs/README.md
- apps/cli/src/channels/channelBridgeAccountConfig.test.ts
Author
|
Superseded by #115 (clean rebase + squash replacement). Keeping this PR for history/context only. |
Author
|
Closing as superseded by #115 (clean replacement PR). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is stacked on top of PR1 (channel bridge core).
Please review against base branch:
feat/channel-bridge-core.Summary
This PR adds Telegram support on top of the provider-agnostic channel bridge core, including:
What’s Included
Telegram adapter + runtime wiring
apps/cli/src/channels/telegram/telegramAdapter.tsapps/cli/src/channels/telegram/telegramAdapter.test.tsapps/cli/src/channels/telegram/telegramWebhookRelay.tsapps/cli/src/channels/telegram/telegramWebhookRelay.test.tsapps/cli/src/channels/startChannelBridgeWorker.tsapps/cli/src/channels/startChannelBridgeWorker.test.tsScoped bridge configuration + persistence
apps/cli/src/channels/channelBridgeAccountConfig.tsapps/cli/src/channels/channelBridgeConfig.tsapps/cli/src/channels/channelBridgeServerKv.tsapps/cli/src/channels/channelBridgeServerConfigOverlay.tsapps/cli/src/channels/channelBindingStore.server.tsCLI management
happier bridgecommand group:apps/cli/src/cli/commands/bridge.tsapps/cli/src/cli/commands/bridge.test.tsapps/cli/src/cli/commandRegistry.tsapps/cli/src/cli/dispatch.tsSecurity / Secret Handling
Implemented local-only secret policy for bridge credentials:
botToken/webhookSecretstay local (settings.json/ env)This establishes the same pattern for future adapters (Discord/Slack/WhatsApp/etc).
Doctor Diagnostics Improvements
Updated doctor behavior to surface bridge health as real pass/fail:
✅ Doctor diagnosis complete!only when no critical failures❌ Doctor diagnosis complete!when any critical failure existswebhook.enabled=true:webhook.secretwebhook.hostwebhook.portDocumentation
docs/telegram-channel-bridge.mddocs/channel-bridge-uat.mdREADME.mdScreenshot evidence
Tests
Added/updated focused tests across:
Representative files:
apps/cli/src/channels/channelBridgeAccountConfig.test.tsapps/cli/src/channels/channelBridgeConfig.test.tsapps/cli/src/channels/channelBridgeServerKv.test.tsapps/cli/src/channels/channelBridgeServerConfigOverlay.test.tsapps/cli/src/channels/channelBindingStore.server.test.tsapps/cli/src/channels/startChannelBridgeWorker.test.tsapps/cli/src/channels/telegram/telegramAdapter.test.tsapps/cli/src/channels/telegram/telegramWebhookRelay.test.tsapps/cli/src/cli/commands/bridge.test.tsapps/cli/src/ui/doctor.test.tsSuggestions:
Summary by CodeRabbit
New Features
Documentation
Improvements
Tests