Skip to content

Commit 834562c

Browse files
committed
feat(orchestration): route @bgagent comments left on the parent epic to the sub-issue they name (aws-samples#247 UX.18)
Live-caught on ABCA-304: a reviewer commented '@bgagent for the footer can you change it to...' on the PARENT epic (the maturing panel lives there, so that's the natural place to comment) and it was SILENTLY DROPPED. The parent epic has no PR of its own, so the comment-trigger fell through to the standalone GSI path, found no task for the parent issue, and ignored it ('issue has no ABCA task — ignoring'). Fix: in handleCommentTrigger, detect when the commented issue is itself an orchestration PARENT (deriveOrchestrationId(issueId) loads an orchestration whose meta.parent_linear_issue_id === issueId — a pure hash, so the parent's own id maps to its orchestration; a sub-issue's doesn't). Route to handleParentEpicCommentTrigger: - 👀 on the comment IMMEDIATELY — a parent comment is never silently dropped again. - parseParentNodeReference(instruction, children) picks the target sub-issue: Linear identifier (ABCA-305) wins outright, else a significant (non-noise) title keyword ('footer' → 'Add a site-wide footer'). Exactly one started node with a PR → iterate it via the shared iterateOrchestrationChild (same pr-iteration + cascade marker + threaded ✅/❌ reply as commenting on the sub-issue directly). - 0/ambiguous match, or matched node has no PR → post a threaded reply on the parent: a best-effort 'did you mean <X>?' suggestion, the list of sub-issues + how to target one (@bgagent ABCA-123:...), and the 'create a sub-issue for NEW work' path. NEVER auto-creates an issue and NEVER silently drops (user's call: ask, don't create). Refactor: extracted the per-child iteration into iterateOrchestrationChild (skipAck/prNumber params) so the direct sub-issue path and the new parent path share one code path. New pure module orchestration-parent-comment.ts (parseParentNodeReference + suggestClosestNode + renderParentDisambiguationReply), 16 unit tests incl. the exact live case. 4 handler-wiring tests (live case, identifier targeting, ambiguous→ask, no-match→ask). Existing A6 sub-issue + standalone trigger tests unchanged + green (36→40 in the orch suite). cdk:compile clean.
1 parent 08d73de commit 834562c

4 files changed

Lines changed: 585 additions & 37 deletions

File tree

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

Lines changed: 149 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,15 @@ import * as crypto from 'crypto';
2121
import { DynamoDBClient } from '@aws-sdk/client-dynamodb';
2222
import { DynamoDBDocumentClient, GetCommand } from '@aws-sdk/lib-dynamodb';
2323
import { createTaskCore } from './shared/create-task-core';
24-
import { reactToComment, reportIssueFailure, EMOJI_STARTED } from './shared/linear-feedback';
24+
import { reactToComment, replyToComment, reportIssueFailure, EMOJI_STARTED } from './shared/linear-feedback';
2525
import { upsertEpicPanel } from './shared/orchestration-rollup';
2626
import { resolveLinearOauthToken } from './shared/linear-oauth-resolver';
2727
import { logger } from './shared/logger';
2828
import { discoverOrchestration } from './shared/orchestration-discovery';
2929
import { readConcurrencyBudget, releaseReadyChildren } from './shared/orchestration-release';
3030
import { deriveOrchestrationId, loadOrchestration, setStatusCommentId, type OrchestrationReleaseContext } from './shared/orchestration-store';
3131
import { buildIterationInstruction, parseCommentTrigger, type CommentTrigger } from './shared/orchestration-comment-trigger';
32+
import { parseParentNodeReference, renderParentDisambiguationReply, suggestClosestNode } from './shared/orchestration-parent-comment';
3233
import { fetchIssueParentId } from './shared/linear-subissue-fetch';
3334
import { resolveTaskByLinearIssue, prNumberFromTask } from './shared/linear-task-by-issue';
3435
import type { Attachment } from './shared/types';
@@ -643,67 +644,182 @@ async function handleCommentTrigger(payload: LinearCommentEvent): Promise<void>
643644
return;
644645
}
645646

