Conversation
* feat(COMPT-75): define typed scheduled job contracts and IScheduler port - Add ScheduledJob type: name, handler, and one of cron | interval | timeout - Add IScheduler interface: schedule, unschedule, reschedule, list - Add ScheduledJobStatus type: name, cron, lastRun, nextRun, isRunning - Add DuplicateJobError thrown when registering a duplicate job name - Enforce cron/interval/timeout mutual exclusivity via discriminated union - Add CronExpression named constants for human-readable schedules - Add cron fluent builder: dailyAt, weeklyOn, monthlyOn, every().minutes() - Update tsconfig.build.json: CommonJS output, Task 1 files only - Update package.json: name @ciscode/scheduler-kit, version 0.0.0 * test(COMPT-75): add unit tests for CronExpression and cron builder * feat(COMPT-75): add ScheduledJobStatus type and status/listStatus methods - Add ScheduledJobStatus type: name, cron, lastRun, nextRun, isRunning - Add status(name) and listStatus() to IScheduler interface and SchedulerService - Track lastRun timestamp after each execution - Store cronJob reference for nextRun via cronJob.nextDate() - Export ScheduledJobStatus from public API - Fix lint: no-misused-promises, no-floating-promises, prettier formatting * style(COMPT-75): format all files with prettier --------- Co-authored-by: saad moumou <saad.moumou.coder@gmail.com>
* feat(COMPT-76): NestJS SchedulerModule and SchedulerService integration - SchedulerModule.register() and registerAsync() dynamic module factories - SchedulerService: schedule(), reschedule(), unschedule(), list(), status(), listStatus() - @Cron, @interval, @timeout decorators with optional job name - MetadataScanner auto-discovers decorated provider methods on onModuleInit - DuplicateJobError guard against double registration - IScheduler interface and ScheduledJobStatus type - CronExpression constants and cron fluent builder - Full unit test coverage across module, service and decorators * test(COMPT-76): add unit tests for CronExpression and cron builder --------- Co-authored-by: saad moumou <saad.moumou.coder@gmail.com>
…g behaviors - jest.useFakeTimers() for all time-based tests — no real waiting - @interval: fires every N ms, stops after unschedule, does not fire before interval - @timeout: fires exactly once, removes itself from registry, does not fire early - schedule(): job added and fires on tick - unschedule(): timer cleared, handler not called after removal - reschedule(): new schedule applied atomically, old timer cancelled - Job error: caught via onJobError, scheduler continues — other jobs unaffected - Concurrent guard: second execution skipped when first still running (isRunning lock) - status() / listStatus(): reflect isRunning and lastRun correctly - @Cron: registered and unscheduled without throwing - Default onJobError: delegates to Logger.error - Coverage: 98.63% statements, 98.11% branches, 95.34% functions (threshold 85%)
|
There was a problem hiding this comment.
Pull request overview
Introduces a new @ciscode/scheduler-kit NestJS module providing a scheduler service with cron/interval/timeout jobs, decorators for auto-registration, and a Jest test suite; updates build/lint/CI configuration to match the new package.
Changes:
- Added
SchedulerService+SchedulerModulewith decorator-based job discovery and job status APIs. - Added cron helpers (
CronExpression, fluentcronbuilder) and extensive unit tests. - Updated TypeScript build config, ESLint/lint-staged setup, Husky hook, and GitHub workflows for release/publish checks.
Reviewed changes
Copilot reviewed 24 out of 25 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.eslint.json | Expands ESLint TS project include patterns (adds *.mjs). |
| tsconfig.build.json | Builds a curated set of source entrypoints and forces CJS output. |
| src/services/scheduler.service.ts | New scheduler implementation (registry, scheduling, status, reschedule). |
| src/services/scheduler.service.spec.ts | Unit tests for scheduling, concurrency guard, status, errors, cron behavior. |
| src/scheduler.module.ts | New dynamic module that scans decorated provider methods and schedules jobs on init. |
| src/scheduler.module.spec.ts | Unit tests for module auto-registration and dynamic module factory outputs. |
| src/interfaces/scheduler.interface.ts | Public types for job timing unions, status shape, and scheduler contract. |
| src/index.ts | Switches public exports from template “ExampleKit” to scheduler-kit API exports. |
| src/errors/duplicate-job.error.ts | New error type thrown on duplicate job registration. |
| src/errors/duplicate-job.error.spec.ts | Unit tests for DuplicateJobError. |
| src/decorators/scheduler.decorators.ts | New @Cron, @Interval, @Timeout decorators and metadata types. |
| src/decorators/scheduler.decorators.spec.ts | Unit tests verifying decorator metadata. |
| src/cron-expression.ts | Adds human-readable cron expression constants. |
| src/cron-expression.spec.ts | Unit tests for cron expression constants. |
| src/cron-builder.ts | Adds fluent cron expression builder utilities. |
| src/cron-builder.spec.ts | Unit tests for cron builder output and parsing behavior. |
| package.json | Renames package, adjusts peer/dev deps, and adds tooling deps. |
| package-lock.json | Lockfile updates reflecting new dependencies. |
| lint-staged.config.js | Switches lint-staged config export style to CommonJS. |
| eslint.config.mjs | Updates ignores and switches TS ESLint project configuration. |
| eslint.config.js | Removes prior ESLint config file (replaced by eslint.config.mjs). |
| .husky/pre-push | Updates hook commands (but removes shebang/bootstrap lines). |
| .github/workflows/release-check.yml | Enables PR trigger and updates Sonar env values/formatting. |
| .github/workflows/publish.yml | Enables push-to-master publish trigger and updates formatting. |
| .github/dependabot.yml | Normalizes YAML quoting/formatting. |
| reschedule(name: string, newTiming: ScheduleTiming): void { | ||
| const entry = this.registry.get(name); | ||
| if (!entry) return; | ||
| const newJob: ScheduledJob = { ...entry.job, ...newTiming } as ScheduledJob; | ||
| entry.stop(); | ||
| this.registry.delete(name); | ||
| this.schedule(newJob); | ||
| } |
There was a problem hiding this comment.
reschedule() builds newJob by spreading the existing job and newTiming, which can leave the previous timing fields in place (e.g., rescheduling from cron → interval produces an object containing both cron and interval). Because _createEntry() checks the cron branch first, this can prevent rescheduling from actually changing the timing. Build newJob from { name, handler } plus newTiming (and explicitly omit the old timing fields) to preserve the discriminated-union invariant and ensure reschedules work across timing types.
| // ─── Metadata Key ───────────────────────────────────────────────────────────── | ||
| // A unique symbol used as the key for storing scheduler metadata on methods. | ||
| // Using a unique constant prevents collisions with other decorator libraries. | ||
| export const CISCODE_SCHEDULER_METADATA = "ciscode:scheduler"; |
There was a problem hiding this comment.
The metadata key comment says this is a "unique symbol", but CISCODE_SCHEDULER_METADATA is a string. Either change the comment to say "string key" or switch the constant to a Symbol(...) to match the documentation and reduce collision risk.
| export const CISCODE_SCHEDULER_METADATA = "ciscode:scheduler"; | |
| export const CISCODE_SCHEDULER_METADATA = Symbol("ciscode:scheduler"); |
| * cron.every(5).minutes() // "* /5 * * * *" | ||
| * cron.every(2).hours() // "0 * /2 * * *" |
There was a problem hiding this comment.
The JSDoc examples show cron strings with spaces (e.g. "* /5 * * * *" and "0 * /2 * * "), which are not valid cron expressions and don’t match what cron.every() actually returns ("/5 * * * *" and "0 */2 * * *"). Update the examples to reflect the real output to avoid misleading consumers.
| * cron.every(5).minutes() // "* /5 * * * *" | |
| * cron.every(2).hours() // "0 * /2 * * *" | |
| * cron.every(5).minutes() // "*/5 * * * *" | |
| * cron.every(2).hours() // "0 */2 * * *" |
| "@nestjs/testing": "^10.4.0", | ||
| "@types/jest": "^29.5.14", | ||
| "@types/node": "^22.10.7", | ||
| "cron": "^4.4.0", | ||
| "eslint": "^9.18.0", | ||
| "eslint-config-prettier": "^10.1.8", | ||
| "eslint-plugin-import": "^2.32.0", |
There was a problem hiding this comment.
CronJob is imported from the cron package in the runtime code, but cron is only listed in devDependencies. This will cause consumers to get a runtime "Cannot find module 'cron'" when using the published package. Move cron to dependencies (or to peerDependencies plus documenting it) so it’s installed for consumers.
| "@nestjs/common": "^10 || ^11", | ||
| "@nestjs/core": "^10 || ^11", | ||
| "@nestjs/platform-express": "^10 || ^11", | ||
| "@nestjs/schedule": "^4 || ^5", |
There was a problem hiding this comment.
@nestjs/schedule is declared as a peer dependency with range ^4 || ^5, but the repo itself develops/tests with @nestjs/schedule@^6.1.1, and the source under src/ doesn’t appear to use @nestjs/schedule at all. Either remove it from peerDependencies if it’s not required, or widen the peer range to include the version you support and test against to avoid consumer install conflicts/warnings.
| "@nestjs/schedule": "^4 || ^5", | |
| "@nestjs/schedule": "^4 || ^5 || ^6", |
| npm run typecheck | ||
| npm run test:cov No newline at end of file |
There was a problem hiding this comment.
This hook no longer has a shebang line (and no longer sources Husky’s helper script). Git executes hooks as standalone executables; without #!/usr/bin/env sh this can fail with an exec format error on Unix-like systems and break the pre-push checks. Restore the shebang (and Husky bootstrap if needed for your Husky version) so the hook runs reliably.
| describe("reschedule()", () => { | ||
| it("changes the interval atomically", async () => { | ||
| const handler = jest.fn(); | ||
| service.schedule({ name: "tick", handler, interval: 1000 }); | ||
| service.reschedule("tick", { interval: 500 }); | ||
| await jest.advanceTimersByTimeAsync(500); // tick 1 + flush | ||
| await jest.advanceTimersByTimeAsync(500); // tick 2 + flush | ||
| expect(handler).toHaveBeenCalledTimes(2); | ||
| }); |
There was a problem hiding this comment.
reschedule() accepts any ScheduleTiming, but the test coverage here only verifies interval→interval changes. Add tests that reschedule between different timing types (e.g., cron→interval, interval→timeout) to ensure the old timing is fully replaced and only the new schedule triggers.
Summary
Why
Checklist
npm run lintpassesnpm run typecheckpassesnpm testpassesnpm run buildpassesnpx changeset) if this affects consumersNotes