Skip to content

docs(adr): ADR-025 — user-scoped connectors and channel routing - #1295

Open
lilyshen0722 wants to merge 2 commits into
mainfrom
docs/adr-025-user-scoped-connector
Open

docs(adr): ADR-025 — user-scoped connectors and channel routing#1295
lilyshen0722 wants to merge 2 commits into
mainfrom
docs/adr-025-user-scoped-connector

Conversation

@lilyshen0722

Copy link
Copy Markdown
Contributor

Proposed. Motivated by today's composition: #1289's private-chat gate × the one-chat-one-pod claim = a user can bridge exactly one pod, ever. Decision: bind the private chat to the user (ADR-001 user scope), route by pod tags + quote-reply + slash commands, judge only for ambiguity, transport stays kernel. Scope-boundary notes to ADR-017/018 included per the ADR-status discipline.

For the new connector-track seats to review adversarially before any implementation.

🤖 Generated with Claude Code

https://claude.ai/code/session_013pc6nGXRS8mHvrwcXMSRDK

The private-only gate (#1289) times the one-chat-one-pod claim caps a
user at one bridged pod ever; rebind the chat to the user and make pod
routing an addressing property (tags, quote-reply, slash commands,
judge for ambiguity only).

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

Copy link
Copy Markdown
Contributor Author

Reviewed at d0ba7a63 against origin/main 25a149d8. Docs-only, so no tests run. Three findings, one of them procedural and cheap to fix now.

1. ADR number collision — two open PRs both claim 025. #1268 adds docs/adr/ADR-025-connector-substrate.md; this adds docs/adr/ADR-025-user-scoped-connectors-and-channel-routing.md. Different filenames, so the file sets are disjoint and git will merge both cleanly — main ends up with two ADR-025s and nothing goes red. main's highest is ADR-024 (git ls-tree origin/main docs/adr/), so 025 is free exactly once. Whichever presses second should be 026. Worth settling before either merges, because renumbering a ratified ADR is worse than renumbering a proposed one.

2. D8's premise is false, and D1 widens its blast radius. D8 says the user-scoped enable path will derive linkedUserId "from the authenticated code-minter exactly as the pod-scoped one now does." The pod-scoped path does not do that. It derives linkedUserId from the PATCH /:id caller who flips liveRelay onintegrations.ts:406, if (config && config.liveRelay === true) nextConfig.linkedUserId = req.user?.id;. The code-minter is a different, separately recorded identity: integrations.ts:238 mints the code inside POST / and :242 stores createdBy: req.user?.id. Nothing reads createdBy for attribution (git grep -n linkedUserId -- backend shows the only writer is :406).

They can be different humans. canDeleteIntegration (:90-98) admits any instance role === 'admin', the pod creator, or the integration creator — three populations, and the Telegram side contributes no identity at all (handleEnableCommand matches on config.connectCode alone, routes/webhooks/telegram.ts:48-52). So the chain is: sender IS the private chat's counterpart (#1289, telegramBridgeService.ts:213) ✔; linkedUserId IS the PATCH caller (:406) ✔; counterpart IS that caller — nothing. Same gap I filed on #1294 and #1268's second amendment.

This matters more here than there. Under the pod-scoped model a mis-derived linkedUserId misattributes messages in one pod. Under D1 the connector is the user's surface for every pod they are in, so the same defect misattributes across all of them. D8 should name the missing link as an open item rather than as an invariant carried over — otherwise the ADR ratifies a guarantee the code has never made.

3. The Context overstates the cap by one word. "a user can bridge exactly one pod, ever" holds for inbound. Outbound is not gated on chatType: findLiveIntegration (telegramBridgeService.ts:102-110) filters on podId + config.liveRelay + config.chatId only, and relayAgentMessageToTelegram goes through it. chatClaim is per-chat, so N group chats today = N pods relaying out, with inbound refused on each. The dead end is real but it is a two-way dead end. Suggest "bridge two-way with exactly one pod" — the argument for D1 survives unchanged and gets more precise.

Verified as stated: the chatClaim one-claim check (routes/webhooks/telegram.ts:73-86); #1289's gate at :213; relayMap carries {tgMessageId, agentUsername, podMessageId} and no podId (models/Integration.ts:175-178), so D3's addition is real work, not a restatement.

One forward note for D5. "sets the chat's active pod (stored on the integration)" means a new config.* key. Integration is strict: true and declares config twice — the TS interface (:86-90) and the runtime Schema (:174-178). A key added to the interface only is silently dropped on write, no error; that is exactly what made #1282's whole feature inert until it was caught pre-merge. Cheap to note in Consequences so the implementer doesn't rediscover it.

Not verified: anything about D6's pod-summarizer reuse or digest cadence; the Connectors-page consequence; the "already 80% built" estimate for D4 beyond the relayMap shape; and whether Telegram-side limits make the 24-bit connect code (crypto.randomBytes(3), matched by code alone, no attempt limiting that I could find in routes/webhooks/telegram.ts) a practical concern — I did not attempt it. Raising it only because D1 raises the code's value from one pod to one user's whole attention surface.

@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Two adversarial reviews from the connector track (findings preserved here; the track continues under real-named seats in the Connectors v2 pod). Both converged independently on:

  • D8 is false as written. The private-only gate exists only INBOUND — chatType is read once in telegramBridgeService, so the ADR's 'private-only gate carried over' claim needs qualifying: it does not constrain outbound at all.
  • linkedUserId is stamped at PATCH, not by the minter (matches what feat(v2): Connectors page + nav rail entry #1290 shipped) — the ADR's D8 wording should say so.
  • 24-bit connect code is too small — brute-forceable; widen it.
  • Routing must key on podId in relayMap (already in D3) — both flagged the enable-path P0 that POST /api/integrations spreads config verbatim (integrations.ts:234), which is the same class as PATCH /api/integrations/:id merges arbitrary config keys — needs an allowlist #1292.

Disjoint-but-real: outbound relay never checks chatType (their F2, fixable today, independent of this ADR). Amendment proposed for D5/D7: precedence should be quote → inline /pod → last-pushed pod within 15 min → active pod → ask, with an ack line on every non-quote route so a misroute is visible immediately (the stale-active-pod misroute, not the cold start, is the real hazard).

Full write-ups were attached in-pod. Vera/Wren/Kai will fold these into the plan.

@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Follow-up on finding 2, because it changes the size of the fix rather than the finding.

D8 is a restatement drift, not a misreading of the code. #1290 and this PR are the same author and the same session (Co-Authored-By: Claude Fable 5, Claude-Session: .../session_013pc6nGXRS8mHvrwcXMSRDK on both), and #1290's own commit body states the behaviour correctly:

Live relay toggle sets linkedUserId to the toggler

That is exactly right, and it is what integrations.ts:406 does. D8 then describes the same line as deriving from "the authenticated code-minter" — a different identity (createdBy, stamped at integrations.ts:242), which nothing reads.

So the correction is a wording change in D8 plus an added open item for the missing counterpart↔caller link. No re-derivation needed, and D1–D7 are untouched by it.

…Commander persona

Sam's decisions 2026-08-26: connector reveals/selects target pods via
slash commands; Commander persona (distinct from Scout) as conversational
routing front-end with profile-level auto-join opt-in. Review findings
folded: D8 inbound-only qualification, 128-bit codes, precedence chain.

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