envDuration accepted 0s and negative values, and RETENTION_SWEEP_INTERVAL feeds time.NewTicker in both the retention
reaper (internal/database/retention.go:122) and the archive sweeper
(internal/delivery/archive_sweeper.go:130). NewTicker panics on a
non-positive period and both tickers are created in goroutines with no
recover, so a bad value aborted the process after startup had already
logged "Configuration loaded".
New envPositiveDuration in internal/config/config.go, structured
exactly like envPort wrapping envPositiveInt: it delegates to envDuration and then rejects <= 0 with an error that wraps ErrNonPositiveValue and names the variable. Same sentinel and same
failure shape as PORT and RECEIVER_RATE_LIMIT.
loadFromEnv uses it for RETENTION_SWEEP_INTERVAL.
Config tests: 0s and -1h cases added to TestRetentionSweepInterval,
asserting via the existing expectStartupErrorFor helper that startup
fails, that the message names RETENTION_SWEEP_INTERVAL, and that the
error wraps config.ErrNonPositiveValue.
Regression cases added to TestSessionIdleTimeout asserting 0s and -1h still start successfully, so a future change cannot quietly turn
the documented "disabled" case into a startup failure.
README: RETENTION_SWEEP_INTERVAL row added to the env table (it was
missing), and the "Invalid values abort startup" section now states the
positive requirement and calls out SESSION_IDLE_TIMEOUT as the
deliberate exception.
envDuration audit
There are exactly two envDuration call sites in the tree:
Variable
Reaches a ticker/timer?
Result
RETENTION_SWEEP_INTERVAL
yes — time.NewTicker in internal/database/retention.go:122 and internal/delivery/archive_sweeper.go:130
fixed here
SESSION_IDLE_TIMEOUT
no
left as-is, deliberately
SESSION_IDLE_TIMEOUT only ever becomes session.idleTimeout, which is
guarded by an explicit if s.idleTimeout <= 0 { return } at both of its
use sites (internal/session/session.go:264 and :390). Non-positive
there means idle expiry is disabled, which is documented in the README,
so it stays on envDuration.
The remaining NewTicker/AfterFunc sites take no
environment-configurable duration: internal/delivery/engine.go:619 uses
the retrySweepInterval constant, internal/delivery/engine.go:222 uses
a computed backoff delay, and the context.WithTimeout calls in internal/delivery/ssrf.go:146 and internal/server/server.go:166 use
constants (and a non-positive timeout there would not panic in any case).
docker build --no-cache-filter=lint,builder --progress=plain . — exit 0,
with both stages confirmed executed rather than CACHED:
#15 [lint 7/8] RUN make fmt-check
#16 [lint 8/8] RUN make lint
#16 61.04 0 issues.
#23 [builder 8/10] RUN make test
#23 65.86 --- PASS: TestRetentionSweepInterval/zero_fails_startup (0.00s)
#23 65.86 --- PASS: TestRetentionSweepInterval/negative_fails_startup (0.00s)
No parallel golangci-lint is running in either run. TODO.md untouched.
Closes https://git.eeqj.de/sneak/webhooker/issues/140.
## What changed
`envDuration` accepted `0s` and negative values, and
`RETENTION_SWEEP_INTERVAL` feeds `time.NewTicker` in both the retention
reaper (`internal/database/retention.go:122`) and the archive sweeper
(`internal/delivery/archive_sweeper.go:130`). `NewTicker` panics on a
non-positive period and both tickers are created in goroutines with no
recover, so a bad value aborted the process after startup had already
logged "Configuration loaded".
- New `envPositiveDuration` in `internal/config/config.go`, structured
exactly like `envPort` wrapping `envPositiveInt`: it delegates to
`envDuration` and then rejects `<= 0` with an error that wraps
`ErrNonPositiveValue` and names the variable. Same sentinel and same
failure shape as `PORT` and `RECEIVER_RATE_LIMIT`.
- `loadFromEnv` uses it for `RETENTION_SWEEP_INTERVAL`.
- Config tests: `0s` and `-1h` cases added to `TestRetentionSweepInterval`,
asserting via the existing `expectStartupErrorFor` helper that startup
fails, that the message names `RETENTION_SWEEP_INTERVAL`, and that the
error wraps `config.ErrNonPositiveValue`.
- Regression cases added to `TestSessionIdleTimeout` asserting `0s` and
`-1h` still start successfully, so a future change cannot quietly turn
the documented "disabled" case into a startup failure.
- README: `RETENTION_SWEEP_INTERVAL` row added to the env table (it was
missing), and the "Invalid values abort startup" section now states the
positive requirement and calls out `SESSION_IDLE_TIMEOUT` as the
deliberate exception.
## `envDuration` audit
There are exactly two `envDuration` call sites in the tree:
| Variable | Reaches a ticker/timer? | Result |
| --- | --- | --- |
| `RETENTION_SWEEP_INTERVAL` | yes — `time.NewTicker` in `internal/database/retention.go:122` and `internal/delivery/archive_sweeper.go:130` | fixed here |
| `SESSION_IDLE_TIMEOUT` | no | left as-is, deliberately |
`SESSION_IDLE_TIMEOUT` only ever becomes `session.idleTimeout`, which is
guarded by an explicit `if s.idleTimeout <= 0 { return }` at both of its
use sites (`internal/session/session.go:264` and `:390`). Non-positive
there means idle expiry is disabled, which is documented in the README,
so it stays on `envDuration`.
The remaining `NewTicker`/`AfterFunc` sites take no
environment-configurable duration: `internal/delivery/engine.go:619` uses
the `retrySweepInterval` constant, `internal/delivery/engine.go:222` uses
a computed backoff delay, and the `context.WithTimeout` calls in
`internal/delivery/ssrf.go:146` and `internal/server/server.go:166` use
constants (and a non-positive timeout there would not panic in any case).
## Gate evidence
`make check` — exit 0, tests executed (not cached):
```
--- PASS: TestRetentionSweepInterval/zero_fails_startup (0.02s)
--- PASS: TestRetentionSweepInterval/negative_fails_startup (0.00s)
--- PASS: TestSessionIdleTimeout/zero_disables_idle_expiry (0.00s)
--- PASS: TestSessionIdleTimeout/negative_disables_idle_expiry (0.01s)
0 issues.
```
`docker build --no-cache-filter=lint,builder --progress=plain .` — exit 0,
with both stages confirmed executed rather than `CACHED`:
```
#15 [lint 7/8] RUN make fmt-check
#16 [lint 8/8] RUN make lint
#16 61.04 0 issues.
#23 [builder 8/10] RUN make test
#23 65.86 --- PASS: TestRetentionSweepInterval/zero_fails_startup (0.00s)
#23 65.86 --- PASS: TestRetentionSweepInterval/negative_fails_startup (0.00s)
```
No `parallel golangci-lint is running` in either run. `TODO.md` untouched.
envDuration accepted 0s and negative values, and
RETENTION_SWEEP_INTERVAL feeds time.NewTicker in both the retention
reaper and the archive sweeper. NewTicker panics on a non-positive
period, and both tickers are created in goroutines with no recover, so
a bad value aborted the process after startup had already logged
"Configuration loaded".
Add envPositiveDuration, mirroring envPort's wrapping of
envPositiveInt, and use it for RETENTION_SWEEP_INTERVAL. It wraps
ErrNonPositiveValue and names the variable, the same failure shape
PORT and RECEIVER_RATE_LIMIT already have.
SESSION_IDLE_TIMEOUT, the only other envDuration caller, stays on
envDuration: non-positive there means idle expiry is disabled, which
is documented behaviour and guarded at both use sites.
PASS. Independently verified: the envDuration audit (exactly two call sites; the only other NewTicker/AfterFunc/WithTimeout sites take constants or a computed backoff), that defaultRetentionSweepInterval also passes the new bound so the unset path cannot reach NewTicker with a non-positive period, that loadFromEnv is the sole production constructor of Config, and that both s.idleTimeout <= 0 guards (internal/session/session.go:264, :390) precede every use of idleTimeout. Mutation-checked: reverting to envDuration fails the two new subtests, and dropping the %w sentinel wrap alone also fails them, so neither assertion is vacuous.
Gate (run locally on 500f39a, not trusting the cached CI log — #119): docker build --no-cache-filter=lint,builder --progress=plain . exit 0, lint stage executed (RUN make lint 73.9s, 0 issues., no parallel golangci-lint), builderRUN make test executed with zero (cached) markers and the four new subtests observed PASS in-container.
Disclosure: script/lint in this repo runs the host golangci-lint, so the make check lint result is not container evidence; the Docker lint stage above is what I relied on.
PASS. Independently verified: the `envDuration` audit (exactly two call sites; the only other `NewTicker`/`AfterFunc`/`WithTimeout` sites take constants or a computed backoff), that `defaultRetentionSweepInterval` also passes the new bound so the unset path cannot reach `NewTicker` with a non-positive period, that `loadFromEnv` is the sole production constructor of `Config`, and that both `s.idleTimeout <= 0` guards (`internal/session/session.go:264`, `:390`) precede every use of `idleTimeout`. Mutation-checked: reverting to `envDuration` fails the two new subtests, and dropping the `%w` sentinel wrap alone also fails them, so neither assertion is vacuous.
Gate (run locally on `500f39a`, not trusting the cached CI log — https://git.eeqj.de/sneak/webhooker/issues/119): `docker build --no-cache-filter=lint,builder --progress=plain .` exit 0, `lint` stage executed (`RUN make lint` 73.9s, `0 issues.`, no `parallel golangci-lint`), `builder` `RUN make test` executed with zero `(cached)` markers and the four new subtests observed PASS in-container.
Disclosure: `script/lint` in this repo runs the host `golangci-lint`, so the `make check` lint result is not container evidence; the Docker `lint` stage above is what I relied on.
clawbot
merged commit 3941f0b0ff into next2026-08-12 12:46:40 +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 #140.
What changed
envDurationaccepted0sand negative values, andRETENTION_SWEEP_INTERVALfeedstime.NewTickerin both the retentionreaper (
internal/database/retention.go:122) and the archive sweeper(
internal/delivery/archive_sweeper.go:130).NewTickerpanics on anon-positive period and both tickers are created in goroutines with no
recover, so a bad value aborted the process after startup had already
logged "Configuration loaded".
envPositiveDurationininternal/config/config.go, structuredexactly like
envPortwrappingenvPositiveInt: it delegates toenvDurationand then rejects<= 0with an error that wrapsErrNonPositiveValueand names the variable. Same sentinel and samefailure shape as
PORTandRECEIVER_RATE_LIMIT.loadFromEnvuses it forRETENTION_SWEEP_INTERVAL.0sand-1hcases added toTestRetentionSweepInterval,asserting via the existing
expectStartupErrorForhelper that startupfails, that the message names
RETENTION_SWEEP_INTERVAL, and that theerror wraps
config.ErrNonPositiveValue.TestSessionIdleTimeoutasserting0sand-1hstill start successfully, so a future change cannot quietly turnthe documented "disabled" case into a startup failure.
RETENTION_SWEEP_INTERVALrow added to the env table (it wasmissing), and the "Invalid values abort startup" section now states the
positive requirement and calls out
SESSION_IDLE_TIMEOUTas thedeliberate exception.
envDurationauditThere are exactly two
envDurationcall sites in the tree:RETENTION_SWEEP_INTERVALtime.NewTickerininternal/database/retention.go:122andinternal/delivery/archive_sweeper.go:130SESSION_IDLE_TIMEOUTSESSION_IDLE_TIMEOUTonly ever becomessession.idleTimeout, which isguarded by an explicit
if s.idleTimeout <= 0 { return }at both of itsuse sites (
internal/session/session.go:264and:390). Non-positivethere means idle expiry is disabled, which is documented in the README,
so it stays on
envDuration.The remaining
NewTicker/AfterFuncsites take noenvironment-configurable duration:
internal/delivery/engine.go:619usesthe
retrySweepIntervalconstant,internal/delivery/engine.go:222usesa computed backoff delay, and the
context.WithTimeoutcalls ininternal/delivery/ssrf.go:146andinternal/server/server.go:166useconstants (and a non-positive timeout there would not panic in any case).
Gate evidence
make check— exit 0, tests executed (not cached):docker build --no-cache-filter=lint,builder --progress=plain .— exit 0,with both stages confirmed executed rather than
CACHED:No
parallel golangci-lint is runningin either run.TODO.mduntouched.PASS. Independently verified: the
envDurationaudit (exactly two call sites; the only otherNewTicker/AfterFunc/WithTimeoutsites take constants or a computed backoff), thatdefaultRetentionSweepIntervalalso passes the new bound so the unset path cannot reachNewTickerwith a non-positive period, thatloadFromEnvis the sole production constructor ofConfig, and that boths.idleTimeout <= 0guards (internal/session/session.go:264,:390) precede every use ofidleTimeout. Mutation-checked: reverting toenvDurationfails the two new subtests, and dropping the%wsentinel wrap alone also fails them, so neither assertion is vacuous.Gate (run locally on
500f39a, not trusting the cached CI log — #119):docker build --no-cache-filter=lint,builder --progress=plain .exit 0,lintstage executed (RUN make lint73.9s,0 issues., noparallel golangci-lint),builderRUN make testexecuted with zero(cached)markers and the four new subtests observed PASS in-container.Disclosure:
script/lintin this repo runs the hostgolangci-lint, so themake checklint result is not container evidence; the Dockerlintstage above is what I relied on.clawbot referenced this pull request2026-08-12 12:56:27 +02:00