Skip to content

Commit 0004e24

Browse files
committed
Fix duplicate comments when commenting inside Plot
1 parent 3c5fc02 commit 0004e24

11 files changed

Lines changed: 138 additions & 18 deletions

File tree

.changeset/bumpy-guests-design.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@plotday/tool-linear": patch
3+
"@plotday/tool-asana": patch
4+
"@plotday/tool-jira": patch
5+
---
6+
7+
Fixed: Duplicate comments when commenting inside Plot

.changeset/old-nights-dress.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@plotday/twister": patch
3+
---
4+
5+
Added: Update Note.key when updating with id

tools/AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,7 @@ Building a tool? Follow this checklist:
279279
4. **❌ Forgetting to store the callback token** - Store it immediately after creating
280280
5. **❌ Passing undefined instead of null** - Use `null` for optional values
281281
6. **❌ Not breaking loops into batches** - Each execution has ~1000 request limit; use `runTask()` for fresh limits
282+
7. **❌ Two-way sync without metadata correlation** - When pushing Plot items to an external system, embed the Plot ID (`Activity.id` / `Note.id`) in the external item's metadata, and update `source`/`key` after creation. In webhook handlers, check metadata for the Plot ID first. This prevents duplicates from a race condition where the webhook arrives before the `source`/`key` update. See SYNC_STRATEGIES.md §6 for a full example.
282283

283284
---
284285

