Found by the integration review of #111. Blocks the 1.0.0 tag: it is a crash-on-misconfiguration of exactly the kind #80 was meant to eliminate.
envDuration (internal/config/config.go:219) accepts 0s and -1h. That value feeds time.NewTicker in two places:
internal/database/retention.go:122
internal/delivery/archive_sweeper.go:130 — this second consumer is new in this milestone
time.NewTicker panics on a non-positive duration, and both calls run in goroutines with no recover. So the process logs "Configuration loaded", reports a successful startup, and then aborts.
PORT and RECEIVER_RATE_LIMIT both got lower bounds under #80. This variable was missed, and the milestone then added a second consumer of it.
Definition of done
A non-positive RETENTION_SWEEP_INTERVAL is rejected in loadFromEnv and aborts startup with an error naming the variable, consistent with how PORT and RECEIVER_RATE_LIMIT fail.
Config tests covering 0s and a negative value, asserting the failure names the variable and wraps the same sentinel error the sibling variables use.
Audit every other envDuration call site for the same gap and state the result. If another duration reaches a NewTicker, NewTimer or similar without a bound, fix it here.
Implementation requirements
Branch from next, PR based on next, single commit, title ending (closes #N).
Do not modify TODO.md.
Gate on make check plus the Docker lint path with the cache defeated (#119).
Found by the integration review of https://git.eeqj.de/sneak/webhooker/pulls/111. Blocks the 1.0.0 tag: it is a crash-on-misconfiguration of exactly the kind https://git.eeqj.de/sneak/webhooker/issues/80 was meant to eliminate.
`envDuration` (`internal/config/config.go:219`) accepts `0s` and `-1h`. That value feeds `time.NewTicker` in two places:
- `internal/database/retention.go:122`
- `internal/delivery/archive_sweeper.go:130` — this second consumer is new in this milestone
`time.NewTicker` panics on a non-positive duration, and both calls run in goroutines with no recover. So the process logs "Configuration loaded", reports a successful startup, and then aborts.
`PORT` and `RECEIVER_RATE_LIMIT` both got lower bounds under https://git.eeqj.de/sneak/webhooker/issues/80. This variable was missed, and the milestone then added a second consumer of it.
## Definition of done
- A non-positive `RETENTION_SWEEP_INTERVAL` is rejected in `loadFromEnv` and aborts startup with an error naming the variable, consistent with how `PORT` and `RECEIVER_RATE_LIMIT` fail.
- Config tests covering `0s` and a negative value, asserting the failure names the variable and wraps the same sentinel error the sibling variables use.
- Audit every other `envDuration` call site for the same gap and state the result. If another duration reaches a `NewTicker`, `NewTimer` or similar without a bound, fix it here.
## Implementation requirements
- Branch from `next`, PR based on `next`, single commit, title ending ` (closes #N)`.
- Do not modify `TODO.md`.
- Gate on `make check` plus the Docker lint path with the cache defeated (https://git.eeqj.de/sneak/webhooker/issues/119).
clawbot
added this to the 1.0.0 milestone 2026-08-12 12:30:00 +02:00
clawbot
self-assigned this 2026-08-12 12:30:00 +02:00
RETENTION_SWEEP_INTERVAL now goes through a new envPositiveDuration, which wraps envDuration the way envPort wraps envPositiveInt and rejects <= 0 with an error wrapping ErrNonPositiveValue that names the variable — the same failure shape as PORT and RECEIVER_RATE_LIMIT.
Audit: there are exactly two envDuration call sites. RETENTION_SWEEP_INTERVAL (ticker in both internal/database/retention.go:122 and internal/delivery/archive_sweeper.go:130) is fixed. SESSION_IDLE_TIMEOUT reaches no ticker or timer — it becomes session.idleTimeout, guarded by if s.idleTimeout <= 0 at both use sites, where non-positive is the documented "disabled" state — so it stays on envDuration. The other NewTicker/AfterFunc/WithTimeout sites use constants or a computed backoff, none environment-configurable.
Verified: make check exit 0 with the new subtests observed running, and docker build --no-cache-filter=lint,builder exit 0 with the lint and builder stages confirmed executed rather than CACHED (0 issues., and the new subtests passing inside the container).
Fixed in https://git.eeqj.de/sneak/webhooker/pulls/142.
`RETENTION_SWEEP_INTERVAL` now goes through a new `envPositiveDuration`, which wraps `envDuration` the way `envPort` wraps `envPositiveInt` and rejects `<= 0` with an error wrapping `ErrNonPositiveValue` that names the variable — the same failure shape as `PORT` and `RECEIVER_RATE_LIMIT`.
Audit: there are exactly two `envDuration` call sites. `RETENTION_SWEEP_INTERVAL` (ticker in both `internal/database/retention.go:122` and `internal/delivery/archive_sweeper.go:130`) is fixed. `SESSION_IDLE_TIMEOUT` reaches no ticker or timer — it becomes `session.idleTimeout`, guarded by `if s.idleTimeout <= 0` at both use sites, where non-positive is the documented "disabled" state — so it stays on `envDuration`. The other `NewTicker`/`AfterFunc`/`WithTimeout` sites use constants or a computed backoff, none environment-configurable.
Verified: `make check` exit 0 with the new subtests observed running, and `docker build --no-cache-filter=lint,builder` exit 0 with the `lint` and `builder` stages confirmed executed rather than `CACHED` (`0 issues.`, and the new subtests passing inside the container).
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.
Found by the integration review of #111. Blocks the 1.0.0 tag: it is a crash-on-misconfiguration of exactly the kind #80 was meant to eliminate.
envDuration(internal/config/config.go:219) accepts0sand-1h. That value feedstime.NewTickerin two places:internal/database/retention.go:122internal/delivery/archive_sweeper.go:130— this second consumer is new in this milestonetime.NewTickerpanics on a non-positive duration, and both calls run in goroutines with no recover. So the process logs "Configuration loaded", reports a successful startup, and then aborts.PORTandRECEIVER_RATE_LIMITboth got lower bounds under #80. This variable was missed, and the milestone then added a second consumer of it.Definition of done
RETENTION_SWEEP_INTERVALis rejected inloadFromEnvand aborts startup with an error naming the variable, consistent with howPORTandRECEIVER_RATE_LIMITfail.0sand a negative value, asserting the failure names the variable and wraps the same sentinel error the sibling variables use.envDurationcall site for the same gap and state the result. If another duration reaches aNewTicker,NewTimeror similar without a bound, fix it here.Implementation requirements
next, PR based onnext, single commit, title ending(closes #N).TODO.md.make checkplus the Docker lint path with the cache defeated (#119).clawbot referenced this issue2026-08-12 12:38:21 +02:00
Fixed in #142.
RETENTION_SWEEP_INTERVALnow goes through a newenvPositiveDuration, which wrapsenvDurationthe wayenvPortwrapsenvPositiveIntand rejects<= 0with an error wrappingErrNonPositiveValuethat names the variable — the same failure shape asPORTandRECEIVER_RATE_LIMIT.Audit: there are exactly two
envDurationcall sites.RETENTION_SWEEP_INTERVAL(ticker in bothinternal/database/retention.go:122andinternal/delivery/archive_sweeper.go:130) is fixed.SESSION_IDLE_TIMEOUTreaches no ticker or timer — it becomessession.idleTimeout, guarded byif s.idleTimeout <= 0at both use sites, where non-positive is the documented "disabled" state — so it stays onenvDuration. The otherNewTicker/AfterFunc/WithTimeoutsites use constants or a computed backoff, none environment-configurable.Verified:
make checkexit 0 with the new subtests observed running, anddocker build --no-cache-filter=lint,builderexit 0 with thelintandbuilderstages confirmed executed rather thanCACHED(0 issues., and the new subtests passing inside the container).clawbot referenced this issue2026-08-17 23:50:11 +02:00