Skip to content

Commit bc27482

Browse files
KrisBraunclaude
andcommitted
fix(twister): require channelId on NewLinkWithNotes so write-back can't silently break
NewLinkWithNotes.channelId (the type saveLink()/saveLinks() accept) was optional, which made it easy to set channelId only inside meta and forget the top-level field. That compiled fine but silently broke outbound write-back: the platform reads a link's channel back from the persisted channelId column, not from meta, when resolving it for connector callbacks like onNoteCreated. A link built that way would send replies to nowhere with no error anywhere. channelId is now required on NewLinkWithNotes, so this fails to compile instead of failing silently in production. Added CreateLinkResult as the return type for Connector.onCreateLink(), where channelId stays optional (the platform auto-fills it from the compose draft when omitted, since there's no equivalent auto-fill for saveLink()/saveLinks() calls). Fixed every connector in this repo affected by the tightened type, several of which had exactly the bug this catches: channelId set only in meta, never at the top level. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 9fc045a commit bc27482

22 files changed

Lines changed: 95 additions & 36 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@plotday/twister": minor
3+
---
4+
5+
Changed: `NewLinkWithNotes.channelId` (the type `integrations.saveLink()`/`saveLinks()` accept) is now required instead of optional. It was easy to set `channelId` only inside `meta` and forget the top-level field — that compiled fine but silently broke outbound write-back (`onNoteCreated` reads the channel back from the persisted `channelId`, not from `meta`) and bulk operations like `archiveLinks({ channelId })` on disable, with no error anywhere. The type system now catches this at compile time instead.
6+
7+
Added: `CreateLinkResult` — the return type for `Connector.onCreateLink()`. Identical to `NewLinkWithNotes` except `channelId` stays optional, since the platform auto-fills it from the compose draft when omitted. Connectors implementing `onCreateLink` should update their return type from `NewLinkWithNotes | null` to `CreateLinkResult | null`.

‎connectors/AGENTS.md‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -301,13 +301,17 @@ Previews (`preview` fields) always use plain text — `snippet` or truncated tit
301301

302302
## Sync metadata injection
303303

304-
Every synced link must carry provider and channel metadata — bulk operations (e.g. `integrations.archiveLinks({ channelId })` on disable) rely on it:
304+
Every synced link must carry provider and channel metadata:
305305

306306
```typescript
307-
link.channelId = resourceId; // first-class field on NewLink
307+
link.channelId = resourceId; // first-class field on NewLinkWithNotes — REQUIRED
308308
link.meta = { ...link.meta, syncProvider: "myprovider" };
309309
```
310310

311+
`channelId` is required on `NewLinkWithNotes` (the type `saveLink()`/`saveLinks()` accept) precisely because it's easy to set only inside `meta` and forget the top-level field — the type system will reject a link built that way. This isn't just about bulk operations like `integrations.archiveLinks({ channelId })` on disable: the platform persists `channelId` to the link's DB row and reads it back from there — not from `meta` — to populate `thread.meta.channelId` for connector callbacks like `onNoteCreated`. A link with `channelId` only inside `meta` will send replies to nobody: the connector's own `onNoteCreated` reads a channelId that was never actually saved, and typically no-ops silently (no error, nothing captured) because it can't resolve a client for the reply. Set `channelId` at the top level on every value you pass to `saveLink()`/`saveLinks()`.
312+
313+
`onCreateLink` is the one exception: its return type is `CreateLinkResult`, where `channelId` is optional — the platform auto-fills it from `draft.channelId` (the channel the user composed into) if you omit it.
314+
311315
## Classifier facets (optional)
312316

313317
Messaging-style connectors may set `link.facets` (`format` / `automation` / `reach` from `@plotday/twister/facets`) as internal classifier signal. Set a dimension only when a heuristic is confident; leave it `null`/omitted otherwise. See `gmail/src/gmail-facets.ts` and `slack/src/slack-facets.ts`.
@@ -520,7 +524,7 @@ For closed-roster DM-style compose set `compose.targets: "contacts"`; for open a
520524
Then implement `onCreateLink` — return the link, do NOT call `integrations.saveLink()` (platform wires it to the originating thread):
521525

