Skip to content

Commit a85ec68

Browse files
committed
fix(linear): start new work from a follow-up comment on a PR-less task (#614)
A follow-up `@bgagent <request>` comment on a completed Linear task that opened no PR was silently dropped. handleStandaloneCommentTrigger is iteration-only: on prNumber === null it checks for a clarify-hold and, if it isn't one, returns with just an info log — the comment vanished. But a task finishes PR-less in several real ways (no change needed, failed-before-commit, or a question/investigation run), and a follow-up on such an issue is almost always NEW work ("then just do X instead"), not iteration. Add maybeStartStandaloneNewWork: when the repo + user are known and the comment carries instruction text, dispatch a fresh coding/new-task-v1 against the same repo using the comment text as the description — reusing the same 👀-ack + threaded reply + fanout terminal ownership as the iteration and clarify-resume paths. Idempotency keyed newwork_<issue>_<comment>. Edge cases: a bare `@bgagent` (no instruction) gets a threaded reply telling the user what to do rather than a vague dispatch or silence; no repo / no user_id falls through to the (now-narrowed) no-op log. Tests: replace the obsolete 'NO PR → no task, no ack' case with the new-work dispatch, bare-mention reply, no-repo, and no-user cases. Full cdk build green (3118 tests). Closes #614.
1 parent 8e31ac1 commit a85ec68

2 files changed

Lines changed: 159 additions & 3 deletions

File tree

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

Lines changed: 117 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2415,7 +2415,17 @@ async function handleStandaloneCommentTrigger(args: {
24152415
if (await maybeResumeClarifyHold({ issueId, task, workspaceId, commentId, replyTargetId, trigger, resolved, registryTableName })) {
24162416
return;
24172417
}
2418-
logger.info('A6 comment (standalone): ABCA task has no resolvable PR/repo — cannot iterate', {
2418+
// #614: a PR-less completed task (no-change-needed, failed-before-commit, or
2419+
// a question/investigation run) is NOT an iteration target — but a follow-up
2420+
// ``@bgagent <request>`` on it is almost always NEW work ("then just do X
2421+
// instead"). When the repo is known, dispatch a fresh new-task-v1 rather than
2422+
// dropping the comment silently (the old dead-end). Falls through to the
2423+
// no-op log below only when we genuinely can't act (no repo/user, or a bare
2424+
// mention with no instruction).
2425+
if (await maybeStartStandaloneNewWork({ issueId, task, workspaceId, commentId, replyTargetId, trigger, resolved, registryTableName })) {
2426+
return;
2427+
}
2428+
logger.info('A6 comment (standalone): PR-less task, no new-work dispatched (no repo/user or empty instruction)', {
24192429
linear_issue_id: issueId, task_id: task.task_id, has_repo: Boolean(task.repo),
24202430
});
24212431
return;
@@ -2470,6 +2480,112 @@ async function handleStandaloneCommentTrigger(args: {
24702480
}
24712481
}
24722482

2483+
/**
2484+
* #614: start NEW work from a follow-up comment on a PR-less completed task.
2485+
*
2486+
* The standalone path only knows how to *iterate* an existing PR. But a task
2487+
* can finish with no PR (no change needed, failed before committing, or a
2488+
* question/investigation run), and a follow-up ``@bgagent <request>`` on such an
2489+
* issue is almost always a fresh ask ("then just do X instead") — not iteration.
2490+
* Before #614 those comments hit a silent ``return`` and vanished. This dispatches
2491+
* a fresh ``coding/new-task-v1`` against the SAME repo, using the comment text as
2492+
* the task description, with the same 👀-ack + threaded reply + fanout terminal
2493+
* ownership as the iteration/clarify paths.
2494+
*
2495+
* Returns true when it handled the comment (a task was dispatched, OR a bare
2496+
* mention was answered with a "nothing to do" reply), false when it cannot act
2497+
* (no repo/user) so the caller falls through to its no-op log.
2498+
*
2499+
* Best-effort: a dispatch failure is logged and still returns true (we already
2500+
* ACKed) — the fanout terminal path reports the outcome.
2501+
*/
2502+
async function maybeStartStandaloneNewWork(args: {
2503+
issueId: string;
2504+
task: { task_id: string; repo?: string; user_id?: string };
2505+
workspaceId: string;
2506+
commentId: string;
2507+
replyTargetId: string;
2508+
trigger: CommentTrigger;
2509+
resolved: { accessToken: string; oauthSecretArn: string; workspaceSlug: string };
2510+
registryTableName: string;
2511+
}): Promise<boolean> {
2512+
const { issueId, task, workspaceId, commentId, replyTargetId, trigger, resolved, registryTableName } = args;
2513+
2514+
// Can't act without a repo to work in or a user to attribute the task to —
2515+
// let the caller no-op-log. (These are the only genuinely unactionable cases.)
2516+
if (!task.repo || !task.user_id) return false;
2517+
2518+
const instruction = trigger.instruction.trim();
2519+
const feedbackCtx = { linearWorkspaceId: workspaceId, registryTableName };
2520+
2521+
// A bare ``@bgagent`` with no text has nothing to start. Unlike iteration
2522+
// (where an empty instruction means "address the latest review"), there is no
2523+
// PR to fall back on here — so acknowledge briefly rather than dispatch a
2524+
// vague task or stay silent. Handled (return true) so we don't no-op-log.
2525+
if (!instruction) {
2526+
await reactToComment(feedbackCtx, commentId, EMOJI_STARTED);
2527+
try {
2528+
await upsertThreadedReply(
2529+
feedbackCtx,
2530+
issueId,
2531+
replyTargetId,
2532+
'This task already finished and has no open PR to iterate on. Reply with what '
2533+
+ "you'd like me to do (e.g. `@bgagent add a note to the README`) and I'll start it.",
2534+
);
2535+
} catch (err) {
2536+
logger.warn('A6 comment (standalone): bare-mention reply failed (non-fatal)', {
2537+
linear_issue_id: issueId, error: err instanceof Error ? err.message : String(err),
2538+
});
2539+
}
2540+
logger.info('A6 comment (standalone): bare mention on PR-less task — replied, no dispatch', {
2541+
linear_issue_id: issueId, task_id: task.task_id,
2542+
});
2543+
return true;
2544+
}
2545+
2546+
// ACK immediately (👀 reaction + threaded "On it"), same as the iteration and
2547+
// clarify-resume paths.
2548+
await reactToComment(feedbackCtx, commentId, EMOJI_STARTED);
2549+
const iterationReplyId = await postIterationAck(workspaceId, registryTableName, issueId, replyTargetId);
2550+
2551+
// Idempotency: key on (issue, comment) so a webhook redelivery of the SAME
2552+
// comment doesn't spawn a second task. Distinct prefix from iterate_/clarify_.
2553+
const idempotencyKey = `newwork_${issueId}_${commentId}`.replace(/[^A-Za-z0-9_-]/g, '').slice(0, MAX_IDEMPOTENCY_KEY_LENGTH);
2554+
const channelMetadata: Record<string, string> = {
2555+
// NO orchestration_id / orchestration_iteration — the reconciler skips this;
2556+
// the fanout dispatcher posts the ✅/❌ reply on terminal. Reply to the thread
2557+
// ROOT (replyTargetId), never to a reply.
2558+
trigger_comment_id: replyTargetId,
2559+
linear_issue_id: issueId,
2560+
linear_workspace_id: workspaceId,
2561+
linear_oauth_secret_arn: resolved.oauthSecretArn,
2562+
linear_workspace_slug: resolved.workspaceSlug,
2563+
...(iterationReplyId && { iteration_reply_comment_id: iterationReplyId }),
2564+
};
2565+
2566+
try {
2567+
const result = await createTaskCore(
2568+
{
2569+
repo: task.repo,
2570+
workflow_ref: 'coding/new-task-v1',
2571+
task_description: instruction,
2572+
},
2573+
{ userId: task.user_id, channelSource: 'linear', channelMetadata, idempotencyKey },
2574+
idempotencyKey,
2575+
);
2576+
logger.info('A6 comment (standalone): fresh new-task dispatched from follow-up on PR-less task', {
2577+
linear_issue_id: issueId, prior_task_id: task.task_id, status_code: result.statusCode,
2578+
});
2579+
} catch (err) {
2580+
logger.error('A6 comment (standalone): createTaskCore threw for new-work dispatch', {
2581+
linear_issue_id: issueId,
2582+
prior_task_id: task.task_id,
2583+
error: err instanceof Error ? err.message : String(err),
2584+
});
2585+
}
2586+
return true;
2587+
}
2588+
24732589
/**
24742590
* PM-1 clarify-resume. A ``coding/new-task-v1`` run can HOLD to ask a
24752591
* clarifying question (no PR, ``code_changed=false``, ``answer_text=<question>``;

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

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ const transitionIssueStateMock = jest.fn();
5656
const upsertStatusCommentMock = jest.fn();
5757
const reactToCommentMock = jest.fn();
5858
const replyToCommentMock = jest.fn();
59+
const upsertThreadedReplyMock = jest.fn();
5960
jest.mock('../../src/handlers/shared/linear-feedback', () => ({
6061
reportIssueFailure: (...args: unknown[]) => reportIssueFailureMock(...args),
6162
swapIssueReaction: (...args: unknown[]) => swapIssueReactionMock(...args),
@@ -64,6 +65,7 @@ jest.mock('../../src/handlers/shared/linear-feedback', () => ({
6465
upsertStatusComment: (...args: unknown[]) => upsertStatusCommentMock(...args),
6566
reactToComment: (...args: unknown[]) => reactToCommentMock(...args),
6667
replyToComment: (...args: unknown[]) => replyToCommentMock(...args),
68+
upsertThreadedReply: (...args: unknown[]) => upsertThreadedReplyMock(...args),
6769
EMOJI_STARTED: 'eyes',
6870
EMOJI_SUCCESS: 'white_check_mark',
6971
EMOJI_FAILURE: 'x',
@@ -473,6 +475,7 @@ describe('linear-webhook-processor — #247 A6 comment trigger', () => {
473475
discoverOrchestrationMock.mockReset();
474476
reactToCommentMock.mockReset().mockResolvedValue(true);
475477
replyToCommentMock.mockReset().mockResolvedValue(true);
478+
upsertThreadedReplyMock.mockReset().mockResolvedValue('reply-1');
476479
});
477480

478481
test('@bgagent on a started sub-issue → pr-iteration task on its PR with cascade marker', async () => {
@@ -594,14 +597,51 @@ describe('linear-webhook-processor — #247 A6 comment trigger', () => {
594597
expect(createTaskCoreMock.mock.calls[0][0].pr_number).toBe(123);
595598
});
596599

597-
test('plain issue whose ABCA task opened NO PR → no task, no ack', async () => {
600+
// #614: a follow-up on a PR-less completed task is NEW work, not a dead-end.
601+
test('PR-less task + instruction → fresh new-task-v1 on the same repo, 👀 ack, NO orchestration markers', async () => {
598602
mockStandaloneOnly({ task_id: 'task-solo', user_id: 'u-solo', repo: 'o/r' }); // no pr
603+
await handler(eventWith(comment())); // '@bgagent change the timeout to 30 min'
604+
605+
expect(createTaskCoreMock).toHaveBeenCalledTimes(1);
606+
const [body, ctx] = createTaskCoreMock.mock.calls[0];
607+
expect(body.workflow_ref).toBe('coding/new-task-v1');
608+
expect(body.pr_number).toBeUndefined(); // NEW work, not an iteration
609+
expect(body.repo).toBe('o/r');
610+
expect(body.task_description).toBe('change the timeout to 30 min');
611+
expect(ctx.userId).toBe('u-solo');
612+
expect(ctx.channelMetadata.trigger_comment_id).toBe('comment-1');
613+
expect(ctx.channelMetadata.linear_issue_id).toBe('sub-issue-1');
614+
expect(ctx.channelMetadata.orchestration_id).toBeUndefined();
615+
expect(ctx.channelMetadata.orchestration_iteration).toBeUndefined();
616+
expect(ctx.idempotencyKey).toContain('newwork_');
617+
// 👀 ack on the comment.
618+
expect(reactToCommentMock).toHaveBeenCalledWith(expect.anything(), 'comment-1', 'eyes');
619+
});
620+
621+
test('PR-less task + BARE @bgagent (no instruction) → no task, but a threaded reply (not silent)', async () => {
622+
mockStandaloneOnly({ task_id: 'task-solo', user_id: 'u-solo', repo: 'o/r' }); // no pr
623+
await handler(eventWith(comment({ data: { id: 'comment-1', body: '@bgagent', issueId: 'sub-issue-1' } })));
624+
625+
expect(createTaskCoreMock).not.toHaveBeenCalled(); // nothing to start
626+
expect(reactToCommentMock).toHaveBeenCalledWith(expect.anything(), 'comment-1', 'eyes');
627+
expect(upsertThreadedReplyMock).toHaveBeenCalledTimes(1); // told the user what to do
628+
});
629+
630+
test('PR-less task with NO repo → genuinely unactionable → no task, no ack', async () => {
631+
mockStandaloneOnly({ task_id: 'task-solo', user_id: 'u-solo' }); // no pr, no repo
632+
await handler(eventWith(comment()));
633+
expect(createTaskCoreMock).not.toHaveBeenCalled();
634+
expect(reactToCommentMock).not.toHaveBeenCalled();
635+
});
636+
637+
test('PR-less task missing user_id → cannot attribute → no task, no ack', async () => {
638+
mockStandaloneOnly({ task_id: 'task-solo', repo: 'o/r' }); // no user_id, no pr
599639
await handler(eventWith(comment()));
600640
expect(createTaskCoreMock).not.toHaveBeenCalled();
601641
expect(reactToCommentMock).not.toHaveBeenCalled();
602642
});
603643

604-
test('plain issue task missing user_id → cannot attribute → no task', async () => {
644+
test('plain issue task missing user_id (has PR) → cannot attribute → no task', async () => {
605645
mockStandaloneOnly({ task_id: 'task-solo', repo: 'o/r', pr_number: 5 }); // no user_id
606646
await handler(eventWith(comment()));
607647
expect(createTaskCoreMock).not.toHaveBeenCalled();

0 commit comments

Comments
 (0)