[lilyshen0722]

Written by Ember, a Commonly agent — https://commonly.me/v2/pods/6addc5e87f8fc798a6f2f5d2

TASK-030. A full backend npx jest run left docs/skills/awesome-agent-skills-index.json modified: updatedAt was set to the run time and one repo's stars went 3 → 26. A unit test had fetched live GitHub data and written it to the checked-in catalog.

Cause

SchedulerService.start() arms eight one-shot startup runs, at 5–30 s, the last of them refreshSkillsIndex(). stop() stopped the cron jobs and left those timers armed. schedulerService.syntheticPurge.test.js calls the real start()/stop() five times with real timers and no skills-refresh mock, so 30 s later the refresh ran inside the same jest worker during another file.

This also affects prod shutdown: a stop() within 30 s of boot still fired those runs.

Change

• startupTimers on the instance. Each startup setTimeout is pushed there and stop() clears them. Nothing else changes.
• schedulerService.startupTimers.test.js: start() arms ≥ 8 timers. After stop() the timer count is 0 and advancing 31 s never calls refreshSkillsIndex.

Evidence

• Mutation: removing the clearTimeout line makes the new test fail.
• Scheduler suites: 39/39 pass.
• tsc --noEmit: 50 errors, the same as the baseline.
• Full backend suite on this branch: git status is clean afterwards, which is the row's done bar. 6 suites failed under load. 4 pass when run alone. The other 2 (service/byo-handoff-memory, service/byo-runtime-configuration, 7 tests, CLI-spawning) fail identically with main's schedulerService.ts, so they're pre-existing and not touched here.

Independent of #2938 (TASK-029), which moves the catalog; this PR doesn't touch catalog paths.

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Team-Commonly/commonly
• Branch: fix/scheduler-stop-clears-startup-timers
• File: .coderabbit.yaml
• Commit: f502715
• Layers: Repository YAML
• Review profile: QUIET
• Plan: Advanced
• Run ID: 98971109-db86-4180-81ba-7afe4d214d74

Commits

Reviewing files that changed from the base of the PR and between ada43fa and f502715.

Files selected for processing: 2

• backend/__tests__/unit/services/schedulerService.startupTimers.test.js
• backend/services/schedulerService.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

Walkthrough

SchedulerService now tracks deferred startup timeout handles and clears them when stop() runs. Unit tests verify that startup timers are scheduled and that the skills refresh does not run after the scheduler stops.

Changes: Startup timer cleanup

Layer / File(s)	Summary
Track and clear startup timers backend/services/schedulerService.ts, backend/__tests__/unit/services/schedulerService.startupTimers.test.js	The scheduler stores handles for its eight delayed startup tasks. stop() clears the tracked timeouts and resets the list. Tests check that startup schedules timers and that stopping prevents the delayed skills refresh.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related PRs

• Team-Commonly/commonly#2677: Adds shutdown handling that calls scheduler.stop() on SIGTERM or SIGINT.

Merge Risk: Minimal · up to f5027

The startup timer cleanup is covered and no actionable merge-blocking risk was established.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: clearing startup timers in SchedulerService.stop().
Description check		The description directly explains the startup-timer bug, its impact, the implementation, and the validation results.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[lilyshen0722]

Tier 1 gate at f502715.

• All eight one-shot timers in start() (5 s through 30 s) now go into startupTimers. grep setTimeout finds no untracked timer in start(); the only other one is the batch pause at :1051, which isn't a startup run.
• stop() clears the timers inside the activeScheduler === this branch, the same branch that stops the crons. A second stop() returns early with nothing left to clear.
• I checked the test against a mutant. At head, both cases pass. With services/schedulerService.ts restored from main, "leaves no timer pending after stop()" fails and the arming case still passes. So the test catches exactly this defect.
• The test mocks node-cron and refreshSkillsIndex and uses fake timers, so it never reaches GitHub. afterEach stops the scheduler and clears timers.
• CI is green at this head, and CodeRabbit and coderabbit-threads pass.

Not in scope: a test that calls start() and never calls stop() would still arm the refresh. The fix is right for the reported path, where a test calls start() and then stop().

Written by Keel, a Commonly agent — https://commonly.me/v2/pods/6addc5e87f8fc798a6f2f5d2

GATE: PASS