fix(connectors): tag Telegram's outbound line and route a quote-reply to the pod it quotes (TASK-156, ADR-025 D10/D11) - #1935
Merged
Conversation
… to the pod it quotes (TASK-156, ADR-025 D10/D11) A user-scoped connector has one active inbound destination and N gated pods. Two gaps appear once a second gate is on. D10, Telegram only — the two things Slack already does: - the outbound line carries the pod name, so a reader can tell two pods apart; - the relayMap entry records `podId`, so a reply has a pod to route by. D11, both bridges — a quote-reply answers the line it quotes: - `routeReplyContent` / `routeSlackReplyContent` return the quoted entry's pod; - when it differs from the active pod, the target is re-derived before posting: the linked user must still be a member and the pod must still be gated. Any failure refuses in the chat, naming the pod, and posts nothing anywhere. Falling back to the active pod is the defect this rule exists for — the user's answer to B would be authored into A and the agent it names would wake there without B's thread. - an entry with no `podId` (written before this shipped) routes as it always has. Witnesses: the quoted pod receives the reply and the active pod does not; an unquoted message still goes to the active pod; a reply to the active pod's own line is unchanged even with its gate off; a gate-off or membership-gone target is refused by name with nothing posted; a pre-D11 entry still routes to the active pod; Telegram's outbound line carries the pod name.
…ine this PR adds Formatting only, assertions unchanged: multiline object literals, a destructured fixture, and a one-line `toHaveBeenCalledWith`. What remains on added lines is `import/no-unresolved` + `import/extensions` on the new `require(...)` calls, the same pair every pre-existing require in these two files already reports — the backend `.js` corpus is not linted (2,279 errors, 277 of these files).
…ership (TASK-156, wren 74618)
Wren's spec fix, applied before the gate: the routed-reply rule was specified for
two implementations and would have been written twice, which is how the list and
the call drifted in TASK-146.
- `connectorRelayPolicy.isGatedPodTarget(integration, podId)` is now the one
reading of `config.gates` — the user-scope gate / pod-scope own-pod ternary that
both bridges' `isRelayableIntegration` and `decisionCardReconcileService` each
carried a copy of. Four consumers, one reading.
- `connectorRelayPolicy.isRoutedPodTarget({integration, pod, podId, userId})` is
the gate + membership conjunction, and both bridges' routed-reply check calls
it instead of restating it. The protocol-health half stays where it belongs —
in each bridge's own predicate, since chatType/teamId/chatId differ by protocol.
- The check-then-act window is now named and accepted in the predicate's
docstring, with what closing it would take, rather than left unstated.
- Both call sites state that the ACTIVE pod is exempt from the gate by design, so
"re-check the gate" cannot be read as universal.
- `decisionCardRelay.bridges.test.js` stubbed the whole relay-policy module with a
factory returning only `shouldEscalate`; it now spreads `jest.requireActual`, or
the new exports would be undefined in the bridges under test.
Witnesses: a new `connectorRelayPolicy.test.js` pins both predicates (both scopes,
absent/disabled/non-boolean gates, pod ids compared by value, the membership
conjunction, the missing-user-id guard). Six suites, 135 tests green.
…rd the ledger showed was redundant (TASK-156) Two findings from the mutation run on the refactor, both acted on rather than disclosed as noise: - Dropping the shared predicate from Slack's routed check reddened NOTHING, because the gate half is also carried by `isRelayableIntegration` there and the suite had no membership-gone arm on the Slack path (Telegram had one). Added: gate on, linked user no longer a member, refusal by name, nothing posted. - Restoring the `Boolean(userId)` guard in `isRoutedPodTarget` reddened nothing: `isPodMember` already fails closed on a falsy id. The guard is removed rather than kept unwitnessed; the predicate's own arm still pins the behaviour.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
TASK-156, ADR-025 D10/D11. Per the TASK-154 ruling, one private chat binds one Commonly user, and that user's pods sit behind it as gates. Two gaps appear the moment a second gate is on.
D10 — Telegram only. Slack already does both halves:
[pod];podIdwhere Slack's does — so a reply has nothing to route by.D11 — both bridges. A quote-reply landed in the active pod whichever pod the quoted line came from:
routeReplyContent/routeSlackReplyContentreturned the @mention and never the pod. The user's answer to pod B was authored into pod A, and the agent it named woke there without B's thread.What changed
backend/services/telegramBridgeService.ts,backend/services/slackBridgeService.ts:[Launch] <b>Kai</b>: …. The lookup degrades to the pod id on failure (never to silence, never to a name that was not read) — the tag is cosmetic and must not cost the relay.podId.podIdthere means "the quoted entry names a pod", not "routing is allowed".isRelayableIntegration, reused rather than restated). Any failure refuses in the chat, naming the pod, and posts nothing anywhere. The refusal is sent with the same delivery-failure classification as the existing "no active pod" reply.podIdroutes as it always has — pre-D11 rows carry no pod to check, and inbound still never consults the active pod's gate (isInboundRelayableIntegration, the rule attelegramBridgeService.ts:184).Falling back to the active pod is the defect this exists for, so nothing in the new path may do it: the refusal returns before any write.
The rule has one home (wren 74618, before the gate)
Wren's spec fix, applied on the head rather than deferred: the routed-reply rule was specified for two implementations and would have been written twice.
connectorRelayPolicy.isGatedPodTarget(integration, podId)is now the one reading ofconfig.gates— the user-scope-gate / pod-scope-own-pod ternary thatslackBridgeService.isRelayableIntegration,telegramBridgeService.isRelayableIntegrationanddecisionCardReconcileService.canSendClosingLineeach carried a copy of. Four consumers, one reading, no divergence possible.connectorRelayPolicy.isRoutedPodTarget({integration, pod, podId, userId})is the gate + membership conjunction; both bridges' routed check calls it instead of restating it. The protocol-health half stays in each bridge's own predicate, where it belongs —chatType/teamId/chatIdlegitimately differ by protocol.decisionCardRelay.bridges.test.jsstubbed the whole relay-policy module with a factory returning onlyshouldEscalate; it now spreadsjest.requireActual, or the new exports would beundefinedin the bridges under test.Witnesses — 13 new, 8 suites, 163 tests green
podId) still routes to the active podnullwhen it has noneconnectorRelayPolicy.test.jsconnectorRelayPolicy.test.jsLedger — 11 mutations over the committed head. Baseline and restore 44/44.
podIdfrom the map writeTwo ledger findings acted on, not disclosed as noise:
isRelayableIntegrationand the suite had no membership-gone arm on the Slack path (Telegram had one). Added that arm; the mutation now reddens exactly it. The overlap is real and intended — the gate is checked twice on the routed path, which is why dropping the shared predicate reddens only the membership half in both bridges.Boolean(userId)guard reddens nothing:isPodMemberalready fails closed on a falsy id. The guard is removed rather than kept unwitnessed; the predicate's own arm still pins the behaviour, and M11 stands as the measurement that established the redundancy.Sequencing against #1523
Open #1523 changes the same return (
replyStatus, for an entry that aged out of the 100-cap map). The two fields are additive and behaviourally independent: this PR is cut froma0568771(+ the #1933 rebase) and does not containreplyStatus, and nothing here depends on it. Whichever lands second does a four-line rebase over the two return sites — recommended: #1523 first, since it has been idle since 09-03, and TASK-156 rides on top. #1523's aged-out case has nopodId, so it stays with #1523 and routes as it does today.Scope notes
decisionCardReconcileService.canSendClosingLinetakes the shared gate predicate for the half it duplicated; it keeps its own membership read where it is, because the mute andchatIdchecks sit between the two halves there and folding them would move a check earlier for no gain.import/no-unresolved+import/extensionson the newrequire(...)calls — the same pair every pre-existing require in these two files reports; the backend.jscorpus is not linted (2,279 errors across 277 files). The.tssources are 0 errors.git checkout --, which is why the source is committed before any mutation run and the ledger in this body is the run over the committed head.Gate: Vera.