Skip to content

Commit 5f08628

Browse files
committed
fix(orchestration): thread the trigger comment's issue id so a parent-routed iteration's ✅/❌ reply lands (aws-samples#247 UX.19)
Live-caught on ABCA-304 immediately after UX.18 shipped: a comment left on the PARENT epic was routed to the footer sub-issue and an iteration spawned (👀 ack + 'routed to sub-issue … pr_number 193'), but the iteration then FAILED (transient GITHUB_UNREACHABLE) and NO ❌ reply appeared — the human saw 👀 then silence. Root cause: replyToIterationComment posted the ✅/❌ reply with issueId = changedSubIssueId (the sub-issue, ABCA-305), but the trigger comment lives on the PARENT epic (ABCA-304). Linear's commentCreate rejects a threaded reply whose parentId belongs to a different issue, so the reply silently failed. (The cascade-skip on FAILED was correct; the reply, which runs BEFORE the success gate, was the casualty.) Fix: thread trigger_comment_issue_id (the issue the human actually commented on) through channel_metadata — - iterateOrchestrationChild adds it (defaults to the sub-issue id; the parent-epic path passes snapshot.meta.parent_linear_issue_id). - parseTerminalTaskRecord reads it into TerminalTaskEvent. - replyToIterationComment uses it as commentCreate's issueId, falling back to changedSubIssueId for pre-UX.19 tasks. Direct sub-issue comments are unaffected (issue id == sub-issue). The standalone fanout reply path already replies on linear_issue_id = the comment's own issue, so no change needed there. Tests: reconciler 'PARENT-routed iteration replies on the PARENT issue not the sub-issue'; webhook wiring asserts trigger_comment_issue_id = the parent. 78 green across reconciler+webhook+matcher suites.
1 parent 834562c commit 5f08628

4 files changed

Lines changed: 65 additions & 4 deletions

File tree

cdk/src/handlers/linear-webhook-processor.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -759,6 +759,9 @@ async function handleParentEpicCommentTrigger(args: {
759759
await iterateOrchestrationChild({
760760
orchestrationId, snapshot, child: childRow,
761761
workspaceId, commentId, replyTargetId, trigger, resolved, registryTableName,
762+
// #247 UX.19: the trigger comment lives on the PARENT epic, not the
763+
// sub-issue — the reconciler must reply with the parent issue id.
764+
triggerCommentIssueId: snapshot.meta.parent_linear_issue_id,
762765
// Already acked on the parent comment above.
763766
skipAck: true,
764767
prNumber,
@@ -783,6 +786,13 @@ async function iterateOrchestrationChild(args: {
783786
workspaceId: string;
784787
commentId: string;
785788
replyTargetId: string;
789+
/**
790+
* The Linear ISSUE the trigger comment lives on — the sub-issue for a direct
791+
* comment, the PARENT epic for a UX.18 parent-routed comment. The reconciler
792+
* replies ✅/❌ using THIS as commentCreate's issueId (#247 UX.19). Defaults to
793+
* the sub-issue id.
794+
*/
795+
triggerCommentIssueId?: string;
786796
trigger: CommentTrigger;
787797
resolved: { oauthSecretArn: string; workspaceSlug: string };
788798
registryTableName: string;
@@ -794,6 +804,7 @@ async function iterateOrchestrationChild(args: {
794804
trigger, resolved, registryTableName,
795805
} = args;
796806
const subIssueId = child.sub_issue_id;
807+
const triggerCommentIssueId = args.triggerCommentIssueId ?? subIssueId;
797808

798809
const prNumber = args.prNumber ?? (child.child_task_id ? await resolveChildPrNumber(child.child_task_id) : null);
799810
if (prNumber === null || prNumber === undefined) {
@@ -827,6 +838,10 @@ async function iterateOrchestrationChild(args: {
827838
// #247 UX.3: the reconciler replies ✅/❌ to the thread ROOT when the
828839
// iteration lands (threaded ack — closes the conversation the human opened).
829840
trigger_comment_id: replyTargetId,
841+
// #247 UX.19: the issue that comment lives on, so the reconciler's reply
842+
// uses the right commentCreate issueId (parent epic for a routed comment;
843+
// the sub-issue for a direct comment).
844+
trigger_comment_issue_id: triggerCommentIssueId,
830845
linear_workspace_id: workspaceId,
831846
linear_oauth_secret_arn: resolved.oauthSecretArn,
832847
linear_workspace_slug: resolved.workspaceSlug,

cdk/src/handlers/orchestration-reconciler.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,15 @@ interface TerminalTaskEvent {
122122
* comment to reply to).
123123
*/
124124
readonly triggerCommentId?: string;
125+
/**
126+
* #247 UX.19: the Linear ISSUE the trigger comment lives on. Usually the
127+
* iterated sub-issue, but for a comment left on the PARENT epic (routed to a
128+
* sub-issue via UX.18) it's the PARENT issue id. The threaded ✅/❌ reply must
129+
* use THIS as commentCreate's issueId — Linear rejects a reply whose parentId
130+
* belongs to a different issue. Absent on older tasks → reply falls back to
131+
* the sub-issue id (the prior behavior).
132+
*/
133+
readonly triggerCommentIssueId?: string;
125134
}
126135

127136
/**
@@ -171,6 +180,9 @@ export function parseTerminalTaskRecord(record: DynamoDBRecord): TerminalTaskEve
171180
const cascadeSubIssueId = isCascadeSource ? cm?.orchestration_sub_issue_id?.S : undefined;
172181
// #247 UX.3: the human comment that triggered this iteration, if any.
173182
const triggerCommentId = isIteration ? cm?.trigger_comment_id?.S : undefined;
183+
// #247 UX.19: the issue that comment lives on (parent epic for a UX.18
184+
// parent-routed comment; the sub-issue for a direct comment).
185+
const triggerCommentIssueId = isIteration ? cm?.trigger_comment_issue_id?.S : undefined;
174186

175187
return {
176188
taskId,
@@ -181,6 +193,7 @@ export function parseTerminalTaskRecord(record: DynamoDBRecord): TerminalTaskEve
181193
...(cascadeSubIssueId !== undefined && { cascadeSubIssueId }),
182194
...(cascadeSubIssueId !== undefined && { cascadeIsIteration: isIteration }),
183195
...(triggerCommentId !== undefined && { triggerCommentId }),
196+
...(triggerCommentIssueId !== undefined && { triggerCommentIssueId }),
184197
};
185198
}
186199

@@ -662,10 +675,13 @@ async function replyToIterationComment(
662675
...(evt.errorMessage !== undefined && { errorMessage: evt.errorMessage }),
663676
taskId: evt.taskId,
664677
});
665-
// The triggering comment lives on the sub-issue (changedSubIssueId is its
666-
// Linear issue id) — Linear requires issueId on commentCreate even for a
667-
// threaded reply.
668-
await replyToComment(ctx, changedSubIssueId, commentId, body);
678+
// The reply's issueId MUST be the issue the trigger comment lives on —
679+
// Linear rejects a threaded reply whose parentId belongs to a different
680+
// issue. For a comment left on the PARENT epic (UX.18 routing) that's the
681+
// parent issue, NOT changedSubIssueId. Fall back to the sub-issue id for
682+
// tasks created before UX.19 (no triggerCommentIssueId persisted).
683+
const replyIssueId = evt.triggerCommentIssueId ?? changedSubIssueId;
684+
await replyToComment(ctx, replyIssueId, commentId, body);
669685
}
670686

671687
/** Build the ✅ ack reply, linking the (re-pushed) PR when resolvable. */

cdk/test/handlers/linear-webhook-processor-orchestration.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,10 @@ describe('linear-webhook-processor — #247 A6 comment trigger', () => {
538538
expect(ctx.channelMetadata.orchestration_sub_issue_id).toBe('sub-footer');
539539
expect(ctx.channelMetadata.orchestration_iteration).toBe('true');
540540
expect(ctx.channelMetadata.linear_issue_id).toBe('sub-footer');
541+
// #247 UX.19: the trigger comment lives on the PARENT epic, so the reply
542+
// must target the parent issue (not the sub-issue) — else Linear rejects it.
543+
expect(ctx.channelMetadata.trigger_comment_issue_id).toBe('PARENT-EPIC');
544+
expect(ctx.channelMetadata.trigger_comment_id).toBe('pc-1');
541545
// No disambiguation reply — we acted.
542546
expect(replyToCommentMock).not.toHaveBeenCalled();
543547
});

cdk/test/handlers/orchestration-reconciler.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ function taskRecord(fields: {
7474
orchestration_iteration?: boolean;
7575
// #247 UX.3: the human comment that triggered an iteration.
7676
trigger_comment_id?: string;
77+
// #247 UX.19: the issue that trigger comment lives on (parent epic when routed).
78+
trigger_comment_issue_id?: string;
7779
// #247 UX.5: raw agent error_message (drives the failure-reply detail).
7880
error_message?: string;
7981
}): DynamoDBRecord {
@@ -96,6 +98,7 @@ function taskRecord(fields: {
9698
}
9799
if (fields.orchestration_iteration) cm.orchestration_iteration = { S: 'true' };
98100
if (fields.trigger_comment_id) cm.trigger_comment_id = { S: fields.trigger_comment_id };
101+
if (fields.trigger_comment_issue_id) cm.trigger_comment_issue_id = { S: fields.trigger_comment_issue_id };
99102
if (Object.keys(cm).length > 0) img.channel_metadata = { M: cm };
100103
return {
101104
eventName: fields.eventName ?? 'MODIFY',
@@ -694,6 +697,29 @@ describe('orchestration-reconciler handler — A6 iteration ack reply (#247 UX.3
694697
expect(body).toMatch(/^ Updated PR #\d+\./);
695698
});
696699

700+
test('#247 UX.19: a PARENT-routed iteration replies on the PARENT issue, not the sub-issue', async () => {
701+
// The human commented on the parent epic (UX.18 routed it to sub-issue A).
702+
// The ✅/❌ reply must use the PARENT issue id as commentCreate's issueId —
703+
// else Linear rejects the reply (parentId belongs to a different issue) and
704+
// the human sees 👀 then silence (live-caught on ABCA-304).
705+
mockCascade([
706+
{ sub_issue_id: 'A', child_status: 'succeeded', child_task_id: 'task-A', child_branch_name: 'branch-A', linear_identifier: 'ENG-1' },
707+
]);
708+
await handler({
709+
Records: [taskRecord({
710+
task_id: 'iter-task-1', status: 'COMPLETED', orchestration_id: 'orch_1',
711+
orchestration_sub_issue_id: 'A', orchestration_iteration: true,
712+
trigger_comment_id: 'parent-cmt-1',
713+
trigger_comment_issue_id: 'PARENT', // comment lives on the parent epic
714+
})],
715+
} as never);
716+
717+
expect(replyToCommentMock).toHaveBeenCalledTimes(1);
718+
const [, issueId, parentCommentId] = replyToCommentMock.mock.calls[0];
719+
expect(issueId).toBe('PARENT'); // NOT 'A' — the reply targets the parent comment's issue
720+
expect(parentCommentId).toBe('parent-cmt-1');
721+
});
722+
697723
test('FAILED iteration (agent crash) → ❌ reply with classified reason + CloudWatch task id (UX.5)', async () => {
698724
mockCascade([
699725
{ sub_issue_id: 'A', child_status: 'succeeded', child_task_id: 'task-A', child_branch_name: 'branch-A', linear_identifier: 'ENG-1' },

0 commit comments

Comments
 (0)