Skip to content

Commit 339d7d3

Browse files
committed
fix(runtime): return the declarative job handler's JobRunOutcome (#14256)
AppPlugin's declarative-job wrapper awaited the bundle handler inside a block-bodied arrow and returned nothing, so it was a `Promise<void>` whatever the handler resolved. #6617's third outcome was therefore unreachable from `defineJob`: a job that ran to completion while its work did not happen was recorded as `success` with `reason` dropped, while all three shipped adapters map a resolved `{ outcome: 'degraded' }` onto a run status distinct from it. Also drops the read-refusal capture from the new pin's rig: measured on the red run, no path in that file provokes the tenancy probe, and a capture nothing provokes asserts a mute. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
1 parent ba6074c commit 339d7d3

3 files changed

Lines changed: 78 additions & 45 deletions

File tree

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
'@objectstack/runtime': patch
3+
---
4+
5+
fix(runtime): a declarative job's `JobRunOutcome` reaches the adapter that records it (#14256)
6+
7+
`AppPlugin`'s declarative-job registration block handed `IJobService.schedule` a
8+
block-bodied arrow that *awaited* the bundle handler and returned nothing, so
9+
the wrapper was a `Promise<void>` whatever the handler resolved. Measured
10+
through an `IJobService` typed only at the contract:
11+
12+
```
13+
HANDLER RESOLVED: {"outcome":"degraded","reason":"STORE_UNAVAILABLE"}
14+
WRAPPER RESOLVED: undefined
15+
```
16+
17+
#6617's third outcome was therefore unreachable from `defineJob`. All three
18+
shipped adapters (`cron-job-adapter`, `interval-job-adapter`, `db-job-adapter`)
19+
map a resolved `{ outcome: 'degraded', reason }` onto a run status distinct from
20+
`success` — they simply never got the value on this path, so a declarative job
21+
that ran to completion while its work did not happen (store unavailable, zero
22+
rows matched) was recorded as `success` with `reason` dropped. The three-outcome
23+
table in `content/docs/automation/jobs.mdx` — the page whose whole subject is
24+
the declarative door — was false on exactly that door. The imperative route (a
25+
handler registered straight on `IJobService.schedule`) was unaffected
26+
throughout; the wrapper is the whole defect.
27+
28+
The repair is to return the handler's resolved value. Patch rather than minor:
29+
no export, type, schema or authoring surface changes, and `JobHandler` has
30+
declared `Promise<void | JobRunOutcome>` since #6617 — this makes the declared
31+
contract true on a route where it was not. Additive in the ruling's sense: a
32+
handler resolving `undefined` (every handler written before #6617) still
33+
resolves `undefined` through the wrapper and still takes the `success` branch.
34+
The one visible change for an existing app is the correction itself — a
35+
declarative handler that *was already* resolving `{ outcome: 'degraded' }` now
36+
lands `sys_job_run.status: 'degraded'` (reason in `error`, `failure_count` flat,
37+
still never retried) where it previously landed `success`.
38+
39+
Pinned by `packages/runtime/src/app-plugin.job-degraded-outcome.test.ts`, which
40+
drives the real `DbJobAdapter` over a real ObjectQL engine carrying the real
41+
`sys_job` / `sys_job_run` declarations and asserts the persisted cell, not the
42+
wrapper's return value.

packages/runtime/src/app-plugin.job-degraded-outcome.test.ts

Lines changed: 16 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -58,23 +58,10 @@ import { DbJobAdapter } from '@objectstack/service-job';
5858
import { SysJob, SysJobRun } from '@objectstack/platform-objects/audit';
5959
import { AppPlugin } from './app-plugin.js';
6060
import type { JobHandlerContext } from './job-handler-context.js';
61-
import {
62-
captureExpectedReadRefusals,
63-
type ExpectedReadRefusalCapture,
64-
} from './expected-read-refusal-noise.js';
6561

6662
/** The reason a #5529-shaped handler reports when its store is unreachable. */
6763
const REASON = 'STORE_UNAVAILABLE';
6864

69-
/**
70-
* [#10629] This fixture provisions the two job tables and nothing else, so the
71-
* engine's own single-tenant probe (`ObjectQL.probeInstallOrganizations`) reads
72-
* a `sys_organization` that was never created. The probe is fail-soft by
73-
* construction, but the driver and the engine each log the fault on the way
74-
* out. Withheld and ASSERTED rather than muted.
75-
*/
76-
const ABSENT_TENANCY_TABLE = 'sys_organization';
77-
7865
interface Harness {
7966
engine: ObjectQL;
8067
adapter: DbJobAdapter;
@@ -90,61 +77,46 @@ const live: Array<{
9077
engine?: ObjectQL;
9178
adapter?: DbJobAdapter;
9279
driver?: SqlDriver;
93-
noise?: ExpectedReadRefusalCapture;
94-
/** The channels this test's path MUST have provoked — see `harness()`. */
95-
requiredChannels?: readonly string[];
9680
}> = [];
9781

9882
afterEach(async () => {
9983
for (const entry of live.splice(0)) {
10084
try { await entry.adapter?.destroy(); } catch { /* noop */ }
10185
try { await entry.engine?.destroy(); } catch { /* noop */ }
10286
try { await entry.driver?.disconnect(); } catch { /* noop */ }
103-
// A capture nobody asserts is a mute. The probe is memoised behind the
104-
// FIRST data operation, so only the paths that actually touch the store
105-
// provoke it — `silentChannels(required)` is the API's own answer to a
106-
// table read on some of a file's paths and not others. The withholding
107-
// is unconditional either way; only the must-have-fired set narrows.
108-
if (entry.noise) {
109-
expect(entry.noise.silentChannels(entry.requiredChannels ?? [ABSENT_TENANCY_TABLE])).toEqual([]);
110-
}
11187
}
11288
});
11389

114-
/** A real engine over the migrated test backend, carrying the REAL `sys_job*`. */
115-
async function bootEngine(): Promise<{ engine: ObjectQL; driver: SqlDriver; noise: ExpectedReadRefusalCapture }> {
90+
/**
91+
* A real engine over the migrated test backend, carrying the REAL `sys_job*`.
92+
*
93+
* ⚠️ [#10629] No expected-read-refusal capture here, deliberately and by
94+
* MEASUREMENT: every read this file performs carries `isSystem`, so the
95+
* engine's single-tenant probe over the unprovisioned `sys_organization` never
96+
* fires and neither refusal channel emits a frame (checked on the red run: zero
97+
* `refused a read on` and zero `Find operation failed` lines for the whole
98+
* file). Installing a capture that nothing provokes would assert a mute.
99+
*/
100+
async function bootEngine(): Promise<{ engine: ObjectQL; driver: SqlDriver }> {
116101
const driver = new SqlDriver({
117102
client: 'better-sqlite3',
118103
connection: { filename: ':memory:' },
119104
useNullAsDefault: true,
120105
});
121-
const noise = captureExpectedReadRefusals([ABSENT_TENANCY_TABLE]);
122-
noise.captureDriver(driver);
123106
await driver.initObjects([SysJob as never, SysJobRun as never]);
124107
const engine = new ObjectQL();
125-
noise.captureEngine(engine);
126108
engine.registerDriver(driver as never, true);
127109
await engine.init();
128110
engine.registry.registerObject(SysJob as never);
129111
engine.registry.registerObject(SysJobRun as never);
130-
return { engine, driver, noise };
112+
return { engine, driver };
131113
}
132114

133-
/**
134-
* @param opts.touchesStore whether this test's path performs a data operation.
135-
* `true` (the default) requires the tenancy probe to have fired and been
136-
* withheld; the one case that swaps `DbJobAdapter` out for a recording
137-
* `IJobService` never reads or writes and passes `false`, which keeps the
138-
* withholding and drops only the must-have-fired requirement.
139-
*/
140-
async function harness(opts: { touchesStore?: boolean } = {}): Promise<Harness> {
141-
const { engine, driver, noise } = await bootEngine();
115+
async function harness(): Promise<Harness> {
116+
const { engine, driver } = await bootEngine();
142117
// The adapter that RECORDS the outcome — the coordinate the card names.
143118
const adapter = new DbJobAdapter({ engine: engine as never });
144-
live.push({
145-
engine, adapter, driver, noise,
146-
requiredChannels: opts.touchesStore === false ? [] : [ABSENT_TENANCY_TABLE],
147-
});
119+
live.push({ engine, adapter, driver });
148120

149121
const readyHooks: Array<() => Promise<void>> = [];
150122
const ctx = {
@@ -306,7 +278,7 @@ describe('#14256 — the controls that keep the assertion honest', () => {
306278
// The reporter's probe, kept as corroboration and NOT as the deliverable:
307279
// it re-states the repair, while the cases above pin its consequence.
308280
// Typed only at the contract, exactly as a third-party `IJobService` is.
309-
const h = await harness({ touchesStore: false });
281+
const h = await harness();
310282
const scheduled: Array<{ name: string; handler: JobHandler }> = [];
311283
const recording: IJobService = {
312284
async schedule(name: string, _schedule: JobSchedule, handler: JobHandler) {

packages/runtime/src/app-plugin.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1033,7 +1033,26 @@ export class AppPlugin implements Plugin {
10331033
ql,
10341034
logger: ctx.logger,
10351035
};
1036-
await handler(jobContext);
1036+
// #14256: RETURN the handler's resolved
1037+
// value. `JobHandler` is
1038+
// `(context) => Promise<void | JobRunOutcome>`
1039+
// and all three shipped adapters map a
1040+
// resolved `{ outcome: 'degraded', reason }`
1041+
// onto a `sys_job_run.status` distinct from
1042+
// `success` (#6617/#5548). A block-bodied
1043+
// arrow that only awaited made this wrapper
1044+
// a `Promise<void>`, so the third outcome
1045+
// was unreachable from `defineJob`: a job
1046+
// that ran to completion while its work did
1047+
// not happen was recorded as `success` with
1048+
// `reason` dropped, and the three-outcome
1049+
// table in `content/docs/automation/jobs.mdx`
1050+
// was false on the declarative door.
1051+
// A handler that resolves `undefined` — every
1052+
// handler written before #6617 — still returns
1053+
// `undefined` here, which is the `success`
1054+
// branch exactly as before.
1055+
return await handler(jobContext);
10371056
},
10381057
// #3494: thread the authored retryPolicy/timeout to the adapter
10391058
(job.retryPolicy || job.timeout)

0 commit comments

Comments
 (0)