fix(test): scheduler sentinel-disable guard asserts a method the refresh path never calls, so the #308 regression is unguarded #167

scheduler_test.go:972 asserts that MarkCollectionStarted is not called, intending to guard the sentinel-disable behaviour: when RecommendationsCacheStaleHours == 0, the background refresh must not run.

The assertion cannot fail, and not for the reasons LeanerCloud/cloud-commitments-cli#1595 fixed.

MarkCollectionStarted has no caller on the scheduler's background-refresh path at all. Its only production caller is handler_recommendations_refresh.go:77. So the test asserts the absence of something that was never going to be present, regardless of whether the guarded behaviour works.

Demonstrated, not inferred

The faithful mutation — making RecommendationsCacheStaleHours == 0 no longer disable the refresh, which is exactly the PR LeanerCloud/cloud-commitments-cli#308 regression — leaves this test green.

So the sentinel-disable behaviour is currently unguarded: if someone reintroduces LeanerCloud/cloud-commitments-cli#308, nothing in the suite notices.

Why this is not part of LeanerCloud/cloud-commitments-cli#1595

LeanerCloud/cloud-commitments-cli#1595 (PR LeanerCloud/cloud-commitments-cli#1735) repaired 77 assertions that could not fail because of MockConfigStore's isExpected short-circuit and testify's empty-expectation diff. Those are now mutation-proven to fail when the behaviour they guard is broken.

This one is a different defect: the assertion is mechanically capable of failing after LeanerCloud/cloud-commitments-cli#1735, but it names the wrong method. No amount of mock repair fixes an assertion pointed at a method the code path never calls. It needs a new guard, not a repaired one.

Suggested fix

Identify what the refresh path actually calls when it runs — the store method or observable effect that is present when the refresh happens and absent when the sentinel disables it — and assert on that instead.

Then prove it: with the guard in place, make RecommendationsCacheStaleHours == 0 stop disabling the refresh and confirm the test fails. If it still passes, the new assertion is pointed at the wrong thing too.

Note for whoever picks this up

Do not simply delete the assertion because it is vacuous. It encodes a real intent — LeanerCloud/cloud-commitments-cli#308 was a real regression — and the intent should survive with a working guard. Deleting it would convert "a test that does not work" into "no test", which is worse in every way except the count.

Found during the mutation-verification sweep for LeanerCloud/cloud-commitments-cli#1595 (PR LeanerCloud/cloud-commitments-cli#1735); explicitly out of scope there and not fixed.