From bde4f6fddafa4f472593386a8b371de52f4907f2 Mon Sep 17 00:00:00 2001 From: Siarhei Date: Sun, 13 Sep 2026 13:34:34 +0200 Subject: [PATCH 1/2] fix(translator): let a manual run lift the debounce it is waiting on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Run now" did nothing for a job queued with an auto-translate debounce. The picker skips a job whose `waitUntil` has not elapsed, and the blockers were only cleared for a job that was processing or had failed — a queued one is neither. So the button cleared nothing, the picker took nothing, and the caller was told the job was already in progress, which it was not. A manual run means now, so it now clears the debounce along with the lock and the spent retry budget. The remaining reasons the picker can decline are ones where "already running" is true: another job for the same document holds the concurrency key. `PayloadJob` gained the `waitUntil` field it always had in the row — part of why this was invisible. Closes #127 --- .../PayloadJobsTaskRunner.test.ts | 27 ++++++++++++++++++- .../PayloadJobsTaskRunner.ts | 11 ++++---- .../task-runner/payload-jobs-runner/types.ts | 2 ++ 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/PayloadJobsTaskRunner.test.ts b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/PayloadJobsTaskRunner.test.ts index 82282e40e..8ef18a538 100644 --- a/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/PayloadJobsTaskRunner.test.ts +++ b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/PayloadJobsTaskRunner.test.ts @@ -473,6 +473,27 @@ describe("PayloadJobsTaskRunner", () => { ); }); + it("lifts the debounce so a queued job really does run now", async () => { + // Auto-translate queues with a `waitUntil` to coalesce rapid edits. The picker skips a job + // whose delay has not elapsed, so without clearing it the "Run now" button runs nothing and + // the caller is told the job is already in progress — which it is not. + const queued = createJob({ + processing: false, + waitUntil: new Date(Date.now() + 60_000).toISOString(), + }); + mockPayload.find.mockResolvedValue({ docs: [queued] }); + + const result = await runner.run("job-123"); + + expect(mockPayload.update).toHaveBeenCalledWith( + expect.objectContaining({ + where: { id: { equals: "job-123" } }, + data: { processing: false, hasError: false, error: null, waitUntil: null }, + }) + ); + expect(result).toEqual({ success: true }); + }); + it("returns already_running when a job is genuinely in flight (fresh lock)", async () => { const runningJob = createJob({ processing: true, @@ -549,7 +570,11 @@ describe("PayloadJobsTaskRunner", () => { where: { id: { equals: "job-123" } }, limit: 1, }); - expect(mockPayload.update).not.toHaveBeenCalled(); + // The clear runs first even for a job with nothing blocking it: see "lifts the debounce so a + // queued job really does run now" for why it is unconditional. + expect(mockPayload.update.mock.invocationCallOrder[0]).toBeLessThan( + mockPayload.jobs.run.mock.invocationCallOrder[0] + ); expect(mockPayload.find).toHaveBeenCalledWith( expect.objectContaining({ where: { diff --git a/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/PayloadJobsTaskRunner.ts b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/PayloadJobsTaskRunner.ts index 4d8a6b254..8a185d71e 100644 --- a/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/PayloadJobsTaskRunner.ts +++ b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/PayloadJobsTaskRunner.ts @@ -187,17 +187,18 @@ export class PayloadJobsTaskRunner implements TaskRunner { if (job.processing && !this.isStale(job.updatedAt)) { return { success: false, error: "already_running" }; } - if (job.processing || job.error) { - await this.clearPickerBlockers(taskId); - } + // Unconditionally: a manual run is "now", so it lifts the debounce a queued job is still waiting + // out as well as the lock and the spent retry budget. Without that the picker takes nothing and + // the caller is told the job is already running, which it is not. + await this.clearPickerBlockers(taskId); // Not `jobs.runByID`: payload 3.84.1 builds the picker guard (processing / hasError / waitUntil) // only on the `where` path, so the id path re-runs a job that exhausted its retries. - const result = (await this.payload.jobs.run({ + const result = await this.payload.jobs.run({ queue: this.config.queueName, where: { id: { equals: taskId } }, limit: 1, - })) as { jobStatus?: Record }; + }); const pickerTookNothing = Object.keys(result?.jobStatus ?? {}).length === 0; if (pickerTookNothing) { diff --git a/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/types.ts b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/types.ts index fbf79f851..e9e5f7593 100644 --- a/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/types.ts +++ b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/types.ts @@ -89,6 +89,8 @@ export type PayloadJob = { updatedAt: string; error?: unknown; processing?: boolean | null; + /** The debounce the job was queued with; the picker skips the job until it has elapsed. */ + waitUntil?: string | null; input?: { /** Document reference (flat text, ID-agnostic). Current shape. */ collection_slug?: string; From 5c8ef146cdc23d0aeea13bd99fd477577128977f Mon Sep 17 00:00:00 2001 From: Siarhei Date: Mon, 14 Sep 2026 12:03:22 +0200 Subject: [PATCH 2/2] test(translator): guard the order the manual-run clear depends on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Making the clear unconditional left it a single statement that is easy to move, and moving it above the guard that refuses a job already in flight passed all 41 unit tests — while resetting `processing` on a live job, which is how two workers end up writing one document. The fresh-lock case now asserts that nothing is written. The retry path is exercised end to end for the first time. `locale-workflow-failure` used to edit the job row by hand and call the queue directly, standing in for the very endpoint under test; it now calls POST /translate/run/:id, and reverting the fix turns it red with a 429. `callEndpoint` had to learn that a 204 carries no body — the reason no integration test had ever called that route. Also dropped a unit test the fix had made idle (it pinned clearing for a failed job, which is no longer a distinct case) and renamed another to what it actually discriminates. --- .../integration/translator/callEndpoint.ts | 7 ++-- .../locale-workflow-failure.int.test.ts | 15 ++++----- .../PayloadJobsTaskRunner.test.ts | 32 ++++--------------- .../PayloadJobsTaskRunner.ts | 4 +-- .../task-runner/payload-jobs-runner/types.ts | 1 - 5 files changed, 17 insertions(+), 42 deletions(-) diff --git a/apps/dev/src/integration/translator/callEndpoint.ts b/apps/dev/src/integration/translator/callEndpoint.ts index 878cd8dbd..0689abce4 100644 --- a/apps/dev/src/integration/translator/callEndpoint.ts +++ b/apps/dev/src/integration/translator/callEndpoint.ts @@ -9,7 +9,8 @@ import type { Payload, PayloadRequest } from "payload"; * handler code path. This is the faithful way to trigger a manual translation (`POST /translate/enqueue`) * or read staleness (`GET /translate/stale/:collection_slug/:collection_id`) from a local-API test. * - * @returns the endpoint's HTTP status + parsed JSON body. + * @returns the endpoint's HTTP status, and the parsed JSON body — `undefined` when the response + * has no body (204). */ export async function callEndpoint( payload: Payload, @@ -31,6 +32,6 @@ export async function callEndpoint( } as unknown as PayloadRequest; const res = await endpoint.handler(req); - const data = await res.json(); - return { status: res.status, data }; + const text = await res.text(); + return { status: res.status, data: text ? JSON.parse(text) : undefined }; } diff --git a/apps/dev/src/integration/translator/locale-workflow-failure.int.test.ts b/apps/dev/src/integration/translator/locale-workflow-failure.int.test.ts index ccda800cc..81477fed5 100644 --- a/apps/dev/src/integration/translator/locale-workflow-failure.int.test.ts +++ b/apps/dev/src/integration/translator/locale-workflow-failure.int.test.ts @@ -96,16 +96,13 @@ describe("when one locale's provider fails", () => { ["fr", "failed"], ]); - // Payload backs a failed job off exponentially, so the picker would skip it for the next few - // seconds. Clearing the delay lets the retry happen now rather than making the spec wait. - await failing.payload.update({ - collection: "payload-jobs" as "pages", - id: (docs[0] as { id: string | number }).id, - data: { waitUntil: null, processing: false } as never, - }); - + // Payload backs a failed job off exponentially; only the manual-run endpoint lifts that, so the + // retry has to go through it. const before = failing.translateCount(); - await failing.payload.jobs.run({ queue: "translations", limit: CRON_BATCH_LIMIT }); + const retry = await callEndpoint(failing.payload, "post", "/translate/run/:id", { + routeParams: { id: String((docs[0] as { id: string | number }).id) }, + }); + expect(retry.status, "the retry endpoint did not accept the job").toBe(204); expect( failing.translateCount() - before, diff --git a/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/PayloadJobsTaskRunner.test.ts b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/PayloadJobsTaskRunner.test.ts index 8ef18a538..7b2175830 100644 --- a/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/PayloadJobsTaskRunner.test.ts +++ b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/PayloadJobsTaskRunner.test.ts @@ -457,30 +457,9 @@ describe("PayloadJobsTaskRunner", () => { }); }); - it("clears what blocks the picker before retrying a failed job", async () => { - mockPayload.find.mockResolvedValue({ - docs: [createJob({ error: { message: "provider down" } })], - }); - - await runner.run("job-123"); - - expect(mockPayload.update).toHaveBeenCalledWith( - expect.objectContaining({ - collection: "payload-jobs", - where: { id: { equals: "job-123" } }, - data: { processing: false, hasError: false, error: null, waitUntil: null }, - }) - ); - }); - - it("lifts the debounce so a queued job really does run now", async () => { - // Auto-translate queues with a `waitUntil` to coalesce rapid edits. The picker skips a job - // whose delay has not elapsed, so without clearing it the "Run now" button runs nothing and - // the caller is told the job is already in progress — which it is not. - const queued = createJob({ - processing: false, - waitUntil: new Date(Date.now() + 60_000).toISOString(), - }); + it("clears the blockers even for a job that is neither running nor failed", async () => { + const stillDebounced = new Date(Date.now() + 60_000).toISOString(); + const queued = createJob({ processing: false, waitUntil: stillDebounced }); mockPayload.find.mockResolvedValue({ docs: [queued] }); const result = await runner.run("job-123"); @@ -505,6 +484,9 @@ describe("PayloadJobsTaskRunner", () => { expect(result).toEqual({ success: false, error: "already_running" }); expect(mockPayload.jobs.run).not.toHaveBeenCalled(); + // Clearing above this guard would reset `processing` on a live job and let a second worker + // take the same document. + expect(mockPayload.update).not.toHaveBeenCalled(); }); it("re-runs a job whose processing lock is stale (killed mid-run)", async () => { @@ -570,8 +552,6 @@ describe("PayloadJobsTaskRunner", () => { where: { id: { equals: "job-123" } }, limit: 1, }); - // The clear runs first even for a job with nothing blocking it: see "lifts the debounce so a - // queued job really does run now" for why it is unconditional. expect(mockPayload.update.mock.invocationCallOrder[0]).toBeLessThan( mockPayload.jobs.run.mock.invocationCallOrder[0] ); diff --git a/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/PayloadJobsTaskRunner.ts b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/PayloadJobsTaskRunner.ts index 8a185d71e..b7a270d23 100644 --- a/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/PayloadJobsTaskRunner.ts +++ b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/PayloadJobsTaskRunner.ts @@ -187,9 +187,7 @@ export class PayloadJobsTaskRunner implements TaskRunner { if (job.processing && !this.isStale(job.updatedAt)) { return { success: false, error: "already_running" }; } - // Unconditionally: a manual run is "now", so it lifts the debounce a queued job is still waiting - // out as well as the lock and the spent retry budget. Without that the picker takes nothing and - // the caller is told the job is already running, which it is not. + // Unconditional: a manual run also lifts the auto-translate debounce, not just the lock. await this.clearPickerBlockers(taskId); // Not `jobs.runByID`: payload 3.84.1 builds the picker guard (processing / hasError / waitUntil) diff --git a/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/types.ts b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/types.ts index e9e5f7593..b6395dba5 100644 --- a/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/types.ts +++ b/packages/payload-plugin-translator/src/server/modules/task-runner/payload-jobs-runner/types.ts @@ -89,7 +89,6 @@ export type PayloadJob = { updatedAt: string; error?: unknown; processing?: boolean | null; - /** The debounce the job was queued with; the picker skips the job until it has elapsed. */ waitUntil?: string | null; input?: { /** Document reference (flat text, ID-agnostic). Current shape. */