max_retries was read through parseNonNegativeInt, which returned 0 for any parse failure. It now goes through one validator shared by the create and edit paths.
What was wrong
Confirmed by execution before the fix. Create form: 999999999 stored verbatim; abc, 2.7, -5 and a 20-digit number all accepted with HTTP 200 and stored as 0. Edit form: a target delivering with max_retries=2, re-saved with max_retries=abc, returned HTTP 200 and was stored as 0 — a working retry configuration destroyed by a typo, silently turning a store-and-forward proxy into fire-and-forget.
The rule applied
A value that is SET BUT UNPARSEABLE is rejected loudly. A default belongs ONLY to an ABSENT value. parseMaxRetries(raw string, fallback int) makes the distinction explicit in the code:
empty or omitted → fallback (0 at creation, the stored count on edit)
unparseable, negative, or above the ceiling → error, 400, nothing written
Blank and whitespace-only count as absent, because they already do for the timeout and retention_days controls on the same forms; a rule those fields did not share would be its own surprise.
Both paths, from one validator
processTargetCreate and applyTargetEdit both call targetMaxRetries, so they cannot come to disagree — TestTargetRetries_CreateAndEditAgreeOnEveryCase submits every input to both and compares the verdicts. The edit path keeps its existing PostForm.Has("max_retries") guard, so a form for a target type that does not render the field still does not touch retries; validation is inside that guard, and runs before anything is assigned to the target.
Wording matches the timeout control on the same submission, which already got this right:
Invalid max retries: retries must be a whole number of attempts, or 0 for fire-and-forget
Invalid max retries: retries out of range: at most 20 retries
The ceiling: 20
Chosen, not invented. Both templates/source_detail.html and templates/target_edit.html have always declared max="20" on the input — the server simply never enforced what the UI advertises, so this closes the gap rather than introducing a new limit, and no template changes.
It is also what the delivery engine can usefully do with the value. Backoff is 2^(n-1) seconds capped at 2^30, so attempt 20 is already about six days after the first; beyond that the value buys no durability. Every attempt writes a delivery_results row that the event log then loads and renders, so an absurd ceiling is itself a resource problem — the issue's 100000 case would be 100000 rows for one delivery.
Audit of every other parseNonNegativeInt caller
The helper was the defect, so it is gone. Its callers, all of them:
Caller
Field
Disposition
processTargetCreate (source_management.go)
max_retries
Fixed. Validates, rejects.
applyTargetEdit (target_edit.go)
max_retries
Fixed. Same validator.
finishResubmit (event_resubmit.go)
page
Justified. See below.
finishReplay (delivery_replay.go)
page
Justified. See below.
The two page readers are not the same bug. A page number says where to send the browser after an action, not what to store: it is never persisted, never delivered, and by the time it is read the resubmit or replay has already completed — answering 400 would report a failure that did not happen. Falling back to page 1 is correct there, and is already what parsePage does for the same parameter on the GET side.
What was wrong was the shape: a general-purpose silently-coercing int parser sitting in the package for the next configuration field to reach for. Those callers now use pageOrFirst, named for what it does, documented as to why it is the exception, and shared with parsePage so the log page number is parsed one way. parseNonNegativeInt no longer exists.
Existing stored values
Input validation, not a migration. Nothing clamps a row. Verified on a running instance with a target set to max_retries=100000 directly in the database: source detail page HTTP 200 showing 100000, edit page HTTP 200 pre-filling value="100000", and the row still 100000 afterwards. Delivery is untouched — no change in internal/delivery.
One consequence, called out deliberately: re-saving such a target from the edit form does have to bring it into range, because the form submits the pre-filled value back and accepting it would mean the ceiling does not apply to the edit path. The 400 names the ceiling, so the fix is one field. This is documented on parseMaxRetries.
Verification
make checkgreen, run with GOFLAGS=-count=1 on the committed tree: exit 0, 570 tests passed, 0 failures, lint 0 issues (in Docker via the script/ entrypoint, as always). The ten new tests all show real durations rather than cached results.
Tests cover, on BOTH create and edit: above-ceiling rejected with the ceiling named; abc, 2.7, -5, a 20-digit number and 1e3 rejected rather than coerced to 0; valid values including 0 and the ceiling itself accepted and stored; an absent field still taking its default, so the two cases stay provably distinct; and a rejected edit leaving both the retry count and the name untouched, proving nothing at all was written.
Live instance, the reported case
The target from the report: max_retries=2, re-saved from the edit form with abc.
--- the reported case: edit form, max_retries=abc ---
Invalid max retries: retries must be a whole number of attempts, or 0 for fire-and-forget
HTTP 400
stored after: max_retries=2 name=evidence-target
The 400 is answered, the working retry count survives, and the name change in the same submission was not written either.
--- edit form, max_retries=999999999 ---
Invalid max retries: retries out of range: at most 20 retries
HTTP 400
stored after: max_retries=2
--- create form, each reported value ---
max_retries=999999999 HTTP 400 rows=0
max_retries=abc HTTP 400 rows=0
max_retries=2.7 HTTP 400 rows=0
max_retries=-5 HTTP 400 rows=0
max_retries=12345678901234567890 HTTP 400 rows=0
max_retries=21 HTTP 400 rows=0
--- valid values still accepted ---
max_retries=0 HTTP 303 stored=0
max_retries=5 HTTP 303 stored=5
max_retries=20 HTTP 303 stored=20
Scope
internal/handlers and its tests only. No change to internal/delivery, internal/database, internal/middleware, the templates, or TODO.md.
Closes https://git.eeqj.de/sneak/webhooker/issues/221.
`max_retries` was read through `parseNonNegativeInt`, which returned 0 for any parse failure. It now goes through one validator shared by the create and edit paths.
## What was wrong
Confirmed by execution before the fix. Create form: `999999999` stored verbatim; `abc`, `2.7`, `-5` and a 20-digit number all accepted with HTTP 200 and stored as **0**. Edit form: a target delivering with `max_retries=2`, re-saved with `max_retries=abc`, returned HTTP 200 and was stored as 0 — a working retry configuration destroyed by a typo, silently turning a store-and-forward proxy into fire-and-forget.
## The rule applied
A value that is SET BUT UNPARSEABLE is rejected loudly. A default belongs ONLY to an ABSENT value. `parseMaxRetries(raw string, fallback int)` makes the distinction explicit in the code:
- empty or omitted → `fallback` (0 at creation, the stored count on edit)
- unparseable, negative, or above the ceiling → error, 400, nothing written
Blank and whitespace-only count as absent, because they already do for the `timeout` and `retention_days` controls on the same forms; a rule those fields did not share would be its own surprise.
## Both paths, from one validator
`processTargetCreate` and `applyTargetEdit` both call `targetMaxRetries`, so they cannot come to disagree — `TestTargetRetries_CreateAndEditAgreeOnEveryCase` submits every input to both and compares the verdicts. The edit path keeps its existing `PostForm.Has("max_retries")` guard, so a form for a target type that does not render the field still does not touch retries; validation is inside that guard, and runs before anything is assigned to the target.
Wording matches the `timeout` control on the same submission, which already got this right:
Invalid max retries: retries must be a whole number of attempts, or 0 for fire-and-forget
Invalid max retries: retries out of range: at most 20 retries
## The ceiling: 20
Chosen, not invented. Both `templates/source_detail.html` and `templates/target_edit.html` have always declared `max="20"` on the input — the server simply never enforced what the UI advertises, so this closes the gap rather than introducing a new limit, and no template changes.
It is also what the delivery engine can usefully do with the value. Backoff is `2^(n-1)` seconds capped at `2^30`, so attempt 20 is already about six days after the first; beyond that the value buys no durability. Every attempt writes a `delivery_results` row that the event log then loads and renders, so an absurd ceiling is itself a resource problem — the issue's 100000 case would be 100000 rows for one delivery.
## Audit of every other `parseNonNegativeInt` caller
The helper was the defect, so it is gone. Its callers, all of them:
| Caller | Field | Disposition |
| --- | --- | --- |
| `processTargetCreate` (`source_management.go`) | `max_retries` | **Fixed.** Validates, rejects. |
| `applyTargetEdit` (`target_edit.go`) | `max_retries` | **Fixed.** Same validator. |
| `finishResubmit` (`event_resubmit.go`) | `page` | **Justified.** See below. |
| `finishReplay` (`delivery_replay.go`) | `page` | **Justified.** See below. |
The two `page` readers are not the same bug. A page number says where to send the browser after an action, not what to store: it is never persisted, never delivered, and by the time it is read the resubmit or replay has already completed — answering 400 would report a failure that did not happen. Falling back to page 1 is correct there, and is already what `parsePage` does for the same parameter on the GET side.
What was wrong was the *shape*: a general-purpose silently-coercing int parser sitting in the package for the next configuration field to reach for. Those callers now use `pageOrFirst`, named for what it does, documented as to why it is the exception, and shared with `parsePage` so the log page number is parsed one way. `parseNonNegativeInt` no longer exists.
## Existing stored values
Input validation, not a migration. Nothing clamps a row. Verified on a running instance with a target set to `max_retries=100000` directly in the database: source detail page HTTP 200 showing 100000, edit page HTTP 200 pre-filling `value="100000"`, and the row still 100000 afterwards. Delivery is untouched — no change in `internal/delivery`.
One consequence, called out deliberately: re-saving such a target from the edit form does have to bring it into range, because the form submits the pre-filled value back and accepting it would mean the ceiling does not apply to the edit path. The 400 names the ceiling, so the fix is one field. This is documented on `parseMaxRetries`.
## Verification
`make check` **green**, run with `GOFLAGS=-count=1` on the committed tree: exit 0, **570 tests passed, 0 failures**, lint `0 issues` (in Docker via the `script/` entrypoint, as always). The ten new tests all show real durations rather than cached results.
Tests cover, on BOTH create and edit: above-ceiling rejected with the ceiling named; `abc`, `2.7`, `-5`, a 20-digit number and `1e3` rejected rather than coerced to 0; valid values including 0 and the ceiling itself accepted and stored; an absent field still taking its default, so the two cases stay provably distinct; and a rejected edit leaving both the retry count and the name untouched, proving nothing at all was written.
### Live instance, the reported case
The target from the report: `max_retries=2`, re-saved from the edit form with `abc`.
--- the reported case: edit form, max_retries=abc ---
Invalid max retries: retries must be a whole number of attempts, or 0 for fire-and-forget
HTTP 400
stored after: max_retries=2 name=evidence-target
The 400 is answered, the working retry count survives, and the name change in the same submission was not written either.
--- edit form, max_retries=999999999 ---
Invalid max retries: retries out of range: at most 20 retries
HTTP 400
stored after: max_retries=2
--- create form, each reported value ---
max_retries=999999999 HTTP 400 rows=0
max_retries=abc HTTP 400 rows=0
max_retries=2.7 HTTP 400 rows=0
max_retries=-5 HTTP 400 rows=0
max_retries=12345678901234567890 HTTP 400 rows=0
max_retries=21 HTTP 400 rows=0
--- valid values still accepted ---
max_retries=0 HTTP 303 stored=0
max_retries=5 HTTP 303 stored=5
max_retries=20 HTTP 303 stored=20
## Scope
`internal/handlers` and its tests only. No change to `internal/delivery`, `internal/database`, `internal/middleware`, the templates, or `TODO.md`.
max_retries was read through parseNonNegativeInt, which returned 0 for
any parse failure. On the create form `abc`, `2.7`, `-5` and a
twenty-digit number were all accepted with HTTP 200 and stored as 0,
and `999999999` was stored verbatim with no ceiling. On the edit form
the same input destroyed a working retry configuration: a target
delivering with max_retries=2, re-saved with a typo in the field, was
silently left at 0 — fire-and-forget on a store-and-forward proxy,
with nothing said.
A value that is set but unparseable must be rejected loudly. A default
belongs only to an absent value. parseMaxRetries makes that
distinction explicit: an empty or omitted field yields the caller's
fallback (0 at creation, the stored count on edit), and anything else
that is not a whole number in range is a 400. Both forms go through
one validator, so they cannot come to disagree.
The ceiling is 20, which both target templates have always declared as
max="20" on the input; only the server never enforced it. Backoff is
2^(n-1) seconds, so attempt 20 is already about six days out, and each
attempt writes a delivery_results row the event log then loads and
renders.
The rejection wording matches the timeout control on the same
submission, which already got this right, and names the ceiling when
the value is out of range.
Existing rows above the ceiling are untouched: they still render on
the source and edit pages and still deliver. This is input validation,
not a migration.
parseNonNegativeInt is removed. Its other two callers read the log
page number for a post-action redirect, where falling back to page 1
is correct — it is navigation, not stored configuration, and the
action has already completed. They now use pageOrFirst, named for what
it does and shared with parsePage on the GET side, so no general
silently-coercing int parser is left for a configuration field to
reach for.
PASS — independent review. Definition of done in #221 is met, verified by execution against a running instance, not by the test suite: all 16 input classes (absent, empty, whitespace-only, 0, 5, 20, 21, 999999999, abc, 2.7, -5, a 20-digit number, +5, 0x10, " 7 ", 1e3) give identical accept/reject verdicts on the create and edit forms; the reported case (a target that had actually delivered at max_retries=2, re-saved with abc plus a rename and a URL change) answers 400 with max_retries, name and config all unchanged — no partial write; a row set to 100000 directly in the DB still renders on both pages, still pre-fills, and still delivers. make check green from a clean clone with GOFLAGS=-count=1 (exit 0, 100s, 570 passes, zero (cached), lint 0 issues in Docker), CI green on e65cb89, merges cleanly into next, commit message and trailers clean.
Anomalies and disclosures, none blocking:
Mutation-probed the tests rather than trusting them: with parseMaxRetries reverted to the old silent-coercion semantics, TestTargetCreate_UnparseableRetriesRejected, TestTargetEdit_UnparseableRetriesRejected, TestTargetCreate_RetriesAboveCeilingRejected and TestTargetEdit_RetriesAboveCeilingRejected all fail. TestTargetRetries_CreateAndEditAgreeOnEveryCase still passes under that mutation — it asserts agreement, not correctness, so it pins nothing on its own. Correct as designed, worth knowing.
A 20-digit value is rejected as retries must be a whole number of attempts rather than retries out of range, because strconv.Atoi returns ErrRange and the code branches only on err != nil. Loud rejection either way; the message is just less precise than it could be. Nit.
+5 stores 5 and " 7 " stores 7 (Atoi sign handling plus the TrimSpace). Lenient, never silently defaults.
Whitespace-only is treated as ABSENT, so on the edit path max_retries=" " keeps the stored count. That is a value the operator submitted taking a default, which is the edge of the iron rule; it is deliberate, matches parseRetentionDays and the timeout control on the same forms, and is covered by a test. Flagging it as the judgement call it is, not as a defect.
Audit of the removed helper verified rather than taken on faith: internal/handlers now contains exactly three integer parses — parseRetentionDays, parseMaxRetries (both reject loudly) and pageOrFirst. A garbage page yields 1 and the page > 1 guards behave exactly as before. Every other form field on these routes (timeout, headers, expiry, signature_scheme, retention_days) already rejects loudly. The disposition table is complete.
Two recommendations, owner's call, explicitly not blockers:
Ceiling of 20 is right. Checked the arithmetic against calcBackoff rather than the PR body: delay is 2^(n-1)s and maxRetries bounds total attempts, so attempt 20 lands 2^19 - 1s out, about 6.1 days. That covers a long weekend with room, and maxBackoffShift = 30 means the cap does not bind below attempt 31. If a longer window is ever wanted, the lever is the backoff curve, not a bigger retry count.
Requiring a legacy above-ceiling row to come into range on resave is acceptable. Confirmed live: re-saving the 100000 row with its own pre-filled value answers Invalid max retries: retries out of range: at most 20 retries and writes nothing. Grandfathering would mean the ceiling does not apply on the edit path at all; the 400 names the limit and the fix is one field.
**PASS** — independent review. Definition of done in https://git.eeqj.de/sneak/webhooker/issues/221 is met, verified by execution against a running instance, not by the test suite: all 16 input classes (absent, empty, whitespace-only, `0`, `5`, `20`, `21`, `999999999`, `abc`, `2.7`, `-5`, a 20-digit number, `+5`, `0x10`, `" 7 "`, `1e3`) give identical accept/reject verdicts on the create and edit forms; the reported case (a target that had actually delivered at `max_retries=2`, re-saved with `abc` plus a rename and a URL change) answers 400 with `max_retries`, `name` and `config` all unchanged — no partial write; a row set to `100000` directly in the DB still renders on both pages, still pre-fills, and still delivers. `make check` green from a clean clone with `GOFLAGS=-count=1` (exit 0, 100s, 570 passes, zero `(cached)`, lint 0 issues in Docker), CI green on `e65cb89`, merges cleanly into `next`, commit message and trailers clean.
Anomalies and disclosures, none blocking:
- Mutation-probed the tests rather than trusting them: with `parseMaxRetries` reverted to the old silent-coercion semantics, `TestTargetCreate_UnparseableRetriesRejected`, `TestTargetEdit_UnparseableRetriesRejected`, `TestTargetCreate_RetriesAboveCeilingRejected` and `TestTargetEdit_RetriesAboveCeilingRejected` all fail. `TestTargetRetries_CreateAndEditAgreeOnEveryCase` still passes under that mutation — it asserts agreement, not correctness, so it pins nothing on its own. Correct as designed, worth knowing.
- A 20-digit value is rejected as `retries must be a whole number of attempts` rather than `retries out of range`, because `strconv.Atoi` returns `ErrRange` and the code branches only on `err != nil`. Loud rejection either way; the message is just less precise than it could be. Nit.
- `+5` stores 5 and `" 7 "` stores 7 (`Atoi` sign handling plus the `TrimSpace`). Lenient, never silently defaults.
- Whitespace-only is treated as ABSENT, so on the edit path `max_retries=" "` keeps the stored count. That is a value the operator submitted taking a default, which is the edge of the iron rule; it is deliberate, matches `parseRetentionDays` and the `timeout` control on the same forms, and is covered by a test. Flagging it as the judgement call it is, not as a defect.
- Audit of the removed helper verified rather than taken on faith: `internal/handlers` now contains exactly three integer parses — `parseRetentionDays`, `parseMaxRetries` (both reject loudly) and `pageOrFirst`. A garbage `page` yields 1 and the `page > 1` guards behave exactly as before. Every other form field on these routes (`timeout`, `headers`, `expiry`, `signature_scheme`, `retention_days`) already rejects loudly. The disposition table is complete.
Two recommendations, owner's call, explicitly not blockers:
- **Ceiling of 20 is right.** Checked the arithmetic against `calcBackoff` rather than the PR body: delay is `2^(n-1)`s and `maxRetries` bounds total attempts, so attempt 20 lands `2^19 - 1`s out, about 6.1 days. That covers a long weekend with room, and `maxBackoffShift = 30` means the cap does not bind below attempt 31. If a longer window is ever wanted, the lever is the backoff curve, not a bigger retry count.
- **Requiring a legacy above-ceiling row to come into range on resave is acceptable.** Confirmed live: re-saving the `100000` row with its own pre-filled value answers `Invalid max retries: retries out of range: at most 20 retries` and writes nothing. Grandfathering would mean the ceiling does not apply on the edit path at all; the 400 names the limit and the fix is one field.
clawbot
merged commit fd5966f807 into next2026-08-24 01:32:48 +02: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.
Closes #221.
max_retrieswas read throughparseNonNegativeInt, which returned 0 for any parse failure. It now goes through one validator shared by the create and edit paths.What was wrong
Confirmed by execution before the fix. Create form:
999999999stored verbatim;abc,2.7,-5and a 20-digit number all accepted with HTTP 200 and stored as 0. Edit form: a target delivering withmax_retries=2, re-saved withmax_retries=abc, returned HTTP 200 and was stored as 0 — a working retry configuration destroyed by a typo, silently turning a store-and-forward proxy into fire-and-forget.The rule applied
A value that is SET BUT UNPARSEABLE is rejected loudly. A default belongs ONLY to an ABSENT value.
parseMaxRetries(raw string, fallback int)makes the distinction explicit in the code:fallback(0 at creation, the stored count on edit)Blank and whitespace-only count as absent, because they already do for the
timeoutandretention_dayscontrols on the same forms; a rule those fields did not share would be its own surprise.Both paths, from one validator
processTargetCreateandapplyTargetEditboth calltargetMaxRetries, so they cannot come to disagree —TestTargetRetries_CreateAndEditAgreeOnEveryCasesubmits every input to both and compares the verdicts. The edit path keeps its existingPostForm.Has("max_retries")guard, so a form for a target type that does not render the field still does not touch retries; validation is inside that guard, and runs before anything is assigned to the target.Wording matches the
timeoutcontrol on the same submission, which already got this right:The ceiling: 20
Chosen, not invented. Both
templates/source_detail.htmlandtemplates/target_edit.htmlhave always declaredmax="20"on the input — the server simply never enforced what the UI advertises, so this closes the gap rather than introducing a new limit, and no template changes.It is also what the delivery engine can usefully do with the value. Backoff is
2^(n-1)seconds capped at2^30, so attempt 20 is already about six days after the first; beyond that the value buys no durability. Every attempt writes adelivery_resultsrow that the event log then loads and renders, so an absurd ceiling is itself a resource problem — the issue's 100000 case would be 100000 rows for one delivery.Audit of every other
parseNonNegativeIntcallerThe helper was the defect, so it is gone. Its callers, all of them:
processTargetCreate(source_management.go)max_retriesapplyTargetEdit(target_edit.go)max_retriesfinishResubmit(event_resubmit.go)pagefinishReplay(delivery_replay.go)pageThe two
pagereaders are not the same bug. A page number says where to send the browser after an action, not what to store: it is never persisted, never delivered, and by the time it is read the resubmit or replay has already completed — answering 400 would report a failure that did not happen. Falling back to page 1 is correct there, and is already whatparsePagedoes for the same parameter on the GET side.What was wrong was the shape: a general-purpose silently-coercing int parser sitting in the package for the next configuration field to reach for. Those callers now use
pageOrFirst, named for what it does, documented as to why it is the exception, and shared withparsePageso the log page number is parsed one way.parseNonNegativeIntno longer exists.Existing stored values
Input validation, not a migration. Nothing clamps a row. Verified on a running instance with a target set to
max_retries=100000directly in the database: source detail page HTTP 200 showing 100000, edit page HTTP 200 pre-fillingvalue="100000", and the row still 100000 afterwards. Delivery is untouched — no change ininternal/delivery.One consequence, called out deliberately: re-saving such a target from the edit form does have to bring it into range, because the form submits the pre-filled value back and accepting it would mean the ceiling does not apply to the edit path. The 400 names the ceiling, so the fix is one field. This is documented on
parseMaxRetries.Verification
make checkgreen, run withGOFLAGS=-count=1on the committed tree: exit 0, 570 tests passed, 0 failures, lint0 issues(in Docker via thescript/entrypoint, as always). The ten new tests all show real durations rather than cached results.Tests cover, on BOTH create and edit: above-ceiling rejected with the ceiling named;
abc,2.7,-5, a 20-digit number and1e3rejected rather than coerced to 0; valid values including 0 and the ceiling itself accepted and stored; an absent field still taking its default, so the two cases stay provably distinct; and a rejected edit leaving both the retry count and the name untouched, proving nothing at all was written.Live instance, the reported case
The target from the report:
max_retries=2, re-saved from the edit form withabc.The 400 is answered, the working retry count survives, and the name change in the same submission was not written either.
Scope
internal/handlersand its tests only. No change tointernal/delivery,internal/database,internal/middleware, the templates, orTODO.md.PASS — independent review. Definition of done in #221 is met, verified by execution against a running instance, not by the test suite: all 16 input classes (absent, empty, whitespace-only,
0,5,20,21,999999999,abc,2.7,-5, a 20-digit number,+5,0x10," 7 ",1e3) give identical accept/reject verdicts on the create and edit forms; the reported case (a target that had actually delivered atmax_retries=2, re-saved withabcplus a rename and a URL change) answers 400 withmax_retries,nameandconfigall unchanged — no partial write; a row set to100000directly in the DB still renders on both pages, still pre-fills, and still delivers.make checkgreen from a clean clone withGOFLAGS=-count=1(exit 0, 100s, 570 passes, zero(cached), lint 0 issues in Docker), CI green one65cb89, merges cleanly intonext, commit message and trailers clean.Anomalies and disclosures, none blocking:
parseMaxRetriesreverted to the old silent-coercion semantics,TestTargetCreate_UnparseableRetriesRejected,TestTargetEdit_UnparseableRetriesRejected,TestTargetCreate_RetriesAboveCeilingRejectedandTestTargetEdit_RetriesAboveCeilingRejectedall fail.TestTargetRetries_CreateAndEditAgreeOnEveryCasestill passes under that mutation — it asserts agreement, not correctness, so it pins nothing on its own. Correct as designed, worth knowing.retries must be a whole number of attemptsrather thanretries out of range, becausestrconv.AtoireturnsErrRangeand the code branches only onerr != nil. Loud rejection either way; the message is just less precise than it could be. Nit.+5stores 5 and" 7 "stores 7 (Atoisign handling plus theTrimSpace). Lenient, never silently defaults.max_retries=" "keeps the stored count. That is a value the operator submitted taking a default, which is the edge of the iron rule; it is deliberate, matchesparseRetentionDaysand thetimeoutcontrol on the same forms, and is covered by a test. Flagging it as the judgement call it is, not as a defect.internal/handlersnow contains exactly three integer parses —parseRetentionDays,parseMaxRetries(both reject loudly) andpageOrFirst. A garbagepageyields 1 and thepage > 1guards behave exactly as before. Every other form field on these routes (timeout,headers,expiry,signature_scheme,retention_days) already rejects loudly. The disposition table is complete.Two recommendations, owner's call, explicitly not blockers:
calcBackoffrather than the PR body: delay is2^(n-1)s andmaxRetriesbounds total attempts, so attempt 20 lands2^19 - 1s out, about 6.1 days. That covers a long weekend with room, andmaxBackoffShift = 30means the cap does not bind below attempt 31. If a longer window is ever wanted, the lever is the backoff curve, not a bigger retry count.100000row with its own pre-filled value answersInvalid max retries: retries out of range: at most 20 retriesand writes nothing. Grandfathering would mean the ceiling does not apply on the edit path at all; the 400 names the limit and the fix is one field.