522526
```typescript
523-
async onCreateLink(draft: CreateLinkDraft): Promise<NewLinkWithNotes | null> {
527+
async onCreateLink(draft: CreateLinkDraft): Promise<CreateLinkResult | null> {
524528
if (draft.type !== "issue") return null;
525529
const client = await this.getClient(draft.channelId);
526530
const payload = await client.createIssue({
@@ -609,7 +613,7 @@ Add to `pnpm-workspace.yaml` if not already covered by a glob.
609613
- [ ] Batch state writes use `this.setMany()`, never a per-item `set()` loop
610614
- [ ] Canonical, globally-unique `source` using immutable ids; mutable keys in `meta` only
611615
- [ ] `note.key` for note-level upserts
612-
- [ ] Set `link.channelId` and inject `syncProvider` into `link.meta`
616+
- [ ] Set `link.channelId` (top-level field, required) and inject `syncProvider` into `link.meta`
613617
- [ ] `contentType: "html"` for HTML — never strip tags locally
614618
- [ ] `created` on notes = external timestamp, not sync time
615619
- [ ] `initialSync` propagated through every entry point and batch; set `unread: false, archived: false` on initial, omit on incremental
@@ -622,7 +626,7 @@ Add to `pnpm-workspace.yaml` if not already covered by a glob.
622626
## Common pitfalls
623627

624628
1. Passing functions/`undefined`/RPC stubs to `this.callback()` → use tokens + `null`.
625-
2. Forgetting sync metadata (`link.channelId`, `meta.syncProvider`) → breaks bulk archive on disable.
629+
2. Setting `channelId` only inside `link.meta` instead of at the top level → `NewLinkWithNotes.channelId` is what the platform actually persists and reads back for connector callbacks (`thread.meta.channelId` in `onNoteCreated`, etc.) and for bulk operations like `integrations.archiveLinks({ channelId })` on disable. A `meta`-only channelId compiles fine structurally but leaves outbound replies (and disable-time cleanup) silently broken — no error, nothing captured. The type system now requires the top-level field on `NewLinkWithNotes`, so this fails to compile instead of failing silently in production.
626630
3. Not propagating `initialSync` through the whole pipeline → notification spam.
627631
4. Mutable ids in `source` (e.g. Jira issue key) → use immutable id, store key in `meta`.
628632
5. `source` that's only unique within one user's account → breaks cross-user dedup; add workspace/tenant/mailbox qualifier.

‎connectors/github/src/issue-sync.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,6 @@ export async function syncIssueBatch(
9494
);
9595

9696
if (link) {
97-
link.channelId = repositoryId;
9897
link.meta = {
9998
...link.meta,
10099
syncProvider: "github",
@@ -209,6 +208,7 @@ async function convertIssueToLink(
209208
}
210209

211210
const link: NewLinkWithNotes = {
211+
channelId: repositoryId,
212212
source: `github:issue:${owner}/${repo}/${issue.number}`,
213213
type: "issue",
214214
title: issue.title,

‎connectors/github/src/pr-sync.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,6 @@ export async function syncPRBatch(
102102
);
103103

104104
if (thread) {
105-
thread.channelId = repositoryId;
106105
thread.meta = {
107106
...thread.meta,
108107
syncProvider: "github",
@@ -219,6 +218,7 @@ async function convertPRToThread(
219218
}
220219

221220
const thread: NewLinkWithNotes = {
221+
channelId: repositoryId,
222222
source: `github:pr:${owner}/${repo}/${pr.number}`,
223223
type: "pull_request",
224224
title: pr.title,

‎connectors/gmail/src/gmail-api.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -855,8 +855,11 @@ export function collectAttachments(
855855
*/
856856
export function transformGmailThread(thread: GmailThread): NewLinkWithNotes {
857857
if (!thread.messages || thread.messages.length === 0) {
858-
// Return empty structure for invalid threads
858+
// Return empty structure for invalid threads. channelId is unknown at
859+
// this call site — the caller always sets the real value right after
860+
// (see "Inject channel ID" in sync.ts) before saving.
859861
return {
862+
channelId: null,
860863
type: "email",
861864
title: "",
862865
notes: [],
@@ -909,8 +912,10 @@ export function transformGmailThread(thread: GmailThread): NewLinkWithNotes {
909912
}
910913
}
911914

912-
// Create link
915+
// Create link. channelId is unknown here — the caller always sets the
916+
// real value right after (see "Inject channel ID" in sync.ts) before saving.
913917
const plotThread: NewLinkWithNotes = {
918+
channelId: null,
914919
source: canonicalUrl,
915920
type: "email",
916921
title: subject || "Email",

‎connectors/gmail/src/gmail.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import {
44
type NoteWriteBackResult,
55
type ToolBuilder,
66
} from "@plotday/twister";
7-
import type { Actor, NewLinkWithNotes, Note, Thread } from "@plotday/twister/plot";
7+
import type { Actor, CreateLinkResult, Note, Thread } from "@plotday/twister/plot";
88
import {
99
AuthProvider,
1010
type AuthToken,
@@ -774,7 +774,7 @@ export class Gmail extends Connector<Gmail> {
774774
*/
775775
override async onCreateLink(
776776
draft: CreateLinkDraft
777-
): Promise<NewLinkWithNotes | null> {
777+
): Promise<CreateLinkResult | null> {
778778
return onCreateLinkFn(this.makeHost(), draft);
779779
}
780780

‎connectors/gmail/src/sync.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import { ActionType } from "@plotday/twister/plot";
2626
import type {
2727
Actor,
2828
ActorId,
29+
CreateLinkResult,
2930
NewLinkWithNotes,
3031
Note,
3132
Thread,
@@ -1968,7 +1969,7 @@ async function onCreateLinkForwardFn(
19681969
host: GmailSyncHost,
19691970
draft: CreateLinkDraft,
19701971
forwardKey: string
1971-
): Promise<NewLinkWithNotes | null> {
1972+
): Promise<CreateLinkResult | null> {
19721973
const api = await getApiAnyFn(host);
19731974
if (!api) {
19741975
console.error(
@@ -2154,7 +2155,7 @@ async function onCreateLinkForwardFn(
21542155
export async function onCreateLinkFn(
21552156
host: GmailSyncHost,
21562157
draft: CreateLinkDraft
2157-
): Promise<NewLinkWithNotes | null> {
2158+
): Promise<CreateLinkResult | null> {
21582159
if (draft.type !== "email") return null;
21592160

21602161
if (draft.forward) {

‎connectors/google-calendar/src/sync.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -725,6 +725,7 @@ export async function processCalendarEventsFn(
725725
};
726726

727727
const link: NewLinkWithNotes = {
728+
channelId: calendarId,
728729
source: canonicalUrl,
729730
sources: buildEventSources({
730731
iCalUID: event.iCalUID,
@@ -760,7 +761,6 @@ export async function processCalendarEventsFn(
760761
...(initialSync ? { archived: false } : {}),
761762
};
762763

763-
link.channelId = calendarId;
764764
link.meta = {
765765
...link.meta,
766766
syncProvider: "google",
@@ -880,6 +880,7 @@ export async function processCalendarEventsFn(
880880
const notes = descriptionNote ? [descriptionNote] : [];
881881

882882
const link: NewLinkWithNotes = {
883+
channelId: calendarId,
883884
source: canonicalUrl,
884885
sources: buildEventSources({
885886
iCalUID: event.iCalUID,
@@ -913,7 +914,6 @@ export async function processCalendarEventsFn(
913914
...(initialSync ? { archived: false } : {}),
914915
};
915916

916-
link.channelId = calendarId;
917917
link.meta = {
918918
...link.meta,
919919
syncProvider: "google",

‎connectors/google-chat/src/google-chat-api.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -657,6 +657,7 @@ export function transformChatThread(
657657
const preview = firstMessage.text?.substring(0, 200) ?? null;
658658

659659
return {
660+
channelId: spaceId,
660661
source: `google-chat:${spaceId}:thread:${threadKey}`,
661662
type: "thread",
662663
title,

‎connectors/google/src/google.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ import type {
6767
Thread,
6868
ToolBuilder,
6969
} from "@plotday/twister";
70-
import type { NewLinkWithNotes, Note } from "@plotday/twister/plot";
70+
import type { CreateLinkResult, Note } from "@plotday/twister/plot";
7171
import type { ScheduleContactStatus } from "@plotday/twister/schedule";
7272
import {
7373
AuthProvider,
@@ -863,7 +863,7 @@ export class Google extends Connector<Google> {
863863
*/
864864
override async onCreateLink(
865865
draft: CreateLinkDraft
866-
): Promise<NewLinkWithNotes | null> {
866+
): Promise<CreateLinkResult | null> {
867867
if (draft.type === "task") {
868868
return tasksOnCreateLinkFn(this.makeTasksHost(), draft);
869869
}

0 commit comments

Comments
 (0)