fix: write success_since on first successful check (no prior state)#40
Conversation
When a site was added to watchers.yml manually without success_since or failed_since (no prior runtime state), the first successful check did not initialize success_since. The guard in all four success branches only checked for domain change (effectiveNewHost !== oldLastKnownMirror) or failure recovery (hadFailureBeforeThisRun), but not for first-time initialization. Added `|| !site.success_since` to all four guards (shouldUpdate success, unchanged success, antibot accepted, heuristic non-pattern) so that success_since is written when no prior timestamp exists. Added 6 unit tests covering each branch and churn-suppression edge cases. Added 6 saveWatchers integration tests for state transition persistence.
There was a problem hiding this comment.
Pull request overview
This PR fixes an initialization gap in watcher state handling: when a site is manually added to watchers.yml without any prior runtime timestamps, the first successful check now correctly writes success_since, aligning state persistence with the intended “success timeline” semantics and avoiding silent missing state.
Changes:
- Update all four success-path guards to also write
success_sincewhen it is missing (|| !site.success_since), ensuring first-time initialization. - Add unit tests for each success-path guard and integration tests validating persistence/round-trip behavior through
saveWatchers. - Bump version to
1.4.5and updateCHANGELOG.mdand builtdist/index.js.
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/index.ts |
Initializes success_since on first successful run when no prior timestamp exists across all success branches. |
__tests__/index.test.ts |
Adds unit tests covering the four success branches plus churn-suppression behavior. |
__tests__/config.test.ts |
Adds integration tests validating state transition persistence via loadWatchers/saveWatchers. |
CHANGELOG.md |
Documents the success_since initialization fix in 1.4.5. |
package.json |
Bumps package version to 1.4.5. |
package-lock.json |
Lockfile version bump to 1.4.5. |
dist/index.js |
Updates compiled output to reflect the success_since guard changes and version bump. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot Check whether the code change matches the description in the pull request. |
Checked. The current commit matches the PR description:
I also ran |
When a site was added to watchers.yml manually without success_since or failed_since (no prior runtime state), the first successful check did not initialize success_since. The guard in all four success branches only checked for domain change (effectiveNewHost !== oldLastKnownMirror) or failure recovery (hadFailureBeforeThisRun), but not for first-time initialization.
Added
|| !site.success_sinceto all four guards (shouldUpdate success, unchanged success, antibot accepted, heuristic non-pattern) so that success_since is written when no prior timestamp exists.Added 6 unit tests covering each branch and churn-suppression edge cases. Added 6 saveWatchers integration tests for state transition persistence.