From 474c49eb51d475f0b830dd270b292d0590587309 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Wed, 26 Aug 2026 03:30:47 -0700 Subject: [PATCH 1/9] =?UTF-8?q?docs(adr-025):=20audit=20the=20connector=20?= =?UTF-8?q?substrate=20=E2=80=94=20inbound=20bridges,=20not=20two-way=20sy?= =?UTF-8?q?nc?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TASK-079. Sam's framing was "we already support partial two-way." Read at origin/main rather than from the integration docs, "partial" turns out to mean request-scoped: both directions exist, but every outbound write in the backend is a reply inside an inbound request's own lifetime. telegramService exports one function with fourteen call sites, all in its own webhook route and no other file; discordService's two outbound POSTs are both Discord interaction endpoints; no Commonly-side event (pod message, reaction, task move) originates an outbound call anywhere. Four more findings with file:line behind each — the provider enum is a closed union that doubles as a dispatch key, `config` is a flat union of all eight providers' fields with a 1000-message buffer inline, connector credentials are plain String with zero encryption anywhere in backend/, and podId is singular so an org-wide connector means N copies of one credential. Six proposed decisions, none ratified. The landscape section is deliberately empty pending cl-strategist's TASK-078 memo; the audit does not depend on it, so it ships now rather than waiting. Also adds a scope-boundary note to ADR-007, which is the "integration strategy" document people reach for first and is about agent SDKs, not chat platforms. Co-Authored-By: Claude Opus 5 --- .../ADR-007-ecosystem-integration-strategy.md | 7 + docs/adr/ADR-025-connector-substrate.md | 198 ++++++++++++++++++ 2 files changed, 205 insertions(+) create mode 100644 docs/adr/ADR-025-connector-substrate.md diff --git a/docs/adr/ADR-007-ecosystem-integration-strategy.md b/docs/adr/ADR-007-ecosystem-integration-strategy.md index 1c4004a863..20645983ea 100644 --- a/docs/adr/ADR-007-ecosystem-integration-strategy.md +++ b/docs/adr/ADR-007-ecosystem-integration-strategy.md @@ -4,6 +4,13 @@ **Author:** Lily Shen **Companion:** [`ADR-004`](ADR-004-commonly-agent-protocol.md), [`ADR-005`](ADR-005-local-cli-wrapper-driver.md), [`ADR-006`](ADR-006-webhook-sdk-and-self-serve-install.md), [`COMMONLY_SCOPE.md`](../COMMONLY_SCOPE.md) +**Scope boundary — this ADR is not about platform connectors.** "Integration" here means agent +*SDKs*: how an agent built with the OpenAI Agents SDK or Vercel Open Agents reaches Commonly. +Discord, Slack, Telegram, GroupMe and the enterprise systems that follow are a different +substrate and belong to [`ADR-025`](ADR-025-connector-substrate.md). If you arrived here looking +for how a conversation crosses between Commonly and an outside chat platform, that is ADR-025, +not this document. + --- ## Context diff --git a/docs/adr/ADR-025-connector-substrate.md b/docs/adr/ADR-025-connector-substrate.md new file mode 100644 index 0000000000..a921a5ae29 --- /dev/null +++ b/docs/adr/ADR-025-connector-substrate.md @@ -0,0 +1,198 @@ +# ADR-025 — The connector substrate: from request-scoped bridges to a two-way driver + +**Status:** **Draft** — audit complete and measured; the decisions below are proposals for Sam. +Nothing here is ratified, and D1–D6 should not be cited as settled. The landscape section is +deliberately unfilled pending cl-strategist's comparison memo (TASK-078); the current-state audit +and the shape proposal do not depend on it, so they are written now rather than held. +**Date:** 2026-08-26 +**Author:** pod-architect (Lily Shen) +**Companions:** [`ADR-001`](ADR-001-installable-taxonomy.md) (the Installable model this should +become a component of), [`ADR-004`](ADR-004-commonly-agent-protocol.md) (CAP — the driver-facing +verbs), [`ADR-006`](ADR-006-webhook-sdk-and-self-serve-install.md) (the webhook driver, which is +the closest existing thing to a connector) + +**Scope boundary — read this before reaching for ADR-007.** [`ADR-007`](ADR-007-ecosystem-integration-strategy.md) +is also called an "integration strategy" and is the document people will find first. It is about a +different subject: agent *SDKs* (OpenAI Agents SDK, Vercel Open Agents) and how an agent built +elsewhere reaches Commonly. This ADR is about *platform connectors* — Discord, Slack, Telegram, +GroupMe, and the enterprise systems that follow — and how a conversation on one side reaches the +other. They do not overlap and neither supersedes the other. ADR-007 is at Draft; so is this. + +--- + +## Context + +Sam's framing: *"we already support partial two-way."* The audit below is an attempt to say +precisely which part, because "partial" turned out to name something narrower than it sounds. + +Everything in this section was read at `origin/main` (`1a29a177`) rather than taken from the +integration docs, and every claim carries the file and line that produced it. + +### Finding 1 — what exists is request-scoped two-way, not state-synced two-way + +Both directions do exist, and this is the distinction that matters for the redesign: + +- **Inbound** is real. `backend/routes/webhooks/{discord,slack,telegram,groupme}.ts` receive + external events, and agents can additionally *pull* external history through + `GET`-side handlers in `backend/routes/agentsRuntime.ts`. +- **Outbound exists only inside an inbound request's own lifetime.** Every outbound write in the + backend is a reply to something that just arrived: + - `services/telegramService.ts` exports exactly one function, `sendMessage`. It has eleven call + sites and all fourteen are inside `routes/webhooks/telegram.ts` — which is also the only + file in the backend that references the service at all. + - `services/discordService.ts` makes two outbound POSTs, both to Discord *interaction* + endpoints (`/webhooks/{applicationId}/{interactionToken}` and + `/interactions/{token}/callback`) — i.e. responses valid only within a live interaction token. + - `services/slackApi.ts` is 55 lines: `postMessage` and `history`. + +**No Commonly-side event originates an outbound call.** A message posted in a pod, a reaction, a +task moving on the board — none of these reach any connected platform. There is no fan-out path +from the message-write path to `Integration`, and grepping the backend for a relay verb +(`sendToDiscord`, `postTo…`, `relayTo…`, `forwardTo…`) returns nothing. + +*Scope of that claim:* it is a statement about this repository's backend. The openclaw gateway is +a separate submodule with its own tool surface and I did not read it for this; if it relays +independently, that changes the picture and should be checked before D1 is ratified. + +So "partial two-way" is precise if read as: **the platform can start a conversation with us; we +cannot start one with the platform.** For an enterprise buyer that is the whole feature — the +value of a Slack connector is that work happening in Commonly shows up in Slack, and today it +does not. + +### Finding 2 — a connector is a schema enum, not an installable + +`models/Integration.ts:96-101` types a connector as a closed enum: + +```ts +type: { + type: String, + required: true, + enum: ['discord', 'telegram', 'slack', 'messenger', 'groupme', 'whatsapp', 'x', 'instagram'], + default: 'discord', +}, +``` + +Adding a connector is therefore a schema change plus an edit to every `switch` on that value. +`routes/agentsRuntime.ts:3193` onward is the representative one — an `if / else if` chain on +`integration.type`, each arm doing its own credential check and its own `require()` of a +provider service, terminating in `Integration type ${integration.type} does not support message +fetching`. This is the shape ADR-001 exists to remove: a connector should be an Installable with +a component, discovered at install time, not a literal in a union type. + +Note the enum already contains types with no service behind them (`messenger`, `whatsapp`) and +types served only by a buffer read (`x`, `instagram`). The enum is a wish list and a dispatch key +at the same time, so nothing distinguishes "declared" from "implemented." + +### Finding 3 — `config` is a union of every provider's fields + +`models/Integration.ts:108-161` is one flat sub-document holding roughly forty keys drawn from +all eight providers at once: `serverId`, `channelId`, `webhookUrl`, `botToken`, `signingSecret`, +`groupId`, `chatId`, `chatType`, `accessToken`, `refreshToken`, `oauthScopes`, `igUserId`, +`followUsernames`, `lastExternalId`, and so on. + +Two consequences. Per-provider validation is impossible — every field is optional for every +provider, so a misconfigured Slack integration and a correct one are the same document shape, +and the failure surfaces at call time as a 400 rather than at save time. And every new connector +widens the record for all existing ones. + +`config.messageBuffer` is also here: an inline array of up to `maxBufferSize` (default **1000**) +messages, stored in the configuration document. Configuration and message data share one record +and one lifecycle. + +### Finding 4 — connector credentials are at rest in plaintext + +`botToken`, `signingSecret`, `secretToken`, `accessToken`, and `refreshToken` are declared as +bare `String` (`models/Integration.ts:115-127`), with no getter/setter, no `select: false`, and +no encryption layer anywhere in the backend — grepping the whole of `backend/` (excluding +`node_modules` and tests) for `encrypt`/`decrypt`/`createCipher` returns zero files. + +This is the single largest enterprise blocker in the audit, and it is worth being plain about the +scope: it is a design gap in how we store third-party credentials, not a known exploit. Any +enterprise security review reaches it in the first hour, and no amount of connector surface area +compensates for it. + +### Finding 5 — a connector binds to exactly one pod + +`models/Integration.ts:95`: `podId` is required and singular. An organisation that wants one +Slack workspace reflected across twenty pods needs twenty integration documents, twenty copies of +the same credential, and twenty things to rotate. ADR-001 already solved this shape for +Installables — one source-of-truth record projecting out to N runtime rows — and connectors did +not inherit it. + +--- + +## Landscape + +**Deliberately empty.** cl-strategist owns TASK-078: a comparison of how tutti, open-agents, +cumora, grok bot, and bloome handle connect / auth / two-way sync / permissions / enterprise +controls, from public documentation and observable behaviour only. When that memo lands, this +section carries it and D2–D4 get re-derived against it. + +Writing a competitive section from memory here would be the failure this ADR is otherwise trying +to name: a confident claim with no reader behind it. + +--- + +## Proposed decisions + +These follow from the audit alone. They are the parts that hold regardless of what the landscape +memo says; anything that depends on it is marked. + +**D1 — Name the current state honestly in the product surface.** Until an outbound path exists, +connectors are *inbound bridges with request-scoped replies*. They should not be described as +two-way sync in any UI, doc, or listing. This costs nothing and prevents the gap being discovered +by a customer. + +**D2 — A connector becomes an Installable component, not an enum member.** Introduce a +`Connector` component type under ADR-001 alongside `Agent`, `Webhook`, and the rest. The provider +enum in `models/Integration.ts` is retired in favour of a manifest-declared provider id, and the +`if/else` dispatch chains become a registry lookup. Additive per design rule 2: the existing +`Integration` rows keep working and are read through an adapter until they are migrated. + +**D3 — A connector declares its directions.** Each connector manifest states which of +`inbound`, `outbound`, and `sync` it actually implements, and the platform enforces it. Today +directionality is implicit in which service functions happen to exist, which is why "partial +two-way" was ambiguous enough to need this audit. A declared direction is checkable in CI. + +**D4 — Outbound needs an event fan-out, and that is the real build.** The missing piece is a +subscriber on the pod event stream that maps a Commonly-side event to a connector's outbound +verb. This is where `AgentEvent`'s existing queue shape is the natural prior art — durable, +retryable, per-target — rather than a synchronous call in the message-write path. Sizing and +delivery semantics depend on the landscape memo; the existence of the component does not. + +**D5 — Credentials move behind a secret reference before any new connector ships.** The +`Integration` document stores a reference, not the material. Whether that is ESO-backed, a +KMS envelope, or an application-level encryption layer is an implementation call I am not making +here, but shipping a sixth plaintext credential is a decision too, and it should be taken +deliberately rather than by default. + +**D6 — Connectors scope like Installables.** One connector record, projected to N pods, per +ADR-001's one-install-fans-out. This is what makes an enterprise install a single administrative +act instead of twenty. + +--- + +## What this ADR does not decide + +- **The wire format for outbound.** Whether outbound reuses CAP's verbs, the webhook driver's + payload shape, or something new is open, and the landscape memo should inform it. +- **Whether federation counts as a connector.** `services/federationService.ts` and + `routes/federation.ts` exist and may belong to this substrate or may be a separate axis. Not + investigated; naming it here so its absence is a gap rather than an oversight. +- **Retention of `config.messageBuffer`.** Finding 3 says it should not live in the config + document; it does not say where it goes. +- **Anything requiring the landscape.** D2's manifest fields, D4's delivery semantics, and the + enterprise-controls surface (audit log, per-channel permissions, data residency) are all + under-specified on purpose until TASK-078 lands. + +--- + +## Open question for Sam + +The audit says the honest headline is *"we have inbound connectors, not two-way sync."* That is a +more negative starting position than "partial two-way support" implies, and it changes what the +redesign is: not an extension of something working, but the first build of the outbound half. + +Do you want this ADR to lead with that framing, or is there an outbound path outside this +backend — in the gateway, or in an integration I have not read — that I have missed? I would +rather be corrected here than have the enterprise pitch inherit the wrong premise. From 82e992b41185801841a97afca86645add126ab88 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Wed, 26 Aug 2026 03:41:15 -0700 Subject: [PATCH 2/9] =?UTF-8?q?docs(adr-025):=20re-derive=20the=20audit=20?= =?UTF-8?q?=E2=80=94=20outbound=20exists,=20synchronisation=20does=20not?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sprint-review falsified the first version's headline and was right. Three corrections, and the method failure behind them is recorded in the ADR because it is the reusable part. 1. Outbound is real. discordService.ts:401 POSTs to a stored channel webhookUrl under routes/integrations.ts:347, and routes/agentsRuntime.ts:3354 lets an AGENT call provider.publishPost under a daily cap with per-agent attribution. Agents already publish to X and Instagram. The first draft said we could not start a conversation with the platform; we can. 2. backend/integrations/ was never opened — it holds a provider registry, per- provider manifests with requiredConfig + configSchema, and the packages/integration-sdk package. "A connector is a schema enum, not an installable" was too strong; the real defect is that the enum and the registry are two live sources of truth for the same question. 3. Enumerating what each provider implements gives the sharper finding: the registry's ONLY outbound verb is publishPost, and it exists on exactly the two social-broadcast providers and none of the four chat providers. Discord's send never became a provider verb, Slack's returns 'not-implemented'. The connectors an enterprise buys are the ones with no conversational outbound. The genuine gap is narrower than "outbound" and more interesting: nothing is driven by a Commonly-side event. Every existing path is an inbound reply, a human button, or an agent's explicit publish. D1 now asks to stop claiming sync rather than to stop claiming outbound. The original grep required a send-verb and an HTTP call on the same source line and found 2 of 10 outbound calls. A conjunctive same-line filter is not a search for a negative, and a directory you did not open cannot be reported as absent. Co-Authored-By: Claude Opus 5 --- docs/adr/ADR-025-connector-substrate.md | 314 +++++++++++++----------- 1 file changed, 172 insertions(+), 142 deletions(-) diff --git a/docs/adr/ADR-025-connector-substrate.md b/docs/adr/ADR-025-connector-substrate.md index a921a5ae29..2f5c279342 100644 --- a/docs/adr/ADR-025-connector-substrate.md +++ b/docs/adr/ADR-025-connector-substrate.md @@ -1,7 +1,8 @@ -# ADR-025 — The connector substrate: from request-scoped bridges to a two-way driver +# ADR-025 — The connector substrate: outbound exists, synchronisation does not -**Status:** **Draft** — audit complete and measured; the decisions below are proposals for Sam. -Nothing here is ratified, and D1–D6 should not be cited as settled. The landscape section is +**Status:** **Draft** — audit re-derived after sprint-review falsified the first version's +headline; the decisions below are proposals for Sam. Nothing here is ratified, and D1–D7 should +not be cited as settled. The landscape section is deliberately unfilled pending cl-strategist's comparison memo (TASK-078); the current-state audit and the shape proposal do not depend on it, so they are written now rather than held. **Date:** 2026-08-26 @@ -23,101 +24,122 @@ other. They do not overlap and neither supersedes the other. ADR-007 is at Draft ## Context Sam's framing: *"we already support partial two-way."* The audit below is an attempt to say -precisely which part, because "partial" turned out to name something narrower than it sounds. - -Everything in this section was read at `origin/main` (`1a29a177`) rather than taken from the -integration docs, and every claim carries the file and line that produced it. - -### Finding 1 — what exists is request-scoped two-way, not state-synced two-way - -Both directions do exist, and this is the distinction that matters for the redesign: - -- **Inbound** is real. `backend/routes/webhooks/{discord,slack,telegram,groupme}.ts` receive - external events, and agents can additionally *pull* external history through - `GET`-side handlers in `backend/routes/agentsRuntime.ts`. -- **Outbound exists only inside an inbound request's own lifetime.** Every outbound write in the - backend is a reply to something that just arrived: - - `services/telegramService.ts` exports exactly one function, `sendMessage`. It has eleven call - sites and all fourteen are inside `routes/webhooks/telegram.ts` — which is also the only - file in the backend that references the service at all. - - `services/discordService.ts` makes two outbound POSTs, both to Discord *interaction* - endpoints (`/webhooks/{applicationId}/{interactionToken}` and - `/interactions/{token}/callback`) — i.e. responses valid only within a live interaction token. - - `services/slackApi.ts` is 55 lines: `postMessage` and `history`. - -**No Commonly-side event originates an outbound call.** A message posted in a pod, a reaction, a -task moving on the board — none of these reach any connected platform. There is no fan-out path -from the message-write path to `Integration`, and grepping the backend for a relay verb -(`sendToDiscord`, `postTo…`, `relayTo…`, `forwardTo…`) returns nothing. - -*Scope of that claim:* it is a statement about this repository's backend. The openclaw gateway is -a separate submodule with its own tool surface and I did not read it for this; if it relays -independently, that changes the picture and should be checked before D1 is ratified. - -So "partial two-way" is precise if read as: **the platform can start a conversation with us; we -cannot start one with the platform.** For an enterprise buyer that is the whole feature — the -value of a Slack connector is that work happening in Commonly shows up in Slack, and today it -does not. - -### Finding 2 — a connector is a schema enum, not an installable - -`models/Integration.ts:96-101` types a connector as a closed enum: - -```ts -type: { - type: String, - required: true, - enum: ['discord', 'telegram', 'slack', 'messenger', 'groupme', 'whatsapp', 'x', 'instagram'], - default: 'discord', -}, -``` - -Adding a connector is therefore a schema change plus an edit to every `switch` on that value. -`routes/agentsRuntime.ts:3193` onward is the representative one — an `if / else if` chain on -`integration.type`, each arm doing its own credential check and its own `require()` of a -provider service, terminating in `Integration type ${integration.type} does not support message -fetching`. This is the shape ADR-001 exists to remove: a connector should be an Installable with -a component, discovered at install time, not a literal in a union type. - -Note the enum already contains types with no service behind them (`messenger`, `whatsapp`) and -types served only by a buffer read (`x`, `instagram`). The enum is a wish list and a dispatch key -at the same time, so nothing distinguishes "declared" from "implemented." - -### Finding 3 — `config` is a union of every provider's fields - -`models/Integration.ts:108-161` is one flat sub-document holding roughly forty keys drawn from -all eight providers at once: `serverId`, `channelId`, `webhookUrl`, `botToken`, `signingSecret`, -`groupId`, `chatId`, `chatType`, `accessToken`, `refreshToken`, `oauthScopes`, `igUserId`, -`followUsernames`, `lastExternalId`, and so on. - -Two consequences. Per-provider validation is impossible — every field is optional for every -provider, so a misconfigured Slack integration and a correct one are the same document shape, -and the failure surfaces at call time as a 400 rather than at save time. And every new connector -widens the record for all existing ones. - -`config.messageBuffer` is also here: an inline array of up to `maxBufferSize` (default **1000**) -messages, stored in the configuration document. Configuration and message data share one record -and one lifecycle. - -### Finding 4 — connector credentials are at rest in plaintext - -`botToken`, `signingSecret`, `secretToken`, `accessToken`, and `refreshToken` are declared as -bare `String` (`models/Integration.ts:115-127`), with no getter/setter, no `select: false`, and -no encryption layer anywhere in the backend — grepping the whole of `backend/` (excluding -`node_modules` and tests) for `encrypt`/`decrypt`/`createCipher` returns zero files. - -This is the single largest enterprise blocker in the audit, and it is worth being plain about the -scope: it is a design gap in how we store third-party credentials, not a known exploit. Any -enterprise security review reaches it in the first hour, and no amount of connector surface area -compensates for it. - -### Finding 5 — a connector binds to exactly one pod - -`models/Integration.ts:95`: `podId` is required and singular. An organisation that wants one -Slack workspace reflected across twenty pods needs twenty integration documents, twenty copies of -the same credential, and twenty things to rotate. ADR-001 already solved this shape for -Installables — one source-of-truth record projecting out to N runtime rows — and connectors did -not inherit it. +precisely which part. + +**Correction, and it is the reason to trust the rest of this section.** The first version of this +ADR claimed there was no outbound path at all. That was wrong, caught by sprint-review, and wrong +for a reason worth recording: I searched for a negative with a grep that required a send-verb and +an HTTP call on the same source line, which found two outbound calls out of ten — and I never +opened `backend/integrations/`, where the provider registry, the per-provider manifests, and the +`packages/integration-sdk` package live. A conjunctive same-line filter is not a search for a +negative, and a directory you did not open cannot be reported as absent. What follows was +re-derived by enumerating every outbound HTTP call in the connector code and every method each +provider actually implements. + +Everything below was read at `origin/main` (`1a29a177`). + +### Finding 1 — outbound exists in three modes, and none of them is event-driven + +There are ten outbound HTTP calls in the connector code (`services/{discord,slack,telegram,groupme}*` +plus `integrations/providers/*`). They fall into three distinct trigger modes: + +1. **Request-scoped replies.** `services/telegramService.ts`'s single `sendMessage` has fourteen + call sites, all inside `routes/webhooks/telegram.ts` — the only file in the backend that + references it. `services/discordService.ts:1116` and `:1187` are both Discord *interaction* + endpoints, valid only within a live interaction token. These are answers to something that + just arrived. +2. **Owner-triggered manual send.** `routes/integrations.ts:347` (`POST /:id/send`, human JWT, + and gated to `pod.createdBy` alone) calls `DiscordService.sendMessage` + (`services/discordService.ts:401`), which POSTs to a *stored* channel `webhookUrl` — a durable + credential, not an interaction token. This is a real outbound path and the first draft of this + ADR missed it. +3. **Agent-originated publish.** `routes/agentsRuntime.ts:3354` resolves a provider from the + registry and calls `provider.publishPost(...)`, under a daily cap + (`INTEGRATION_PUBLISH_DAILY_LIMIT`) with per-agent attribution written back to + `config.lastAgentPublishBy`. Agents can and do originate outbound posts. + +**What is uniformly absent is mode 4: a Commonly-side *event* originating an outbound call.** A +pod message, a reaction, or a task moving on the board reaches nothing. Every path above is +triggered by an inbound request, a human pressing a button, or an agent explicitly deciding to +publish. Nothing mirrors. + +So "partial two-way" is accurate, and the precise missing piece is narrower and more interesting +than "outbound": it is **synchronisation**. + +### Finding 2 — the registry's only outbound verb is `publishPost`, and no chat provider has it + +`backend/integrations/` already contains the abstraction ADR-001 would ask for: a provider +registry (`integrations/index.ts`, backed by `packages/integration-sdk/src/registry.js`), and +per-provider manifests carrying `requiredConfig`, a generated `configSchema`, and a declared +`capabilities` list (`integrations/manifests.ts`). + +Enumerating what each of the six providers actually implements: + +| provider | validateConfig | ingestEvent | syncRecent | health | publishPost | +|---|---|---|---|---|---| +| discord | ✓ | ✓ | ✓ | ✓ | — | +| slack | ✓ | ✓ | ✓ | ✓ | — | +| telegram | ✓ | ✓ | ✓ | ✓ | — | +| groupme | ✓ | ✓ | ✓ | ✓ | — | +| x | ✓ | ✓ | ✓ | ✓ | **✓** | +| instagram | ✓ | ✓ | ✓ | ✓ | **✓** | + +Four of the five verbs are inbound or health. The one outbound verb, `publishPost`, is implemented +by exactly the two *social broadcast* providers and by none of the four *chat* providers. The SDK's +shared types (`packages/integration-sdk/src/types.js`) are `NormalizedMessage` and +`NormalizedSummaryInput` — both inbound shapes; there is no normalized outbound message at all. + +**This is the enterprise finding.** Discord's outbound send exists (Finding 1, mode 2) but lives +*outside* the registry, in a service method reachable from one legacy owner-only route. It never +became a provider verb, so nothing else in the system can reach it and no other chat provider had +a shape to copy. Slack's send is literally `return res.json({ success: true, result: 'not-implemented' })` +at `routes/integrations.ts:360`. The connectors an enterprise actually buys — Slack, Teams-shaped, +Discord — are the ones with no conversational outbound in the abstraction. + +### Finding 3 — the provider enum and the registry disagree about what a connector is + +`models/Integration.ts:96-101` types a connector as a closed enum of eight strings, and +`routes/agentsRuntime.ts:3193` dispatches on it with an `if / else if` chain that does its own +per-provider credential check and its own `require()`. Meanwhile `integrations/index.ts` resolves +providers from a registry keyed by the same string. + +Both mechanisms are live, in the same file in at least one case. So a connector is *simultaneously* +a registry entry and a schema literal, and the enum contains entries (`messenger`, `whatsapp`) with +no provider registered at all — the enum is a wish list and a dispatch key at once, and nothing +distinguishes "declared" from "implemented." + +The first draft called this "a schema enum, not an installable." That was too strong: the registry +exists and is good. The accurate defect is the **duplication** — two sources of truth for the same +question, one of which cannot be extended without a schema migration. + +### Finding 4 — `config` is a union of every provider's fields + +`models/Integration.ts:108-161` is one flat sub-document holding roughly forty keys drawn from all +eight providers at once. Note this coexists with the manifests' per-provider `configSchema`: the +schema knows Slack needs `botToken`, `signingSecret`, `channelId`, but the storage model accepts +any of the forty for any provider. The validation exists and the persistence layer does not enforce it. + +`config.messageBuffer` also lives here: an inline array of up to `maxBufferSize` (default **1000**) +messages inside the configuration document, so config and message data share one record and one +lifecycle. + +### Finding 5 — connector credentials are at rest in plaintext + +`botToken`, `signingSecret`, `secretToken`, `accessToken`, and `refreshToken` are declared as bare +`String` (`models/Integration.ts:115-127`), with no getter/setter, no `select: false`, and no +encryption layer anywhere — grepping the whole of `backend/` (excluding `node_modules` and tests) +for `encrypt`/`decrypt`/`createCipher` returns zero files. + +This is the largest enterprise blocker in the audit. It is a design gap in how we store third-party +credentials, not a known exploit, and it is the kind of thing any enterprise security review reaches +in its first hour. + +### Finding 6 — a connector binds to exactly one pod + +`models/Integration.ts:95`: `podId` is required and singular. An organisation wanting one Slack +workspace reflected across twenty pods needs twenty documents and twenty copies of one credential +to rotate. ADR-001 solved this shape for Installables — one source-of-truth record projecting to N +runtime rows — and connectors did not inherit it. --- @@ -135,53 +157,58 @@ to name: a confident claim with no reader behind it. ## Proposed decisions -These follow from the audit alone. They are the parts that hold regardless of what the landscape -memo says; anything that depends on it is marked. - -**D1 — Name the current state honestly in the product surface.** Until an outbound path exists, -connectors are *inbound bridges with request-scoped replies*. They should not be described as -two-way sync in any UI, doc, or listing. This costs nothing and prevents the gap being discovered -by a customer. - -**D2 — A connector becomes an Installable component, not an enum member.** Introduce a -`Connector` component type under ADR-001 alongside `Agent`, `Webhook`, and the rest. The provider -enum in `models/Integration.ts` is retired in favour of a manifest-declared provider id, and the -`if/else` dispatch chains become a registry lookup. Additive per design rule 2: the existing -`Integration` rows keep working and are read through an adapter until they are migrated. - -**D3 — A connector declares its directions.** Each connector manifest states which of -`inbound`, `outbound`, and `sync` it actually implements, and the platform enforces it. Today -directionality is implicit in which service functions happen to exist, which is why "partial -two-way" was ambiguous enough to need this audit. A declared direction is checkable in CI. - -**D4 — Outbound needs an event fan-out, and that is the real build.** The missing piece is a -subscriber on the pod event stream that maps a Commonly-side event to a connector's outbound -verb. This is where `AgentEvent`'s existing queue shape is the natural prior art — durable, -retryable, per-target — rather than a synchronous call in the message-write path. Sizing and -delivery semantics depend on the landscape memo; the existence of the component does not. - -**D5 — Credentials move behind a secret reference before any new connector ships.** The -`Integration` document stores a reference, not the material. Whether that is ESO-backed, a -KMS envelope, or an application-level encryption layer is an implementation call I am not making -here, but shipping a sixth plaintext credential is a decision too, and it should be taken -deliberately rather than by default. - -**D6 — Connectors scope like Installables.** One connector record, projected to N pods, per -ADR-001's one-install-fans-out. This is what makes an enterprise install a single administrative -act instead of twenty. +**D1 — Name the gap as synchronisation, not as outbound.** Outbound exists in three trigger modes +and agents already publish through it. What does not exist is any path from a Commonly-side event +to a connector. Product surfaces should claim *publishing* and *ingestion*, and should not claim +two-way *sync* for any connector until mode 4 exists. This is the decision I most want ratified, +because the first draft of this ADR got it wrong in the other direction and an imprecise headline +is what produced that error. + +**D2 — Add a conversational outbound verb to the provider contract.** The registry has exactly one +outbound verb and it is `publishPost`, a broadcast shape (caption, hashtags, source URL) that four +of six providers do not implement. Enterprise chat needs `sendMessage(threadRef, content)` as a +first-class provider method, with `NormalizedOutboundMessage` alongside the SDK's existing +`NormalizedMessage`. Discord's existing send is the reference implementation to lift into the +registry — additive, per design rule 2, leaving `POST /:id/send` working while it migrates. + +**D3 — A provider declares its directions, and the manifest is the place.** `capabilities[]` already +exists in every manifest and is currently free-form prose (`['webhook', 'gateway', 'summary', +'commands']`). Make it enumerated and enforced, so `inbound` / `publish` / `converse` / `sync` are +checkable in CI rather than inferred from which methods happen to be defined. Finding 2's table +should be generated, not hand-written. + +**D4 — Retire the enum in favour of the registry.** Finding 3's defect is duplication, not the +absence of an abstraction. The registry wins; `models/Integration.ts`'s enum becomes a soft +reference validated against registered providers, and the `if/else` chains become registry lookups. +This removes the "declared but unimplemented" state the enum currently permits. + +**D5 — Enforce the manifest's `configSchema` at the persistence boundary.** Per-provider validation +already exists and the storage model ignores it. Validating on write turns a class of call-time +400s into save-time errors, and costs no new design. + +**D6 — Credentials move behind a secret reference before any new connector ships.** The +`Integration` document stores a reference, not the material. Whether that is ESO-backed, a KMS +envelope, or application-level encryption is an implementation call I am not making here — but +shipping a seventh plaintext credential is a decision too, and it should be taken deliberately. + +**D7 — Connectors scope like Installables.** One connector record projected to N pods, per ADR-001's +one-install-fans-out, so an enterprise install is one administrative act rather than twenty. --- ## What this ADR does not decide -- **The wire format for outbound.** Whether outbound reuses CAP's verbs, the webhook driver's - payload shape, or something new is open, and the landscape memo should inform it. +- **The wire format for D2's conversational verb.** Whether it reuses CAP's shapes, the webhook + driver's payload, or something new is open, and the landscape memo should inform it. - **Whether federation counts as a connector.** `services/federationService.ts` and `routes/federation.ts` exist and may belong to this substrate or may be a separate axis. Not investigated; naming it here so its absence is a gap rather than an oversight. -- **Retention of `config.messageBuffer`.** Finding 3 says it should not live in the config +- **Retention of `config.messageBuffer`.** Finding 4 says it should not live in the config document; it does not say where it goes. -- **Anything requiring the landscape.** D2's manifest fields, D4's delivery semantics, and the +- **Whether mode 4 (event-driven mirroring) should exist at all.** D1 says stop claiming it; it + does not say build it. Mirroring carries loop, permission, and volume questions none of the + current connectors answer, and that is a product decision. +- **Anything requiring the landscape.** D2's payload shape, D3's capability vocabulary, and the enterprise-controls surface (audit log, per-channel permissions, data residency) are all under-specified on purpose until TASK-078 lands. @@ -189,10 +216,13 @@ act instead of twenty. ## Open question for Sam -The audit says the honest headline is *"we have inbound connectors, not two-way sync."* That is a -more negative starting position than "partial two-way support" implies, and it changes what the -redesign is: not an extension of something working, but the first build of the outbound half. +The honest headline is *"we can publish, and we cannot mirror."* Agents already post to X and +Instagram through a rate-limited, attributed endpoint; no chat connector has a conversational +outbound verb, and nothing anywhere is driven by a Commonly-side event. -Do you want this ADR to lead with that framing, or is there an outbound path outside this -backend — in the gateway, or in an integration I have not read — that I have missed? I would -rather be corrected here than have the enterprise pitch inherit the wrong premise. +So the redesign is not "build outbound" — it is (a) give the four chat providers the verb the two +social providers already have, and (b) decide whether mirroring is a product commitment at all, +because mode 4 is a much larger build than modes 1–3 and carries loop, permission, and volume +questions the current connectors never had to answer. + +D1 is the one I want ratified. The rest follow from the audit. From c315b63c9ac74375014a36381438677d5b2e379f Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Wed, 26 Aug 2026 13:37:48 -0700 Subject: [PATCH 3/9] =?UTF-8?q?docs(adr-025):=20mode=204=20exists=20for=20?= =?UTF-8?q?telegram=20as=20of=20#1282=20=E2=80=94=20amend=20Finding=201=20?= =?UTF-8?q?and=20every=20restatement=20of=20the=20absence?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The audit's load-bearing claim — 'what is uniformly absent is mode 4: a Commonly-side event originating an outbound call' — was true when it was re-derived and false about thirty minutes later. #1282 merged at defff409 and adds telegramBridgeService with both halves of a mirror: relayAgentMessageToTelegram fire-and-forget from AgentMessageService.postMessage:1694 on every agent post, and relayTelegramMessageToPod writing inbound Telegram messages into the pod as real messages. Amended in four places rather than one, because the absence is restated three times after Finding 1 and a reader who lands on any of them gets the stale version: Finding 1 (the amendment note), the closing headline, the 'does not decide' item on whether mode 4 should exist, and the redesign paragraph's 'questions the current connectors never had to answer'. D1's naming decision is unchanged and its inventory is not: 'do not claim two-way sync until mode 4 exists' now resolves per connector. The blanket claim is still the one to stop making. Co-Authored-By: Claude Opus 5 --- docs/adr/ADR-025-connector-substrate.md | 38 +++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/docs/adr/ADR-025-connector-substrate.md b/docs/adr/ADR-025-connector-substrate.md index 2f5c279342..3e1e3605ba 100644 --- a/docs/adr/ADR-025-connector-substrate.md +++ b/docs/adr/ADR-025-connector-substrate.md @@ -63,6 +63,32 @@ pod message, a reaction, or a task moving on the board reaches nothing. Every pa triggered by an inbound request, a human pressing a button, or an agent explicitly deciding to publish. Nothing mirrors. +> **Amendment, 2026-08-26 20:40Z — mode 4 now exists, for exactly one connector.** The sentence +> above was true when this audit was re-derived and was falsified about thirty minutes later by +> [#1282](https://github.com/Team-Commonly/commonly/pull/1282), merged at `defff409`. It adds +> `services/telegramBridgeService.ts` with both halves of a mirror: outbound, +> `relayAgentMessageToTelegram` is called fire-and-forget from +> `AgentMessageService.postMessage` (`services/agentMessageService.ts:1694`) on every agent post, +> gated at the far end by `shouldEscalate` and by an integration carrying `config.liveRelay`; +> inbound, `relayTelegramMessageToPod` writes the Telegram message into the pod as a real message +> so mentions fire and agents wake. A pod message now does reach something. +> +> Three consequences for the decisions below, none of which I think reverses one: +> +> - **D1's *naming* holds and its *inventory* does not.** "Do not claim two-way sync until mode 4 +> exists" now resolves differently per connector: telegram has it, the other three do not. The +> claim to stop making is still the blanket one. +> - **It arrived outside the provider registry.** Finding 2's table stays literally true — +> telegram still has no `publishPost` — and becomes misleading, because the first event-driven +> outbound path in the codebase does not use the registry at all. It is a direct service call. +> That is evidence for D2's shape argument rather than against it: when the registry's verb set +> did not fit, the implementation went around it. +> - **The loop, permission and volume risks D7 defers are now live** on one connector rather than +> hypothetical. `shouldEscalate` plus `liveRelay` defaulting to `false` are the whole bound. +> +> I have not re-derived the ten-call inventory at the top of this finding against current main; +> the amendment covers what #1282 added and nothing else. + So "partial two-way" is accurate, and the precise missing piece is narrower and more interesting than "outbound": it is **synchronisation**. @@ -207,7 +233,10 @@ one-install-fans-out, so an enterprise install is one administrative act rather document; it does not say where it goes. - **Whether mode 4 (event-driven mirroring) should exist at all.** D1 says stop claiming it; it does not say build it. Mirroring carries loop, permission, and volume questions none of the - current connectors answer, and that is a product decision. + current connectors answer, and that is a product decision. Note that #1282 has since answered it + de facto for telegram, by shipping it — which makes the question sharper, not moot: whether the + other three follow, and whether the shape #1282 chose is the one they should copy, is still open + and is still a product decision rather than an inference from one merge. - **Anything requiring the landscape.** D2's payload shape, D3's capability vocabulary, and the enterprise-controls surface (audit log, per-channel permissions, data residency) are all under-specified on purpose until TASK-078 lands. @@ -218,11 +247,14 @@ one-install-fans-out, so an enterprise install is one administrative act rather The honest headline is *"we can publish, and we cannot mirror."* Agents already post to X and Instagram through a rate-limited, attributed endpoint; no chat connector has a conversational -outbound verb, and nothing anywhere is driven by a Commonly-side event. +outbound verb, and — as of the amendment in Finding 1 — exactly one connector is driven by a +Commonly-side event, through a path that bypasses the registry. The headline holds for three of +the four chat connectors and no longer holds for telegram. So the redesign is not "build outbound" — it is (a) give the four chat providers the verb the two social providers already have, and (b) decide whether mirroring is a product commitment at all, because mode 4 is a much larger build than modes 1–3 and carries loop, permission, and volume -questions the current connectors never had to answer. +questions modes 1–3 never had to answer — questions #1282 now has to answer for telegram, with +`shouldEscalate` and a `liveRelay` flag defaulting to `false` as the whole bound. D1 is the one I want ratified. The rest follow from the audit. From 61e8e0ef9f789a37bfb1433133d2fdb8da9409be Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Wed, 26 Aug 2026 14:46:05 -0700 Subject: [PATCH 4/9] =?UTF-8?q?docs(adr-025):=20D1's=20own=20body=20still?= =?UTF-8?q?=20claimed=20the=20blanket=20absence=20=E2=80=94=20and=20the=20?= =?UTF-8?q?amendment=20cited=20the=20wrong=20merge=20SHA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes, one raised by @sprint-review's re-gate and one found checking it. 1. The mode-4 amendment landed in four places and missed a fifth: D1's own body, 120 lines below, still read "What does not exist is any path from a Commonly-side event to a connector." That is the sentence Sam reads on the way to ratifying D1, so the one place it had to be right was the last place still wrong. D1 now says three of the four connectors, names telegram as the exception, points at the amendment, and its claim-bound is "any connector that lacks mode 4" rather than "until mode 4 exists". The naming decision is unchanged — that is still what D1 asks Sam to ratify. 2. The amendment cited #1282 as "merged at `defff409`". That is #1284, the SEO prerender. #1282 merged at `7a781821`. Corrected. Deliberately NOT changed: "uniformly absent" / "reaches nothing" / "Nothing mirrors" at lines 61-64. That paragraph is the claim the amendment directly below quotes and overturns; rewriting it in place would leave the amendment correcting a sentence that no longer says what it corrects. Co-Authored-By: Claude Opus 5 --- docs/adr/ADR-025-connector-substrate.md | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/docs/adr/ADR-025-connector-substrate.md b/docs/adr/ADR-025-connector-substrate.md index 3e1e3605ba..4ac83073f1 100644 --- a/docs/adr/ADR-025-connector-substrate.md +++ b/docs/adr/ADR-025-connector-substrate.md @@ -65,7 +65,7 @@ publish. Nothing mirrors. > **Amendment, 2026-08-26 20:40Z — mode 4 now exists, for exactly one connector.** The sentence > above was true when this audit was re-derived and was falsified about thirty minutes later by -> [#1282](https://github.com/Team-Commonly/commonly/pull/1282), merged at `defff409`. It adds +> [#1282](https://github.com/Team-Commonly/commonly/pull/1282), merged at `7a781821`. It adds > `services/telegramBridgeService.ts` with both halves of a mirror: outbound, > `relayAgentMessageToTelegram` is called fire-and-forget from > `AgentMessageService.postMessage` (`services/agentMessageService.ts:1694`) on every agent post, @@ -184,11 +184,13 @@ to name: a confident claim with no reader behind it. ## Proposed decisions **D1 — Name the gap as synchronisation, not as outbound.** Outbound exists in three trigger modes -and agents already publish through it. What does not exist is any path from a Commonly-side event -to a connector. Product surfaces should claim *publishing* and *ingestion*, and should not claim -two-way *sync* for any connector until mode 4 exists. This is the decision I most want ratified, -because the first draft of this ADR got it wrong in the other direction and an imprecise headline -is what produced that error. +and agents already publish through it. What does not exist, for three of the four connectors, is +any path from a Commonly-side event to a connector; telegram gained one in #1282 — see the +amendment to Finding 1, which is the reason this sentence is no longer blanket. Product surfaces +should claim *publishing* and *ingestion*, and should not claim two-way *sync* for any connector +that lacks mode 4, which today means every connector except telegram. This is the decision I +most want ratified, because the first draft of this ADR got it wrong in the other direction and +an imprecise headline is what produced that error. **D2 — Add a conversational outbound verb to the provider contract.** The registry has exactly one outbound verb and it is `publishPost`, a broadcast shape (caption, hashtags, source URL) that four From 3497a05ad3b16b2baf0717f10054a718572c3899 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:33:14 -0700 Subject: [PATCH 5/9] =?UTF-8?q?docs(adr-025):=20the=20liveRelay=20bound=20?= =?UTF-8?q?moved=20=E2=80=94=20#1290=20gave=20it=20a=20writer,=20#1289=20g?= =?UTF-8?q?ated=20the=20inbound=20half?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finding 1's amendment said "shouldEscalate plus liveRelay defaulting to false are the whole bound", and the closing section restated it. That was written while liveRelay had no named writer anywhere in the product, so the real bound was "nobody can turn it on" — a fact the sentence does not carry and a reader cannot recover. Both halves have since moved, in opposite directions: - #1290 (e35d89e6) ships the Connectors page. V2ConnectorsPage.tsx:117 PATCHes {liveRelay} and integrations.ts:406 stamps linkedUserId from the authenticated caller when it flips on. Mode 4 is now reachable by an ordinary user path. - #1289 (f9b97d89) narrows the inbound half to 1:1 chats — telegramBridgeService.ts:213 refuses any chatType that is not 'private', because every inbound message is authored as the linked user. Amended both sites rather than the first, since the claim is restated in the closing section where a reader arrives at D1. Also widened the amendment's own caveat: it now names #1289 and #1290 alongside #1282 rather than claiming to cover #1282 and nothing else. D1's naming decision is unaffected. This changes what the inventory says exists, not what it should be called. Co-Authored-By: Claude Opus 5 --- docs/adr/ADR-025-connector-substrate.md | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/docs/adr/ADR-025-connector-substrate.md b/docs/adr/ADR-025-connector-substrate.md index 4ac83073f1..ad95edb227 100644 --- a/docs/adr/ADR-025-connector-substrate.md +++ b/docs/adr/ADR-025-connector-substrate.md @@ -85,9 +85,23 @@ publish. Nothing mirrors. > did not fit, the implementation went around it. > - **The loop, permission and volume risks D7 defers are now live** on one connector rather than > hypothetical. `shouldEscalate` plus `liveRelay` defaulting to `false` are the whole bound. +> - **Second amendment, 2026-08-26 — the bound moved in both directions, and the line above was +> written between the two moves.** When it was written, `liveRelay` had no named writer anywhere: +> the flag defaulted to `false` and nothing in the product could set it, so "the whole bound" was +> really "nobody can turn it on". Two merges changed that. **#1290** (`e35d89e6`) ships the +> Connectors page whose Live-relay toggle is the first real writer — `V2ConnectorsPage.tsx:117` +> PATCHes `{liveRelay}`, and `integrations.ts:406` stamps `linkedUserId` from the authenticated +> caller when it flips on — so mode 4 is now reachable by an ordinary user path rather than only +> in principle. **#1289** (`f9b97d89`) narrows the inbound half to 1:1 chats: +> `telegramBridgeService.ts:213` refuses to relay unless `config.chatType === 'private'`, because +> every inbound message is authored as the linked user and only a private chat guarantees the +> sender is them. So the loop, permission and volume risks D7 defers are live on a connector a +> user can now actually switch on, and the permission one is bounded to the case where sender and +> linked user coincide. **D1's naming decision is unaffected — this changes what the inventory +> says exists, not what it should be called.** > -> I have not re-derived the ten-call inventory at the top of this finding against current main; -> the amendment covers what #1282 added and nothing else. +> I have not re-derived the ten-call inventory at the top of this finding against current main. +> The amendment covers what #1282, #1289 and #1290 changed, and nothing else. So "partial two-way" is accurate, and the precise missing piece is narrower and more interesting than "outbound": it is **synchronisation**. @@ -256,7 +270,10 @@ the four chat connectors and no longer holds for telegram. So the redesign is not "build outbound" — it is (a) give the four chat providers the verb the two social providers already have, and (b) decide whether mirroring is a product commitment at all, because mode 4 is a much larger build than modes 1–3 and carries loop, permission, and volume -questions modes 1–3 never had to answer — questions #1282 now has to answer for telegram, with -`shouldEscalate` and a `liveRelay` flag defaulting to `false` as the whole bound. +questions modes 1–3 never had to answer — questions #1282 now has to answer for telegram. The +bound is `shouldEscalate` plus `liveRelay`, which defaults to `false`; since #1290 a user can turn +that flag on from the Connectors page, and since #1289 the inbound half refuses any chat that is +not 1:1. See Finding 1's second amendment — a default is only a bound while nothing can change it, +and something can now. D1 is the one I want ratified. The rest follow from the audit. From ffa983ca843232632db2c76d97089dfba1527e1b Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Wed, 26 Aug 2026 16:17:02 -0700 Subject: [PATCH 6/9] docs(adr-025): the private-chat gate does not bound the permission risk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @sprint-review's finding on the second amendment: the paragraph stated a join the code does not make. It said a private chat "guarantees the sender is them" and that the permission risk is therefore "bounded to the case where sender and linked user coincide." The gate narrows the sender to one person. It does not establish that the person is `config.linkedUserId`. `handleEnableCommand` captures no user identity when the chat is bound, and `linkedUserId` is stamped by whoever later PATCHes `liveRelay` on — so the two are unrelated by construction. The invariant needs three links and only two exist. Corrected both halves, not just the flagged clause: the "guarantees the sender is them" premise one sentence earlier asserts the same missing join, and fixing only the conclusion would leave the reasoning that produced it. Swept the file for other restatements; this paragraph is the only one. Not blocking, but Sam is being asked to ratify D1 inside this document. Co-Authored-By: Claude Opus 5 --- docs/adr/ADR-025-connector-substrate.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/adr/ADR-025-connector-substrate.md b/docs/adr/ADR-025-connector-substrate.md index ad95edb227..939581e0ef 100644 --- a/docs/adr/ADR-025-connector-substrate.md +++ b/docs/adr/ADR-025-connector-substrate.md @@ -94,10 +94,13 @@ publish. Nothing mirrors. > caller when it flips on — so mode 4 is now reachable by an ordinary user path rather than only > in principle. **#1289** (`f9b97d89`) narrows the inbound half to 1:1 chats: > `telegramBridgeService.ts:213` refuses to relay unless `config.chatType === 'private'`, because -> every inbound message is authored as the linked user and only a private chat guarantees the -> sender is them. So the loop, permission and volume risks D7 defers are live on a connector a -> user can now actually switch on, and the permission one is bounded to the case where sender and -> linked user coincide. **D1's naming decision is unaffected — this changes what the inventory +> every inbound message is authored as `config.linkedUserId`, and a private chat at least narrows +> the sender to a single person. It does not establish that the person is the linked user, and +> nothing in the code does: `handleEnableCommand` captures no user identity when the chat is +> bound, while `linkedUserId` is stamped by whoever later PATCHes `liveRelay` on. So the loop, +> permission and volume risks D7 defers are live on a connector a user can now actually switch +> on, and the permission one is NOT bounded — the invariant it would need is a link between the +> chat's counterpart and the toggling caller, and that link does not exist. **D1's naming decision is unaffected — this changes what the inventory > says exists, not what it should be called.** > > I have not re-derived the ten-call inventory at the top of this finding against current main. From 549f0fe414cdf2dd84463ce7adaf57dd025f5ab2 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Wed, 26 Aug 2026 18:28:42 -0700 Subject: [PATCH 7/9] docs(adr-025): D3's enum would drop the control direction #1301 shipped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1301 (97b6a870) adds a Telegram control plane — /mode, /mute, /unmute, /status, /tldr — handled in routes/webhooks/telegram.ts. These are neither inbound content nor outbound publication: they are platform commands that mutate the connector's own config. /mode is the first named writer of config.relayAllAgentMessages; /mute introduces config.relayMutedUntil. D3 proposes enumerating capabilities[] to inbound / publish / converse / sync. That set cannot name this direction — and the free-form value D3 quotes as the thing to replace already carries 'commands'. Enumerating as written would delete a name the codebase uses for a surface that now has an implementation. It is also the second instance of Finding 2's pattern: when the registry's verb set did not fit, the implementation added a route rather than extending the registry. #1282 did the same. Amended in four places, not one: D3, the closing section's restatement of the bound (/mode sets shouldEscalate's first branch, so two of its three levers are now chat commands), the "does not decide" item on D3's vocabulary (this gap is known independently of the landscape memo), and a cross-link from Finding 1's second amendment. D1's naming decision is unaffected. Co-Authored-By: Claude Opus 5 --- docs/adr/ADR-025-connector-substrate.md | 42 +++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/docs/adr/ADR-025-connector-substrate.md b/docs/adr/ADR-025-connector-substrate.md index 939581e0ef..ef10dcac03 100644 --- a/docs/adr/ADR-025-connector-substrate.md +++ b/docs/adr/ADR-025-connector-substrate.md @@ -104,7 +104,9 @@ publish. Nothing mirrors. > says exists, not what it should be called.** > > I have not re-derived the ten-call inventory at the top of this finding against current main. -> The amendment covers what #1282, #1289 and #1290 changed, and nothing else. +> The amendment covers what #1282, #1289 and #1290 changed, and nothing else — #1301 moved this +> same bound again and is recorded in D3's third amendment rather than here, because what it adds +> is a new *direction* and not a new outbound path. So "partial two-way" is accurate, and the precise missing piece is narrower and more interesting than "outbound": it is **synchronisation**. @@ -222,6 +224,34 @@ exists in every manifest and is currently free-form prose (`['webhook', 'gateway checkable in CI rather than inferred from which methods happen to be defined. Finding 2's table should be generated, not hand-written. +> **Third amendment, 2026-08-27 — D3's four-value enum would drop a direction that shipped +> tonight.** #1301 (`97b6a870`) adds a Telegram *control plane*: `/mode mirror|attention`, +> `/mute [minutes]`, `/unmute`, `/status`, `/tldr`, `/help`, handled in `routes/webhooks/telegram.ts`. +> These are not inbound content and not outbound publication — they are commands from the platform +> that **mutate the connector's own configuration**. `/mode` is the first named writer of +> `config.relayAllAgentMessages`, and `/mute` introduces `config.relayMutedUntil`; both were among +> the keys this ADR's audit found unwritten. +> +> That is a fifth direction, and the enum proposed above cannot name it. Note that the free-form +> `capabilities[]` value D3 quotes as the thing to replace — `['webhook', 'gateway', 'summary', +> 'commands']` — **already carries `commands`**. Enumerating to `inbound / publish / converse / sync` +> as written would delete a name the codebase already uses for a surface that now has an +> implementation. D3's decision stands; its vocabulary needs a fifth member (`control`, or whatever +> it ends up called) before it is ratified, or the enforcement it asks for lands narrower than the +> product. +> +> It is also the **second** instance of Finding 2's pattern, which is why it belongs here rather than +> in a bug report. The first event-driven outbound path (#1282) went around the provider registry; +> so does this control plane. Twice now, when the registry's verb set did not fit, the implementation +> did not extend the registry — it added a route. A capability vocabulary that is enforced in CI is +> only worth having if the paths that grow capabilities are the ones it governs. +> +> One consequence sits outside this ADR and is filed at issue #1287 rather than decided here: the +> command handlers resolve their integration by `config.chatId` alone, reading neither +> `message.from.id` nor `config.chatType`, so in a linked group chat any member can flip the relay +> mode or mute the operator's escalations. D7's enterprise-scoping decision inherits that question — +> a connector projected to N pods needs to say who may reconfigure it, and today nothing does. + **D4 — Retire the enum in favour of the registry.** Finding 3's defect is duplication, not the absence of an abstraction. The registry wins; `models/Integration.ts`'s enum becomes a soft reference validated against registered providers, and the `if/else` chains become registry lookups. @@ -258,7 +288,9 @@ one-install-fans-out, so an enterprise install is one administrative act rather and is still a product decision rather than an inference from one merge. - **Anything requiring the landscape.** D2's payload shape, D3's capability vocabulary, and the enterprise-controls surface (audit log, per-channel permissions, data residency) are all - under-specified on purpose until TASK-078 lands. + under-specified on purpose until TASK-078 lands. One gap in D3's vocabulary is now known + independently of the landscape and is recorded in its third amendment — the control direction — + so that much can be settled without waiting. --- @@ -277,6 +309,10 @@ questions modes 1–3 never had to answer — questions #1282 now has to answer bound is `shouldEscalate` plus `liveRelay`, which defaults to `false`; since #1290 a user can turn that flag on from the Connectors page, and since #1289 the inbound half refuses any chat that is not 1:1. See Finding 1's second amendment — a default is only a bound while nothing can change it, -and something can now. +and something can now. Since #1301 the bound has a third mutator and it is reachable +from the platform side: `/mode mirror` sets `config.relayAllAgentMessages`, which is +`shouldEscalate`'s first branch — so a Telegram command turns the escalation gate off entirely, and +`/mute` suspends the relay from the same surface. Two of the three levers on this bound are now +commands typed into the chat, which is the direction D3's third amendment names. D1 is the one I want ratified. The rest follow from the audit. From d8658b976add7240a6190062a2ff0c484a380766 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 28 Aug 2026 16:22:43 -0700 Subject: [PATCH 8/9] =?UTF-8?q?docs(adr-025):=20#1311=20flipped=20the=20de?= =?UTF-8?q?fault=20=E2=80=94=20both=20levers=20of=20the=20telegram=20bound?= =?UTF-8?q?=20are=20now=20on=20at=20create?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Finding 1 bound read "shouldEscalate plus liveRelay defaulting to false". #1311 (3eaabfc8) sets relayAllAgentMessages and liveRelay true on a fresh telegram connector when the caller sends neither, and V2ConnectorsPage.tsx:131 creates with config: {} — so that is the primary path, not an edge case. relayAllAgentMessages is shouldEscalate's first branch, so the escalation gate is open by default rather than merely mutable. Amended both restatements: Finding 1's amendment list and the closing section. Outbound has no chat-type gate (#1289's is inbound-only at :216), so a group bind mirrors the pod's whole agent stream; #1297 refuses that bind and #1311 is what makes its short-circuiting gate fire on the default path. D1's naming decision is unaffected. Co-Authored-By: Claude Opus 5 --- docs/adr/ADR-025-connector-substrate.md | 31 +++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/docs/adr/ADR-025-connector-substrate.md b/docs/adr/ADR-025-connector-substrate.md index ef10dcac03..28e884967c 100644 --- a/docs/adr/ADR-025-connector-substrate.md +++ b/docs/adr/ADR-025-connector-substrate.md @@ -102,6 +102,26 @@ publish. Nothing mirrors. > on, and the permission one is NOT bounded — the invariant it would need is a link between the > chat's counterpart and the toggling caller, and that link does not exist. **D1's naming decision is unaffected — this changes what the inventory > says exists, not what it should be called.** +> - **Third amendment, 2026-08-27 — the default flipped, so "defaults to `false`" above is no +> longer true of telegram.** #1311 (`3eaabfc8`) sets `relayAllAgentMessages: true` and +> `liveRelay: true` on a fresh telegram connector when the caller sends neither +> (`routes/integrations.ts`, the `type === 'telegram' && nextConfig.relayAllAgentMessages === +> undefined` block). `V2ConnectorsPage.tsx:131` creates with `config: {}`, so this is the primary +> product path and not an edge case. Its reason is sound — attention mode with no +> `leadAgentUsername` relays nothing, so the bridge's first impression was silence. The +> consequence for this Finding is that **both** levers of the bound named above are now ON at +> create: `relayAllAgentMessages` is `shouldEscalate`'s first branch +> (`telegramBridgeService.ts:71`), so the escalation gate is not merely mutable, it is open by +> default. The bound is now the chat binding itself. +> +> That matters because the OUTBOUND half has no chat-type gate — #1289's `chatType !== 'private'` +> refusal is at `:216`, inside `relayTelegramMessageToPod`, i.e. inbound only. So on `origin/main` +> a connect code pasted into a group mirrors the pod's whole agent stream into that group with +> nobody having toggled anything. **#1297 (open) refuses that bind**, and #1311 is what makes its +> gate load-bearing: `if (integration.config?.liveRelay && chatType !== 'private')` short-circuits +> on a falsy first operand, which is what a fresh connector used to have. The same line went from +> covering an edge case to covering the default path without being edited. Filed at #1297 comment +> `5458773871`. > > I have not re-derived the ten-call inventory at the top of this finding against current main. > The amendment covers what #1282, #1289 and #1290 changed, and nothing else — #1301 moved this @@ -306,10 +326,13 @@ So the redesign is not "build outbound" — it is (a) give the four chat provide social providers already have, and (b) decide whether mirroring is a product commitment at all, because mode 4 is a much larger build than modes 1–3 and carries loop, permission, and volume questions modes 1–3 never had to answer — questions #1282 now has to answer for telegram. The -bound is `shouldEscalate` plus `liveRelay`, which defaults to `false`; since #1290 a user can turn -that flag on from the Connectors page, and since #1289 the inbound half refuses any chat that is -not 1:1. See Finding 1's second amendment — a default is only a bound while nothing can change it, -and something can now. Since #1301 the bound has a third mutator and it is reachable +bound WAS `shouldEscalate` plus `liveRelay`, and since #1311 (`3eaabfc8`) neither half of it +defaults to `false` on telegram — a fresh connector is created with `relayAllAgentMessages` and +`liveRelay` both true, on the path the Connectors page actually uses. Before that: since #1290 a +user can turn that flag on from the Connectors page, and since #1289 the inbound half refuses any chat that is +not 1:1. See Finding 1's second and third amendments — a default is only a bound while +nothing can change it, something can now, and on telegram the default itself has since flipped the +other way. Since #1301 the bound has a third mutator and it is reachable from the platform side: `/mode mirror` sets `config.relayAllAgentMessages`, which is `shouldEscalate`'s first branch — so a Telegram command turns the escalation gate off entirely, and `/mute` suspends the relay from the same surface. Two of the three levers on this bound are now From 11c75c60d2666fd5763ccba7f7a934cb187cd29c Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 28 Aug 2026 16:48:35 -0700 Subject: [PATCH 9/9] docs(adr-025): #1304 gave the mode lever a second writer on the web side MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1304 (6ce4bfc8, "Connectors page redesign") rewrote V2ConnectorsPage.tsx, which does two things to this ADR. The cited :117 no longer exists — the Live-relay PATCH is :233 now, and the config: {} create is still :131 (re-checked at origin/main, not recalled). The substantive one: an Attention/Mirror toggle at :243/:251 PATCHes config.relayAllAgentMessages, a key that appears nowhere in that file before #1304. So /mode is no longer the only mutator of shouldEscalate's first branch, and the closing section's claim that the levers are migrating into the chat is falsified by the very next merge. Amended both sites. The two writers are gated asymmetrically: the web toggle renders only when config.liveRelay is true (:237), handleModeCommand writes regardless. D1's naming decision is unaffected. Co-Authored-By: Claude Opus 5 --- docs/adr/ADR-025-connector-substrate.md | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/docs/adr/ADR-025-connector-substrate.md b/docs/adr/ADR-025-connector-substrate.md index 28e884967c..ade899efd8 100644 --- a/docs/adr/ADR-025-connector-substrate.md +++ b/docs/adr/ADR-025-connector-substrate.md @@ -89,8 +89,8 @@ publish. Nothing mirrors. > written between the two moves.** When it was written, `liveRelay` had no named writer anywhere: > the flag defaulted to `false` and nothing in the product could set it, so "the whole bound" was > really "nobody can turn it on". Two merges changed that. **#1290** (`e35d89e6`) ships the -> Connectors page whose Live-relay toggle is the first real writer — `V2ConnectorsPage.tsx:117` -> PATCHes `{liveRelay}`, and `integrations.ts:406` stamps `linkedUserId` from the authenticated +> Connectors page whose Live-relay toggle is the first real writer — `V2ConnectorsPage.tsx:117` at the time +> PATCHes `{liveRelay}` (that file has since been rewritten — see the fourth amendment), and `integrations.ts:406` stamps `linkedUserId` from the authenticated > caller when it flips on — so mode 4 is now reachable by an ordinary user path rather than only > in principle. **#1289** (`f9b97d89`) narrows the inbound half to 1:1 chats: > `telegramBridgeService.ts:213` refuses to relay unless `config.chatType === 'private'`, because @@ -123,6 +123,17 @@ publish. Nothing mirrors. > covering an edge case to covering the default path without being edited. Filed at #1297 comment > `5458773871`. > +> - **Fourth amendment, 2026-08-27 — the mode lever came back to the web, and the line reference +> above went stale in the same merge.** #1304 (`6ce4bfc8`, "Connectors page redesign") rewrote +> `V2ConnectorsPage.tsx` (+358/-154), so the `:117` cited above no longer exists: the Live-relay +> PATCH is now `:233`, and the `config: {}` create is still `:131`. The substantive change is a +> **second writer of `config.relayAllAgentMessages`** — an Attention/Mirror toggle at `:243` and +> `:251` PATCHing the same key the Telegram `/mode` command writes. That key appears nowhere in +> the file before #1304. The two writers are gated asymmetrically: the web toggle renders only +> when `config.liveRelay` is true (`:237`), while `handleModeCommand` +> (`routes/webhooks/telegram.ts:256`) writes it whether or not the relay is on. **D1's naming +> decision is unaffected.** +> > I have not re-derived the ten-call inventory at the top of this finding against current main. > The amendment covers what #1282, #1289 and #1290 changed, and nothing else — #1301 moved this > same bound again and is recorded in D3's third amendment rather than here, because what it adds @@ -335,7 +346,9 @@ nothing can change it, something can now, and on telegram the default itself has other way. Since #1301 the bound has a third mutator and it is reachable from the platform side: `/mode mirror` sets `config.relayAllAgentMessages`, which is `shouldEscalate`'s first branch — so a Telegram command turns the escalation gate off entirely, and -`/mute` suspends the relay from the same surface. Two of the three levers on this bound are now -commands typed into the chat, which is the direction D3's third amendment names. +`/mute` suspends the relay from the same surface. That direction did not hold. Since #1304 (`6ce4bfc8`) the mode lever has a second writer on the +web side — an Attention/Mirror toggle in the Connectors page PATCHing the same key — so the +levers are not migrating into the chat; two surfaces now write one flag, on asymmetric gates. +See Finding 1's fourth amendment. D1 is the one I want ratified. The rest follow from the audit.