maintenance_mode is a real config key (config.example.yml:6, internal/config/config.go:38,103), it is reported in the healthcheck JSON (internal/healthcheck/healthcheck.go:79), and Server.MaintenanceMode() exists (internal/server/server.go:148-151).
Server.MaintenanceMode() has no callers — grep across internal/ and cmd/ finds only the definition and test references. No route, middleware, or handler consults it.
So turning on maintenance mode changes exactly one thing: the healthcheck JSON says true. Every request is still served normally. An operator who sets it believing traffic will be drained gets no such behavior, silently — the failure mode is that they find out during the maintenance they were trying to perform.
Definition of done
When maintenance_mode is true, image-serving routes return 503 Service Unavailable with a Retry-After header and the standard JSON error body used elsewhere.
The healthcheck endpoint keeps responding (it must stay reachable to report the state), and /metrics likewise — decide deliberately and state the choice in the PR.
Decide whether the auth UI (/, /generate) stays available during maintenance; recommend yes, so an operator can still work.
Implemented as middleware in internal/server/routes.go rather than a check sprinkled through handlers.
Failing tests first: with the flag on, an image request returns 503 with Retry-After, and the healthcheck still returns 200 reporting maintenance: true.
make check green.
Alternative
If maintenance mode is only ever meant as a signal for an external load balancer to drain, then the code is already correct and the gap is documentation. In that case: document it in README.md and config.example.yml as advisory-only, and remove the unused Server.MaintenanceMode() accessor. Pick one — the current state (a real-looking accessor nobody calls) reads as an unfinished feature.
Verified against `main` at `61f42e6`.
`maintenance_mode` is a real config key (`config.example.yml:6`, `internal/config/config.go:38,103`), it is reported in the healthcheck JSON (`internal/healthcheck/healthcheck.go:79`), and `Server.MaintenanceMode()` exists (`internal/server/server.go:148-151`).
`Server.MaintenanceMode()` has **no callers** — grep across `internal/` and `cmd/` finds only the definition and test references. No route, middleware, or handler consults it.
So turning on maintenance mode changes exactly one thing: the healthcheck JSON says `true`. Every request is still served normally. An operator who sets it believing traffic will be drained gets no such behavior, silently — the failure mode is that they find out during the maintenance they were trying to perform.
## Definition of done
1. When `maintenance_mode` is true, image-serving routes return `503 Service Unavailable` with a `Retry-After` header and the standard JSON error body used elsewhere.
2. The healthcheck endpoint keeps responding (it must stay reachable to report the state), and `/metrics` likewise — decide deliberately and state the choice in the PR.
3. Decide whether the auth UI (`/`, `/generate`) stays available during maintenance; recommend yes, so an operator can still work.
4. Implemented as middleware in `internal/server/routes.go` rather than a check sprinkled through handlers.
5. Failing tests first: with the flag on, an image request returns 503 with `Retry-After`, and the healthcheck still returns 200 reporting `maintenance: true`.
6. `make check` green.
## Alternative
If maintenance mode is only ever meant as a signal for an external load balancer to drain, then the code is already correct and the gap is documentation. In that case: document it in `README.md` and `config.example.yml` as advisory-only, and remove the unused `Server.MaintenanceMode()` accessor. Pick one — the current state (a real-looking accessor nobody calls) reads as an unfinished feature.
clawbot
added this to the 1.0.0 milestone 2026-08-09 03:45:38 +02:00
Plan, checked against current next: Server.MaintenanceMode() still has no caller, and turning on maintenance_mode only changes the health check's maintenance_mode field.
One middleware in internal/server/routes.go, applied to a route group that holds only the image routes (/v1/image/ GET and HEAD, /v1/e/). While maintenance_mode is on it answers 503 with a Retry-After of a fixed number of seconds (one constant) and the same JSON error body (error, status, timestamp) the image handlers send. It calls Server.MaintenanceMode(), so the accessor gets its caller.
The health check (/.well-known/healthcheck.json) stays 200 and reports "maintenance_mode": true. The image's Docker HEALTHCHECK and upaas both read it, and a 503 there would make upaas mark the deploy failed. This goes in README.md and config.example.yml.
/metrics stays available: it has its own password, touches no image, and is how an operator watches the instance during the maintenance.
The login and URL generator pages (/, /generate, /logout) stay available: they only sign URLs and never fetch or process an image, so an operator can keep working.
Tests first, in their own commit: with the flag on, both image routes answer 503 with Retry-After and the JSON body, and the health check and login page still answer 200.
No new setting: PIXA_MAINTENANCE_MODE already exists (#128).
Model: opus-5-5
Plan, checked against current `next`: `Server.MaintenanceMode()` still has no caller, and turning on `maintenance_mode` only changes the health check's `maintenance_mode` field.
- One middleware in `internal/server/routes.go`, applied to a route group that holds only the image routes (`/v1/image/` GET and HEAD, `/v1/e/`). While `maintenance_mode` is on it answers 503 with a `Retry-After` of a fixed number of seconds (one constant) and the same JSON error body (`error`, `status`, `timestamp`) the image handlers send. It calls `Server.MaintenanceMode()`, so the accessor gets its caller.
- The health check (`/.well-known/healthcheck.json`) stays 200 and reports `"maintenance_mode": true`. The image's Docker `HEALTHCHECK` and upaas both read it, and a 503 there would make upaas mark the deploy failed. This goes in `README.md` and `config.example.yml`.
- `/metrics` stays available: it has its own password, touches no image, and is how an operator watches the instance during the maintenance.
- The login and URL generator pages (`/`, `/generate`, `/logout`) stay available: they only sign URLs and never fetch or process an image, so an operator can keep working.
- Tests first, in their own commit: with the flag on, both image routes answer 503 with `Retry-After` and the JSON body, and the health check and login page still answer 200.
- No new setting: `PIXA_MAINTENANCE_MODE` already exists (https://git.eeqj.de/sneak/pixa/issues/128).
Model: opus-5-5
Question for sneak: approve one edit to an existing test file in #154, and if you agree, this kind of edit from now on?
newTestServer in internal/server/login_rate_limit_internal_test.go builds the server the login-limit tests run against. The new maintenance-mode tests also request the health check through it, so the PR adds the health check to what it builds (eight lines of setup). No test's checks change. #61 (comment) asks you about a one-line edit to the same helper.
Options: (1) approve this edit and, standing, any edit to an existing test helper that only adds setup the new code needs and changes no assertion (recommended; together with the standing approval asked on #64 (comment), you would then be asked only when a check itself changes); (2) approve this edit only; (3) refuse.
Until you answer, the PR goes through review but is not merged; other work carries on.
Model: opus-5-5
Question for sneak: approve one edit to an existing test file in https://git.eeqj.de/sneak/pixa/pulls/154, and if you agree, this kind of edit from now on?
`newTestServer` in `internal/server/login_rate_limit_internal_test.go` builds the server the login-limit tests run against. The new maintenance-mode tests also request the health check through it, so the PR adds the health check to what it builds (eight lines of setup). No test's checks change. https://git.eeqj.de/sneak/pixa/issues/61#issuecomment-104876 asks you about a one-line edit to the same helper.
Options: (1) approve this edit and, standing, any edit to an existing test helper that only adds setup the new code needs and changes no assertion (recommended; together with the standing approval asked on https://git.eeqj.de/sneak/pixa/issues/64#issuecomment-105132, you would then be asked only when a check itself changes); (2) approve this edit only; (3) refuse.
Until you answer, the PR goes through review but is not merged; other work carries on.
Model: opus-5-5
sneak
was assigned by clawbot2026-09-29 06:26:28 +02:00
Addition to the question above: #157 (issue #70) adds one constant, testContentTypeWebP = "image/webp", to the shared test helper file internal/imgcache/testutil_internal_test.go; no test changes. Option (1) above would cover it as well; option (2) or (3) means I ask you about it separately.
Model: opus-5-5
Addition to the question above: https://git.eeqj.de/sneak/pixa/pulls/157 (issue https://git.eeqj.de/sneak/pixa/issues/70) adds one constant, `testContentTypeWebP = "image/webp"`, to the shared test helper file `internal/imgcache/testutil_internal_test.go`; no test changes. Option (1) above would cover it as well; option (2) or (3) means I ask you about it separately.
Model: opus-5-5
Standing approval recorded (sneak, #71 (comment), option 1): the setup lines #154 adds to newTestServer and the constant #157 adds to internal/imgcache/testutil_internal_test.go are approved, and from now on any edit to an existing test helper that only adds setup the new code needs and changes no check needs no question. Together with #64 (comment), the owner is asked only when a test's checks change.
Model: opus-5-5
Standing approval recorded (sneak, https://git.eeqj.de/sneak/pixa/issues/71#issuecomment-106558, option 1): the setup lines https://git.eeqj.de/sneak/pixa/pulls/154 adds to `newTestServer` and the constant https://git.eeqj.de/sneak/pixa/pulls/157 adds to `internal/imgcache/testutil_internal_test.go` are approved, and from now on any edit to an existing test helper that only adds setup the new code needs and changes no check needs no question. Together with https://git.eeqj.de/sneak/pixa/issues/64#issuecomment-106095, the owner is asked only when a test's checks change.
Model: opus-5-5
sneak
was unassigned by clawbot2026-09-29 10:49:11 +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.
Verified against
mainat61f42e6.maintenance_modeis a real config key (config.example.yml:6,internal/config/config.go:38,103), it is reported in the healthcheck JSON (internal/healthcheck/healthcheck.go:79), andServer.MaintenanceMode()exists (internal/server/server.go:148-151).Server.MaintenanceMode()has no callers — grep acrossinternal/andcmd/finds only the definition and test references. No route, middleware, or handler consults it.So turning on maintenance mode changes exactly one thing: the healthcheck JSON says
true. Every request is still served normally. An operator who sets it believing traffic will be drained gets no such behavior, silently — the failure mode is that they find out during the maintenance they were trying to perform.Definition of done
maintenance_modeis true, image-serving routes return503 Service Unavailablewith aRetry-Afterheader and the standard JSON error body used elsewhere./metricslikewise — decide deliberately and state the choice in the PR./,/generate) stays available during maintenance; recommend yes, so an operator can still work.internal/server/routes.gorather than a check sprinkled through handlers.Retry-After, and the healthcheck still returns 200 reportingmaintenance: true.make checkgreen.Alternative
If maintenance mode is only ever meant as a signal for an external load balancer to drain, then the code is already correct and the gap is documentation. In that case: document it in
README.mdandconfig.example.ymlas advisory-only, and remove the unusedServer.MaintenanceMode()accessor. Pick one — the current state (a real-looking accessor nobody calls) reads as an unfinished feature.Plan, checked against current
next:Server.MaintenanceMode()still has no caller, and turning onmaintenance_modeonly changes the health check'smaintenance_modefield.internal/server/routes.go, applied to a route group that holds only the image routes (/v1/image/GET and HEAD,/v1/e/). Whilemaintenance_modeis on it answers 503 with aRetry-Afterof a fixed number of seconds (one constant) and the same JSON error body (error,status,timestamp) the image handlers send. It callsServer.MaintenanceMode(), so the accessor gets its caller./.well-known/healthcheck.json) stays 200 and reports"maintenance_mode": true. The image's DockerHEALTHCHECKand upaas both read it, and a 503 there would make upaas mark the deploy failed. This goes inREADME.mdandconfig.example.yml./metricsstays available: it has its own password, touches no image, and is how an operator watches the instance during the maintenance./,/generate,/logout) stay available: they only sign URLs and never fetch or process an image, so an operator can keep working.Retry-Afterand the JSON body, and the health check and login page still answer 200.PIXA_MAINTENANCE_MODEalready exists (#128).Model: opus-5-5
Built as planned above in #154. The
Retry-Afteris 300 seconds.Model: opus-5-5
Question for sneak: approve one edit to an existing test file in #154, and if you agree, this kind of edit from now on?
newTestServerininternal/server/login_rate_limit_internal_test.gobuilds the server the login-limit tests run against. The new maintenance-mode tests also request the health check through it, so the PR adds the health check to what it builds (eight lines of setup). No test's checks change. #61 (comment) asks you about a one-line edit to the same helper.Options: (1) approve this edit and, standing, any edit to an existing test helper that only adds setup the new code needs and changes no assertion (recommended; together with the standing approval asked on #64 (comment), you would then be asked only when a check itself changes); (2) approve this edit only; (3) refuse.
Until you answer, the PR goes through review but is not merged; other work carries on.
Model: opus-5-5
Addition to the question above: #157 (issue #70) adds one constant,
testContentTypeWebP = "image/webp", to the shared test helper fileinternal/imgcache/testutil_internal_test.go; no test changes. Option (1) above would cover it as well; option (2) or (3) means I ask you about it separately.Model: opus-5-5
1 yes approved and standing rule approved
Standing approval recorded (sneak, #71 (comment), option 1): the setup lines #154 adds to
newTestServerand the constant #157 adds tointernal/imgcache/testutil_internal_test.goare approved, and from now on any edit to an existing test helper that only adds setup the new code needs and changes no check needs no question. Together with #64 (comment), the owner is asked only when a test's checks change.Model: opus-5-5
clawbot referenced this issue2026-09-29 11:01:45 +02:00