Multi-account connectors and per-trigger account pinning. Ten findings
from a high-effort review of 5c35f6c5d…HEAD. Four are
marked verified — I re-checked those against the code myself;
the rest are the reviewer's, unconfirmed by me.
01
Slack resolves an event to one connection, so a pinned trigger drops
High
Verified
Fixed
apps/api/src/app/api/integrations/slack/events/process-automation-event/process-automation-event.ts:52
Slack calls accountConnection("slack", team_id) —
singular, ORDER BY updated_at DESC LIMIT 1 — while
Linear (processDelivery.ts:97) and Notion
(notion/webhook/route.ts:102) fan out over
accountConnections. I confirmed that asymmetry.
What breaks: two members of one org both
connect workspace T. A adds a Slack trigger, auto-pinned to A's
connection. B's token refresh bumps updated_at, so every
event for T now arrives carrying B's connection id, and the new
or(isNull, eq(connectionId, …)) filter drops A's
trigger. A's automation stops firing with no UI signal, and it flips
back and forth as tokens refresh.
Fixed. processAutomationEvent now resolves with
accountConnections and records the event once per
connection on the workspace, so each account's pinned triggers get
their own delivery. Sequential rather than concurrent, for the
reason Linear's fan-out documents — neon-http opens a connection per
query, and asking for all of them at one instant starves the pool.
One connection failing is caught and logged so it cannot cost the
others their delivery. The fan-out itself lives in its own module with a type-only import, so its four tests need no database stub — registering one reached whichever suite loaded next and broke it.
Which field each provider maps to a connection
The column being matched is
connections.external_account_id, populated at connect
time from the connector's identity.account.id —
except Sentry, which matches on connection state instead.
Two things fall out of this table. Slack is the only
user-scoped connector resolving through a singular lookup,
which is exactly this finding. And Sentry keys on installation state
rather than the account column, so a pin would compare against a
connection the account id never identified — moot today, because
Sentry and Teams are org-scoped and cannot hold two connections, but
it would matter if either ever became user-scoped.
02
The default pin picks an arbitrary account
Medium
Verified
Fixed
apps/desktop/…/TriggersEditor/TriggersEditor.tsx:70 ·
packages/trpc/src/router/integration/connection-status.ts:43
A new trigger is pinned to
connectorAccounts(config)[0]?.id, but that list comes
from a findMany with no orderBy — I
confirmed there is none. Postgres returns heap order, which changes
whenever a row is updated, and a token refresh is an update.
What breaks: a user with work + personal
Slack gets a trigger bound to an arbitrary one of them, and two
triggers added minutes apart can land on different accounts.
Fixed. Both reads now order by
(createdAt, id) — connection-status for
the picker and the default pin, and connectors.status
for the account list. Fixing the Slack fan-out did not make this
moot: fan-out decides which connections receive an event, this
decides which one a new trigger listens to.
03
The backfill can overwrite a deliberate “Any account”
Medium
packages/trpc/src/lib/connectors/trigger-pins.ts:42
NULL is overloaded: it means both “never set” and the
“Any account” the chip now writes.
pinTriggersToExistingAccount updates every trigger with
connectionId IS NULL, so a deliberate choice can be
silently replaced.
Reachable how: the backfill only runs on
the one-to-two transition, and the chip only appears at two or more
accounts — so the path is: hold two accounts, set a trigger to Any,
disconnect one, connect another. Narrower than it first reads, but
real.
Distinguish legacy NULL from deliberate NULL — a one-shot backfill
marker, or restrict the update to triggers created before the column
shipped.
04
The backfill runs outside the connect transaction and can fail the connect
Medium
Fixed
packages/trpc/src/lib/connectors/upsert.ts:143
It is awaited after the connection row is committed, in no
transaction, and its rejection propagates out of
upsertConnection.
What breaks: a transient DB error makes
the OAuth callback report failure even though the account is
connected. On retry, previousConnectionIds now contains
the new row, so accountToPinTo returns null and the
backfill never runs again.
Fixed. The prior-account read, the connection write and the
pin now commit together in one dbWs.transaction.
Catching and logging was not enough on its own: it keeps the connect
from reporting failure, but a failed pin still leaves the new
account connected with every existing trigger unpinned — matching
both accounts — and the retry cannot repair it, because by then the
new row is in previousConnectionIds. Encryption stays
outside the transaction; it touches no database.
05
An older client silently clears every pin, and pins are not validated
Medium
Partly fixed
packages/trpc/src/router/automation/triggerSet.ts:120
connectionId: trigger.connectionId ?? null overwrites
unconditionally, and the field is .nullish() in
draftTriggerSchema. Desktop and API ship independently,
so an older desktop build round-tripping a trigger without the field
clears the pin on the next save. Nothing checks that the uuid names a
connection in the caller's org either, so a bad id persists as a
trigger that never fires and never errors.
Half fixed. saveTriggerSet now distinguishes
absent from null — undefined is a client that does not know the
field and leaves the pin alone, null is the “any account” the editor
writes. Server-side ownership validation of the uuid is
still open; a pin naming another org's connection persists
and matches nothing.
06
A needs-reauth account vanishes from the picker
Medium
packages/trpc/src/router/integration/connection-status.ts:79
accounts only receives rows with
disconnectedAt === null, while
connected/needsReauth still fold in expired
rows. With A live and B needing reauth, the connector reports
needsReauth: false and the picker lists only A — so a
trigger pinned to B renders as “Unknown account” with no reconnect
affordance, and no way to tell that B just needs re-authorizing.
Include needs-reauth rows in accounts with a flag, or
surface them separately.
07
A dangling pin after disconnect stops the automation silently
Medium
apps/desktop/…/TriggerSentence/components/AccountChip/AccountChip.tsx:36
connectors.disconnect is a soft delete. Disconnecting
the pinned account while another remains leaves the trigger pointing
at a dead id: the chip reads “Unknown account”,
saveTriggerSet accepts it, and the automation stops
firing with nothing in summarizeTriggerProblems and
nothing in the automations list.
Make a dangling pin a trigger problem that blocks save, or at minimum
warns.
08
Try now inherits a stale draft
Low
Verified
apps/desktop/…/PluginDetail/PluginDetail.tsx:46
tryNow calls updateDraft({ prompt })
without resetDraft() first. I confirmed the convention:
every other seed-then-open caller resets — the PR header, the GitHub
issues view, and the issue page all call
resetDraft() before seeding.
What breaks: a user who earlier started a
workspace from a PR still has linkedPR,
selectedProjectId, hostId and attachments
in the store, so Try now opens the create surface linked to an
unrelated PR.
Call resetDraft() before seeding the prompt.
09
“Primary” is a label with nothing behind it
Low
Verified
apps/desktop/…/ConnectedAccounts/ConnectedAccounts.tsx:52 ·
packages/trpc/src/router/connectors/index.ts:73
The subtitle is index === 0 over
connectors.status, which I confirmed also has no
orderBy. There is no backend concept of a primary
account, so the badge moves to a different account after a refetch.
When externalUserLabel is null the non-first rows also
repeat the title verbatim as their subtitle.
Drop the label, or give it real backing and a deterministic order.
10
A cast hides a stale trigger-kind key until runtime
Low
packages/trpc/src/lib/connectors/trigger-pins.ts:47
kinds as AutomationTriggerKind[] casts values derived
from TRIGGER_KIND_CONNECTOR, typed
Record<string, string | null>. A typo'd or stale
key becomes an invalid enum literal in the inArray at
runtime instead of a compile error; the registry test only covers
kinds present in TRIGGER_PROVIDERS.
Type the map as
Record<AutomationTriggerKind, string | null> and
drop the cast.
Findings 01, 02, 08 and 09 are mine, introduced by this PR, and 01 makes
the feature unsafe for Slack specifically. I would fix 01, 02, 04 and 08
before handing this over, and send the rest as known and tracked.