From 6f52902d26d4e1c4c1684458688a370ee3042cf9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 09:24:42 +0000 Subject: [PATCH 1/3] feat(spec): optional degraded-outcome report on JobHandler (#6617) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit JobHandler had two states — threw or did not — so a run that completed without accomplishing its work was recorded as success. Widen the return type to Promise so a handler can OPTIONALLY report "ran to completion, work did not happen". Additive on both sides: existing Promise handlers are unchanged byte for byte, and existing IJobService implementations are unchanged because this widens a return type rather than adding a context member. degraded is not a failure and does not retry — retry stays throw-driven. Spec half of #5548's B-minimal ruling; the DbJobAdapter/sys_job_run mapping is #5548's half and is deliberately not wired here. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PiRUoQkTSBBmpyXBY3cVn2 --- .changeset/job-handler-degraded-outcome.md | 48 ++++++ .../spec/src/contracts/job-service.test.ts | 137 +++++++++++++++++- packages/spec/src/contracts/job-service.ts | 66 ++++++++- 3 files changed, 248 insertions(+), 3 deletions(-) create mode 100644 .changeset/job-handler-degraded-outcome.md diff --git a/.changeset/job-handler-degraded-outcome.md b/.changeset/job-handler-degraded-outcome.md new file mode 100644 index 0000000000..c045e05352 --- /dev/null +++ b/.changeset/job-handler-degraded-outcome.md @@ -0,0 +1,48 @@ +--- +"@objectstack/spec": minor +--- + +feat(spec): give `JobHandler` an optional "ran, but the work did not happen" report (#6617) + +`JobHandler` had exactly two states — it threw, or it did not — so a run that +completed without accomplishing anything was recorded as `success`, +indistinguishable from one that did the work. The motivating case is #5529's +wait-wake handler: when its store is unavailable it fires the shot at nothing, +returns normally, and `sys_job_run` says the wake succeeded. + +This is the **spec half** of the maintainer's B-minimal ruling on #5548. The +handler's return type widens: + +```ts +export interface JobRunOutcome { + outcome: 'completed' | 'degraded'; + reason?: string; +} + +export type JobHandler = + (context: { jobId: string; data?: unknown }) => Promise; +``` + +**`degraded` is not a failure and does not trigger a retry.** Failure and retry +remain driven exclusively by a rejected promise (`runWithPolicy` retries on +throw), so a resolved outcome — whatever it says — never re-runs the job and +never surfaces as an error. A handler that wants a retry must still throw. That +separation is the point of the ruling: making these handlers throw instead was +rejected precisely because it would change failure semantics that third-party +`IJobService` implementations already build retry behaviour on. + +**Nothing to migrate — this is additive on both sides.** + +- Existing `Promise< void >` handlers are unchanged, byte for byte. Reporting + nothing means exactly what it means today: no throw implies success. +- Existing `IJobService` implementations are unchanged. This widens a *return* + type rather than adding a member to the handler context, so no implementation + has to grow anything; an adapter that ignores the resolved value keeps its + current behaviour. (A `ctx.reportOutcome` callback would have forced every + third-party implementation to construct a new context member — which is why + the return-value shape was chosen.) + +Consuming the report — mapping `degraded` onto a `sys_job_run.status` distinct +from `success` — is the services half, tracked in #5548, and is deliberately not +wired here: the shipped adapters currently discard the resolved value, which is +what makes landing the contract first safe. diff --git a/packages/spec/src/contracts/job-service.test.ts b/packages/spec/src/contracts/job-service.test.ts index 11497f2967..c2655fdb12 100644 --- a/packages/spec/src/contracts/job-service.test.ts +++ b/packages/spec/src/contracts/job-service.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import type { IJobService, JobHandler, JobExecution } from './job-service'; +import type { IJobService, JobHandler, JobRunOutcome, JobExecution } from './job-service'; describe('Job Service Contract', () => { it('should allow a minimal IJobService implementation with required methods', () => { @@ -109,3 +109,138 @@ describe('Job Service Contract', () => { expect(jobs).toContain('cleanup_logs'); }); }); + +/** + * [#6617] The degraded-outcome reporting channel — the spec half of #5548's + * B-minimal ruling. + * + * These are COMPILE-TIME pins first and runtime assertions second: the package + * type-checks its tests through `tsconfig.test.json`, so an assignability pin + * here is a real check rather than a phantom one. Reverse verification for the + * whole block is `JobHandler`'s return type narrowed back to `Promise` — + * which reddens the `degraded` direction only, and leaves every legacy pin + * green. That asymmetry IS additivity. + */ +describe('[#6617] JobHandler degraded-outcome channel', () => { + /** + * The handler type EXACTLY as it stood before #6617. Pinning it as a + * standalone declaration is what makes the additivity claim falsifiable: + * if the union ever stops being a widening, these assignments break. + */ + type PreIssue6617JobHandler = (context: { jobId: string; data?: unknown }) => Promise; + + it('(a) an existing Promise< void > handler is unchanged — additivity, implementer side', async () => { + // Written the way every handler in the repo is written today: no return. + const legacy: PreIssue6617JobHandler = async (ctx) => { + void ctx.jobId; + }; + + // The pin: the PRE-change type is still assignable to the post-change one. + // A narrowing (rather than a widening) of JobHandler breaks this line. + const stillAJobHandler: JobHandler = legacy; + + // …and inline literals keep inferring, with no annotation ceremony. + const inlineLegacy: JobHandler = async () => {}; + + await expect(stillAJobHandler({ jobId: 'sync_metadata' })).resolves.toBeUndefined(); + await expect(inlineLegacy({ jobId: 'sync_metadata' })).resolves.toBeUndefined(); + }); + + it('(b) a handler reporting { outcome: degraded, reason } is type-legal', async () => { + // THE pin that must go red when the union is narrowed back to Promise< void >. + const degraded: JobHandler = async () => ({ + outcome: 'degraded', + reason: 'STORE_UNAVAILABLE', + }); + + const completed: JobHandler = async () => ({ outcome: 'completed' }); + + await expect(degraded({ jobId: 'wait_wake' })).resolves.toEqual({ + outcome: 'degraded', + reason: 'STORE_UNAVAILABLE', + }); + await expect(completed({ jobId: 'wait_wake' })).resolves.toEqual({ outcome: 'completed' }); + }); + + it('reason is optional, and the outcome union admits exactly two members', async () => { + const bare: JobRunOutcome = { outcome: 'degraded' }; + expect(bare.reason).toBeUndefined(); + + // @ts-expect-error 'skipped' is not a member of the outcome union — the + // third state is `degraded`, and new states are a spec decision, not a + // free-text field. (This directive is live: the tests are type-checked.) + const bogus: JobRunOutcome = { outcome: 'skipped' }; + expect(bogus.outcome).toBe('skipped'); + }); + + it('an adapter that IGNORES the resolved value keeps todays behaviour exactly', async () => { + // This is `DbJobAdapter.wrap()`'s shape as it stands on main (L161-176): + // `await handler(ctx)` discards the resolved value, and only a THROW takes + // the failure branch. #5548 is what teaches it to read the value. + const recorded: string[] = []; + const wrap = (handler: JobHandler): JobHandler => async (ctx) => { + try { + await handler(ctx); + recorded.push('success'); + } catch { + recorded.push('failed'); + } + }; + + await wrap(async () => {})({ jobId: 'legacy' }); + await wrap(async () => ({ outcome: 'degraded', reason: 'nothing to wake' }))({ jobId: 'new' }); + + // Both land on `success` — an unwired adapter cannot regress, which is the + // safety argument for landing the spec half first. + expect(recorded).toEqual(['success', 'success']); + }); + + it('a consumer that DOES read the value distinguishes all three states', async () => { + // The mapping #5548 will implement, pinned here at the contract level so + // the spec half states what the services half owes. + const classify = async (handler: JobHandler): Promise<'success' | 'degraded' | 'failed'> => { + let result: void | JobRunOutcome; + try { + result = await handler({ jobId: 'j' }); + } catch { + return 'failed'; + } + // Reporting nothing is success — "不回报 = 现状". + return result && result.outcome === 'degraded' ? 'degraded' : 'success'; + }; + + expect(await classify(async () => {})).toBe('success'); + expect(await classify(async () => ({ outcome: 'completed' }))).toBe('success'); + expect(await classify(async () => ({ outcome: 'degraded', reason: 'x' }))).toBe('degraded'); + expect(await classify(async () => { throw new Error('boom'); })).toBe('failed'); + }); + + it('degraded does not travel the retry path — retry is throw-driven', async () => { + // Mirrors `runWithPolicy`: retries are driven by a REJECTED promise, so a + // resolved degraded report can never re-run the job. + let attempts = 0; + const runWithRetry = async (handler: JobHandler, maxRetries: number) => { + for (let i = 0; i <= maxRetries; i++) { + try { + return await handler({ jobId: 'j' }); + } catch (err) { + if (i === maxRetries) throw err; + } + } + }; + + const degraded: JobHandler = async () => { + attempts++; + return { outcome: 'degraded', reason: 'STORE_UNAVAILABLE' }; + }; + + const outcome = await runWithRetry(degraded, 3); + expect(attempts).toBe(1); // ← not 4: degraded is not a failure + expect(outcome).toEqual({ outcome: 'degraded', reason: 'STORE_UNAVAILABLE' }); + + let thrownAttempts = 0; + await expect(runWithRetry(async () => { thrownAttempts++; throw new Error('boom'); }, 3)) + .rejects.toThrow('boom'); + expect(thrownAttempts).toBe(4); // ← a throw still retries, unchanged + }); +}); diff --git a/packages/spec/src/contracts/job-service.ts b/packages/spec/src/contracts/job-service.ts index 876eb2e50d..b4d61899bb 100644 --- a/packages/spec/src/contracts/job-service.ts +++ b/packages/spec/src/contracts/job-service.ts @@ -45,9 +45,71 @@ export interface JobSchedule { } /** - * Job handler function + * What a job handler reports about a run that finished without throwing — + * the OPTIONAL third state of {@link JobHandler} (#6617, the spec half of the + * #5548 B-minimal ruling). + * + * A handler that resolves this instead of `undefined` distinguishes *"ran and + * did the work"* from *"ran to completion but did not accomplish it"*. The + * motivating case is #5529's wait-wake handler: when its store is unavailable + * it fires the shot at nothing, completes normally, and today is recorded as + * indistinguishable from a wake that actually woke something. + * + * `reason` is a short operator-facing note (`'STORE_UNAVAILABLE'`, `'0 rows + * matched'`) — free text for an audit surface, never a machine-dispatched code. + */ +export interface JobRunOutcome { + /** + * `'completed'` — the run did its work. Identical in meaning to resolving + * `undefined`; spell it out when a handler computes the verdict either way. + * + * `'degraded'` — the run finished, but its work did not happen. **This is + * not a failure** (see {@link JobHandler}). + */ + outcome: 'completed' | 'degraded'; + /** Why the run was degraded — short, human-readable, for the audit trail. */ + reason?: string; +} + +/** + * Job handler function. + * + * **Three outcomes, of which the third is optional and additive** (#6617): + * + * | The handler… | Means | Recorded as | + * |:---|:---|:---| + * | throws / rejects | the run **failed** | `failed` — and the retry policy applies | + * | resolves `undefined` (or `{ outcome: 'completed' }`) | the run **succeeded** | `success` | + * | resolves `{ outcome: 'degraded', reason? }` | ran to completion, **work did not happen** | a status distinct from `success` (#5548) | + * + * ⚠️ **`degraded` is NOT a failure and does NOT trigger a retry.** Retry and + * failure are driven exclusively by a *rejected* promise (`runWithPolicy` + * retries on throw), so a resolved outcome — whatever it says — never re-runs + * the job and never surfaces as an error. A handler that wants the run retried + * must throw, exactly as before. This separation is the whole point of the + * ruling: option A (make these handlers throw) was rejected precisely because + * it would change the failure semantics that third-party `IJobService` + * implementations already build retry behaviour on. + * + * **Additivity — the compatibility contract this type owes** (the ruling's + * 「可加性条款」, and the acceptance criterion of #6617): + * + * - An existing `Promise` handler is **unchanged, byte for byte**. It + * reports nothing, and reporting nothing is today's behaviour exactly: + * *no throw ⇒ success*. + * - An existing `IJobService` implementation is **unchanged**. This is a + * widened *return* type, not a new member on the handler context, so no + * implementation has to grow anything — an adapter that simply ignores the + * resolved value keeps its current semantics. (A `ctx.reportOutcome` + * callback would have forced every implementation to construct a new context + * member; that is why the return-value shape was chosen.) + * + * The reporting channel is deliberately opt-in on **both** ends. Consuming it + * — mapping `degraded` onto a `sys_job_run.status` distinct from `success` — + * is #5548's half and is **not yet wired**: the shipped adapters currently + * discard the resolved value, which is precisely why doing so is safe. */ -export type JobHandler = (context: { jobId: string; data?: unknown }) => Promise; +export type JobHandler = (context: { jobId: string; data?: unknown }) => Promise; /** * Retry policy for a scheduled job (mirrors the authorable `RetryPolicySchema`, From a0f2f978310d9f9f24953f101efb1f5c973c4f21 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 10:20:49 +0000 Subject: [PATCH 2/3] fix(service-job): stop runWithPolicy erasing the run outcome (#6617) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The widened JobHandler may resolve a JobRunOutcome, and a wrapper typed `() => Promise` rejects that — TypeScript's return-type void special case does not reach through Promise, so both the cron and interval adapters failed to build. runWithPolicy/withTimeout are now generic with T = void: every existing caller still infers void and behaviour is unchanged, but the wrapper no longer erases what the run resolved to. Retry stays throw-driven, so a resolved degraded report returns on the first attempt. Also commits the regenerated api-surface shard (+ JobRunOutcome, the only delta: 0 breaking, 1 added). The sys_job_run mapping that CONSUMES the outcome remains #5548's half. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PiRUoQkTSBBmpyXBY3cVn2 --- .changeset/job-handler-degraded-outcome.md | 10 +++ .../src/run-with-policy.outcome.test.ts | 66 +++++++++++++++++++ .../service-job/src/run-with-policy.ts | 23 +++++-- packages/spec/api-surface/contracts.json | 1 + 4 files changed, 93 insertions(+), 7 deletions(-) create mode 100644 packages/services/service-job/src/run-with-policy.outcome.test.ts diff --git a/.changeset/job-handler-degraded-outcome.md b/.changeset/job-handler-degraded-outcome.md index c045e05352..2397f85a04 100644 --- a/.changeset/job-handler-degraded-outcome.md +++ b/.changeset/job-handler-degraded-outcome.md @@ -1,5 +1,6 @@ --- "@objectstack/spec": minor +"@objectstack/service-job": minor --- feat(spec): give `JobHandler` an optional "ran, but the work did not happen" report (#6617) @@ -42,6 +43,15 @@ rejected precisely because it would change failure semantics that third-party third-party implementation to construct a new context member — which is why the return-value shape was chosen.) +**One consumer needed widening too, and it is additive as well.** +`runWithPolicy` in `@objectstack/service-job` typed its run as +`() => Promise< void >`, which rejects a handler that may resolve an outcome — +TypeScript's return-type `void` special case does not reach through +`Promise< void >`. It is now generic with `T = void`, so every existing call +still infers `T = void` and behaviour is unchanged; what changed is that the +retry wrapper no longer *erases* what the run resolved to. Retry semantics are +untouched: only a rejected promise retries. + Consuming the report — mapping `degraded` onto a `sys_job_run.status` distinct from `success` — is the services half, tracked in #5548, and is deliberately not wired here: the shipped adapters currently discard the resolved value, which is diff --git a/packages/services/service-job/src/run-with-policy.outcome.test.ts b/packages/services/service-job/src/run-with-policy.outcome.test.ts new file mode 100644 index 0000000000..45d9a9f067 --- /dev/null +++ b/packages/services/service-job/src/run-with-policy.outcome.test.ts @@ -0,0 +1,66 @@ +import { describe, it, expect } from 'vitest'; +import type { JobHandler, JobRunOutcome } from '@objectstack/spec'; +import { runWithPolicy } from './run-with-policy.js'; + +/** + * [#6617] `runWithPolicy` must not ERASE what a job run resolved to. + * + * `JobHandler` may now resolve a {@link JobRunOutcome}. A wrapper typed + * `() => Promise` rejects that at compile time — TypeScript's + * return-type `void` special case does not reach through `Promise` — + * so the wrapper is generic with `T = void`. These cases pin the two halves: + * the value survives the wrapper, and retry stays throw-driven so a degraded + * report never re-runs the job. + * + * The `sys_job_run` mapping that CONSUMES the outcome is #5548's half and is + * deliberately not implemented here. + */ +describe('[#6617] runWithPolicy passes the run outcome through', () => { + it('returns a degraded outcome unchanged, with no retry policy', async () => { + const handler: JobHandler = async () => ({ outcome: 'degraded', reason: 'STORE_UNAVAILABLE' }); + + const result = await runWithPolicy('wait_wake', () => handler({ jobId: 'wait_wake' })); + + expect(result).toEqual({ outcome: 'degraded', reason: 'STORE_UNAVAILABLE' }); + }); + + it('returns a degraded outcome unchanged THROUGH the retry path, without retrying', async () => { + let attempts = 0; + const handler: JobHandler = async () => { + attempts++; + return { outcome: 'degraded', reason: 'nothing to wake' } satisfies JobRunOutcome; + }; + + const result = await runWithPolicy( + 'wait_wake', + () => handler({ jobId: 'wait_wake' }), + { retryPolicy: { maxRetries: 3, backoffMs: 1 } }, + ); + + // Resolved is resolved: degraded is not a failure, so one attempt only. + expect(attempts).toBe(1); + expect(result).toEqual({ outcome: 'degraded', reason: 'nothing to wake' }); + }); + + it('a legacy void handler still resolves undefined — unchanged', async () => { + const legacy: JobHandler = async () => {}; + + const result = await runWithPolicy('sync_metadata', () => legacy({ jobId: 'sync_metadata' })); + + expect(result).toBeUndefined(); + }); + + it('a throwing handler still retries and rethrows — unchanged', async () => { + let attempts = 0; + + await expect( + runWithPolicy( + 'flaky', + async () => { attempts++; throw new Error('boom'); }, + { retryPolicy: { maxRetries: 2, backoffMs: 1 } }, + ), + ).rejects.toThrow('boom'); + + expect(attempts).toBe(3); // initial + 2 retries + }); +}); diff --git a/packages/services/service-job/src/run-with-policy.ts b/packages/services/service-job/src/run-with-policy.ts index 2dbe2fb93b..0bc0802c5a 100644 --- a/packages/services/service-job/src/run-with-policy.ts +++ b/packages/services/service-job/src/run-with-policy.ts @@ -40,14 +40,14 @@ function sleep(ms: number): Promise { }); } -function withTimeout(run: () => Promise, jobId: string, timeoutMs?: number): Promise { +function withTimeout(run: () => Promise, jobId: string, timeoutMs?: number): Promise { if (!timeoutMs || timeoutMs <= 0) return run(); let timer: ReturnType | undefined; const guard = new Promise((_, reject) => { timer = setTimeout(() => reject(new JobTimeoutError(jobId, timeoutMs)), timeoutMs); (timer as any)?.unref?.(); }); - return Promise.race([run(), guard]).finally(() => clearTimeout(timer)) as Promise; + return Promise.race([run(), guard]).finally(() => clearTimeout(timer)) as Promise; } /** @@ -67,12 +67,22 @@ function withTimeout(run: () => Promise, jobId: string, timeoutMs?: number * convergence (#4661) and are enforced here, not merely declared: jitter is * what stops a fleet of jobs that failed on the same outage from retrying in * lockstep. + * + * Generic in the run's resolved value, defaulting to `void` (#6617). Every + * existing caller passes a `() => Promise` and still infers `T = void`, + * so this is purely additive — what changed is that the wrapper no longer + * ERASES what the run resolved to. It has to stop erasing it because + * {@link JobHandler} can now resolve a `JobRunOutcome`, and a wrapper typed + * `() => Promise` rejects that: TypeScript's return-type `void` special + * case does not reach through `Promise`. Retry semantics are untouched — + * only a REJECTED promise retries, so a resolved outcome (degraded or not) + * returns on the first attempt exactly as a resolved `undefined` always did. */ -export async function runWithPolicy( +export async function runWithPolicy( jobId: string, - run: () => Promise, + run: () => Promise, options?: JobScheduleOptions, -): Promise { +): Promise { const timeoutMs = options?.timeout; if (!options?.retryPolicy) { return withTimeout(run, jobId, timeoutMs); @@ -93,8 +103,7 @@ export async function runWithPolicy( await sleep(delay); } try { - await withTimeout(run, jobId, timeoutMs); - return; + return await withTimeout(run, jobId, timeoutMs); } catch (err) { lastError = err; } diff --git a/packages/spec/api-surface/contracts.json b/packages/spec/api-surface/contracts.json index 205b12e561..3f7c4a8ee4 100644 --- a/packages/spec/api-surface/contracts.json +++ b/packages/spec/api-surface/contracts.json @@ -163,6 +163,7 @@ "JobExecution (type)", "JobHandler (type)", "JobRetryPolicy (interface)", + "JobRunOutcome (interface)", "JobSchedule (interface)", "JobScheduleOptions (interface)", "KNOWLEDGE_SERVICE (const)", From fab0403f6a4935c0aee9b51e6731eb439ab21f27 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 11:20:58 +0000 Subject: [PATCH 3/3] test(service-job): import the job contracts from the spec subpath (#6617) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prime Directive #4 — @objectstack/spec subpaths, matching the sibling adapters; JobHandler/JobRunOutcome are not on the package root. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PiRUoQkTSBBmpyXBY3cVn2 --- .../services/service-job/src/run-with-policy.outcome.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/services/service-job/src/run-with-policy.outcome.test.ts b/packages/services/service-job/src/run-with-policy.outcome.test.ts index 45d9a9f067..f2029ea237 100644 --- a/packages/services/service-job/src/run-with-policy.outcome.test.ts +++ b/packages/services/service-job/src/run-with-policy.outcome.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import type { JobHandler, JobRunOutcome } from '@objectstack/spec'; +import type { JobHandler, JobRunOutcome } from '@objectstack/spec/contracts'; import { runWithPolicy } from './run-with-policy.js'; /**