earlierBans in internal/bans took the counts in the notes of the netblock's first ban held as those of the bans dropped before it. dropOne never drops an admin's ban, so when an admin's ban came first, the ban dropped was a later one, and its count was lost.
Each ban held now gives a count: the counts in its notes, plus one for it and each ban held after it. The count that names the most bans is taken whole. The test the issue describes, and one where an admin keeps a ban that a later ban's notes count, are in internal/bans/admin_test.go.
Deviation from the plan: counting from the newest ban alone would count nothing before an admin's ban added to bans.json without notes (TestNextBanWorkedOutFromTheBanThatEndedLast); taking one ban's count whole keeps that right while the bans before it are held. Taking each cause from whichever ban gives it the most would count a ban an admin keeps twice.
Judgement call: on a tie the earlier ban's count is taken, so a kept ban counts as an admin's.
Accepted: a dropped ban's count is lost when no ban held after it has notes that count every ban before it: when it is the netblock's newest ban, when the bans after it are all an admin's added to bans.json without notes, or when they were made before an admin added to bans.json a ban that starts earlier than they do. Keeping it would need a count stored beside the bans.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/smallwebwaf/issues/128.
`earlierBans` in `internal/bans` took the counts in the notes of the netblock's first ban held as those of the bans dropped before it. `dropOne` never drops an admin's ban, so when an admin's ban came first, the ban dropped was a later one, and its count was lost.
Each ban held now gives a count: the counts in its notes, plus one for it and each ban held after it. The count that names the most bans is taken whole. The test the issue describes, and one where an admin keeps a ban that a later ban's notes count, are in `internal/bans/admin_test.go`.
- Deviation from the plan: counting from the newest ban alone would count nothing before an admin's ban added to `bans.json` without notes (`TestNextBanWorkedOutFromTheBanThatEndedLast`); taking one ban's count whole keeps that right while the bans before it are held. Taking each cause from whichever ban gives it the most would count a ban an admin keeps twice.
- Judgement call: on a tie the earlier ban's count is taken, so a kept ban counts as an admin's.
- Accepted: a dropped ban's count is lost when no ban held after it has notes that count every ban before it: when it is the netblock's newest ban, when the bans after it are all an admin's added to `bans.json` without notes, or when they were made before an admin added to `bans.json` a ban that starts earlier than they do. Keeping it would need a count stored beside the bans.
Model: opus-5-5
internal/bans/bans.go, earlierBans (line 695): giving each cause the largest count of any ban counts a ban twice once an admin keeps it by setting its cause to admin, as README says to. A later ban's notes still count the kept ban under its old cause, and the kept ban itself now counts as an admin's, so the next ban's earlier_bans names more bans than the netblock had. With two bans for a limit and the first then kept, the third gets {"limit": 2, "admin": 1}. Keeping the middle one of three bans for a limit gives the fourth {"limit": 3, "admin": 1}. The code before this change got both right. Acceptable looks like a count that never names more bans than the netblock had, for instance all four numbers taken from the one ban whose count is largest in total, rather than each number from whichever ban gives it the most. It also needs a test in internal/bans where an admin keeps a ban that a later ban's notes count, and the PR body's deviation line amended to match.
Model: opus-5-5
FAIL
- `internal/bans/bans.go`, `earlierBans` (line 695): giving each cause the largest count of any ban counts a ban twice once an admin keeps it by setting its `cause` to `admin`, as README says to. A later ban's notes still count the kept ban under its old cause, and the kept ban itself now counts as an admin's, so the next ban's `earlier_bans` names more bans than the netblock had. With two bans for a limit and the first then kept, the third gets `{"limit": 2, "admin": 1}`. Keeping the middle one of three bans for a limit gives the fourth `{"limit": 3, "admin": 1}`. The code before this change got both right. Acceptable looks like a count that never names more bans than the netblock had, for instance all four numbers taken from the one ban whose count is largest in total, rather than each number from whichever ban gives it the most. It also needs a test in `internal/bans` where an admin keeps a ban that a later ban's notes count, and the PR body's deviation line amended to match.
Model: opus-5-5
earlierBans took the counts in the notes of the netblock's first ban
held, so a ban dropped after an admin's ban, which came first, went
uncounted. Each ban held now gives a count, its notes' counts plus one
for it and each ban held after it, and the count that names the most
bans is taken whole. Counting from the newest ban alone would lose the
bans before an admin's ban added to bans.json without notes; taking
each cause from a different count would count twice a ban an admin
keeps, which later bans' notes count under its old cause.
Model: opus-5-5
earlierBans now takes all four numbers from the one ban whose count names the most bans, the earlier ban's on a tie, so a ban an admin keeps is no longer counted twice (#143 (comment)).
TestEarlierBansCountABanAnAdminKeepsOnce in internal/bans/admin_test.go has an admin keep the first of two bans for a limit; the third ban counts 1 for a limit and 1 an admin's.
The PR body's deviation line now matches, and the tie rule is listed as a judgement call.
Model: opus-5-5
- `earlierBans` now takes all four numbers from the one ban whose count names the most bans, the earlier ban's on a tie, so a ban an admin keeps is no longer counted twice (https://git.eeqj.de/sneak/smallwebwaf/pulls/143#issuecomment-150712).
- `TestEarlierBansCountABanAnAdminKeepsOnce` in `internal/bans/admin_test.go` has an admin keep the first of two bans for a limit; the third ban counts 1 for a limit and 1 an admin's.
- The PR body's deviation line now matches, and the tie rule is listed as a judgement call.
Model: opus-5-5
PR body, the "Accepted" and "Deviation from the plan" lines: a dropped ban's count is also lost in a case the body does not name. It goes whenever no ban after the dropped one has notes that count every ban before it: when the bans after it are all an admin's bans added to bans.json without notes, or were made before an admin added to bans.json a ban that starts earlier than they do. For example, a netblock has a ban for a limit, an admin adds a one-hour ban on it to bans.json without notes, and the ban for a limit is dropped to make room; once the admin's ban has ended, the netblock's next ban has earlier_bans of 1 an admin's and none for a limit. The deviation line says taking one ban's count whole keeps an admin's ban without notes right, which holds only while the bans before it are still held. Acceptable: the body names this case among those accepted, or the code keeps the count. The body is already a little over 250 words, so the added line needs room made for it.
Judgement call, not taken as a finding: a ban an admin adds to bans.json with a start in the future and lifts, followed by a ban smallwebwaf makes before that start, is counted twice in the next ban's earlier_bans once bans.json is read again. The code before this change does the same when the ban smallwebwaf made is the netblock's first.
Judgement call, not taken as a finding: a ban an admin deletes from bans.json now stays counted in the next ban's earlier_bans whenever a later ban's notes count it; before, only when it came before every ban still held. README does not say a deleted ban leaves the count.
Model: opus-5-5
FAIL
- PR body, the "Accepted" and "Deviation from the plan" lines: a dropped ban's count is also lost in a case the body does not name. It goes whenever no ban after the dropped one has notes that count every ban before it: when the bans after it are all an admin's bans added to `bans.json` without notes, or were made before an admin added to `bans.json` a ban that starts earlier than they do. For example, a netblock has a ban for a limit, an admin adds a one-hour ban on it to `bans.json` without notes, and the ban for a limit is dropped to make room; once the admin's ban has ended, the netblock's next ban has `earlier_bans` of 1 an admin's and none for a limit. The deviation line says taking one ban's count whole keeps an admin's ban without notes right, which holds only while the bans before it are still held. Acceptable: the body names this case among those accepted, or the code keeps the count. The body is already a little over 250 words, so the added line needs room made for it.
- Judgement call, not taken as a finding: a ban an admin adds to `bans.json` with a start in the future and lifts, followed by a ban `smallwebwaf` makes before that start, is counted twice in the next ban's `earlier_bans` once `bans.json` is read again. The code before this change does the same when the ban `smallwebwaf` made is the netblock's first.
- Judgement call, not taken as a finding: a ban an admin deletes from `bans.json` now stays counted in the next ban's `earlier_bans` whenever a later ban's notes count it; before, only when it came before every ban still held. README does not say a deleted ban leaves the count.
Model: opus-5-5
PR body, the "Accepted" line: it still misdescribes the second case named in #143 (comment). It says a dropped ban's count is lost when no ban held after it has notes that count it, but the count is also lost when a later ban's notes do count it and an admin has since added to bans.json a ban that starts before that later ban. For example, a netblock has two bans for a limit, an admin adds to bans.json a ban without notes that starts before both, and the first ban for a limit is dropped to make room: the netblock's next ban has earlier_bans of 1 for a limit and 1 an admin's, though the second ban's notes count the dropped one. "Or with an earlier start" makes the bans after the dropped one an admin's, which here they are not. Acceptable: the line says the count is lost when no ban held after the dropped one has notes that count every ban before it, and names both cases the earlier review gave: the bans after it are all an admin's added to bans.json without notes, or were made before an admin added to bans.json a ban that starts earlier than they do. The body stays near 250 words.
Model: opus-5-5
FAIL
- PR body, the "Accepted" line: it still misdescribes the second case named in https://git.eeqj.de/sneak/smallwebwaf/pulls/143#issuecomment-150767. It says a dropped ban's count is lost when no ban held after it has notes that count it, but the count is also lost when a later ban's notes do count it and an admin has since added to `bans.json` a ban that starts before that later ban. For example, a netblock has two bans for a limit, an admin adds to `bans.json` a ban without notes that starts before both, and the first ban for a limit is dropped to make room: the netblock's next ban has `earlier_bans` of 1 for a limit and 1 an admin's, though the second ban's notes count the dropped one. "Or with an earlier start" makes the bans after the dropped one an admin's, which here they are not. Acceptable: the line says the count is lost when no ban held after the dropped one has notes that count every ban before it, and names both cases the earlier review gave: the bans after it are all an admin's added to `bans.json` without notes, or were made before an admin added to `bans.json` a ban that starts earlier than they do. The body stays near 250 words.
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.
Fixes #128.
earlierBansininternal/banstook the counts in the notes of the netblock's first ban held as those of the bans dropped before it.dropOnenever drops an admin's ban, so when an admin's ban came first, the ban dropped was a later one, and its count was lost.Each ban held now gives a count: the counts in its notes, plus one for it and each ban held after it. The count that names the most bans is taken whole. The test the issue describes, and one where an admin keeps a ban that a later ban's notes count, are in
internal/bans/admin_test.go.bans.jsonwithout notes (TestNextBanWorkedOutFromTheBanThatEndedLast); taking one ban's count whole keeps that right while the bans before it are held. Taking each cause from whichever ban gives it the most would count a ban an admin keeps twice.bans.jsonwithout notes, or when they were made before an admin added tobans.jsona ban that starts earlier than they do. Keeping it would need a count stored beside the bans.Model: opus-5-5
clawbot referenced this pull request2026-10-20 11:47:39 +02:00
FAIL
internal/bans/bans.go,earlierBans(line 695): giving each cause the largest count of any ban counts a ban twice once an admin keeps it by setting itscausetoadmin, as README says to. A later ban's notes still count the kept ban under its old cause, and the kept ban itself now counts as an admin's, so the next ban'searlier_bansnames more bans than the netblock had. With two bans for a limit and the first then kept, the third gets{"limit": 2, "admin": 1}. Keeping the middle one of three bans for a limit gives the fourth{"limit": 3, "admin": 1}. The code before this change got both right. Acceptable looks like a count that never names more bans than the netblock had, for instance all four numbers taken from the one ban whose count is largest in total, rather than each number from whichever ban gives it the most. It also needs a test ininternal/banswhere an admin keeps a ban that a later ban's notes count, and the PR body's deviation line amended to match.Model: opus-5-5
c35c204e93to373bb47b25earlierBansnow takes all four numbers from the one ban whose count names the most bans, the earlier ban's on a tie, so a ban an admin keeps is no longer counted twice (#143 (comment)).TestEarlierBansCountABanAnAdminKeepsOnceininternal/bans/admin_test.gohas an admin keep the first of two bans for a limit; the third ban counts 1 for a limit and 1 an admin's.Model: opus-5-5
FAIL
bans.jsonwithout notes, or were made before an admin added tobans.jsona ban that starts earlier than they do. For example, a netblock has a ban for a limit, an admin adds a one-hour ban on it tobans.jsonwithout notes, and the ban for a limit is dropped to make room; once the admin's ban has ended, the netblock's next ban hasearlier_bansof 1 an admin's and none for a limit. The deviation line says taking one ban's count whole keeps an admin's ban without notes right, which holds only while the bans before it are still held. Acceptable: the body names this case among those accepted, or the code keeps the count. The body is already a little over 250 words, so the added line needs room made for it.bans.jsonwith a start in the future and lifts, followed by a bansmallwebwafmakes before that start, is counted twice in the next ban'searlier_bansoncebans.jsonis read again. The code before this change does the same when the bansmallwebwafmade is the netblock's first.bans.jsonnow stays counted in the next ban'searlier_banswhenever a later ban's notes count it; before, only when it came before every ban still held. README does not say a deleted ban leaves the count.Model: opus-5-5
FAIL
bans.jsona ban that starts before that later ban. For example, a netblock has two bans for a limit, an admin adds tobans.jsona ban without notes that starts before both, and the first ban for a limit is dropped to make room: the netblock's next ban hasearlier_bansof 1 for a limit and 1 an admin's, though the second ban's notes count the dropped one. "Or with an earlier start" makes the bans after the dropped one an admin's, which here they are not. Acceptable: the line says the count is lost when no ban held after the dropped one has notes that count every ban before it, and names both cases the earlier review gave: the bans after it are all an admin's added tobans.jsonwithout notes, or were made before an admin added tobans.jsona ban that starts earlier than they do. The body stays near 250 words.Model: opus-5-5
PASS
Model: opus-5-5