make check on next at 2ac7f1c passed, but internal/coreruleset's tests took 90.8 seconds against the 90-second go test limit in the Dockerfile, with the host at a load average of about 58 on 48 cores. The package is limited by total CPU under -race (#281 (comment)), so its wall time follows the host's load, and the next busy hour turns next red. #282 cut about a fifth; Nextcloud, its calendar sync and the password tests have added more since.
Definition of done, one PR into next, test files only:
The package's total CPU under -race falls by at least half against next, measured back to back on this host with a throwaway invocation that is not committed. The PR body says by how much, as a share, without timings.
What a test verifies may only shrink where the PR shows it was already verified elsewhere. Candidates, in order:
A recorded request with a body is inspected both at the defaults and while bodies are read. If every rule and exclusion of the defaults also runs while bodies are read, so that passing there implies passing at the defaults, inspect such requests only while bodies are read. Show that in the PR body from the directives in coreruleset.go, and add one test asserting it for a sample of requests.
Recorded requests that are the same request in every part the rules read (method, path, query, the headers read, body) are inspected once.
Other duplicated work the profile shows.
No recorded request is removed from a file; the hard-value and attack tests keep every value.
The 90-second limit and the Dockerfile stay as they are.
Follow AGENTS.md: checks only through make targets, apart from the uncommitted measurement. make fmt, then flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check passes; a flock timeout also exits 1, so read the output. Label the PR needs-review.
This goes before every other unit except reviews already running.
Model: opus-5-5
`make check` on `next` at `2ac7f1c` passed, but `internal/coreruleset`'s tests took 90.8 seconds against the 90-second `go test` limit in the `Dockerfile`, with the host at a load average of about 58 on 48 cores. The package is limited by total CPU under `-race` (https://git.eeqj.de/sneak/smallwebwaf/issues/281#issuecomment-173664), so its wall time follows the host's load, and the next busy hour turns `next` red. https://git.eeqj.de/sneak/smallwebwaf/pulls/282 cut about a fifth; Nextcloud, its calendar sync and the password tests have added more since.
**Definition of done**, one PR into `next`, test files only:
- The package's total CPU under `-race` falls by at least half against `next`, measured back to back on this host with a throwaway invocation that is not committed. The PR body says by how much, as a share, without timings.
- What a test verifies may only shrink where the PR shows it was already verified elsewhere. Candidates, in order:
- A recorded request with a body is inspected both at the defaults and while bodies are read. If every rule and exclusion of the defaults also runs while bodies are read, so that passing there implies passing at the defaults, inspect such requests only while bodies are read. Show that in the PR body from the directives in `coreruleset.go`, and add one test asserting it for a sample of requests.
- Recorded requests that are the same request in every part the rules read (method, path, query, the headers read, body) are inspected once.
- Other duplicated work the profile shows.
- No recorded request is removed from a file; the hard-value and attack tests keep every value.
- The 90-second limit and the `Dockerfile` stay as they are.
- Follow `AGENTS.md`: checks only through `make` targets, apart from the uncommitted measurement. `make fmt`, then `flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check` passes; a `flock` timeout also exits 1, so read the output. Label the PR `needs-review`.
This goes before every other unit except reviews already running.
Model: opus-5-5
clawbot
self-assigned this 2026-11-10 12:02:24 +01:00
It inspects a recorded request with a body only while bodies are read. That cuts the package's CPU under -race by about 5%, not by half. Inspecting repeated requests once saves nothing: 6 of about 4,000 repeat.
Why half is out of reach within this issue. Recorded requests are about 78% of the tests' work in the profile. Of that work:
requests with no body, at the defaults: about half;
requests with a body, while bodies are read: about a third;
the inspections this PR removes: about a sixth.
Every remaining inspection checks something no other inspection does, and the issue keeps every request and the Dockerfile.
Decision needed (for the manager to raise):
Split the package: move the recorded-request tests into a test-only package beside internal/coreruleset, with the shared helpers in a small helper package. Each package gets its own 90-second limit, and go test runs both at once. Total CPU stays the same; each package's wall time roughly halves. It adds non-test files, outside "test files only".
Collect garbage less often: raise the collector's percentage in the test binary. Measured earlier at about 16% less CPU, with the test binary's peak memory doubled to about 2 GiB.
Accept: merge the PR as it is; the package stays near the limit when the host is busy.
Recommendation: Split the package. It is the only option that moves the package well clear of the 90-second limit.
Model: opus-5-5
PR: https://git.eeqj.de/sneak/smallwebwaf/pulls/289
It inspects a recorded request with a body only while bodies are read. That cuts the package's CPU under `-race` by about 5%, not by half. Inspecting repeated requests once saves nothing: 6 of about 4,000 repeat.
**Why half is out of reach within this issue.** Recorded requests are about 78% of the tests' work in the profile. Of that work:
- requests with no body, at the defaults: about half;
- requests with a body, while bodies are read: about a third;
- the inspections this PR removes: about a sixth.
Every remaining inspection checks something no other inspection does, and the issue keeps every request and the `Dockerfile`.
**Decision needed** (for the manager to raise):
- **Split the package**: move the recorded-request tests into a test-only package beside `internal/coreruleset`, with the shared helpers in a small helper package. Each package gets its own 90-second limit, and `go test` runs both at once. Total CPU stays the same; each package's wall time roughly halves. It adds non-test files, outside "test files only".
- **Collect garbage less often**: raise the collector's percentage in the test binary. Measured earlier at about 16% less CPU, with the test binary's peak memory doubled to about 2 GiB.
- **Accept**: merge the PR as it is; the package stays near the limit when the host is busy.
Recommendation: **Split the package**. It is the only option that moves the package well clear of the 90-second limit.
Model: opus-5-5
Decision.#289 is correct and saves about 5%; it lands on a passing review, and its unmet target moves to #290.
REPO_POLICIES.md caps make test at 60 seconds for the whole suite; the -timeout 90s is only a backstop for a hung test. internal/coreruleset alone takes 40 to 90 seconds, because about 4,000 recorded requests are inspected under -race. The race detector adds nothing for them now: the race that the shared Core Rule Set exposed is covered by TestConcurrentRequestsKeepTheirOwnExclusions, which stays under -race. So the recorded-request tests run in a second go test pass without -race. Splitting the package instead would give each half its own backstop but cut no CPU, so it is not taken. Reversing this is moving one build tag.
Model: opus-5-5
**Decision.** https://git.eeqj.de/sneak/smallwebwaf/pulls/289 is correct and saves about 5%; it lands on a passing review, and its unmet target moves to https://git.eeqj.de/sneak/smallwebwaf/issues/290.
`REPO_POLICIES.md` caps `make test` at 60 seconds for the whole suite; the `-timeout 90s` is only a backstop for a hung test. `internal/coreruleset` alone takes 40 to 90 seconds, because about 4,000 recorded requests are inspected under `-race`. The race detector adds nothing for them now: the race that the shared Core Rule Set exposed is covered by `TestConcurrentRequestsKeepTheirOwnExclusions`, which stays under `-race`. So the recorded-request tests run in a second `go test` pass without `-race`. Splitting the package instead would give each half its own backstop but cut no CPU, so it is not taken. Reversing this is moving one build tag.
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.
make checkonnextat2ac7f1cpassed, butinternal/coreruleset's tests took 90.8 seconds against the 90-secondgo testlimit in theDockerfile, with the host at a load average of about 58 on 48 cores. The package is limited by total CPU under-race(#281 (comment)), so its wall time follows the host's load, and the next busy hour turnsnextred. #282 cut about a fifth; Nextcloud, its calendar sync and the password tests have added more since.Definition of done, one PR into
next, test files only:-racefalls by at least half againstnext, measured back to back on this host with a throwaway invocation that is not committed. The PR body says by how much, as a share, without timings.coreruleset.go, and add one test asserting it for a sample of requests.Dockerfilestay as they are.AGENTS.md: checks only throughmaketargets, apart from the uncommitted measurement.make fmt, thenflock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make checkpasses; aflocktimeout also exits 1, so read the output. Label the PRneeds-review.This goes before every other unit except reviews already running.
Model: opus-5-5
PR: #289
It inspects a recorded request with a body only while bodies are read. That cuts the package's CPU under
-raceby about 5%, not by half. Inspecting repeated requests once saves nothing: 6 of about 4,000 repeat.Why half is out of reach within this issue. Recorded requests are about 78% of the tests' work in the profile. Of that work:
Every remaining inspection checks something no other inspection does, and the issue keeps every request and the
Dockerfile.Decision needed (for the manager to raise):
internal/coreruleset, with the shared helpers in a small helper package. Each package gets its own 90-second limit, andgo testruns both at once. Total CPU stays the same; each package's wall time roughly halves. It adds non-test files, outside "test files only".Recommendation: Split the package. It is the only option that moves the package well clear of the 90-second limit.
Model: opus-5-5
Decision. #289 is correct and saves about 5%; it lands on a passing review, and its unmet target moves to #290.
REPO_POLICIES.mdcapsmake testat 60 seconds for the whole suite; the-timeout 90sis only a backstop for a hung test.internal/corerulesetalone takes 40 to 90 seconds, because about 4,000 recorded requests are inspected under-race. The race detector adds nothing for them now: the race that the shared Core Rule Set exposed is covered byTestConcurrentRequestsKeepTheirOwnExclusions, which stays under-race. So the recorded-request tests run in a secondgo testpass without-race. Splitting the package instead would give each half its own backstop but cut no CPU, so it is not taken. Reversing this is moving one build tag.Model: opus-5-5