From 50c156e025cce2870b73cdaeff3883cf62b20617 Mon Sep 17 00:00:00 2001 From: xmtp-coder-agent <> Date: Tue, 21 Apr 2026 07:01:58 +0000 Subject: [PATCH] Handle missing task in comment workflow as no-op MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a comment is posted on an issue or PR with no associated Coder task, runComment previously threw NonRetryableError from locate-task. That surfaces the workflow instance as `errored` in listings even though the situation is benign (e.g. a comment on an issue that was never assigned to the agent). Return null from the locate-task step when findTaskByName returns null and short-circuit runComment, matching failed-check.ts's behavior for the same case. Malformed SDK shapes still throw NonRetryableError — that failure mode is distinct and should remain loud. Fixes #116 Co-Authored-By: Claude Opus 4.7 --- src/workflows/steps/comment.test.ts | 49 ++++++++++++++++++++--------- src/workflows/steps/comment.ts | 48 ++++++++++++++++------------ 2 files changed, 63 insertions(+), 34 deletions(-) diff --git a/src/workflows/steps/comment.test.ts b/src/workflows/steps/comment.test.ts index bb86084..78fffcb 100644 --- a/src/workflows/steps/comment.test.ts +++ b/src/workflows/steps/comment.test.ts @@ -255,24 +255,45 @@ describe("runComment — task-name derivation (regression guard)", () => { // ── Task-not-found + structured-message coverage ───────────────────────────── describe("runComment — error paths + message formatting", () => { - test("throws NonRetryableError when task not found (PR path)", async () => { - const { NonRetryableError } = await import("cloudflare:workflows"); + test("PR comment with no matching task → silent short-circuit (no throw, no send)", async () => { + // Previously this path threw NonRetryableError, which surfaced the + // instance as `errored` in workflow listings even though a comment on + // an issue/PR without an associated Coder task is benign. const step = makeStep(); const coder = makeCoder({ findTaskByName: vi.fn(async () => null) }); const github = makeGithub(); - await expect( - runComment({ - step: step as never, - coder: coder as never, - github: github as never, - config, - event: commentEvent("pull_request"), - }), - // Assert the specific error type so a regression that throws a - // generic Error (and thus gets retried by the engine instead of - // terminating the instance) fails this test. - ).rejects.toThrowError(NonRetryableError); + await runComment({ + step: step as never, + coder: coder as never, + github: github as never, + config, + event: commentEvent("pull_request"), + }); + + expect(step.calls).toEqual(["find-linked-issues", "locate-task"]); + expect(coder.sendTaskInput).not.toHaveBeenCalled(); + expect(github.addReactionToComment).not.toHaveBeenCalled(); + expect(github.addReactionToReviewComment).not.toHaveBeenCalled(); + }); + + test("issue comment with no matching task → silent short-circuit (no throw, no send)", async () => { + const step = makeStep(); + const coder = makeCoder({ findTaskByName: vi.fn(async () => null) }); + const github = makeGithub(); + + await runComment({ + step: step as never, + coder: coder as never, + github: github as never, + config, + event: commentEvent("issue"), + }); + + expect(step.calls).toEqual(["locate-task"]); + expect(coder.sendTaskInput).not.toHaveBeenCalled(); + expect(github.addReactionToComment).not.toHaveBeenCalled(); + expect(github.addReactionToReviewComment).not.toHaveBeenCalled(); }); test("PR review comment: sendTaskInput receives structured formatPRCommentMessage with file:line", async () => { diff --git a/src/workflows/steps/comment.ts b/src/workflows/steps/comment.ts index 9a7ab06..9ce3234 100644 --- a/src/workflows/steps/comment.ts +++ b/src/workflows/steps/comment.ts @@ -70,9 +70,9 @@ async function resolveIssueNumber( * Workflow step factory for `comment_posted` (both PR and issue kinds). * * Flow: [find-linked-issues for PR comments] → locate-task → ensureTaskReady - * → send-task-input → react-to-comment. Throws `NonRetryableError` if the - * task doesn't exist — there's nothing to send to, and retrying won't create - * it. + * → send-task-input → react-to-comment. Silently short-circuits when no task + * exists for the issue — a comment on an unassigned issue/PR is benign and + * shouldn't surface the workflow instance as `errored`. * * The task input is wrapped in the same `[INSTRUCTIONS]` / `[COMMENT]` template * the pre-migration action classes used (`formatPRCommentMessage` / @@ -96,23 +96,31 @@ export async function runComment(ctx: RunCommentContext): Promise { issueNumber, ); - const located = await step.do("locate-task", async () => { - const raw = await coder.findTaskByName(taskName); - if (!raw) { - throw new NonRetryableError(`task ${taskName} not found`); - } - // `findTaskByName` returns `ExperimentalCoderSDKTask | null` (narrowed via - // Zod inside CoderService). Validate the scalar fields we depend on before - // projecting into the step return — defensive against upstream SDK shape - // changes. - const task = raw as unknown as { id?: unknown; owner_id?: unknown }; - if (typeof task.id !== "string" || typeof task.owner_id !== "string") { - throw new NonRetryableError( - `locate-task: task ${taskName} missing id/owner_id scalars`, - ); - } - return { taskId: task.id, owner: task.owner_id }; - }); + const located = await step.do<{ taskId: string; owner: string } | null>( + "locate-task", + async () => { + const raw = await coder.findTaskByName(taskName); + // No task exists for this issue/PR — a comment without an associated + // Coder task is benign (e.g. a comment on an issue that was never + // assigned to the agent). Short-circuit silently, matching + // `failed-check.ts`'s behavior for the same case. + if (!raw) { + return null; + } + // `findTaskByName` returns `ExperimentalCoderSDKTask | null` (narrowed via + // Zod inside CoderService). Validate the scalar fields we depend on before + // projecting into the step return — defensive against upstream SDK shape + // changes. + const task = raw as unknown as { id?: unknown; owner_id?: unknown }; + if (typeof task.id !== "string" || typeof task.owner_id !== "string") { + throw new NonRetryableError( + `locate-task: task ${taskName} missing id/owner_id scalars`, + ); + } + return { taskId: task.id, owner: task.owner_id }; + }, + ); + if (located == null) return; const taskId = TaskIdSchema.parse(located.taskId);