fix(services): the DbJobAdapter class JSDoc stops promising a sys_job_run row recordRuns: false never writes (#9631)

The class-level "persisted side effects" list said `every execution writes a
sys_job_run row`. `wrap()` gates that insert on `recordRuns`:

  current = { id: this.recordRuns ? await this.startRun(...) : undefined, ... };

and `settle()` only updates a row when one exists, so with `recordRuns: false`
no execution writes one. `tsup` emits this comment into the package's built
`index.d.ts`, which is the class-level editor tooltip an npm consumer reads —
the same "published documentation asserting behaviour the runtime does not
have" class as #9611, in the same file and the same emitted declaration.

The corrected bullet defers to `DbJobAdapterOptions.recordRuns` with `{@link}`
instead of restating the rule. Two true-but-divergent descriptions of one flag
is the next version of this defect, and the field's own JSDoc is where the
meaning belongs; the class list only says where it shows up. It also names the
one row the flag does not govern — `replay()`'s synthetic `trigger: 'replay'`
row, written either way, measured rather than assumed. Without that clause the
corrected sentence would be false today for exactly the reason #9633 records.

The fourth bullet was already true and gains the matching negative:
`bumpJob` is called from `settle` OUTSIDE the `if (run.id)` guard, so the
`sys_job` counters are updated whether or not a run row exists. Left implicit,
a reader carries the `recordRuns` caveat down onto it.

No behaviour change.

Five cases pin the flag, reusing this file's existing engine double rather
than minting a new one. Nothing in the package referenced `recordRuns` in any
direction before, so the wording corrected here and the field wording
corrected on #9611 were both accurate but unenforced. The discriminator
asserts the execution REALLY RAN — handler fired, `sys_job.run_count` bumped —
and that no row was written anyway; without that half, "0 rows" would also
pass for a job that never fired.

The replay case pins today's behaviour because that is what the JSDoc now
states, not as an endorsement: #9633 holds the open disposition, and if it
lands the case and the bullet it mirrors change together.

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-project-manager

.changeset/service-job-class-jsdoc-recordruns.md

@@ -0,0 +1,26 @@
+---
+"@objectstack/service-job": patch
+---
+
+fix(services): the `DbJobAdapter` class JSDoc stops promising a `sys_job_run` row that `recordRuns: false` never writes (#9631)
+
+`tsup` emits this comment into `packages/services/service-job/dist/index.d.ts`, so it is
+the class-level editor tooltip an npm consumer of `@objectstack/service-job` reads. Its
+third "persisted side effects" bullet said **every execution writes a `sys_job_run` row**;
+`wrap()` gates that insert on `recordRuns`, which defaults to `true` but writes nothing at
+all when set to `false`. The same emitted `index.d.ts` states the truthful field-level rule
+for `recordRuns` sixty lines above, so the published declaration disagreed with itself
+about one flag — a reader hovering either one got a different answer.
+
+No runtime behaviour changes. This is a patch because the entire deliverable is text inside
+a published package's `.d.ts`: with no version bump the corrected tooltip never reaches npm
+and the fix is unmet in the only channel it is about.
+
+The corrected bullet defers to `DbJobAdapterOptions.recordRuns` via `{@link}` rather than
+restating the rule, so the two cannot drift apart again, and it names the one row the flag
+does not govern — `replay()`'s synthetic `trigger: 'replay'` row, written either way. The
+fourth bullet gains the matching negative: the `sys_job` counters are bumped
+unconditionally, `recordRuns` gating only the per-attempt rows.
+
+Five cases now pin the flag in both directions. Nothing in this package referenced
+`recordRuns` before, so both corrected sentences were accurate but unenforced.

packages/services/service-job/src/db-job-adapter.test.ts

@@ -234,3 +234,89 @@ describe('DbJobAdapter — kernel rebuild (#8362)', () => {
     await rebuilt.db.destroy();
   });
 });
