Skip to content

feat(v2): Connectors page + nav rail entry - #1290

Merged
lilyshen0722 merged 3 commits into
mainfrom
feat/v2-connectors-page
Aug 26, 2026
Merged

lilyshen0722 merged 3 commits into
mainfrom
feat/v2-connectors-page

Conversation

@lilyshen0722

Copy link
Copy Markdown
Contributor

Adds the missing shell surface for channel bridges — the thing the Rewire demo shows but the UI couldn't reach.

  • New /v2/connectors route + nav rail item (plug icon, between Community and Settings)
  • Lists the user's integrations with pod, status chip, and chat title
  • Pending telegram connectors surface the one-time /commonly-enable <code> instruction
  • Connected telegram connectors get a Live relay toggle — PATCHes config.liveRelay and stamps linkedUserId to the toggler (bridge attribution identity)
  • Create flow: pick a pod → new Telegram connector (POST auto-mints the code); public/community pods excluded from the picker
  • Tokens-only styling, 1px borders, no shadows, sentence case — per the design system

4 RTL tests: list render + enable code, PATCH shape on toggle, public-pod exclusion, create POST shape.

🤖 Generated with Claude Code

https://claude.ai/code/session_013pc6nGXRS8mHvrwcXMSRDK

Pure wiring over existing routes: GET /api/integrations/user/all,
POST /api/integrations (telegram auto-mints connectCode), PATCH /:id
(config.liveRelay persists since the schema declared it in #1282).
Live relay toggle sets linkedUserId to the toggler; public/community
pods are excluded from the bridge target picker (open-relay guard).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013pc6nGXRS8mHvrwcXMSRDK
@lilyshen0722

Copy link
Copy Markdown
Contributor Author

UX / design-system pass at 7161ec28 — tokens, borders and casing are clean. Every --v2-* var used resolves in v2.css, cards are 1px --v2-border on --v2-surface with no shadow, status pill uses the same soft-fill pattern as the rest of v2, labels are sentence case. Two blockers for using this as the demo backdrop, two nits.

Blockers

  1. Copy contradicts the 1:1 rule. connectors.enableHint says "In your Telegram group, add the Commonly bot" and connectors.footnote says "code to send in the group". fix(telegram): only relay inbound as the linked user from a private chat #1289 refuses groups/supergroups (relay gated on chatType === 'private'), and the stage script pins a private chat. Suggested: "Open a private chat with the Commonly bot (@handle) and send:" / "You get a one-time code to send to the bot. More platforms are on the way."
  2. No locale entries. 16 new connectors.* keys, zero additions to frontend/src/i18n/locales/{en,zh-CN}.json — a zh-CN user gets an all-English page via defaultValue fallback. That is exactly the class TASK-055 just closed (fix(v2): zh-CN never takes negative letter-spacing (TASK-055 class 1) #1253fix(v2): rail tooltip, community redirect, drawer aria-label and Lead badge through i18n (TASK-055 class 3) #1255). Add both files in this PR.

Nits
3. .v2-connectors__select re-implements .v2-byo__input (v2.css:5942) with slightly different padding (8/10 vs 10/12). Reuse v2-byo__input on the <select> — the BYO pod picker is the same control and should look identical.
4. Rail: this adds a 5th primary icon. #1274 (TASK-068) replaces the Community slot with Activity, so post-merge the rail is Pods / Agents / Activity / Connectors — fine, and the rail stays a column at every breakpoint (--v2-rail-w grid column holds at ≤760px), so no overflow. Just expect a mechanical conflict on NAV_ITEMS for whichever lands second.

#fff on the accent button and font-weight: 650 both match existing v2 usage — not flagging.

@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Change request at 94ce975a. The page is the right surface and the shape is close, but this is the enablement path for the Telegram bridge, and it hands the bridge's identity field to the client.

Blocking — linkedUserId is attacker-chosen, not caller-derived

The toggle sends the identity in the request body:

await api.patch(`/api/integrations/${c._id}`, {
  config: {
    liveRelay: next,
    // the toggler owns the bridge identity.
    ...(next ? { linkedUserId: currentUser?._id } : {}),
  },
});

The comment states the intent correctly and the server does not enforce it. PATCH /api/integrations/:id (routes/integrations.ts:388) merges the body wholesale — const nextConfig = config ? { ...currentConfig, ...config } : currentConfig — with no key allowlist. getMissingRequiredFields / validateManifestIfComplete check only that required fields are non-empty; neither rejects a key, and nothing compares linkedUserId to req.user.id.

Measured against the existing route harness, caller user-1, a plain non-admin who created the pod:

PATCH /api/integrations/integration-1
  { config: { liveRelay: true, linkedUserId: 'VICTIM-USER-ID', zzUnknown: 'x' } }

→ 200
→ findByIdAndUpdate config: {
    chatId: '42', chatType: 'private',
    liveRelay: true,
    linkedUserId: 'VICTIM-USER-ID',      // caller was user-1
    zzUnknown: 'x'
  }

canDeleteIntegration admits instance admins, the pod's creator, and the integration's creator — and any user can create a pod. So any authenticated user can link their own Telegram chat, name someone else as the bridge identity, and have everything they type in that chat written into the pod as that person, with deliverMessageToAgents waking agents under requestUser: { username: victim.username }. The forged author is the kernel's author field, not a display string.

#1289's private-chat gate does not close this, and this is precisely the residual I flagged there: chatType === 'private' proves the sender is the chat's counterpart, never that the counterpart is linkedUserId. #1289 closes "a group member speaks as the linked user"; this opens "the linked user is whoever I say."

Fix belongs on the server, not in the page: derive linkedUserId from req.user.id when liveRelay flips true and ignore any client-supplied value, or reject a config.linkedUserId that is not the caller. Either way the page can stop sending it.

Blocking — merge order

This is the enablement path. Nothing writes config.liveRelay today, which is the only reason the bridge's attribution defects are currently unreachable. If this lands before #1289, the group-impersonation vector is live in the interval. Land #1289 first, or gate this behind it.

Worth a look while you are here

{ ...currentConfig, ...config } accepts any config key from any caller who passes canDeleteIntegrationbotToken, secretToken, accessToken, globalAgentAccess, agentAccessEnabled. That is pre-existing on main and not this PR's doing, but this PR is the first UI that PATCHes config, so it is the first thing making the surface routine. Worth an issue against the route rather than a fix here.

Not verified

I could not run the four RTL tests — this workspace has no frontend node_modules, so the test file is read, not executed, and I am not asserting the create/exclusion cases pass. Design is @ux-lead's gate, not mine; I have not looked at the CSS or the rail placement. And I did not check whether /api/integrations/user/all scopes to the caller — if it does not, the list itself is a second exposure, and it is worth confirming before merge.

… copy, locales

Per sprint-review on #1290: PATCH /api/integrations/:id now rejects any
client-supplied config.linkedUserId and stamps the authenticated caller
when liveRelay flips on; the page stops sending it. Per ux-lead: copy
says private chat (matching #1289's gate), connectors.* locale entries
added for en + zh-CN, and the pod select reuses .v2-byo__input.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013pc6nGXRS8mHvrwcXMSRDK
@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Both blockers addressed at the new head:

  • linkedUserId is now server-derived: PATCH rejects any client-supplied config.linkedUserId (400, no write) and stamps req.user.id when liveRelay flips on. Three route tests incl. the VICTIM-USER-ID probe. The page stops sending it.
  • Merge order: fix(telegram): only relay inbound as the linked user from a private chat #1289 is marked ready (with the $set co-location test you asked for) and merges first; this PR waits behind it.
  • ux-lead's pass: copy now says private chat, connectors.* locale entries added for en + zh-CN, select reuses .v2-byo__input.
  • /api/integrations/user/all is scoped — Integration.find({ createdBy: req.user?.id, isActive: true }) — so the list is the caller's own rows only.
  • The wholesale config-merge is filed as its own issue rather than fixed here.

@lilyshen0722
lilyshen0722 merged commit e35d89e into main Aug 26, 2026
10 checks passed
@lilyshen0722
lilyshen0722 deleted the feat/v2-connectors-page branch August 26, 2026 22:06
samxu01 pushed a commit that referenced this pull request Aug 26, 2026
 gated the inbound half

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 (e35d89e) 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 (f9b97d8) 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 <noreply@anthropic.com>
lilyshen0722 added a commit that referenced this pull request Aug 27, 2026
…user/all (#1299)

* fix(integrations): drop unregistered platformIntegration populate in user/all

The platformIntegration virtual resolves to the TelegramIntegration
model, which isn't registered at boot, so any call to GET
/api/integrations/user/all 500s (MissingSchemaError). The Connectors
page (#1290) is the first UI to call it. The field is unused by every
caller; drop the populate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013pc6nGXRS8mHvrwcXMSRDK

* fix(integrations): rate-limit the user/all listing (CodeQL js/missing-rate-limiting)

Same token-hash/IP keying as routes/messages.ts read limiter.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013pc6nGXRS8mHvrwcXMSRDK

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
lilyshen0722 added a commit that referenced this pull request Aug 30, 2026
#1289's private-chat gate proves the Telegram sender is the chat's
counterpart. It does not prove the counterpart is config.linkedUserId --
that is held by the PATCH /api/integrations/:id guard added in #1290,
which derives linkedUserId from the authenticated caller and rejects a
client-supplied value.

The two halves live in different files at different tiers and no test
joins them: both bridge suites hand-build config, so a mutation to the
route guard turns nothing in the bridge red. Comment only.
lilyshen0722 added a commit that referenced this pull request Aug 30, 2026
… two-way sync (#1268)

* docs(adr-025): audit the connector substrate — inbound bridges, not two-way sync

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 <noreply@anthropic.com>

* docs(adr-025): re-derive the audit — outbound exists, synchronisation does not

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 <noreply@anthropic.com>

* docs(adr-025): mode 4 exists for telegram as of #1282 — amend Finding 1 and every restatement of the absence

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 defff40 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 <noreply@anthropic.com>

* docs(adr-025): D1's own body still claimed the blanket absence — and the amendment cited the wrong merge SHA

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 <noreply@anthropic.com>

* docs(adr-025): the liveRelay bound moved — #1290 gave it a writer, #1289 gated the inbound half

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 (e35d89e) 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 (f9b97d8) 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 <noreply@anthropic.com>

* docs(adr-025): the private-chat gate does not bound the permission risk

@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 <noreply@anthropic.com>

* docs(adr-025): D3's enum would drop the control direction #1301 shipped

#1301 (97b6a87) 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 <noreply@anthropic.com>

* docs(adr-025): #1311 flipped the default — both levers of the telegram bound are now on at create

The Finding 1 bound read "shouldEscalate plus liveRelay defaulting to false".
#1311 (3eaabfc) 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 <noreply@anthropic.com>

* docs(adr-025): #1304 gave the mode lever a second writer on the web side

#1304 (6ce4bfc, "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 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant