Disclosed by the review of #132, which judged it below the bar for rework there.
#132 moved the production store onto store.MaxAge(...) so the securecookie codecs share the 7-day cap. internal/middleware/middleware_test.go:67 and :850 still build lookalike stores by assigning store.Options directly, which leaves their codecs at gorilla's 30-day default.
The cause is reachability, not oversight: NewStore is exported through internal/session/export_test.go, so package middleware cannot reach the production constructor and has to hand-roll one.
No false green today — nothing in middleware asserts on codec age — but the divergence is invisible and will silently outlive the next change to store construction.
Definition of done
NewStore moves from internal/session/export_test.go into the existing internal/session/testing.go so other packages can use it.
internal/middleware/middleware_test.go:67 and :850 construct their stores through it.
No test in the repo constructs a cookie store by assigning store.Options.
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).
Disclosed by the review of https://git.eeqj.de/sneak/webhooker/pulls/132, which judged it below the bar for rework there.
https://git.eeqj.de/sneak/webhooker/pulls/132 moved the production store onto `store.MaxAge(...)` so the securecookie codecs share the 7-day cap. `internal/middleware/middleware_test.go:67` and `:850` still build lookalike stores by assigning `store.Options` directly, which leaves their codecs at gorilla's 30-day default.
The cause is reachability, not oversight: `NewStore` is exported through `internal/session/export_test.go`, so package `middleware` cannot reach the production constructor and has to hand-roll one.
No false green today — nothing in `middleware` asserts on codec age — but the divergence is invisible and will silently outlive the next change to store construction.
## Definition of done
- `NewStore` moves from `internal/session/export_test.go` into the existing `internal/session/testing.go` so other packages can use it.
- `internal/middleware/middleware_test.go:67` and `:850` construct their stores through it.
- No test in the repo constructs a cookie store by assigning `store.Options`.
## 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
self-assigned this 2026-08-14 06:17:56 +02:00
Plan. As the definition of done says: move the test store constructor from internal/session/export_test.go into internal/session/testing.go, so other packages can build a store the way production does; have the middleware tests build their stores through it; and leave no test that builds a cookie store by setting store.Options by hand. The line numbers in the issue are from August; find the current sites. Nothing else changes.
Model: opus-5-5
Plan. As the definition of done says: move the test store constructor from `internal/session/export_test.go` into `internal/session/testing.go`, so other packages can build a store the way production does; have the middleware tests build their stores through it; and leave no test that builds a cookie store by setting `store.Options` by hand. The line numbers in the issue are from August; find the current sites. Nothing else changes.
Model: opus-5-5
Done in #434. session.NewStore moved from internal/session/export_test.go into internal/session/testing.go, and the two middleware test helpers that set store.Options by hand now build their store through it. No test in the repo builds a cookie store that way any more, and no assertion was removed or loosened.
Model: opus-5-5
Done in https://git.eeqj.de/sneak/webhooker/pulls/434. `session.NewStore` moved from `internal/session/export_test.go` into `internal/session/testing.go`, and the two middleware test helpers that set `store.Options` by hand now build their store through it. No test in the repo builds a cookie store that way any more, and no assertion was removed or loosened.
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.
Disclosed by the review of #132, which judged it below the bar for rework there.
#132 moved the production store onto
store.MaxAge(...)so the securecookie codecs share the 7-day cap.internal/middleware/middleware_test.go:67and:850still build lookalike stores by assigningstore.Optionsdirectly, which leaves their codecs at gorilla's 30-day default.The cause is reachability, not oversight:
NewStoreis exported throughinternal/session/export_test.go, so packagemiddlewarecannot reach the production constructor and has to hand-roll one.No false green today — nothing in
middlewareasserts on codec age — but the divergence is invisible and will silently outlive the next change to store construction.Definition of done
NewStoremoves frominternal/session/export_test.gointo the existinginternal/session/testing.goso other packages can use it.internal/middleware/middleware_test.go:67and:850construct their stores through it.store.Options.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).Plan. As the definition of done says: move the test store constructor from
internal/session/export_test.gointointernal/session/testing.go, so other packages can build a store the way production does; have the middleware tests build their stores through it; and leave no test that builds a cookie store by settingstore.Optionsby hand. The line numbers in the issue are from August; find the current sites. Nothing else changes.Model: opus-5-5
Done in #434.
session.NewStoremoved frominternal/session/export_test.gointointernal/session/testing.go, and the two middleware test helpers that setstore.Optionsby hand now build their store through it. No test in the repo builds a cookie store that way any more, and no assertion was removed or loosened.Model: opus-5-5