+
+// ─── #9631 — `recordRuns`, the flag two published `.d.ts` comments describe ──
+//
+// Until this block, NOTHING in this package referenced `recordRuns` in either
+// direction: not that `true` writes a row, not that `false` writes none, not
+// that the default is `true`. The class JSDoc and the field JSDoc both describe
+// the flag to every npm consumer through the emitted `index.d.ts`, and both
+// were free to drift from the code — which is exactly what #9611 and #9631
+// each found one of. These cases exist so the sentences stop being unenforced.
+//
+// The discriminator is case 2: it asserts the execution REALLY RAN (the handler
+// fired and `sys_job.run_count` bumped) and that no row was written anyway.
+// Without that half, "0 rows" would also pass for a job that never fired, which
+// is the way a test like this goes quietly blind.
+describe('DbJobAdapter — recordRuns (#9631)', () => {
+  const adapters: DbJobAdapter[] = [];
+  const build = (options?: { recordRuns?: boolean }) => {
+    const engine = makeFakeEngine();
+    const adapter = new DbJobAdapter({ engine, options });
+    adapters.push(adapter);
+    return { engine, adapter };
+  };
+  afterEach(async () => {
+    while (adapters.length) await adapters.pop()!.destroy();
+  });
+
+  it('defaults to true: a triggered execution writes one sys_job_run row', async () => {
+    const { engine, adapter } = build(); // no options at all — the documented default
+    await adapter.schedule('d', { type: 'cron', expression: '* * * * *' }, async () => {});
+    await adapter.trigger('d');
+    expect(engine.tables.get('sys_job_run') ?? []).toHaveLength(1);
+  });
+
+  it('recordRuns: false writes NO sys_job_run row, though the execution really ran', async () => {
+    const { engine, adapter } = build({ recordRuns: false });
+    let ran = 0;
+    await adapter.schedule('off', { type: 'cron', expression: '* * * * *' }, async () => { ran++; });
+    await adapter.trigger('off');
+
+    expect(ran, 'the handler must actually have run — otherwise "no rows" proves nothing').toBe(1);
+    expect(engine.tables.get('sys_job_run') ?? []).toHaveLength(0);
+  });
+
+  it('recordRuns: false does NOT gate the sys_job counters — only the per-attempt rows', async () => {
+    // The class JSDoc's fourth bullet: `bumpJob` is called from `settle`
+    // outside the `if (run.id)` guard, so the job row is updated either way.
+    const { engine, adapter } = build({ recordRuns: false });
+    await adapter.schedule('c', { type: 'cron', expression: '* * * * *' }, async () => {
+      throw new Error('boom');
+    });
+    await adapter.trigger('c');
+
+    const job = (engine.tables.get('sys_job') ?? [])[0];
+    expect(job.last_status).toBe('failed');
+    expect(job.run_count).toBe(1);
+    expect(job.failure_count).toBe(1);
+    expect(engine.tables.get('sys_job_run') ?? []).toHaveLength(0);
+  });
+
+  it('recordRuns: true is the same as the default', async () => {
+    const { engine, adapter } = build({ recordRuns: true });
+    await adapter.schedule('on', { type: 'cron', expression: '* * * * *' }, async () => {});
+    await adapter.trigger('on');
+    const runs = engine.tables.get('sys_job_run') ?? [];
+    expect(runs).toHaveLength(1);
+    expect(runs[0]).toMatchObject({ job_name: 'on', trigger: 'schedule', status: 'success' });
+  });
+
+  it("replay() writes its synthetic row even when recordRuns is false — the exception the JSDoc names", async () => {
+    // This pins TODAY'S behaviour, which is what the class JSDoc now states;
+    // it is not an endorsement of it. #9633 holds the open disposition on
+    // whether `replay()` should honour the flag. If that lands, this case and
+    // the class-JSDoc bullet it mirrors change together — which is the whole
+    // point of writing it down here: the sentence cannot go stale in silence
+    // again.
+    const { engine, adapter } = build({ recordRuns: false });
+    await adapter.schedule('rp', { type: 'cron', expression: '* * * * *' }, async () => {});
+    await adapter.replay('rp');
+
+    const runs = engine.tables.get('sys_job_run') ?? [];
+    // Exactly one: the synthetic replay row. The wrapped execution the replay
+    // drives underneath it is gated by the flag and writes nothing.
+    expect(runs.map((r) => r.trigger)).toEqual(['replay']);
+    expect(runs[0].status).toBe('success');
+  });
+});

packages/services/service-job/src/db-job-adapter.ts

@@ -69,8 +69,14 @@ function uid(prefix: string): string {
  * Persisted side effects:
  *   - `schedule(name, …)` upserts a `sys_job` row (active=true)
  *   - `cancel(name)` marks the row inactive
- *   - every execution writes a `sys_job_run` row
- *   - every execution updates `sys_job.last_run_at / last_status / run_count / failure_count`
+ *   - every execution writes a `sys_job_run` row per attempt — unless
+ *     {@link DbJobAdapterOptions.recordRuns} is `false`, the on/off switch for
+ *     run history, which writes none of them. The one row it does not govern is
+ *     {@link DbJobAdapter.replay}'s synthetic `trigger: 'replay'` row, written
+ *     either way.
+ *   - every execution updates `sys_job.last_run_at / last_status / run_count /
+ *     failure_count` — unconditionally: `recordRuns` gates the per-attempt rows
+ *     above, never these counters.
  *
  * The persistence is best-effort: a DB failure is logged but does not
  * break job execution. This keeps a healthy job system resilient to