You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The export change is fine, but the new tests need tightening before merge. I ran mutations against src/lib/scheduler/index.ts and the suite stayed green through all four of these:
Replacing the AutomationAlreadyRunningError guard with if (false) — deleting the concurrent-run handling entirely. The test at line 86 asserts only expect(runAutomation).toHaveBeenCalled(), which is equally true in the generic-error path (line 108 asserts the same thing), so nothing distinguishes the branch. Seed two due automations and assert toHaveBeenCalledTimes(2) plus no failure row from automationRun.create.
Adding throw error to the inner catch, so one failing automation aborts the whole batch. The test at line 100 is named "…and continuing", but the fixture has a single automation, so the continuing half never runs — the outer try/catch swallows the throw and the assertion still holds. Two automations with the first rejecting, then assert the second still ran.
Setting userId: "wrong-user" and resumeId: undefined in the payload mapping (lines 64/70). The test at line 97 never checks what runAutomation was called with. This is the one place the scheduler hands a userId to the scraper, so a cross-user mix-up would ship silently. Add expect(runAutomation).toHaveBeenCalledWith(expect.objectContaining({ id, userId, resumeId, jobBoard, matchThreshold })) with those fields on the fixture.
Deleting the where: { status: "active", nextRunAt: { lte: now } } clause, so every automation — paused ones included — runs on every tick. The first test is named "returns early if no automations are due" but never pins down what "due" means. scheduler-reaper.spec.ts:33-37 has the pattern for inspecting call args; assert where.status === "active" and that where.nextRunAt.lte is a Date.
Also worth dropping the // @ts-ignore on line 82 — it isn't needed (npx tsc --noEmit exits 0 without it) and would hide a future signature change to AutomationAlreadyRunningError or runAutomation.
Happy to merge once these are in.
Reviewed by Claude Code (Opus 5)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
runDueAutomationsso the runner can be tested directlyTesting
npm run testnpm run lintnpx tsc --noEmitnpm run build