refactor(api): extract the Telegram Bot adapter into its own package - #397
Merged
Merged
Conversation
The channel-neutral contracts already live in @wateaminbox/shared, but the Telegram provider sat inside apps/api, so nothing stopped it from reaching for application internals. Move it to @wateaminbox/adapter-telegram, which imports only @wateaminbox/shared, node:crypto, and bun:test. adapter.ts, api.ts, and normalize.ts move unchanged - they had no app-internal imports to begin with. Only transport.ts was coupled, through three functions, so it now takes injected ports instead: resolveOutboundContext -> credential read and conversation lookup resolveAttachmentUrl -> media presigning Both are bound in apps/api/.../telegram-bot/ports.ts and injected by the registry, which remains the composition root. Tenant database and media storage knowledge stays with the application; Bot API knowledge stays in the package. Two things beyond a straight move: parseTelegramThreadTarget moves into the package. The <chatId>[:thread:<topicId>] encoding is Telegram's, even though the host owns the column that stores it. The local-failure protocol is now explicit. TELEGRAM_LOCAL_FAILURE_CODES and TelegramLocalFailureError are exported, and the host raises from that closed set rather than throwing bare strings that classifyTelegramSendFailure happened to match. The error message is still the code, so classification is unchanged - anything outside the set degrades to "uncertain" and strands a send instead of failing it, which is why the set is closed. resolveOutboundContext had no test before: transport.ts's only test covered failure classification. Cover it with ports.integration.test.ts, asserting the forum-topic split and that each unsendable conversation refuses with its specific code - malformed thread id, unmapped thread, archived conversation, paused account, another account's conversation, missing credential. run-tests.sh enumerates test directories explicitly, so the moved tests would have silently left CI without a new line. The root typecheck chain builds the package before tsc, since apps/api resolves its dist types. The integration runner gains a synthetic credential keyring, fixed and non-secret, because the adapter reads credentials through the configured cipher. WhatsApp is deliberately left in place: its transport depends on NATS, message formatting, the command outbox, and session state, and its normalizer imports MessageEvent from lib/nats.
Main's credential-key handling (#391, #393) landed on the three provider files this branch moved into @wateaminbox/adapter-telegram, and git followed the renames - carrying `../../../services/channel-credential.service.js` imports into a package that may not have them. Both intents are kept, re-plumbed across the package boundary rather than either side being dropped. Ingress. Main rethrows ChannelCredentialKeyError so the route answers 503 instead of telling a legitimate sender its secret is wrong. The adapter cannot recognise an application error type, so it takes an `isCredentialUnavailable` predicate alongside the resolver it already took; the registry supplies `(error) => error instanceof ChannelCredentialKeyError`. The original instance is rethrown untouched, so channel-ingress.ts keeps main's `instanceof` check unchanged - which matters because that route is provider-generic and must not learn about provider packages. Sending. `telegram_credential_key_unavailable` joins TELEGRAM_LOCAL_FAILURE_CODES, so classification needs no new branch: the closed set already maps a local failure code to permanent_failure. The translation lives in the host's port, where `readBotToken` turns ChannelCredentialKeyError into the adapter's code. Missing it anywhere would let the raw error reach classifyTelegramSendFailure as unrecognised and be called "uncertain" - never retried - which is the bug main fixed, so the package test asserts the classification and the ports integration test asserts the translation by storing a credential under a retired key version. Also adopted main's CHANNEL_INGRESS_PUBLIC_URL in place of pointing APP_URL at the development tunnel, which is the same separation done properly: a webhook needs a publicly routable address while APP_URL builds invite and OAuth links.
Both image builds enumerate every workspace manifest by hand and then run
`bun install --frozen-lockfile`, so a new package that the lockfile knows about
but the build context does not is a hard failure:
error: Workspace dependency "@wateaminbox/adapter-telegram" not found
error: @wateaminbox/adapter-telegram@workspace:* failed to resolve
Nothing on the host catches this. `bun run build` succeeds there because every
workspace directory exists; only the image sees a tree assembled from an
explicit list. Web fails the same way even though its bundle never imports the
adapter - the install has to resolve the whole workspace.
The API also needs the source and a build step, since `bun build --target bun`
inlines the adapter into apps/api/dist. The runtime stage needs nothing further
for the same reason, which is why no dist is copied there.
Verified by building both images: the API image starts from a tree that
resolves, and its bundle contains the adapter.
The ts-unit and ts-integration jobs build internal TypeScript dependencies by
name, and the new package was not among them. Its package manifest points at
`./dist/index.js`, so on a clean checkout every importer failed to resolve:
error: Cannot find module '@wateaminbox/adapter-telegram'
from apps/api/src/channel-spine/registry.ts
49 unit tests failed this way. A local run hid it because the dist was already
on disk from an earlier build; deleting it reproduces CI exactly.
The unit job also names each test location, so the package's own 25 tests are
added explicitly - the same omission that would have let them leave CI silently.
The static job is unaffected: it runs turbo, which builds the whole workspace.
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
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.
The channel-neutral contracts already live in
@wateaminbox/shared, but theTelegram provider sat inside
apps/api, so nothing stopped it from reaching forapplication internals. This moves it to
@wateaminbox/adapter-telegram, whoseonly imports are
@wateaminbox/shared,node:crypto, andbun:test.The cut
adapter.ts,api.ts, andnormalize.tsmove unchanged — they had noapp-internal imports to begin with. Only
transport.tswas coupled, throughthree functions, so it now takes injected ports:
resolveOutboundContextresolveAttachmentUrlBoth are bound in
apps/api/.../telegram-bot/ports.tsand injected by theregistry, which remains the composition root. Tenant database and media storage
knowledge stays with the application; Bot API knowledge stays in the package.
Two things beyond a straight move:
parseTelegramThreadTargetmoves into the package. The<chatId>[:thread:<topicId>]encoding is Telegram's, even though the host ownsthe column that stores it.
TELEGRAM_LOCAL_FAILURE_CODESand
TelegramLocalFailureErrorare exported, and the host raises from thatclosed set rather than throwing bare strings that
classifyTelegramSendFailurehappened to match. The message is still the code, so classification is
unchanged — anything outside the set degrades to
uncertainand strands a sendinstead of failing it, which is why the set is closed.
Merging main
Main's credential-key handling (#391, #393) landed on the three files this branch
moved, and git followed the renames — carrying
services/channel-credential.service.jsimports into a package that may not havethem. Both intents are kept, re-plumbed across the boundary:
an
isCredentialUnavailablepredicate alongside the resolver it already took;the registry supplies
(error) => error instanceof ChannelCredentialKeyError.The original instance is rethrown untouched, so
channel-ingress.tskeepsmain's
instanceofcheck unchanged — which matters, because that route isprovider-generic and must not learn about provider packages.
telegram_credential_key_unavailablejoins the closed code set, soclassification needed no new branch; the translation lives in the host's
readBotToken. Missing it anywhere would let the raw error reachclassifyTelegramSendFailureas unrecognised and be calleduncertain— neverretried — which is the bug main fixed. The package test asserts the
classification and the ports integration test asserts the translation, by
storing a credential under a retired key version.
New test coverage
resolveOutboundContextis the only logic this refactor rewrote, and it had notest before —
transport.ts's only test covered failure classification. The newports.integration.test.tscovers the forum-topic split and six refusals thatmust each arrive as a specific code: malformed thread id, unmapped thread,
archived conversation, paused account, another account's conversation, missing
credential, and a credential under a retired key version.
Workspace wiring
apps/apidependency, roottypecheckchain (the package must build beforetscorapps/apicannot resolve itsdisttypes), and aknip.jsonentry.scripts/run-tests.shenumerates test directories explicitly, so the movedtests would have silently left CI without a new line.
scripts/run-integration-tests-ts.shgains a synthetic, non-secret credentialkeyring, because the adapter reads credentials through the configured cipher.
Testing
bun run buildbun run typecheckpackages/adapter-telegramtestsapps/apiunit testsThe ports integration test was mutation-checked: inverting the account-status
guard and dropping the credential-key translation each fail it.
Not included
WhatsApp stays in
apps/api: its transport depends on NATS, message formatting,the command outbox, and session state, and its normalizer imports
MessageEventfrom
lib/nats. Extracting it means untangling the worker protocol.