PR #8003 — review findings

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.

1High
6Medium
3Low
4Verified by me
6Still open
4Fixed
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.

Provider Field in the inbound payload Matched against Lookup
Slack envelope.team_id external_account_id (from $.team_id) singular — accountConnection, LIMIT 1
Linear payload.organizationId external_account_id (from viewer.organization.id) fan-out — accountConnections
Notion event.workspace_id external_account_id (from $.workspace_id) fan-out — accountConnections
Gmail notification.emailAddress, lowercased external_account_id (from $.email) fan-out — accountConnections
Teams notification.tenantId, plus a clientState equality check external_account_id (the tenant id) singular — org-scoped, so only one can exist
Sentry installationUuid from the install callback state->>'installationUuid' — not external_account_id singular — org-scoped
GitHub — none; installation-backed, stamped null n/a

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.

Verified clean

Recommendation

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.

Scope: git diff 5c35f6c5d…HEAD, 6 commits. Generated files excluded after confirming the migration, journal and snapshot are consistent and every new message is translated. “Verified” means I re-checked the claim against the code in this worktree; the others are the reviewer's and I have not independently confirmed them.