Skip to content

The DbJobAdapter class JSDoc still says "every execution writes a sys_job_run row" — the third published .d.ts comment in the same file that recordRuns: false contradicts #9631

Description

@os-project-manager

Found while fixing #9611 (PR linked below). ⛔ Filed rather than absorbed: #9611's dispatch scope fence is explicit — "two comments and the pinning test; do not improve adjacent JSDoc you happen to notice, file it".

Same defect class as #9611, in the same file, emitted into the same published index.d.ts.

The claim and the code

packages/services/service-job/src/db-job-adapter.ts, the DbJobAdapter class JSDoc:

 * 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`

The third bullet is false whenever recordRuns is false. db-job-adapter.ts:305 (pre-#9611 numbering) gates the insert:

current = { id: this.recordRuns ? await this.startRun(name, defaultTrigger, attempt) : undefined, settled: false };

and settle() only updates a row when one exists (if (run.id) await this.finishRun(...)). With recordRuns: false, no execution writes a sys_job_run row.

The fourth bullet is fine — bumpJob is called unconditionally.

Why it matters in this channel

tsup emits this comment into packages/services/service-job/dist/index.d.ts (around line 92 after #9611), so it is the class-level editor tooltip an npm consumer of @objectstack/service-job reads. After #9611 lands, the same emitted file states the truthful field-level rule for recordRuns about sixty lines above a class-level bullet asserting the opposite. That is strictly better than before — one of the two was corrected — but the file now visibly disagrees with itself, which is the cheapest possible signal that this bullet is the leftover.

Suggested disposition

Comment-only, no behaviour change: qualify the bullet, e.g. "every execution writes a sys_job_run row unless recordRuns is false". Same reasoning as #9611 — the defect is that the documentation lies, not that the adapter is wrong.

Related coverage gap, recorded here rather than filed separately

Nothing in packages/services/service-job/src/*.test.ts references recordRuns at all: no test pins that true writes a row, that false writes none, or that the default is true. #9611 pinned the MemoryCacheAdapter FIFO comment with discriminating tests but its acceptance named only that one; the corrected recordRuns wording is therefore still an unenforced claim. Whoever takes this card is the natural place to add the pin, since the two touch the same sentences.

Refs: #9611 · #9532 · #9517


Generated by Claude Code

Activity

  1. os-project-manager commented on Aug 18, 2026

    @os-project-manager
    CollaboratorAuthor

    Claiming this card.

    • session: session_01Y26DJEHSBhhAQ6wwfsHNza (dev seat dispatched by the PM loop)
    • branch: claude/issue-9631-dbjobadapter-class-jsdoc

    Branched from origin/main (40162f1e2) — PR #9635 (for #9611) has not merged yet, so the corrected field-level wording is not on main at branch time. I read that wording from the PR's head commit 9b07a5e34 and will match it rather than invent a second description of the same flag; the two diffs touch the same file, so the ordering is noted in the PR body.


    Generated by Claude Code

  2. os-project-manager commented on Aug 18, 2026

    @os-project-manager
    CollaboratorAuthor
    {
      "issue": 9631,
      "status": "done",
      "branch": "claude/issue-9631-dbjobadapter-class-jsdoc",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/9646",
      "premise_still_valid": true,
      "summary": "The card's premise held on re-measurement: the class-level 'every execution writes a sys_job_run row' bullet is false whenever recordRuns is false, since wrap() gates startRun on the flag and settle() only calls finishRun under `if (run.id)`. Corrected the bullet to defer to DbJobAdapterOptions.recordRuns via {@link} rather than restate the rule — the card's sharpest constraint was not to mint a second divergent description of one flag, and deferring makes divergence structurally impossible rather than merely avoided this round. The corrected sentence names the replay exception, because the obvious wording ('no row when false') would be wrong today for exactly the reason #9633 records. The fourth bullet was already true and gained its matching negative (bumpJob sits outside the `if (run.id)` guard, so the sys_job counters are bumped either way) — named in the PR body with its evidence, since left implicit a reader carries the recordRuns caveat down onto it. Added five pin tests: the package referenced recordRuns in NO direction before, so both this wording and the field wording #9611 corrected were accurate but unenforced. No behaviour change. ONE DISPATCH PREMISE MOVED: PR #9635 has NOT merged — it is still an open draft and main (07e630e58, re-checked after my push) still carries the old 'Soft cap' field comment. I branched from main as instructed, did not branch from its branch, and matched its wording read from head commit 9b07a5e34. The two diffs sit ~30 lines apart in db-job-adapter.ts and share no other file, so either merge order is clean.",
      "tests": "Union run at dc61fbaee, the final commit; working tree clean; gates derived via `node scripts/pm/dispatch-gates.mjs` from `git merge-base` (40162f1e2) per #9320. `pnpm --filter @objectstack/service-job test` -> 'Test Files 8 passed (8) / Tests 80 passed (80)' (75 pre-existing + 5 new). `pnpm --filter @objectstack/service-job typecheck` -> tsc --noEmit clean (script name echoed in output, so not the zero-match silent-green trap). REVERSE VERIFICATION, direction predicted before running: ablation applied to the COMMITTED state (recordRuns gate removed from wrap(), startRun unconditional); predicted red in exactly cases 2/3/5 with 1/4 and all 75 pre-existing green; observed exactly that -> 'Tests 3 failed | 77 passed (80)', 'expected [ { ...(10) } ] to have a length of +0 but got 1' and \"expected [ 'replay', 'schedule' ] to deeply equal [ 'replay' ]\". The load-bearing half is that all 75 pre-existing tests stayed GREEN under the ablation — the flag could stop being honoured entirely and this suite would not notice, which is how the comment drifted. These are source-level vitest cases, not a dogfood/dist run, so the ablation needs no rebuild to take effect (stated because the contract requires an ablation to declare its rebuild status either way). Restored via `git checkout claude/issue-9631-... -- <path>`; grep -c ABLATION-9631 = 0. EMITTED-ARTIFACT ACCEPTANCE (the channel that is the card): rebuilt the package and checked BOTH published declarations — in dist/index.d.ts AND dist/index.d.cts, the unqualified bullet regex '^ \\* +- every execution writes a `sys_job_run` row$' counts 0 and the qualified replacement counts 1. Gates all green: check:nul-bytes (6177 files), check:changeset-gate-self-tests, check:objectui-changeset, check:test-source-alias (72 pkgs), check:type-source-resolution (76 pkgs), check-adr-0087-registration, check-changeset-no-major, check-empty-changeset (1 declaring changeset), docs-audit/check-affected-docs (242 cases), check:query-options-erasure (ratchet holds), check:engine-double-contract (319 pinned, NO new double — the pins reuse the file's existing engine double rather than minting one), check:where-matcher (255 matchers), check:type-check-coverage (64/77), check:type-check-debt --re-measure (33 entries re-measured in 230.3s, none above its number). The last one needed the built closure, so `turbo run build --filter=./packages/* --filter=./packages/*/*` ran first: 70/70 successful — a refusal would have meant NOT MEASURED, not a pass. Every heavy step ran under flock /tmp/os-heavy-verify.lock.",
      "open_questions": [],
      "out_of_scope_findings": [
        "NOT filed as a new issue — recorded as a comment on the existing #9633 (issuecomment-5329452743), stated plainly because the report template presumes a new number: packages/services/service-job/README.md:157 describes the same flag as '`false` keeps the in-memory history only', which the replay row falsifies exactly as it does the JSDoc. That README is npm-rendered, so it is the same published channel. It is not standalone-fileable because its correctness is decided entirely by #9633's disposition — option 1 (replay honours the flag) makes the line true with no edit at all, option 2 needs the carve-out. Attaching it to the card that decides it beats minting an orphan. Not fixed in place: the bounded in-place exemption fails its fourth condition — a README edit pulls in the published-README gate family this diff does not otherwise touch.",
        "No other finding. #9633 (replay writes its synthetic row regardless of recordRuns) was already filed by the #9611 run and is deliberately NOT addressed here; this PR only stops the JSDoc from contradicting it. Flagging one coupling for triage: my fifth test case pins the current replay-writes-anyway behaviour BECAUSE that is what the corrected JSDoc now states, not as an endorsement. If #9633 lands disposition 1, that case and the class-JSDoc clause it mirrors must change together — the case carries a comment saying so and pointing at #9633, which is the coupling that stops this sentence drifting a fourth time."
      ]
    }

    Generated by Claude Code

  3. os-project-manager commented on Aug 18, 2026

    @os-project-manager
    CollaboratorAuthor

    Landed and verified on main — PR #9646 merged as 73010f180 at 14:59:18Z.

    Probed the merged tree, control + anti-control:

    probe expected observed
    grep -c 'row per attempt' in db-job-adapter.ts 1 — the qualified replacement is there 1 ✅
    grep -cE '^\s*\*\s+- every execution writes a \sys_job_run` row$'` 0 — the unqualified promise is gone 0 ✅

    The anti-control is the half that matters: a qualified bullet appearing next to a surviving unqualified one would read as green while still shipping the false sentence to npm.

    ⭐ The part of this card that outlived it

    Two things it produced are worth carrying forward:

    The blindness measurement. All 75 pre-existing tests stayed green under the ablation that removed the recordRuns gate entirely — the flag could stop being honoured and this package's suite would not have noticed. That is the account of why the comment drifted, and it has now been reproduced independently on a second defect in the same file (#9633's ablation A, same 75, same result).

    The fifth test case did its job. It pinned replay() writing its row under recordRuns: false — behaviour it knew #9633 was about to change — with a comment pointing at that card. #9633's fix contradicts it, which turns the coupling into a red assertion instead of a note somebody has to remember. That is why the ordering between the two PRs was a scheduling decision this morning rather than an incident this afternoon.

    Now that this has landed, PR #9675 (#9633) is unblocked and its dev has been sent back to rebase — the corrected class-JSDoc clause naming the replay exception comes out there, in the same diff that makes the exception cease to exist.


    Generated by Claude Code

  4. added a commit that references this issue on Aug 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions