Skip to content

fix(connectors): relay takes the pod write path's membership, so a departed creator cannot relay (TASK-161) - #1940

Merged
lilyshen0722 merged 1 commit into
mainfrom
kai/task161-strict-membership
Sep 27, 2026
Merged

lilyshen0722 merged 1 commit into
mainfrom
kai/task161-strict-membership

Conversation

@lilyshen0722

@lilyshen0722 lilyshen0722 commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

A pod's creator who leaves the pod keeps relay, because two lines disagree about who may write. Filed by Vera, ruled by Wren (option A), scope and census on the row. Cut from c0f58a9a.

Revised head 9b36d921 (was 934dffdf, before that bfb8dc67) — Vera's 74663 hold folded in, then her 74671 re-gate: the three swaps whose witnesses were missing.

Revised head after Vera's 74663 hold — the reconciler is folded in (it is the site this PR makes newly wrong) and the two installables.ts connector sites are folded in with it, so the census claim below is now true rather than narrowed. Corrections to this body and to the row are in “What the first head got wrong” at the end.

The failure path

leavePod (podController.ts:598-600) filters members and never clears createdBy. createMessage (messageController.ts:207-218) and the socket post path (server.ts:503) check pod.members alone, so that person gets a 401. Connector relay checked utils/isPodMember, whose first clause counts createdBy as a member. Consequences, all measured on 19d1d3e2:

  1. Their Telegram/Slack messages still relay into the pod.
  2. The pod's messages still relay out to their private chat (isRoutedPodTarget = gate + membership, both pass).
  3. They can still turn new gates on, and aim their active inbound destination at it (integrations.ts:647/:670) — under a comment at :638-641 stating the invariant this breaks: "it must be a pod that owner can still write to."
  4. They can still install a connector into it (installables.ts:571, which seeds that pod's gate ON) and confirm a Slack bind into it (:406).

The creator clause is not wrong for what its comment claims ("Pod.members does not always list them" — a false negative at creation time). It was never a ruling that leaving leaves membership intact.

The fix

Wren's amendment put the definition beside isPodMember in utils/isPodMember.ts, with connectorRelayPolicy re-exporting it, so TASK-162's platform readers (pgMessageController.isMemberWithFallback, podWriteAccessService) import the same export rather than growing a second one. That is isListedPodMember: pod.members, nothing else — named for the mechanism, and the exact check createMessage runs.

Every connector site found by census now reads it through connectorRelayPolicy: no connector site calls the permissive predicate any more. That claim is bounded and measured — the census, the callers still on the permissive predicate, and the two exceptions are named below, so nothing here rests on the shape of a grep.

The census, measured

Remaining permissive callers in backend/ outside tests (Vera 74666's grep on bfb8dc67, which I re-checked on this head):

site what it gates stays permissive because
routes/activity.ts:336, :363 seed / create activities in a pod not a connector; the general creator-bypass question
routes/podInvites.ts:60, :102, :142 invite create / redeem / list same
services/activityService.ts:1297 activity write same
services/decisionRequestService.ts:373 chooseDecision — a human ruling from the app named below; needs the production-data question, not this row

Two exceptions to “every connector site”, both named rather than implied:

  1. externalFeedService.persistExternalPosts (:107) calls no predicate at all (wren 74669, now TASK-164). It writes posts into the pod as integration.createdBy (:137, from :129) and never reads membership, so a departed owner's scheduled X/Instagram sync still lands there where createPost would 403 them. I verified the claim on this head rather than transcribing it: externalFeedService.ts contains no membership read of any kind. It is out of this PR because it needs a membership read and a pause rule, not a swap.
  2. server.ts:503 keeps its own copy of the strict rule — already strict, no creator clause, used at :536 for the socket post path and exported at :814 with no non-test importer (wren 74666, now TASK-165). So after this PR the strict rule has one definition in utils and one pre-existing copy; TASK-165 points server.ts at the shared export, with an arm on the socket post path.

So the body's claim is: no connector site calls the permissive predicate, and the strict rule has one definition in utils plus one pre-existing copy in server.ts, tracked as TASK-165.

site what it authorises
isRoutedPodTarget outbound relay (both bridges), routed-quote-reply, decision-card delivery
slackBridgeService, telegramBridgeService inbound authorship of the relayed message
decisionCardReply, decisionCardReconcileService a card ruling on behalf of the member, and the closing line
integrations.ts ×3 the connector's own pod, its active inbound destination, every gate key
installables.ts ×2 the install route, and the Slack bind confirm
installable/eventHandlers.ts the selector half (below)
installable/installableReconciler.ts the gate sweep (below)

The selector half. activeHandlersForPod unioned pod.createdBy into memberIds, so on the departed-creator cell the selector and the predicate agreed — Vera's point that agreement is not authorisation, and the reason TASK-160's matrix was green on the wrong answer. The union is gone. That is also the mutation connector-ops measured as a survivor on #1938 (73/73) and deferred to this row: it now reddens (M2), and the matrix gained the creator cell so the exclusion is asserted rather than left to a comment.

The gate sweep — the hold. sweepOrphanedGates's own comment says it prunes obsolete keys "so the owner's gate list does not promise a pod they left", and it read the permissive predicate, so a departed creator's gate was never pruned. Before this PR that promise was merely stale (the gate still delivered); after it, the Connectors page shows an ON switch for a pod whose relay now refuses every message. Same predicate here, so the sweep and the relay cannot disagree again; the new arm is a non-active gate, because only this sweep touches one.

Witnesses — each fails before the fix

witness arm
(i) inbound refused refuses a quote-reply into a pod the linked user created and left, refuses an unquoted inbound message when the ACTIVE pod's creator left it (telegram), refuses inbound authorship when the linked user created the active pod and left (slack)
(ii) outbound refused refuses a pod whose creator is not listed in members (isRoutedPodTarget)
(iii) gate/enable refused (403) refuses a gate for a pod the linked owner created and then left, and the same for the active destination
install refused (403) rejects a departed CREATOR before any install row is claimed
bind refused (403) refuses to confirm a bind whose pod the caller created and left — slack_pod_access_denied had no arm at all before this
gate pruned prunes a NON-ACTIVE gate for a pod the owner created and then left
the selector half the matrix's creator not listed cells, asserted for the concrete expectation as well as agreement
create refused (403) refuses to create a connection for a pod the caller created and then left (POST /api/integrations, :393)
card reply refused, ledger untouched refuses a card reply from a pod creator who left, recording nothing (decisionCardReply.ts:84) — wren's ruling named this as witness (v); the arm did not exist until 74671
no closing line sends no closing line to a creator who left, while a listed sibling still receives one (decisionCardReconcileService.ts:94) — with the listed sibling as the control, so a 0 cannot be an arm that never ran
the complement still admits a creator who is also listed — the clause is gone, not the creator

Ledger — 8 mutations, baseline and restore 183/183

mutation red
M1 the permissive creator clause comes back (the row's mutation) 9 — every witness class
M2 pod.createdBy unioned back into memberIds 1 — the matrix creator cell, alone
M3 telegram inbound reads the permissive predicate 1 — that placement's arm, alone
M4 isRoutedPodTarget reads the permissive predicate 3 — the policy half's arms
M5 control: the strict predicate always admits 23 — every membership refusal, so the arms are live
M6 the reconciler's drift restored (predicate and select) 1 — the non-active gate arm, alone
M7 the install route reads the permissive predicate 1 — its arm, alone
M8 the Slack bind confirm reads the permissive predicate 1 — its arm, alone
M9 the create route reads the permissive predicate (integrations.ts:393) 1 — its arm, alone
M10 the card reply reads the permissive predicate (decisionCardReply.ts:84) 1 — its arm, alone
M11 the closing-line gate reads the permissive predicate (decisionCardReconcileService.ts:94) 1 — its arm, alone

M9–M11 are the three swaps Vera's 74671 found unwitnessed: restoring the permissive predicate at each site reddened nothing across unit/routes and both service suites, so the change was correct and unguarded. Each now has its own arm and its own one-armed mutation.

No survivors. Four instrument disclosures, all measured rather than assumed:

  • M5's first draft was an instrument failure, not a result (return true followed by unreachable code broke the compile; jest printed Tests: 0 total), so it was rewritten to compile and re-run.
  • M10's first form was an over-red, not a finding. The mutation restores the permissive predicate by requiring it directly, and my first path was require('./utils/isPodMember') inside services/ — module-not-found at call time, so 33 arms reddened, including ones with nothing to do with membership. Corrected to require('../utils/isPodMember'): 1 red, alone. An instrument that reddens everything is as void as one that reddens nothing, and this is the same class as Vera's own ts-jest disclosure below.
  • N failed counts both provider passes; the name list is unique. decisionCardReply.bridges is a describe.each(['telegram', 'slack']), so an arm that fails fails twice under one name — M10 reads 2 failed and one name, M1 13 failed and twelve.
  • One failure in the wide run, not reproduced: unit/routes/tasksApi.updateRenewsLease failed in the 295-suite run and passed 25/25 on immediate re-run of that suite alone. It is the repo's known latent flake (TASK-149 half b, never reproduced), and this diff does not touch the task surface; recorded rather than dropped.
  • M6 has to be a two-edit mutation. Reverting the predicate alone is invisible, because the strict form narrows its own query to .select('members') and the creator clause then has no field to read — the first version of this arm survived that revert. The mutation restores both halves of the drift. That masking is disclosed, not relied on: put the predicate back with a wide select and the arm reddens.

Fixture fallout — disclosed, not tidied

Three route suites stubbed the whole membership module with jest.mock('../utils/isPodMember', () => jest.fn(() => true)); that stub no longer reaches the connector sites, and it is also why those suites were not exercising the write gate at all. They build real pods with the caller listed, so the stub is deleted rather than re-pointed: integrations.discordTokenCopy, integrations.manifestStatus, integrations.routingState. integrations.validation's mocked pod listed nobody and leaned on the same clause; it now lists its caller.

integrations.linkedUserId had two 200 arms asserting "pods they belong to" while the fixture was members: [] with createdBy: user-1 — i.e. the arms passed because of the bypass they were meant to be indifferent to. Those fixtures now list the owner, and the departed-creator call is a new pair of 403 arms instead. installableInstallationService's pause/prune arm had the same shape and now lists its owner too.

Scope, and what is deliberately left

  • Out of scope, measured (Vera 74657): activityService, decisionRequestService, routes/activity.ts, routes/podInvites.ts keep isPodMember — see the census table above. They are app/agent surfaces behind a user JWT, and every unlisted creator in the census is a bot, which cannot hold one. Not connector paths. TASK-164 (the feed writer) and TASK-165 (server.ts's copy) carry the two exceptions.
  • Named, not fixed: decisionRequestService.chooseDecision (:373) also reads the permissive predicate. Its connector caller (decisionCardReply) is strict as of this PR, so the relay path is closed; what remains is a human creator who left ruling on a decision from the app, which is the general creator-bypass question (TASK-162's neighbourhood), not this row's. Its reachable population is the 2 human ghost-row holders in Vera's census — I have no production access to measure whether any of them has a pending decision, so it is recorded rather than claimed.
  • TASK-162's binding constraint is satisfied here: one definition, in utils, re-exported for connectors, so its platform readers call the same export.
  • Verified: unit/services + unit/routes = 295 suites / 2774 tests, 1 flake above (295 unit/routes+unit/services suites: 139+156) and the ledger's own 11-suite instrument at 275/275 baseline and restore; both changed non-test .ts files 0 lint problems; routes/integrations.ts's 33 max-len warnings pre-existing and identical at HEAD~1; 0 diagnostics on added lines in every changed file except decisionCardReconcileService.test.js, where the 28 are the file's own idiom — measured at 194 diagnostics at HEAD~1 → 222 at HEAD (154 of 194 are object-property-newline, the rule that fires on my 24), so they match the neighbours rather than introducing a rule.

What the first head got wrong

  • “No connector site calls isPodMember any more” was false when I wrote it. installables.ts:406 (Slack bind confirm) and :571 (install) were connector sites that my census — wren's four, taken as the census — did not include. Fixed here rather than re-worded, and the arm-less slack_pod_access_denied now has one.
  • The reconciler was the site this PR made newly wrong, and I had filed it under “out of scope, measured”. Vera measured the consequence instead: a gate that still delivered became a gate that cannot. Folded in, with the witness.
  • “8 mutations, no survivors” was true of the eight I ran and not of the change. Three sites (integrations.ts:393, decisionCardReply.ts:84, decisionCardReconcileService.ts:94) had been swapped and were unwitnessed; decisionCardReply is the one wren's ruling had already named as witness (v). All three are witnessed above. That is the second completeness claim on this PR to outrun its ledger, which is why the ledger is now stated per-site rather than per-class, and why M9–M11 exist at all.

Gate: Vera.

@samxu01
samxu01 force-pushed the kai/task161-strict-membership branch from bfb8dc6 to 934dffd Compare September 27, 2026 04:29
…parted creator cannot relay (TASK-161)

`leavePod` filters `members` and never clears `createdBy`. The app's write path
(`createMessage`, the socket post path) checks `pod.members` only and 401s that
person; connector relay checked `utils/isPodMember`, whose first clause counts
`createdBy` as a member. So a pod's creator who left it could not post in the pod
from Commonly but could still relay into it from Telegram or Slack, still receive
its messages in their private chat, and could still turn new gates on for it —
under a comment in `integrations.ts` stating the invariant that breaks
("it must be a pod that owner can still write to").

Wren ruled option A: connector sites take the pod write path's membership, one
definition. That definition is `isListedPodMember`, beside `isPodMember` in
`utils/isPodMember.ts` — `pod.members` alone, named for the mechanism
(`Pod.members` "does not always list" the creator, which is what the permissive
clause answers, and never a ruling that leaving leaves membership intact).

`connectorRelayPolicy` re-exports it and every connector site in the census reads
it through that module; no connector site calls `isPodMember` any more:

- `isRoutedPodTarget` — outbound relay for both bridges, the routed-quote-reply
  check, and decision-card delivery.
- `slackBridgeService` / `telegramBridgeService` inbound authorship.
- `decisionCardReply` (a card ruling written on behalf of a departed member) and
  `decisionCardReconcileService` (the closing line).
- `routes/integrations.ts` — the connector's own pod, its active inbound
  destination, and every requested gate key.
- `routes/installables.ts` — the install route, which is how a connector enters a
  pod and seeds that pod's gate, and the Slack bind confirm.
- `installable/eventHandlers.ts` `activeHandlersForPod` — dropped the
  `pod.createdBy` union from `memberIds`, the selector half of the same rule.
- `installable/installableReconciler.ts` `sweepOrphanedGates` — the half that
  closes the loop this PR would otherwise open: its own comment says it prunes
  gates "so the owner's gate list does not promise a pod they left", and the
  permissive predicate left it blind to the departed creator, whose ON switch
  this PR makes undeliverable (Vera 74663).

Fixture fallout, disclosed rather than tidied: three route suites stubbed the
whole membership module with `jest.fn(() => true)`, which is also why they were
not exercising the write gate at all. They build real pods with the caller
listed, so the stub is deleted rather than re-pointed, and they now run the
predicate. `integrations.validation.test.js`'s mocked pod listed nobody and
leaned on the same clause; it now lists its caller. `integrations.linkedUserId`
had two 200 arms whose pods claimed `members: []` while asserting "pods they
belong to" — those fixtures now list the owner, so they still mean what they say.

Out of scope, measured: `activityService`, `decisionRequestService`,
`routes/activity.ts` and `routes/podInvites.ts` keep
`isPodMember` — they are the app/agent surfaces Vera
measured as unreachable by the bypass (bots cannot hold a user JWT), not
connector paths.
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