Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 35 additions & 14 deletions src/workflows/steps/comment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
48 changes: 28 additions & 20 deletions src/workflows/steps/comment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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` /
Expand All @@ -96,23 +96,31 @@ export async function runComment(ctx: RunCommentContext): Promise<void> {
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);

Expand Down
Loading