647+
const commentedIssueId = subIssueId;
648+
const commentId = payload.data.id;
649+
// The ✅/❌ ack must reply to the thread ROOT — Linear rejects a reply whose
650+
// parentId is itself a reply. When the trigger is a thread-reply, data.parentId
651+
// is the root; otherwise the comment IS the root. The 👀 still goes on the
652+
// actual comment the human wrote (reactions work at any thread depth).
653+
const replyTargetId = payload.data.parentId ?? commentId;
654+
655+
// #247 UX.18: is the commented issue itself a PARENT epic? deriveOrchestrationId
656+
// is a pure hash of the issue id, so the parent's own id maps to ITS
657+
// orchestration; a sub-issue's id hashes to nothing. The maturing panel lives
658+
// on the parent, so reviewers comment THERE ("@bgagent for the footer, …") —
659+
// route that to the sub-issue it names. (Was a silent drop: the parent has no
660+
// PR, so it fell to the standalone GSI path → miss → ignored.)
661+
const ownOrchestrationId = deriveOrchestrationId(commentedIssueId);
662+
const parentSnapshot = await loadOrchestration(ddb, ORCHESTRATION_TABLE, ownOrchestrationId);
663+
if (parentSnapshot && parentSnapshot.meta.parent_linear_issue_id === commentedIssueId) {
664+
await handleParentEpicCommentTrigger({
665+
orchestrationId: ownOrchestrationId,
666+
snapshot: parentSnapshot,
667+
workspaceId, commentId, replyTargetId, trigger, resolved,
668+
registryTableName: WORKSPACE_REGISTRY_TABLE,
669+
});
670+
return;
671+
}
672+
646673
// Sub-issue → parent → orchestration. When ANY of these don't hold (no
647674
// parent, parent isn't an orchestration, or this isn't a STARTED child),
648675
// the issue may still be a plain (non-orchestration) issue that ABCA opened
649676
// a PR for — fall through to the standalone path (#247 UX.3), which iterates
650677
// on that PR with the same 👀/reply ack but no dependency cascade.
651-
const parentId = await fetchIssueParentId(resolved.accessToken, subIssueId);
678+
const parentId = await fetchIssueParentId(resolved.accessToken, commentedIssueId);
652679
const orchestrationId = parentId ? deriveOrchestrationId(parentId) : null;
653680
const snapshot = orchestrationId
654681
? await loadOrchestration(ddb, ORCHESTRATION_TABLE, orchestrationId)
655682
: null;
656-
const child = snapshot?.children.find((c) => c.sub_issue_id === subIssueId);
683+
const child = snapshot?.children.find((c) => c.sub_issue_id === commentedIssueId);
657684
if (!snapshot || !child || !child.child_task_id) {
658685
await handleStandaloneCommentTrigger({
659-
subIssueId, workspaceId, commentId: payload.data.id,
660-
// Reply to the thread ROOT (Linear rejects replying to a reply); the 👀
661-
// still goes on the actual comment.
662-
replyTargetId: payload.data.parentId ?? payload.data.id,
686+
subIssueId: commentedIssueId, workspaceId, commentId,
687+
replyTargetId,
663688
trigger, resolved,
664689
registryTableName: WORKSPACE_REGISTRY_TABLE,
665690
});
666691
return;
667692
}
668693

