The inconsistency alert was sent on every DNS check for as long as two nameservers disagreed. detectInconsistencies now also takes the previous hostname state, compares every pair of nameservers, and alerts for a pair that differs now unless both were in that state and already differed there. The choice of pairs is in newlyDisagreeingPairs, tested on record data built in the test. The alert itself is tested by passing record data for successive checks to the hostname change detection with the notifier stand-in; no resolver is involved.
What a reader might trip over:
The state loaded at startup is the previous state for the first check, so a disagreement saved before a restart is not reported again. Nothing new is stored.
A nameserver that was not in the previous check (newly added, or back after dropping out) and answers differently is reported once, on the check where it appears, and not again while it keeps disagreeing.
Every pair is compared, not only neighbours in sorted order of name as before. One nameserver moving away from several that agreed with it sends one alert per such pair.
The new tests are in internal/watcher/inconsistency_test.go with their own values. They use the notifier stand-in from watcher_test.go, which stays under #159, and not mockResolver.
Disclosures:
Judgement call, from review (#161 (comment)): this replaces the plan's rule (#158 (comment)) that a pair is alerted only when both nameservers were in the previous state.
Model: opus-5-5
Implements https://git.eeqj.de/sneak/dnswatcher/issues/158.
The inconsistency alert was sent on every DNS check for as long as two nameservers disagreed. `detectInconsistencies` now also takes the previous hostname state, compares every pair of nameservers, and alerts for a pair that differs now unless both were in that state and already differed there. The choice of pairs is in `newlyDisagreeingPairs`, tested on record data built in the test. The alert itself is tested by passing record data for successive checks to the hostname change detection with the notifier stand-in; no resolver is involved.
What a reader might trip over:
- The state loaded at startup is the previous state for the first check, so a disagreement saved before a restart is not reported again. Nothing new is stored.
- A nameserver that was not in the previous check (newly added, or back after dropping out) and answers differently is reported once, on the check where it appears, and not again while it keeps disagreeing.
- Every pair is compared, not only neighbours in sorted order of name as before. One nameserver moving away from several that agreed with it sends one alert per such pair.
- The new tests are in `internal/watcher/inconsistency_test.go` with their own values. They use the notifier stand-in from `watcher_test.go`, which stays under https://git.eeqj.de/sneak/dnswatcher/issues/159, and not `mockResolver`.
Disclosures:
- Judgement call, from review (https://git.eeqj.de/sneak/dnswatcher/pulls/161#issuecomment-104653): this replaces the plan's rule (https://git.eeqj.de/sneak/dnswatcher/issues/158#issuecomment-104282) that a pair is alerted only when both nameservers were in the previous state.
Model: opus-5-5
internal/watcher/watcher.go, newlyDisagreeingPairs: a new disagreement is never reported when the nameserver between the two in sorted order already disagreed. Example: b already returns different records; a and c agree, then c changes to a third value. No inconsistency alert is sent, on that check or later; before this change it was reported. Four nameservers with b stuck and a changing behave the same. This fails the first point of the definition of done in #158 and makes the new README sentence ("Two nameservers that agreed on the previous check now return different record sets ... sent once, on the check where they start to disagree") and the new TODO.md entry untrue. Acceptable: whenever any two nameservers that agreed on the previous check now differ, an alert is sent on that check wherever they sit in sorted order (for example, compare every pair against the previous state), with a test case for it.
internal/watcher/inconsistency_test.go: no test fails if the inconsistency alert stops being sent; only the choice of pairs is tested. The PR body says testing the alert would need the stubbed resolver; it would not: the hostname change detection takes the previous state and current records directly, and the notifier stand-in stays under #159. Acceptable: a test that passes record data for successive checks to the hostname change detection with the notifier stand-in and no resolver, and checks that a disagreement lasting several checks gives exactly one inconsistency notification and one already in the loaded state gives none.
Model: opus-5-5
1. `internal/watcher/watcher.go`, `newlyDisagreeingPairs`: a new disagreement is never reported when the nameserver between the two in sorted order already disagreed. Example: `b` already returns different records; `a` and `c` agree, then `c` changes to a third value. No inconsistency alert is sent, on that check or later; before this change it was reported. Four nameservers with `b` stuck and `a` changing behave the same. This fails the first point of the definition of done in https://git.eeqj.de/sneak/dnswatcher/issues/158 and makes the new README sentence ("Two nameservers that agreed on the previous check now return different record sets ... sent once, on the check where they start to disagree") and the new `TODO.md` entry untrue. Acceptable: whenever any two nameservers that agreed on the previous check now differ, an alert is sent on that check wherever they sit in sorted order (for example, compare every pair against the previous state), with a test case for it.
2. `internal/watcher/inconsistency_test.go`: no test fails if the inconsistency alert stops being sent; only the choice of pairs is tested. The PR body says testing the alert would need the stubbed resolver; it would not: the hostname change detection takes the previous state and current records directly, and the notifier stand-in stays under https://git.eeqj.de/sneak/dnswatcher/issues/159. Acceptable: a test that passes record data for successive checks to the hostname change detection with the notifier stand-in and no resolver, and checks that a disagreement lasting several checks gives exactly one inconsistency notification and one already in the loaded state gives none.
Model: opus-5-5
newlyDisagreeingPairs now compares every pair of nameservers against the previous state; new case: c changes while b already differs, alert for a and c.
TestInconsistencyAlert passes record data for three checks to the hostname change detection with the notifier stand-in and no resolver: a lasting disagreement gives one alert, one in the loaded state gives none. README, TODO.md and PR body updated; the untested-alert disclosure is dropped.
Model: opus-5-5
1. `newlyDisagreeingPairs` now compares every pair of nameservers against the previous state; new case: `c` changes while `b` already differs, alert for `a` and `c`.
2. `TestInconsistencyAlert` passes record data for three checks to the hostname change detection with the notifier stand-in and no resolver: a lasting disagreement gives one alert, one in the loaded state gives none. README, `TODO.md` and PR body updated; the untested-alert disclosure is dropped.
Model: opus-5-5
internal/watcher/watcher.go, newlyDisagreeingPairs: a nameserver that was not in the previous check (newly added to the delegation, or back after dropping out of it) and answers differently from the others is never reported, neither on that check nor on any later one while the disagreement lasts. Before this change it was reported, and no other hostname alert covers it: the record change and recovery checks also skip a nameserver missing from the previous state. A newly added nameserver serving a stale zone therefore produces no alert about its differing records, contrary to the README's "Every observable state change produces a notification". The definition of done in #158 does not need this silence; it only asks that a disagreement not repeat. Acceptable: alert for a pair that differs now unless both nameservers were in the previous state and already differed there, so a new nameserver that disagrees is reported once, on the check where it appears, and not again while it keeps disagreeing; the test case "nameserver new on this check does not alert" changed to expect that alert once and no repeat; the README and TODO.md sentences adjusted to match.
Judgement call: this overrides the rule in the plan (#158 (comment)) that a pair is alerted only when both nameservers were in the previous state; the PR followed the plan.
Model: opus-5-5
1. `internal/watcher/watcher.go`, `newlyDisagreeingPairs`: a nameserver that was not in the previous check (newly added to the delegation, or back after dropping out of it) and answers differently from the others is never reported, neither on that check nor on any later one while the disagreement lasts. Before this change it was reported, and no other hostname alert covers it: the record change and recovery checks also skip a nameserver missing from the previous state. A newly added nameserver serving a stale zone therefore produces no alert about its differing records, contrary to the README's "Every observable state change produces a notification". The definition of done in https://git.eeqj.de/sneak/dnswatcher/issues/158 does not need this silence; it only asks that a disagreement not repeat. Acceptable: alert for a pair that differs now unless both nameservers were in the previous state and already differed there, so a new nameserver that disagrees is reported once, on the check where it appears, and not again while it keeps disagreeing; the test case "nameserver new on this check does not alert" changed to expect that alert once and no repeat; the README and `TODO.md` sentences adjusted to match.
Judgement call: this overrides the rule in the plan (https://git.eeqj.de/sneak/dnswatcher/issues/158#issuecomment-104282) that a pair is alerted only when both nameservers were in the previous state; the PR followed the plan.
Model: opus-5-5
detectInconsistencies alerted for neighbouring pairs of nameservers whose
records differed, on every DNS check, for as long as they differed. It now
also takes the previous hostname state, compares every pair of nameservers,
and alerts for a pair that differs unless both were in that state and
already differed there, so a nameserver new on a check that answers
differently is reported once. The state loaded at startup is the previous
state for the first check, so a disagreement saved before a restart is not
reported again. The choice of pairs is tested on record data, and the alert
through the hostname change detection with the notifier stand-in and no
resolver. The README describes the new behaviour.
Model: opus-5-5
newlyDisagreeingPairs now alerts for a pair that differs unless both nameservers were in the previous check and already differed there, so a nameserver new on a check that answers differently is reported once. The new-nameserver case in TestNewlyDisagreeingPairs now expects that alert and no repeat, and TestInconsistencyAlert has the same case through the notifier stand-in. README, TODO.md and PR body updated.
Model: opus-5-5
1. `newlyDisagreeingPairs` now alerts for a pair that differs unless both nameservers were in the previous check and already differed there, so a nameserver new on a check that answers differently is reported once. The new-nameserver case in `TestNewlyDisagreeingPairs` now expects that alert and no repeat, and `TestInconsistencyAlert` has the same case through the notifier stand-in. README, `TODO.md` and PR body updated.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Implements #158.
The inconsistency alert was sent on every DNS check for as long as two nameservers disagreed.
detectInconsistenciesnow also takes the previous hostname state, compares every pair of nameservers, and alerts for a pair that differs now unless both were in that state and already differed there. The choice of pairs is innewlyDisagreeingPairs, tested on record data built in the test. The alert itself is tested by passing record data for successive checks to the hostname change detection with the notifier stand-in; no resolver is involved.What a reader might trip over:
internal/watcher/inconsistency_test.gowith their own values. They use the notifier stand-in fromwatcher_test.go, which stays under #159, and notmockResolver.Disclosures:
Model: opus-5-5
internal/watcher/watcher.go,newlyDisagreeingPairs: a new disagreement is never reported when the nameserver between the two in sorted order already disagreed. Example:balready returns different records;aandcagree, thencchanges to a third value. No inconsistency alert is sent, on that check or later; before this change it was reported. Four nameservers withbstuck andachanging behave the same. This fails the first point of the definition of done in #158 and makes the new README sentence ("Two nameservers that agreed on the previous check now return different record sets ... sent once, on the check where they start to disagree") and the newTODO.mdentry untrue. Acceptable: whenever any two nameservers that agreed on the previous check now differ, an alert is sent on that check wherever they sit in sorted order (for example, compare every pair against the previous state), with a test case for it.internal/watcher/inconsistency_test.go: no test fails if the inconsistency alert stops being sent; only the choice of pairs is tested. The PR body says testing the alert would need the stubbed resolver; it would not: the hostname change detection takes the previous state and current records directly, and the notifier stand-in stays under #159. Acceptable: a test that passes record data for successive checks to the hostname change detection with the notifier stand-in and no resolver, and checks that a disagreement lasting several checks gives exactly one inconsistency notification and one already in the loaded state gives none.Model: opus-5-5
8fd2fadbd8to8ea6191566newlyDisagreeingPairsnow compares every pair of nameservers against the previous state; new case:cchanges whilebalready differs, alert foraandc.TestInconsistencyAlertpasses record data for three checks to the hostname change detection with the notifier stand-in and no resolver: a lasting disagreement gives one alert, one in the loaded state gives none. README,TODO.mdand PR body updated; the untested-alert disclosure is dropped.Model: opus-5-5
internal/watcher/watcher.go,newlyDisagreeingPairs: a nameserver that was not in the previous check (newly added to the delegation, or back after dropping out of it) and answers differently from the others is never reported, neither on that check nor on any later one while the disagreement lasts. Before this change it was reported, and no other hostname alert covers it: the record change and recovery checks also skip a nameserver missing from the previous state. A newly added nameserver serving a stale zone therefore produces no alert about its differing records, contrary to the README's "Every observable state change produces a notification". The definition of done in #158 does not need this silence; it only asks that a disagreement not repeat. Acceptable: alert for a pair that differs now unless both nameservers were in the previous state and already differed there, so a new nameserver that disagrees is reported once, on the check where it appears, and not again while it keeps disagreeing; the test case "nameserver new on this check does not alert" changed to expect that alert once and no repeat; the README andTODO.mdsentences adjusted to match.Judgement call: this overrides the rule in the plan (#158 (comment)) that a pair is alerted only when both nameservers were in the previous state; the PR followed the plan.
Model: opus-5-5
8ea6191566todf68bbc615newlyDisagreeingPairsnow alerts for a pair that differs unless both nameservers were in the previous check and already differed there, so a nameserver new on a check that answers differently is reported once. The new-nameserver case inTestNewlyDisagreeingPairsnow expects that alert and no repeat, andTestInconsistencyAlerthas the same case through the notifier stand-in. README,TODO.mdand PR body updated.Model: opus-5-5
Review passed on
df68bbc.Model: opus-5-5
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.