fix(services): two published .d.ts JSDoc comments stop describing behaviour their code does not have

.changeset/service-jsdoc-declared-equals-actual.md

@@ -0,0 +1,52 @@
+---
+"@objectstack/service-cache": patch
+"@objectstack/service-job": patch
+---
+
+fix(services): two published `.d.ts` JSDoc comments stop describing behaviour their code does not have — `MemoryCacheAdapter` eviction is FIFO, not LRU, and `recordRuns` is an on/off switch, not a retention cap (#9611)
+
+Both comments are emitted by `tsup` into each package's built `index.d.ts`, so they
+are **the editor tooltip an npm consumer sees** — the same "published documentation
+asserting behaviour the runtime does not have" class as #9517 and #9532, in a third
+channel that no gate reads. No runtime behaviour changes in either package; this is a
+patch because the corrected text only reaches consumers through a release.
+
+**1. `MemoryCacheAdapter` — "LRU-style eviction" was never LRU.**
+
+The class comment advertised `TTL-based expiry and LRU-style eviction`. The eviction
+path takes `this.store.keys().next().value` — the first key in `Map` insertion order —
+and `get()` returns `entry.value` without ever deleting and re-setting the key, so a
+read does not move an entry back. Nor does an overwrite: `Map.set` on a key already
+present keeps its original insertion slot. Eviction has therefore always been
+**oldest-inserted (FIFO)**, which is a materially different hit-rate profile from the
+one the tooltip promised anyone sizing a cache.
+
+The comment was corrected rather than the code, deliberately: `maxSize` defaults to `0`
+(unlimited), so the eviction path is off by default and nothing shipped is getting FIFO
+where it expected LRU, and there is no measured pull for LRU. Minting a real behaviour
+change to make a stale sentence true inverts the fix — the defect is that the
+documentation lies, not that the cache is wrong. (A real LRU already exists in the repo,
+`packages/metadata/src/utils/lru-cache.ts`, for the callers that need one.)
+
+Four tests now pin the corrected sentence so it stops being an unenforced claim. Each is
+written as a **discriminator against LRU**: it reads (or overwrites) the oldest entry
+before overflowing the cache and then asserts that entry was evicted anyway — a hot key
+dies on age, an untouched newer key survives. The pre-existing eviction tests could not
+tell the two policies apart, which is how the wrong comment sat green.
+
+**2. `DbJobAdapterOptions.recordRuns` — the comment described a different field.**
+
+`/** Soft cap on sys_job_run rows recorded per job (defaults to none — handled by
+retention jobs) */` made three claims and the code contradicts all three: the field is a
+`boolean`, not a count; it defaults to `true`, not "none" (`args.options?.recordRuns ??
+true`); and it gates whether a `sys_job_run` row is written at all, rather than being
+trimmed later by retention. The sentence reads as if it belongs to the numeric
+`JobRunRetention` knob that ADR-0057 retired — a copy-paste that outlived its source.
+
+The consequence the new wording keeps in sight: **a reader who sets `recordRuns: false`
+expecting "no cap" gets run history switched off.** The replacement states the real
+meaning (one row per attempt, inserted at start and updated on settle, default `true`)
+and both things that are *not* affected by the flag — the `sys_job` row's own
+`last_status` / `run_count` / `failure_count` counters, which `bumpJob` updates
+regardless, and `replay()`, which writes its synthetic `trigger: 'replay'` row without
+consulting the flag at all.

packages/services/service-cache/src/memory-cache-adapter.test.ts

@@ -105,3 +105,87 @@ describe('MemoryCacheAdapter', () => {
  expect(await cache.has('expiring')).toBe(false);
  });
 });
