Skip to content

test(service-job): make the timeout suite's cron unable to self-fire - #8750

Merged
qq9340100 merged 1 commit into
mainfrom
claude/issue-8628-db-job-timeout-minute-boundary
Aug 14, 2026
Merged

test(service-job): make the timeout suite's cron unable to self-fire#8750
qq9340100 merged 1 commit into
mainfrom
claude/issue-8628-db-job-timeout-minute-boundary

Conversation

@qq9340100

Copy link
Copy Markdown
Collaborator

Fixes#8628

db-job-adapter.timeout.test.ts registered a real croner job on '* * * * *' and then asserted an exact sys_job_run row count. Nothing suppressed the registration's own schedule, so whenever a run straddled a minute boundary croner fired it independently, a second row landed, and CI reddened on a package the offending PR had usually not touched.

What changed

The exact-count assertion is kept verbatim — it is the only thing that can catch a genuine double-scheduling regression, which is what this suite exists to catch. What is removed is the dependency on where the wall clock happens to be.

The fixture now uses '0 0 30 2 *': croner parses it, but February 30th never occurs, so nextRun() is null and the registration carries no schedule of its own, leaving the explicit trigger() as the only writer. trigger() never consults the schedule — CronJobAdapter.trigger executes the stored record directly — so nothing the case exercises depends on the registration being schedulable.

'0 0 29 2 *' would not do: croner resolves Feb 29 to the next leap year (measured: 2028-02-29T00:00:00.000Z).

No production code is touched. The card permitted a minimal clock-injection seam if measurement showed none existed; it turned out none is needed, since pinning the schedule removes the wall-clock dependency entirely. CronJobAdapter constructs new Cron(...) directly and exposes no clock seam — that stays true, and this PR does not add one.

The property is pinned, not just commented

Two assertions make the inertness enforced, so a later tidy-up back to an every-minute spelling cannot silently reintroduce a flake that passes locally and reds only in CI:

  • the shared fixture's nextRun() must be null;
  • the croner job the cron case actually registers must be both present (the case still exercises the real adapter) and unschedulable.

Verified by reverting the spelling with the pins in place: both fail immediately, with no boundary forcing at all — AssertionError: expected 2026-08-14T17:45:00.000Z to be null.

Evidence: the failure mode can no longer occur

"It passes now" is worth little on a flake, so the boundary was forced rather than waited for. Only Date is faked; timers stay real, so croner's genuine scheduling path runs. Same forcing on both legs, on the real test file:

Leg A — origin/main's file, boundary forced. Reproduces the reported CI signature byte-for-byte, same line:

FAIL src/db-job-adapter.timeout.test.ts > the timeout policy still applies through an injected cron adapter (#7734) > a cron-scheduled run lands a timeout row even though the adapter no longer sees the policy
AssertionError: expected [ { …(10) }, { …(7) } ] to have a length of 1 but got 2
❯ src/db-job-adapter.timeout.test.ts:221:18
Tests 1 failed | 9 passed (10)

Leg B — this branch, identical forcing.Tests 11 passed (11).

A standalone sweep of the same case body isolated the window: positioned 2/5/10/15 ms before a boundary the old spelling yields 2 rows and the new spelling 1; at 20/25/30 ms both yield 1, because the case completes before the boundary arrives.

Verification

All at HEAD 35685766e, working tree clean, run after the final commit.

  • pnpm --filter @objectstack/service-job test — 8 files, 72 tests passed (71 before; the added one is the fixture pin)
  • pnpm --filter @objectstack/service-job typecheck — clean
  • Gates, re-derived against the actual changed path with scripts/pm/dispatch-gates.mjs: check:test-source-alias, check:type-source-resolution, check:cross-package-test-inputs, check:query-options-erasure, check:type-check-coverage, check:nul-bytes — all pass
  • check:where-matcher — passes (the card flagged it; no new matches(row, where) double is introduced, the fake engine is unchanged)
  • check:type-check-debt — passes after building the full closure (turbo run build, 70 tasks): "33 ledger entries re-measured, none above its recorded number". It refused before the build, and a refusal is NOT MEASURED, so it was built and re-run rather than waived. service-job is in neither ledger.

Scope

Test-only, one file, inside packages/services/service-job/src/**. No changeset — nothing user-visible is released by this change; skip-changeset applied.

The sibling suite carries the same hazard in four cases and is filed separately as #8748 — deliberately not addressed here, per the card scoping the sweep out.


Generated by Claude Code

…8628)
`db-job-adapter.timeout.test.ts` registered a REAL croner job on
`'* * * * *'` and then asserted an exact `sys_job_run` row count. Nothing
suppressed the registration's own schedule, so whenever a run straddled a
minute boundary croner fired it independently, a second row landed, and CI
reddened on a package the offending PR had usually not touched.
The exact-count assertion is kept verbatim — it is the only thing that can
catch a genuine double-scheduling regression. What is removed is the
dependency on where the wall clock happens to be: the fixture now uses
`'0 0 30 2 *'`, which croner parses but can never fire (February 30th does
not exist, so `nextRun()` is null), leaving the explicit `trigger()` as the
only writer. Feb 29 would NOT do — croner resolves it to the next leap year.
Two assertions pin the property so it cannot be silently undone by a later
tidy-up back to an every-minute spelling, which would pass locally and red
only in CI: the shared fixture's `nextRun()` must be null, and the croner
job the cron case actually registers must be both present and unschedulable.
Measured, not assumed: with only `Date` faked (real timers, croner's real
scheduling path) and the registration positioned 2/5/10/15ms before a minute
boundary, the old spelling produces 2 rows and the new one produces 1.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NaS1PAHJcPfAA2acnV53Tn
@vercel

vercelBot commented Aug 14, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 14, 2026 6:07pm

Request Review

@qq9340100qq9340100 added skip-changeset PR has no user-facing published change; bypasses the changeset gate and removed tests labels Aug 14, 2026 — with Claude
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

No hand-written docs reference the 0 changed package(s). ✅

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/sskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants

@qq9340100@claude