669-
// Resolve the sub-issue's PR number (prefer pr_number, else parse pr_url).
670-
const prNumber = await resolveChildPrNumber(child.child_task_id);
694+
await iterateOrchestrationChild({
695+
orchestrationId: orchestrationId!,
696+
snapshot, child,
697+
workspaceId, commentId, replyTargetId, trigger, resolved,
698+
registryTableName: WORKSPACE_REGISTRY_TABLE,
699+
});
700+
}
701+
702+
/**
703+
* #247 UX.18 — an ``@bgagent`` comment left on the PARENT epic. The maturing
704+
* panel lives on the parent, so a reviewer's natural move is to comment there.
705+
* The parent has no PR of its own, so we route the request to the sub-issue it
706+
* names (by identifier or title keyword) and iterate THAT sub-issue's PR. When
707+
* the comment names no single sub-issue, we 👀 + post a "which one?" reply
708+
* (with a best-effort suggestion + the create-a-sub-issue path) — NEVER a
709+
* silent drop, and NEVER auto-creating new work (user's call).
710+
*/
711+
async function handleParentEpicCommentTrigger(args: {
712+
orchestrationId: string;
713+
snapshot: NonNullable<Awaited<ReturnType<typeof loadOrchestration>>>;
714+
workspaceId: string;
715+
commentId: string;
716+
replyTargetId: string;
717+
trigger: CommentTrigger;
718+
resolved: { accessToken: string; oauthSecretArn: string; workspaceSlug: string };
719+
registryTableName: string;
720+
}): Promise<void> {
721+
const { orchestrationId, snapshot, workspaceId, commentId, replyTargetId, trigger, resolved, registryTableName } = args;
722+
const feedbackCtx = { linearWorkspaceId: workspaceId, registryTableName };
723+
// ACK immediately — a parent comment is never silently dropped again.
724+
await reactToComment(feedbackCtx, commentId, EMOJI_STARTED);
725+
726+
// Only STARTED children with a task are iterable candidates; match against all
727+
// real nodes for the disambiguation list, but iterate only a started one.
728+
const match = parseParentNodeReference(trigger.instruction, snapshot.children);
729+
const target = match.reason === null ? match.matches[0] : null;
730+
731+
if (!target || !target.child_task_id) {
732+
// No confident single match (or matched a not-yet-started node) → ask.
733+
const reason = match.reason === 'ambiguous' ? 'ambiguous' : 'none';
734+
const suggestion = reason === 'none' ? suggestClosestNode(trigger.instruction, snapshot.children) : null;
735+
const body = renderParentDisambiguationReply(reason, snapshot.children, suggestion);
736+
await replyToComment(feedbackCtx, snapshot.meta.parent_linear_issue_id, replyTargetId, body);
737+
logger.info('A6 comment (parent epic): no single iterable sub-issue matched — asked', {
738+
orchestration_id: orchestrationId, reason, match_count: match.matches.length,
739+
});
740+
return;
741+
}
742+
743+
const prNumber = await resolveChildPrNumber(target.child_task_id);
671744
if (prNumber === null) {
672-
logger.warn('A6 comment: sub-issue has no resolvable PR — cannot iterate', {
673-
orchestration_id: orchestrationId,
674-
sub_issue_id: subIssueId,
675-
child_task_id: child.child_task_id,
745+
const body = renderParentDisambiguationReply('none', snapshot.children, target);
746+
await replyToComment(feedbackCtx, snapshot.meta.parent_linear_issue_id, replyTargetId, body);
747+
logger.info('A6 comment (parent epic): matched sub-issue has no PR yet — asked', {
748+
orchestration_id: orchestrationId, sub_issue_id: target.sub_issue_id,
676749
});
677750
return;
678751
}
679752

680-
// Past the fall-through guard, this is a started orchestration child:
681-
// orchestrationId is non-null. Bind it so the channel metadata types check.
682-
const orchId = orchestrationId!;
753+
// Resolve the FULL child row (the matcher returns a trimmed view without
754+
// ``repo``) so the iteration carries the sub-issue's repo.
755+
const childRow = snapshot.children.find((c) => c.sub_issue_id === target.sub_issue_id)!;
756+
757+
// Route to the matched sub-issue exactly as if the human had commented there.
758+
// The 👀 is already on the parent comment; the ✅/❌ reply threads back to it.
759+
await iterateOrchestrationChild({
760+
orchestrationId, snapshot, child: childRow,
761+
workspaceId, commentId, replyTargetId, trigger, resolved, registryTableName,
762+
// Already acked on the parent comment above.
763+
skipAck: true,
764+
prNumber,
765+
});
766+
logger.info('A6 comment (parent epic): routed to sub-issue', {
767+
orchestration_id: orchestrationId, sub_issue_id: target.sub_issue_id, pr_number: prNumber,
768+
});
769+
}
770+
771+
/**
772+
* Spawn a ``coding/pr-iteration-v1`` task for one orchestration sub-issue from
773+
* an ``@bgagent`` comment (#247 A6 + UX.18). Shared by the direct sub-issue
774+
* path (comment on the sub-issue) and the parent-epic path (comment on the
775+
* epic, routed here). Acks the trigger comment with 👀 (unless already acked),
776+
* marks the task as a cascade SOURCE so the reconciler re-stacks dependents,
777+
* and threads ✅/❌ back to ``replyTargetId`` on completion.
778+
*/
779+
async function iterateOrchestrationChild(args: {
780+
orchestrationId: string;
781+
snapshot: NonNullable<Awaited<ReturnType<typeof loadOrchestration>>>;
782+
child: { sub_issue_id: string; repo: string; child_task_id?: string };
783+
workspaceId: string;
784+
commentId: string;
785+
replyTargetId: string;
786+
trigger: CommentTrigger;
787+
resolved: { oauthSecretArn: string; workspaceSlug: string };
788+
registryTableName: string;
789+
skipAck?: boolean;
790+
prNumber?: number;
791+
}): Promise<void> {
792+
const {
793+
orchestrationId, snapshot, child, workspaceId, commentId, replyTargetId,
794+
trigger, resolved, registryTableName,
795+
} = args;
796+
const subIssueId = child.sub_issue_id;
797+
798+
const prNumber = args.prNumber ?? (child.child_task_id ? await resolveChildPrNumber(child.child_task_id) : null);
799+
if (prNumber === null || prNumber === undefined) {
800+
logger.warn('A6 comment: sub-issue has no resolvable PR — cannot iterate', {
801+
orchestration_id: orchestrationId, sub_issue_id: subIssueId, child_task_id: child.child_task_id,
802+
});
803+
return;
804+
}
683805

684806
// Attribute to the orchestration's release user (the comment author may not
685807
// be a linked platform user; the orchestration already ran under this id).
686808
const platformUserId = snapshot.meta.release_context.platform_user_id;
687809

688-
// #247 UX.3: ACK the request the instant we commit to acting on it. 👀 on
689-
// the TRIGGERING comment (not the issue) is the zero-clutter "on it" signal;
690-
// the matching ✅/❌ threaded reply lands from the reconciler when the
691-
// iteration task completes (it reads trigger_comment_id off channel_metadata).
692-
const commentId = payload.data.id;
693-
// The ✅/❌ ack must reply to the thread ROOT — Linear rejects a reply whose
694-
// parentId is itself a reply. When the trigger is a thread-reply, data.parentId
695-
// is the root; otherwise the comment IS the root. The 👀 still goes on the
696-
// actual comment the human wrote (reactions work at any thread depth).
697-
const replyTargetId = payload.data.parentId ?? commentId;
698-
const feedbackCtx = { linearWorkspaceId: workspaceId, registryTableName: WORKSPACE_REGISTRY_TABLE };
699-
await reactToComment(feedbackCtx, commentId, EMOJI_STARTED);
810+
// #247 UX.3: ACK the request the instant we commit to acting on it. 👀 on the
811+
// TRIGGERING comment is the zero-clutter "on it" signal. The parent-epic path
812+
// already acked, so it passes skipAck.
813+
if (!args.skipAck) {
814+
await reactToComment({ linearWorkspaceId: workspaceId, registryTableName }, commentId, EMOJI_STARTED);
815+
}
700816

701817
// Idempotency: one iteration per (sub-issue, comment). The comment id is
702818
// unique per comment, so a webhook retry of the same comment dedups.
703819
const idempotencyKey = `iterate_${subIssueId}_${commentId}`.replace(/[^A-Za-z0-9_-]/g, '').slice(0, 128);
704820

705821
const channelMetadata: Record<string, string> = {
706-
orchestration_id: orchId,
822+
orchestration_id: orchestrationId,
707823
orchestration_sub_issue_id: subIssueId,
708824
// Mark this as a cascade SOURCE so the reconciler re-stacks dependents
709825
// when the iteration completes (A6.2 reads this flag).
@@ -714,9 +830,9 @@ async function handleCommentTrigger(payload: LinearCommentEvent): Promise<void>
714830
linear_workspace_id: workspaceId,
715831
linear_oauth_secret_arn: resolved.oauthSecretArn,
716832
linear_workspace_slug: resolved.workspaceSlug,
833+
// The agent addresses the real sub-issue (reactions/comments).
834+
linear_issue_id: subIssueId,
717835
};
718-
// The agent addresses the real sub-issue (reactions/comments).
719-
channelMetadata.linear_issue_id = subIssueId;
720836

721837
try {
722838
const result = await createTaskCore(
@@ -730,15 +846,11 @@ async function handleCommentTrigger(payload: LinearCommentEvent): Promise<void>
730846
idempotencyKey,
731847
);
732848
logger.info('A6 comment: iteration task created for sub-issue PR', {
733-
orchestration_id: orchestrationId,
734-
sub_issue_id: subIssueId,
735-
pr_number: prNumber,
736-
status_code: result.statusCode,
849+
orchestration_id: orchestrationId, sub_issue_id: subIssueId, pr_number: prNumber, status_code: result.statusCode,
737850
});
738851
} catch (err) {
739852
logger.error('A6 comment: createTaskCore threw for iteration', {
740-
orchestration_id: orchestrationId,
741-
sub_issue_id: subIssueId,
853+
orchestration_id: orchestrationId, sub_issue_id: subIssueId,
742854
error: err instanceof Error ? err.message : String(err),
743855
});
744856
}

0 commit comments

Comments
 (0)