+
+// ─── eviction is insertion-order (FIFO), and the class JSDoc says so ─────────
+//
+// The class comment used to advertise "LRU-style eviction" while `get()` has
+// never re-inserted the key it reads, so eviction has always been oldest-
+// INSERTED, not least-recently-USED. The comment was corrected rather than the
+// code; these tests are what stops that corrected sentence from being another
+// unenforced claim.
+//
+// Every case below is written to be a DISCRIMINATOR: each one passes under the
+// shipped FIFO store and fails under an LRU store, because each performs a
+// read (or an overwrite) on the oldest entry and then asserts that the entry
+// was evicted anyway. A test that merely fills the cache past `maxSize` without
+// touching anything first cannot tell the two policies apart, which is how the
+// pre-existing eviction tests above sat green over a comment they contradicted.
+describe('MemoryCacheAdapter — insertion-order (FIFO) eviction', () => {
+ it('a read does NOT refresh eviction order — the read-hot oldest entry is still evicted', async () => {
+ const cache = new MemoryCacheAdapter({ maxSize: 2 });
+ await cache.set('a', 1);
+ await cache.set('b', 2);
+
+ // Read 'a' repeatedly: under LRU this promotes it to most-recently-used and
+ // makes 'b' the eviction candidate. Under FIFO it changes nothing at all.
+ expect(await cache.get('a')).toBe(1);
+ expect(await cache.get('a')).toBe(1);
+ expect(await cache.has('a')).toBe(true);
+
+ await cache.set('c', 3);
+
+ expect(await cache.has('a')).toBe(false); // oldest-inserted, evicted despite being hot
+ expect(await cache.get('b')).toBe(2); // untouched, survives — the inverse of LRU
+ expect(await cache.get('c')).toBe(3);
+ });
+
+ it('an overwrite does NOT refresh eviction order either — Map.set keeps the original slot', async () => {
+ const cache = new MemoryCacheAdapter({ maxSize: 2 });
+ await cache.set('a', 1);
+ await cache.set('b', 2);
+
+ // Overwriting an existing key never evicts (it is not a new key) and never
+ // moves the entry: `Map.set` on a present key keeps its insertion position.
+ await cache.set('a', 10);
+ expect(await cache.get('a')).toBe(10);
+
+ await cache.set('c', 3);
+
+ expect(await cache.has('a')).toBe(false); // freshly written, still the oldest slot
+ expect(await cache.get('b')).toBe(2);
+ expect(await cache.get('c')).toBe(3);
+ });
+
+ it('evicts strictly in insertion order across a longer run, reads notwithstanding', async () => {
+ const cache = new MemoryCacheAdapter({ maxSize: 3 });
+ await cache.set('a', 1);
+ await cache.set('b', 2);
+ await cache.set('c', 3);
+
+ // Make 'a' the hottest key in the cache and 'c' the coldest.
+ await cache.get('a');
+ await cache.get('a');
+ await cache.get('a');
+
+ await cache.set('d', 4); // evicts 'a' (first in), not 'c' (least recently used)
+ expect(await cache.has('a')).toBe(false);
+ expect(await cache.has('c')).toBe(true);
+
+ await cache.set('e', 5); // evicts 'b' — the queue keeps advancing by age
+ expect(await cache.has('b')).toBe(false);
+
+ expect(await cache.get('c')).toBe(3);
+ expect(await cache.get('d')).toBe(4);
+ expect(await cache.get('e')).toBe(5);
+ expect((await cache.stats()).keyCount).toBe(3);
+ });
+
+ it('leaves the eviction path off entirely at the default maxSize of 0 (unlimited)', async () => {
+ const cache = new MemoryCacheAdapter(); // maxSize defaults to 0
+ for (let i = 0; i < 50; i++) await cache.set(`k${i}`, i);
+
+ expect(await cache.get('k0')).toBe(0); // the very first key is still there
+ expect(await cache.get('k49')).toBe(49);
+ expect((await cache.stats()).keyCount).toBe(50);
+ });
+});

packages/services/service-cache/src/memory-cache-adapter.ts

@@ -30,7 +30,13 @@ export interface MemoryCacheAdapterOptions {
 /**
  * In-memory cache adapter implementing ICacheService.
  *
- * Uses a Map-backed store with TTL-based expiry and LRU-style eviction.
+ * Uses a Map-backed store with TTL-based expiry and **insertion-order (FIFO)
+ * eviction**: once `maxSize` is reached, a new key evicts the oldest-inserted
+ * entry. This is deliberately **not** LRU — neither reading an entry nor
+ * overwriting its value moves it back in the queue, so a hot key is evicted on
+ * age like any other (pinned by the FIFO tests in `memory-cache-adapter.test.ts`).
+ * `maxSize` defaults to `0` (unlimited), which leaves the eviction path off.
+ *
  * Suitable for single-process environments, development, and testing.
  */
 export class MemoryCacheAdapter implements ICacheService {

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

@@ -31,7 +31,19 @@ export interface JobLoggerLike {
 export interface DbJobAdapterOptions {
  /** Maximum executions kept in memory per job (default 100) */
  maxExecutions?: number;
- /** Soft cap on sys_job_run rows recorded per job (defaults to none — handled by retention jobs) */
+ /**
+ * Record each scheduled or triggered execution as a `sys_job_run` row —
+ * inserted at the start of every attempt and updated to its terminal status
+ * when that attempt settles. Default **`true`**.
+ *
+ * This is an on/off switch for run history, NOT a retention cap: setting it to
+ * `false` means no per-attempt rows are written at all, so `sys_job_run` holds
+ * nothing for these executions and `listExecutionsByStatus` has nothing to
+ * read. Two things are unaffected either way — the `sys_job` row's own
+ * `last_status` / `run_count` / `failure_count` counters, and
+ * {@link DbJobAdapter.replay}, which writes its synthetic `trigger: 'replay'`
+ * row regardless of this flag.
+ */
  recordRuns?: boolean;
 }