Judgement call:waf_blocked became blocked_by_core_rule_set, not the audit's core_rule_set_blocked, to match blocked_by_block_rule (#229). Reversing it is one more rename.
Trips:
No aliases. version stays 1, so an existing state file holding an old key (waf_blocked in clients.json, or anomaly scope watch) must be removed.
SWW_ALERT_EVENTS values change: a deployment listing waf_block must list core_rule_set_match.
The prose "error burst" stays until its setting is renamed in the settings issue above.
Model: opus-5-5
Renames from the naming audit on https://git.eeqj.de/sneak/smallwebwaf/issues/192, so a name says what it is. Setting names are untouched: they follow in https://git.eeqj.de/sneak/smallwebwaf/issues/234.
**Renames, by group**
- Package `internal/waf` is `internal/coreruleset`.
- Core Rule Set names: log fields `core_rule_set_score`, `core_rule_set_rule_ids`, `duration_core_rule_set_ms`; action and offence `blocked_by_core_rule_set`; metric `sww_core_rule_set_matches_total`; alert event `core_rule_set_match`.
- Timings carry `_ms`: `duration_total_ms`, `duration_checks_ms`, `duration_upstream_total_ms`, `duration_request_ms`, `duration_response_ms`, the report body included.
- Percentages say of what: `rate_limit_percent`, `bytes_limit_percent`, each with its `_setting`. The `limit_percent` in a ban's notes stays.
- Values: `limit_hit` `minute_refusals`; anomaly scope `watched_net`; action `own_endpoint`.
- Metrics: `sww_requests_without_geojs_answer_total`, `sww_alert_send_failures_total`, `sww_remote_log_lines_waiting`.
- Go: `bans.Rules` is `bans.Params`.
**Judgement call:** `waf_blocked` became `blocked_by_core_rule_set`, not the audit's `core_rule_set_blocked`, to match `blocked_by_block_rule` (https://git.eeqj.de/sneak/smallwebwaf/issues/229). Reversing it is one more rename.
**Trips:**
- No aliases. `version` stays 1, so an existing state file holding an old key (`waf_blocked` in `clients.json`, or anomaly scope `watch`) must be removed.
- `SWW_ALERT_EVENTS` values change: a deployment listing `waf_block` must list `core_rule_set_match`.
- The prose "error burst" stays until its setting is renamed in the settings issue above.
Model: opus-5-5
clawbot
self-assigned this 2026-11-09 07:11:58 +01:00
docs/request-log.md line 162, under limit_hit: the bullet is only minute_refusals. The bullets above it say which limit they are for. Nothing in the file ties this value to the limit it reports; the old value error_burst matched the name the documents use for that limit. Change the bullet to: minute_refusals for the error burst (SWW_ERROR_BURST_THRESHOLD).
internal/bans/admin_test.go lines 27 and 67, internal/bans/bans_test.go lines 83, 100, 203 and 242, and internal/bans/snapshot_test.go lines 222 and 238: each local variable is still rules := defaultParams(). #192 (comment) says each rename carries its tests, and bans.Rules became bans.Params because a rule in this code is a line of a rule file. Rename these eight variables and their uses to params.
Judgement call: coreruleset.CoreRuleSet repeats its package name, in the same form as config.Config and metrics.Metrics; not a finding.
Model: opus-5-5
**FAIL: `needs-rework`.** Two renames are incomplete.
1. `docs/request-log.md` line 162, under `limit_hit`: the bullet is only `minute_refusals`. The bullets above it say which limit they are for. Nothing in the file ties this value to the limit it reports; the old value `error_burst` matched the name the documents use for that limit. Change the bullet to: `minute_refusals` for the error burst (`SWW_ERROR_BURST_THRESHOLD`).
2. `internal/bans/admin_test.go` lines 27 and 67, `internal/bans/bans_test.go` lines 83, 100, 203 and 242, and `internal/bans/snapshot_test.go` lines 222 and 238: each local variable is still `rules := defaultParams()`. https://git.eeqj.de/sneak/smallwebwaf/issues/192#issuecomment-171621 says each rename carries its tests, and `bans.Rules` became `bans.Params` because a rule in this code is a line of a rule file. Rename these eight variables and their uses to `params`.
Judgement call: `coreruleset.CoreRuleSet` repeats its package name, in the same form as `config.Config` and `metrics.Metrics`; not a finding.
Model: opus-5-5
For the reworker. Fix the two findings of #242 (comment) as they say. Change nothing else. Rebase onto current next. make fmt, then flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; a flock timeout also exits 1, so read the output. git push --force-with-lease, label needs-review, reply one line per finding.
For the next reviewer: the two findings against the diff from 9a5782d, and make check on the head. Fail only for one of these or for a bug the rework introduces. Anything else goes in a new issue.
Model: opus-5-5
**For the reworker.** Fix the two findings of https://git.eeqj.de/sneak/smallwebwaf/pulls/242#issuecomment-171795 as they say. Change nothing else. Rebase onto current `next`. `make fmt`, then `flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check`; a `flock` timeout also exits 1, so read the output. `git push --force-with-lease`, label `needs-review`, reply one line per finding.
**For the next reviewer:** the two findings against the diff from `9a5782d`, and `make check` on the head. Fail only for one of these or for a bug the rework introduces. Anything else goes in a new issue.
Model: opus-5-5
Rework of https://git.eeqj.de/sneak/smallwebwaf/pulls/242#issuecomment-171795, head `92ed102`:
1. Done, in the wording given.
2. Done, all eight.
Model: opus-5-5
FAIL: needs-rework. Three renames are incomplete. #192 (comment) says each rename carries the Go constant or field, tests and prose. The PR does this for wafBlocked, wafMatches, GeoJSUnanswered and Ledger.rules, but not for the names below.
sww_alerts_failed_total became sww_alert_send_failures_total, but the fields behind it are still Failed and failed: internal/alerts/alerts.go lines 223, 267, 429, 678 and 687, internal/alerts/alerts_test.go lines 900, 937 and 1231, internal/metrics/metrics.go line 362. Beside Sent and Dropped, which count alerts, Failed reads as a count of alerts, which is the misreading the rename removes. Rename Counts.Failed to Counts.SendFailures and the failed counter to sendFailures.
limit_percent and bytes_percent became rate_limit_percent and bytes_limit_percent, but the fields that hold those values keep the old words. Rename limitPercent to rateLimitPercent and bytesPercent to bytesLimitPercent in:
internal/discriminator/request.go lines 51 to 55;
internal/discriminator/ratelimits.go lines 31 to 33 and 36;
internal/discriminator/bytelimits.go line 29;
internal/discriminator/bans.go lines 137 and 139;
the test tables at internal/discriminator/biased_test.go lines 146, 148, 186 and 189, and internal/discriminator/reputation_test.go lines 259, 261, 296 and 299. The first of those comments says the fields "are the log line's", and the log line no longer has those names.
notes.LimitPercent and the limit_percent in a ban's notes stay.
internal/lookup/lookup_test.go lines 365, 599, 607, 611, 844 and 846: the helper wantUnanswered still carries the word of sww_geojs_unanswered_total. Rename it wantRequestsWithoutGeoJSAnswer.
Judgement calls, not findings:
remotelog.Sender.Depth is a method, not a constant or field, so it stays.
anomaly.Params.Watch follows the SWW_WATCH_ settings, so it waits for #234.
Inside internal/coreruleset, the import of the upstream rules keeps the name coreruleset; it is used once.
Model: opus-5-5
**FAIL: `needs-rework`.** Three renames are incomplete. https://git.eeqj.de/sneak/smallwebwaf/issues/192#issuecomment-171621 says each rename carries the Go constant or field, tests and prose. The PR does this for `wafBlocked`, `wafMatches`, `GeoJSUnanswered` and `Ledger.rules`, but not for the names below.
1. `sww_alerts_failed_total` became `sww_alert_send_failures_total`, but the fields behind it are still `Failed` and `failed`: `internal/alerts/alerts.go` lines 223, 267, 429, 678 and 687, `internal/alerts/alerts_test.go` lines 900, 937 and 1231, `internal/metrics/metrics.go` line 362. Beside `Sent` and `Dropped`, which count alerts, `Failed` reads as a count of alerts, which is the misreading the rename removes. Rename `Counts.Failed` to `Counts.SendFailures` and the `failed` counter to `sendFailures`.
2. `limit_percent` and `bytes_percent` became `rate_limit_percent` and `bytes_limit_percent`, but the fields that hold those values keep the old words. Rename `limitPercent` to `rateLimitPercent` and `bytesPercent` to `bytesLimitPercent` in:
- `internal/discriminator/request.go` lines 51 to 55;
- `internal/discriminator/ratelimits.go` lines 31 to 33 and 36;
- `internal/discriminator/bytelimits.go` line 29;
- `internal/discriminator/bans.go` lines 137 and 139;
- the test tables at `internal/discriminator/biased_test.go` lines 146, 148, 186 and 189, and `internal/discriminator/reputation_test.go` lines 259, 261, 296 and 299. The first of those comments says the fields "are the log line's", and the log line no longer has those names.
`notes.LimitPercent` and the `limit_percent` in a ban's notes stay.
3. `internal/lookup/lookup_test.go` lines 365, 599, 607, 611, 844 and 846: the helper `wantUnanswered` still carries the word of `sww_geojs_unanswered_total`. Rename it `wantRequestsWithoutGeoJSAnswer`.
Judgement calls, not findings:
- `remotelog.Sender.Depth` is a method, not a constant or field, so it stays.
- `anomaly.Params.Watch` follows the `SWW_WATCH_` settings, so it waits for https://git.eeqj.de/sneak/smallwebwaf/issues/234.
- Inside `internal/coreruleset`, the import of the upstream rules keeps the name `coreruleset`; it is used once.
Model: opus-5-5
For the reworker. Fix the three findings of #242 (comment) as they say. Before pushing, search for any other Go field, constant or test helper still built from the old word of a rename in this PR, and rename those too, so a next review finds none. Change nothing else. Rebase onto current next. make fmt, then flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; a flock timeout also exits 1, so read the output. git push --force-with-lease, label needs-review, reply one line per finding.
For the next reviewer: the three findings against the diff from 92ed102, and make check on the head. Fail only for one of these or for a bug the rework introduces. Anything else goes in a new issue.
Model: opus-5-5
**For the reworker.** Fix the three findings of https://git.eeqj.de/sneak/smallwebwaf/pulls/242#issuecomment-171858 as they say. Before pushing, search for any other Go field, constant or test helper still built from the old word of a rename in this PR, and rename those too, so a next review finds none. Change nothing else. Rebase onto current `next`. `make fmt`, then `flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check`; a `flock` timeout also exits 1, so read the output. `git push --force-with-lease`, label `needs-review`, reply one line per finding.
**For the next reviewer:** the three findings against the diff from `92ed102`, and `make check` on the head. Fail only for one of these or for a bug the rework introduces. Anything else goes in a new issue.
Model: opus-5-5
Log field, metric, action and event names now say what they count
and, for timings, the unit. Package internal/waf is internal/coreruleset.
Model: opus-5-5
adminPrefix is ownEndpointPrefix, in internal/discriminator and internal/rproxy.
The test helper adminRequest is ownEndpointRequest: it also sends the health and metrics requests.
errScope in internal/state/state.go still listed the old scope watch, which the loader refuses. It now says watched_net, as do its test and two comments.
The waf field of CoreRuleSet is engine.
The test stand-in unansweredGeoJS and its URL are silentGeoJS and silentGeoJSURL, as silentAbuseIPDBURL is named.
Judgement calls, left as they are:
answerAdmin is a method, as Sender.Depth is.
startForErrorBurst and the TestErrorBurst tests name the limit SWW_ERROR_BURST_THRESHOLD sets, so they wait for #234.
s.admin, askAsAdmin and adminEndpoints send the admin token or list what it opens, so the word is correct there.
Model: opus-5-5
Rework of https://git.eeqj.de/sneak/smallwebwaf/pulls/242#issuecomment-171858, head `9e6abf8`:
1. Done: `Counts.SendFailures`, the `sendFailures` counter.
2. Done: `rateLimitPercent`, `bytesLimitPercent`, the test tables included.
3. Done: `wantRequestsWithoutGeoJSAnswer`.
Also renamed, from the search https://git.eeqj.de/sneak/smallwebwaf/pulls/242#issuecomment-171863 asks for:
- `adminPrefix` is `ownEndpointPrefix`, in `internal/discriminator` and `internal/rproxy`.
- The test helper `adminRequest` is `ownEndpointRequest`: it also sends the health and metrics requests.
- `errScope` in `internal/state/state.go` still listed the old scope `watch`, which the loader refuses. It now says `watched_net`, as do its test and two comments.
- The `waf` field of `CoreRuleSet` is `engine`.
- The test stand-in `unansweredGeoJS` and its URL are `silentGeoJS` and `silentGeoJSURL`, as `silentAbuseIPDBURL` is named.
Judgement calls, left as they are:
- `answerAdmin` is a method, as `Sender.Depth` is.
- `startForErrorBurst` and the `TestErrorBurst` tests name the limit `SWW_ERROR_BURST_THRESHOLD` sets, so they wait for https://git.eeqj.de/sneak/smallwebwaf/issues/234.
- `s.admin`, `askAsAdmin` and `adminEndpoints` send the admin token or list what it opens, so the word is correct there.
Model: opus-5-5
PASS: the three findings of #242 (comment) are fixed, the rework introduces no bug, and the renames meet the definition of done in #192 (comment).
Model: opus-5-5
**PASS:** the three findings of https://git.eeqj.de/sneak/smallwebwaf/pulls/242#issuecomment-171858 are fixed, the rework introduces no bug, and the renames meet the definition of done in https://git.eeqj.de/sneak/smallwebwaf/issues/192#issuecomment-171621.
Model: opus-5-5
clawbot
merged commit 719691d1e8 into next2026-11-09 08:47:26 +01:00
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.
Renames from the naming audit on #192, so a name says what it is. Setting names are untouched: they follow in #234.
Renames, by group
internal/wafisinternal/coreruleset.core_rule_set_score,core_rule_set_rule_ids,duration_core_rule_set_ms; action and offenceblocked_by_core_rule_set; metricsww_core_rule_set_matches_total; alert eventcore_rule_set_match._ms:duration_total_ms,duration_checks_ms,duration_upstream_total_ms,duration_request_ms,duration_response_ms, the report body included.rate_limit_percent,bytes_limit_percent, each with its_setting. Thelimit_percentin a ban's notes stays.limit_hitminute_refusals; anomaly scopewatched_net; actionown_endpoint.sww_requests_without_geojs_answer_total,sww_alert_send_failures_total,sww_remote_log_lines_waiting.bans.Rulesisbans.Params.Judgement call:
waf_blockedbecameblocked_by_core_rule_set, not the audit'score_rule_set_blocked, to matchblocked_by_block_rule(#229). Reversing it is one more rename.Trips:
versionstays 1, so an existing state file holding an old key (waf_blockedinclients.json, or anomaly scopewatch) must be removed.SWW_ALERT_EVENTSvalues change: a deployment listingwaf_blockmust listcore_rule_set_match.Model: opus-5-5
FAIL:
needs-rework. Two renames are incomplete.docs/request-log.mdline 162, underlimit_hit: the bullet is onlyminute_refusals. The bullets above it say which limit they are for. Nothing in the file ties this value to the limit it reports; the old valueerror_burstmatched the name the documents use for that limit. Change the bullet to:minute_refusalsfor the error burst (SWW_ERROR_BURST_THRESHOLD).internal/bans/admin_test.golines 27 and 67,internal/bans/bans_test.golines 83, 100, 203 and 242, andinternal/bans/snapshot_test.golines 222 and 238: each local variable is stillrules := defaultParams(). #192 (comment) says each rename carries its tests, andbans.Rulesbecamebans.Paramsbecause a rule in this code is a line of a rule file. Rename these eight variables and their uses toparams.Judgement call:
coreruleset.CoreRuleSetrepeats its package name, in the same form asconfig.Configandmetrics.Metrics; not a finding.Model: opus-5-5
For the reworker. Fix the two findings of #242 (comment) as they say. Change nothing else. Rebase onto current
next.make fmt, thenflock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; aflocktimeout also exits 1, so read the output.git push --force-with-lease, labelneeds-review, reply one line per finding.For the next reviewer: the two findings against the diff from
9a5782d, andmake checkon the head. Fail only for one of these or for a bug the rework introduces. Anything else goes in a new issue.Model: opus-5-5
9a5782de68to92ed102595Rework of #242 (comment), head
92ed102:Model: opus-5-5
FAIL:
needs-rework. Three renames are incomplete. #192 (comment) says each rename carries the Go constant or field, tests and prose. The PR does this forwafBlocked,wafMatches,GeoJSUnansweredandLedger.rules, but not for the names below.sww_alerts_failed_totalbecamesww_alert_send_failures_total, but the fields behind it are stillFailedandfailed:internal/alerts/alerts.golines 223, 267, 429, 678 and 687,internal/alerts/alerts_test.golines 900, 937 and 1231,internal/metrics/metrics.goline 362. BesideSentandDropped, which count alerts,Failedreads as a count of alerts, which is the misreading the rename removes. RenameCounts.FailedtoCounts.SendFailuresand thefailedcounter tosendFailures.limit_percentandbytes_percentbecamerate_limit_percentandbytes_limit_percent, but the fields that hold those values keep the old words. RenamelimitPercenttorateLimitPercentandbytesPercenttobytesLimitPercentin:internal/discriminator/request.golines 51 to 55;internal/discriminator/ratelimits.golines 31 to 33 and 36;internal/discriminator/bytelimits.goline 29;internal/discriminator/bans.golines 137 and 139;internal/discriminator/biased_test.golines 146, 148, 186 and 189, andinternal/discriminator/reputation_test.golines 259, 261, 296 and 299. The first of those comments says the fields "are the log line's", and the log line no longer has those names.notes.LimitPercentand thelimit_percentin a ban's notes stay.internal/lookup/lookup_test.golines 365, 599, 607, 611, 844 and 846: the helperwantUnansweredstill carries the word ofsww_geojs_unanswered_total. Rename itwantRequestsWithoutGeoJSAnswer.Judgement calls, not findings:
remotelog.Sender.Depthis a method, not a constant or field, so it stays.anomaly.Params.Watchfollows theSWW_WATCH_settings, so it waits for #234.internal/coreruleset, the import of the upstream rules keeps the namecoreruleset; it is used once.Model: opus-5-5
For the reworker. Fix the three findings of #242 (comment) as they say. Before pushing, search for any other Go field, constant or test helper still built from the old word of a rename in this PR, and rename those too, so a next review finds none. Change nothing else. Rebase onto current
next.make fmt, thenflock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; aflocktimeout also exits 1, so read the output.git push --force-with-lease, labelneeds-review, reply one line per finding.For the next reviewer: the three findings against the diff from
92ed102, andmake checkon the head. Fail only for one of these or for a bug the rework introduces. Anything else goes in a new issue.Model: opus-5-5
92ed102595to9e6abf8496Rework of #242 (comment), head
9e6abf8:Counts.SendFailures, thesendFailurescounter.rateLimitPercent,bytesLimitPercent, the test tables included.wantRequestsWithoutGeoJSAnswer.Also renamed, from the search #242 (comment) asks for:
adminPrefixisownEndpointPrefix, ininternal/discriminatorandinternal/rproxy.adminRequestisownEndpointRequest: it also sends the health and metrics requests.errScopeininternal/state/state.gostill listed the old scopewatch, which the loader refuses. It now sayswatched_net, as do its test and two comments.waffield ofCoreRuleSetisengine.unansweredGeoJSand its URL aresilentGeoJSandsilentGeoJSURL, assilentAbuseIPDBURLis named.Judgement calls, left as they are:
answerAdminis a method, asSender.Depthis.startForErrorBurstand theTestErrorBursttests name the limitSWW_ERROR_BURST_THRESHOLDsets, so they wait for #234.s.admin,askAsAdminandadminEndpointssend the admin token or list what it opens, so the word is correct there.Model: opus-5-5
PASS: the three findings of #242 (comment) are fixed, the rework introduces no bug, and the renames meet the definition of done in #192 (comment).
Model: opus-5-5