tools/asana/src/asana.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -475,16 +475,20 @@ export class Asana extends Tool<Asana> implements ProjectTool {
475475
authToken: string,
476476
meta: ActivityMeta,
477477
body: string
478-
): Promise<void> {
478+
): Promise<string | void> {
479479
const taskGid = meta.taskGid as string | undefined;
480480
if (!taskGid) {
481481
throw new Error("Asana task GID not found in activity meta");
482482
}
483483
const client = await this.getClient(authToken);
484484

485-
await client.tasks.addComment(taskGid, {
485+
const result = await client.tasks.addComment(taskGid, {
486486
text: body,
487487
});
488+
489+
if (result?.gid) {
490+
return `story-${result.gid}`;
491+
}
488492
}
489493

490494
/**

tools/jira/src/jira.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -605,8 +605,9 @@ export class Jira extends Tool<Jira> implements ProjectTool {
605605
async addIssueComment(
606606
authToken: string,
607607
meta: import("@plotday/twister").ActivityMeta,
608-
body: string
609-
): Promise<void> {
608+
body: string,
609+
noteId?: string,
610+
): Promise<string | void> {
610611
const issueKey = meta.issueKey as string | undefined;
611612
if (!issueKey) {
612613
throw new Error("Jira issue key not found in activity meta");
@@ -616,10 +617,15 @@ export class Jira extends Tool<Jira> implements ProjectTool {
616617
// Convert plain text to Atlassian Document Format (ADF)
617618
const adfBody = this.convertTextToADF(body);
618619

619-
await client.issueComments.addComment({
620+
const result = await client.issueComments.addComment({
620621
issueIdOrKey: issueKey,
621622
comment: adfBody,
623+
properties: noteId ? [{ key: "plotNoteId", value: noteId }] : undefined,
622624
});
625+
626+
if (result?.id) {
627+
return `comment-${result.id}`;
628+
}
623629
}
624630

625631
/**
@@ -812,13 +818,21 @@ export class Jira extends Tool<Jira> implements ProjectTool {
812818
? comment.body
813819
: this.extractTextFromADF(comment.body);
814820

821+
// Check for Plot note ID in comment properties (set when comment was created from Plot)
822+
const plotNoteId = comment.properties?.find(
823+
(p: any) => p.key === "plotNoteId"
824+
)?.value;
825+
815826
// Create activity update with single comment note
816827
const activity: NewActivityWithNotes = {
817828
...(source ? { source } : {}),
818829
type: ActivityType.Action, // Required field (will match existing activity)
819830
notes: [
820831
{
821832
key: `comment-${comment.id}`,
833+
// If this comment originated from Plot, identify by note ID so we update the existing note
834+
// rather than creating a duplicate
835+
...(plotNoteId ? { id: plotNoteId } : {}),
822836
activity: source ? { source } : undefined,
823837
content: commentText,
824838
created: comment.created ? new Date(comment.created) : undefined,

tools/linear/src/linear.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -489,17 +489,13 @@ export class Linear extends Tool<Linear> implements ProjectTool {
489489
if (!currentAssigneeActorId) {
490490
updateFields.assigneeId = null;
491491
} else {
492-
const actors = await this.tools.plot.getActors([
493-
currentAssigneeActorId,
494-
]);
492+
const actors = await this.tools.plot.getActors([currentAssigneeActorId]);
495493
const actor = actors[0];
496494
const email = actor?.email;
497495

498496
if (email) {
499497
// Check cache first
500-
let linearUserId = await this.get<string>(
501-
`linear_user:${email}`
502-
);
498+
let linearUserId = await this.get<string>(`linear_user:${email}`);
503499

504500
if (!linearUserId) {
505501
// Query Linear for user by email
@@ -578,17 +574,22 @@ export class Linear extends Tool<Linear> implements ProjectTool {
578574
authToken: string,
579575
meta: ActivityMeta,
580576
body: string
581-
): Promise<void> {
577+
): Promise<string | void> {
582578
const issueId = meta.linearId as string | undefined;
583579
if (!issueId) {
584580
throw new Error("Linear issue ID not found in activity meta");
585581
}
586582
const client = await this.getClient(authToken);
587583

588-
await client.createComment({
584+
const payload = await client.createComment({
589585
issueId,
590586
body,
591587
});
588+
589+
const comment = await payload.comment;
590+
if (comment?.id) {
591+
return `comment-${comment.id}`;
592+
}
592593
}
593594

594595
/**

twister/cli/templates/AGENTS.template.md

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,43 @@ const activity: NewActivity = {
570570
- **Incremental sync**: New activities appear as unread, and archived state is preserved (respects user's archiving decisions)
571571
- **Reinstall**: Acts as initial sync, so previously archived activities are unarchived (fresh start)
572572

573+
### Two-Way Sync: Avoiding Race Conditions
574+
575+
When implementing two-way sync where items created in Plot are pushed to an external system (e.g. Notes becoming comments), a race condition can occur: the external system may send a webhook for the newly created item before you've updated the Activity/Note with the external key. The webhook handler won't find the item by external key and may create a duplicate.
576+
577+
**Solution:** Embed the Plot `Activity.id` / `Note.id` in the external item's metadata when creating it, and update `Activity.source` / `Note.key` after creation. When processing webhooks, check for the Plot ID in metadata first.
578+
579+
```typescript
580+
async pushNoteAsComment(note: Note, externalItemId: string): Promise<void> {
581+
// Create external item with Plot ID in metadata for webhook correlation
582+
const externalComment = await externalApi.createComment(externalItemId, {
583+
body: note.content,
584+
metadata: { plotNoteId: note.id },
585+
});
586+
587+
// Update Note with external key AFTER creation
588+
// A webhook may arrive before this completes — that's OK (see onWebhook below)
589+
await this.tools.plot.updateNote({
590+
id: note.id,
591+
key: `comment-${externalComment.id}`,
592+
});
593+
}
594+
595+
async onWebhook(payload: WebhookPayload): Promise<void> {
596+
const comment = payload.comment;
597+
598+
// Use Plot ID from metadata if present (handles race condition),
599+
// otherwise fall back to upserting by activity source and key
600+
await this.tools.plot.createNote({
601+
...(comment.metadata?.plotNoteId
602+
? { id: comment.metadata.plotNoteId }
603+
: { activity: { source: payload.itemUrl } }),
604+
key: `comment-${comment.id}`,
605+
content: comment.body,
606+
});
607+
}
608+
```
609+
573610
## Error Handling
574611

575612
Always handle errors gracefully and communicate them to users:

twister/docs/SYNC_STRATEGIES.md

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -754,6 +754,45 @@ async cleanupOldMappings(): Promise<void> {
754754
}
755755
```
756756

757+
### 6. Avoid Race Conditions in Two-Way Sync
758+
759+
When implementing two-way sync where items can be created in Plot and pushed to an external system (e.g. Notes becoming comments), update `Activity.source` / `Note.key` **after** creating the external item. If the external system supports setting custom metadata, include the `Activity.id` / `Note.id` in the metadata when creating the external item. Then, when processing an incoming webhook, check for the Plot ID in the metadata first and use it if present.
760+
761+
This eliminates a race condition where a webhook for an item you're creating arrives before you've updated the Activity/Note with the external key. Without this pattern, the webhook handler won't find the item by external key and may create a duplicate.
762+
763+
```typescript
764+
async pushNoteAsComment(note: Note, externalItemId: string): Promise<void> {
765+
// Create the comment in the external system, embedding the Note ID in metadata
766+
const externalComment = await externalApi.createComment(externalItemId, {
767+
body: note.content,
768+
metadata: { plotNoteId: note.id }, // Embed Plot ID for webhook correlation
769+
});
770+
771+
// Update the Note with the external key AFTER creation
772+
// A webhook may arrive between these two steps — that's OK because
773+
// the webhook handler checks metadata first (see below)
774+
await this.tools.plot.updateNote({
775+
id: note.id,
776+
key: `comment-${externalComment.id}`,
777+
});
778+
}
779+
780+
async onWebhook(payload: WebhookPayload): Promise<void> {
781+
const comment = payload.comment;
782+
783+
// Use the Plot ID from metadata if present (handles the race condition
784+
// where the webhook arrives before we've set the external key on the Note),
785+
// otherwise fall back to upserting by activity source and key
786+
await this.tools.plot.createNote({
787+
...(comment.metadata?.plotNoteId
788+
? { id: comment.metadata.plotNoteId }
789+
: { activity: { source: payload.itemUrl } }),
790+
key: `comment-${comment.id}`,
791+
content: comment.body,
792+
});
793+
}
794+
```
795+
757796
## Summary
758797

759798
- **Strategy 1** (Create Once): Simplest, no deduplication, use for one-time items

twister/src/common/projects.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -156,11 +156,13 @@ export type ProjectTool = {
156156
* @param authToken - Authorization token for access
157157
* @param meta - Activity metadata containing the tool's issue/task identifier
158158
* @param body - The comment text content
159-
* @returns Promise that resolves when the comment is added
159+
* @param noteId - Optional Plot note ID, used by tools that support comment metadata (e.g. Jira)
160+
* @returns The external comment key (e.g. "comment-123") for dedup, or void
160161
*/
161162
addIssueComment?(
162163
authToken: string,
163164
meta: ActivityMeta,
164-
body: string
165-
): Promise<void>;
165+
body: string,
166+
noteId?: string,
167+
): Promise<string | void>;
166168
};

twister/src/plot.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1059,7 +1059,7 @@ export type NewNote = Partial<
10591059
* Type for updating existing notes.
10601060
* Must provide either id or key to identify the note to update.
10611061
*/
1062-
export type NoteUpdate = ({ id: Uuid } | { key: string }) &
1062+
export type NoteUpdate = ({ id: Uuid; key?: string } | { key: string }) &
10631063
Partial<Pick<Note, "private" | "archived" | "content" | "links">> & {
10641064
/**
10651065
* Format of the note content. Determines how the note is processed:

0 commit comments

Comments
 (0)