Implements the cleanup half of ADR-0021 — the config infrastructure (TTL, retention factor, max retention, cleanup interval) had been merged earlier without the actual cleanup logic. This PR closes that gap.
What ships
Core mechanism
RemoveExpiredSecrets() int on JWTSecretManager — drops every non-primary secret whose ExpiresAt is non-nil and in the past. Returns count of removed. Primary secret is never removed regardless of expiration (ADR-0021 invariant).
StartCleanupLoop(ctx, interval) — spawns a goroutine running RemoveExpiredSecrets at the configured interval. Stops cleanly when ctx is cancelled.
All struct operations are now mutex-protected (sync.Mutex). Race detector clean.
Wiring
pkg/server/server.goRun() calls StartJWTSecretCleanupLoop(rootCtx, ...) after the signal context is set up — the loop stops on graceful shutdown.
AuthService interface extended with StartJWTSecretCleanupLoop and RemoveExpiredJWTSecrets.
Status update
ADR-0021 → Implemented. Heading also corrected from 10. to 21. (was a doc error from earlier commit).
Tests
4 new unit tests in pkg/user/jwt_manager_test.go:
TestRemoveExpiredSecrets_ExpiredNonPrimaryRemoved
TestRemoveExpiredSecrets_PrimaryNeverRemoved
TestRemoveExpiredSecrets_NonExpiredKept
TestStartCleanupLoop_FiresAndStops (drives the goroutine end-to-end with low TTL)
go test -race ./pkg/user/... passes.
Full BDD suite (auth/config/greet/health/info/jwt) still green.
BDD scenarios tagged @todo / @skip in features/jwt/jwt_secret_retention.feature remain so — they reference an admin endpoint /api/v1/admin/jwt/secrets that is explicitly out of scope for this PR.
Verifier verdict (skill-driven)
APPROVE_WITH_NITS
Dim A (code quality): APPROVE_WITH_NITS — StartCleanupLoop is 34 lines (just over the 30-line guideline; could be split into setupLoopCtx + runTickerLoop helpers but not critical).
Dim B (docs homogeneity): APPROVE — ADR-0021 numbering fixed (was 10., now 21.); status accurately reflects what landed; out-of-scope items explicitly documented.
Dim C (test reliability): APPROVE — race detector clean, behavior coverage (positive + invariant + negative + goroutine), time.Sleep uses are justified by the goroutine-timing nature of TestStartCleanupLoop_FiresAndStops (low TTL keeps the test < 100ms).
Out of scope (deferred)
Admin endpoint /api/v1/admin/jwt/secrets — referenced by @todo BDD scenarios but no concrete need yet. Reopen when ops require introspection.
Cleanup metrics (count emitted to /metrics) — log-based observability for now (log.Info().Int("removed", n)).
Manual cleanup endpoint — RemoveExpiredJWTSecrets() is exposed on the service but not as an HTTP endpoint.
Test plan
go test -race ./pkg/user/... passes
Full BDD suite passes
go vet ./... clean
Pre-commit hooks (go mod tidy + go fmt + swag fmt) successful
Reviewer to verify the loop actually runs in production (start the server, check logs after 1h)
## Sprint 3 of autonomous trainer day 2026-05-05
Implements the cleanup half of [ADR-0021](adr/0021-jwt-secret-retention-policy.md) — the config infrastructure (TTL, retention factor, max retention, cleanup interval) had been merged earlier without the actual cleanup logic. This PR closes that gap.
## What ships
### Core mechanism
- **`RemoveExpiredSecrets() int`** on `JWTSecretManager` — drops every non-primary secret whose `ExpiresAt` is non-nil and in the past. Returns count of removed. **Primary secret is never removed** regardless of expiration (ADR-0021 invariant).
- **`StartCleanupLoop(ctx, interval)`** — spawns a goroutine running `RemoveExpiredSecrets` at the configured interval. Stops cleanly when `ctx` is cancelled.
- All struct operations are now mutex-protected (`sync.Mutex`). Race detector clean.
### Wiring
- `pkg/server/server.go` `Run()` calls `StartJWTSecretCleanupLoop(rootCtx, ...)` after the signal context is set up — the loop stops on graceful shutdown.
- `AuthService` interface extended with `StartJWTSecretCleanupLoop` and `RemoveExpiredJWTSecrets`.
### Status update
- ADR-0021 → `Implemented`. Heading also corrected from `10.` to `21.` (was a doc error from earlier commit).
## Tests
- 4 new unit tests in `pkg/user/jwt_manager_test.go`:
- `TestRemoveExpiredSecrets_ExpiredNonPrimaryRemoved`
- `TestRemoveExpiredSecrets_PrimaryNeverRemoved`
- `TestRemoveExpiredSecrets_NonExpiredKept`
- `TestStartCleanupLoop_FiresAndStops` (drives the goroutine end-to-end with low TTL)
- `go test -race ./pkg/user/...` passes.
- Full BDD suite (auth/config/greet/health/info/jwt) still green.
- BDD scenarios tagged `@todo` / `@skip` in `features/jwt/jwt_secret_retention.feature` remain so — they reference an admin endpoint `/api/v1/admin/jwt/secrets` that is **explicitly out of scope** for this PR.
## Verifier verdict (skill-driven)
**APPROVE_WITH_NITS**
- **Dim A** (code quality): APPROVE_WITH_NITS — `StartCleanupLoop` is 34 lines (just over the 30-line guideline; could be split into `setupLoopCtx` + `runTickerLoop` helpers but not critical).
- **Dim B** (docs homogeneity): APPROVE — ADR-0021 numbering fixed (was `10.`, now `21.`); status accurately reflects what landed; out-of-scope items explicitly documented.
- **Dim C** (test reliability): APPROVE — race detector clean, behavior coverage (positive + invariant + negative + goroutine), `time.Sleep` uses are justified by the goroutine-timing nature of `TestStartCleanupLoop_FiresAndStops` (low TTL keeps the test < 100ms).
## Out of scope (deferred)
- **Admin endpoint `/api/v1/admin/jwt/secrets`** — referenced by `@todo` BDD scenarios but no concrete need yet. Reopen when ops require introspection.
- **Cleanup metrics** (count emitted to /metrics) — log-based observability for now (`log.Info().Int("removed", n)`).
- **Manual cleanup endpoint** — `RemoveExpiredJWTSecrets()` is exposed on the service but not as an HTTP endpoint.
## Test plan
- [x] `go test -race ./pkg/user/...` passes
- [x] Full BDD suite passes
- [x] `go vet ./...` clean
- [x] Pre-commit hooks (go mod tidy + go fmt + swag fmt) successful
- [ ] Reviewer to verify the loop actually runs in production (start the server, check logs after 1h)
Implements the cleanup half of ADR-0021 (which had only config infrastructure
landed). Non-primary expired secrets are removed by a goroutine that runs at
auth.jwt.secret_retention.cleanup_interval (default 1h). Primary secret is
never removed regardless of expiration — invariant preserved.
Changes:
- pkg/user/jwt_manager.go : add sync.Mutex protection; add
RemoveExpiredSecrets() int and StartCleanupLoop(ctx, interval) methods.
Reset() now also cancels any running cleanup goroutine.
- pkg/user/auth_service.go : delegate to manager via new AuthService methods
StartJWTSecretCleanupLoop and RemoveExpiredJWTSecrets.
- pkg/user/user.go : extend AuthService interface accordingly.
- pkg/server/server.go Run() : start cleanup loop tied to rootCtx so it
stops on graceful shutdown.
- pkg/jwt/* : same treatment on the secondary (less-used) implementation
for consistency.
- adr/0021-jwt-secret-retention-policy.md : Status → Implemented + fix
numbering (was incorrectly "10.").
Tests:
- 4 new unit tests in pkg/user/jwt_manager_test.go covering
RemoveExpiredSecrets (expired removed, primary preserved, future kept)
and StartCleanupLoop (fires + stops on context cancel).
- go test -race ./pkg/user/... passes.
- Full BDD suite (auth/config/greet/health/info/jwt) still green.
- BDD scenarios at @todo / @skip remain so — they require an admin
endpoint /api/v1/admin/jwt/secrets which is explicitly out of scope.
Verifier verdict: APPROVE_WITH_NITS — StartCleanupLoop is 34 lines (just
over the 30-line guideline); 2 time.Sleeps in TestStartCleanupLoop_FiresAndStops
are justified by the goroutine-timing nature of the test.
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.
Sprint 3 of autonomous trainer day 2026-05-05
Implements the cleanup half of ADR-0021 — the config infrastructure (TTL, retention factor, max retention, cleanup interval) had been merged earlier without the actual cleanup logic. This PR closes that gap.
What ships
Core mechanism
RemoveExpiredSecrets() intonJWTSecretManager— drops every non-primary secret whoseExpiresAtis non-nil and in the past. Returns count of removed. Primary secret is never removed regardless of expiration (ADR-0021 invariant).StartCleanupLoop(ctx, interval)— spawns a goroutine runningRemoveExpiredSecretsat the configured interval. Stops cleanly whenctxis cancelled.sync.Mutex). Race detector clean.Wiring
pkg/server/server.goRun()callsStartJWTSecretCleanupLoop(rootCtx, ...)after the signal context is set up — the loop stops on graceful shutdown.AuthServiceinterface extended withStartJWTSecretCleanupLoopandRemoveExpiredJWTSecrets.Status update
Implemented. Heading also corrected from10.to21.(was a doc error from earlier commit).Tests
pkg/user/jwt_manager_test.go:TestRemoveExpiredSecrets_ExpiredNonPrimaryRemovedTestRemoveExpiredSecrets_PrimaryNeverRemovedTestRemoveExpiredSecrets_NonExpiredKeptTestStartCleanupLoop_FiresAndStops(drives the goroutine end-to-end with low TTL)go test -race ./pkg/user/...passes.@todo/@skipinfeatures/jwt/jwt_secret_retention.featureremain so — they reference an admin endpoint/api/v1/admin/jwt/secretsthat is explicitly out of scope for this PR.Verifier verdict (skill-driven)
APPROVE_WITH_NITS
StartCleanupLoopis 34 lines (just over the 30-line guideline; could be split intosetupLoopCtx+runTickerLoophelpers but not critical).10., now21.); status accurately reflects what landed; out-of-scope items explicitly documented.time.Sleepuses are justified by the goroutine-timing nature ofTestStartCleanupLoop_FiresAndStops(low TTL keeps the test < 100ms).Out of scope (deferred)
/api/v1/admin/jwt/secrets— referenced by@todoBDD scenarios but no concrete need yet. Reopen when ops require introspection.log.Info().Int("removed", n)).RemoveExpiredJWTSecrets()is exposed on the service but not as an HTTP endpoint.Test plan
go test -race ./pkg/user/...passesgo vet ./...clean