ci: bump image digests to 7a3dfb06db

Built and scanned by 7a3dfb06db. ArgoCD syncs from this commit.

[skip ci]

.gitea/workflows/ci.yaml

@@ -28,6 +28,17 @@ env:
   REGISTRY: gitea.oci-oci.duckdns.org
   IMAGE_NS: gitea_admin
   UV_VERSION: "0.11.29"
+  # Every uv job logged this, five times a run:
+  #
+  #   warning: Failed to hardlink files; falling back to full copy. This may lead to
+  #   degraded performance.
+  #
+  # The uv cache (the runner's cache volume) and the target venv are on different
+  # filesystems here, so hardlinking cannot work and uv copies 86 packages anyway. Saying
+  # `copy` up front does not make it slower — it is already copying — it just stops the
+  # warning from being noise that trains people to skim CI logs. The Dockerfiles set the
+  # same variable for the same reason.
+  UV_LINK_MODE: copy
   GITLEAKS_VERSION: "8.30.1"
   GITLEAKS_SHA256: "e4a487ee7ccd7d3a7f7ec08657610aa3606637dab924210b3aee62570fb4b080"
 
@@ -92,7 +103,7 @@ jobs:
     needs: [unit]
     services:
       postgres:
-        image: postgres:18@sha256:32ca0af8e77bfb8c6610c488e4691f83f972a3e9e64d3b02facf3ab111ad5500
+        image: mirror.gcr.io/library/postgres:18@sha256:32ca0af8e77bfb8c6610c488e4691f83f972a3e9e64d3b02facf3ab111ad5500
         env:
           POSTGRES_PASSWORD: postgres
           POSTGRES_DB: svcforge
@@ -373,6 +384,21 @@ jobs:
             -t svcforge/${{ matrix.svc }}:ci \
             .
 
+      - name: import smoke test
+        # Import the service's entrypoint module INSIDE the built image, which every unit and
+        # integration test that passes cannot do: they import from the source tree, where
+        # every file exists. The image is a different filesystem — each Dockerfile copies only
+        # its own `services/<svc>/`, so a shared module added at the `services/` root, or any
+        # dependency the Dockerfile forgets, is present in the tests and absent in the image.
+        #
+        # That gap shipped a reconciler that crashed on boot with
+        # `ModuleNotFoundError: No module named 'services._runtime'` while every gate was
+        # green. Importing main here loads the whole transitive graph and fails the build
+        # before the digest is pushed, instead of after ArgoCD has rolled it out.
+        run: |
+          docker run --rm --entrypoint python svcforge/${{ matrix.svc }}:ci \
+            -c "import services.${{ matrix.svc }}.main"
+
       - name: trivy
         # Run trivy directly rather than via aquasecurity/trivy-action, for the same reason
         # gitleaks is run directly above: the command is the documented one, pinned by
@@ -405,11 +431,30 @@ jobs:
         # comment here claimed, so the retention story matters. Trivy replaces the DB in
         # place rather than accumulating versions, so the volume stays at roughly one DB,
         # and the weekly prune CronJob reclaims it at the cost of one re-download.
+        env:
+          TRIVY: ghcr.io/aquasecurity/trivy:0.72.0@sha256:cffe3f5161a47a6823fbd23d985795b3ed72a4c806da4c4df16266c02accdd6f
         run: |
+          # Pull by digest, then give it a local tag, and run the TAG.
+          #
+          # An image pulled by digest carries no tag, which makes it dangling the moment its
+          # container exits — so the reclaim step below deleted it at the end of every matrix
+          # leg and the next leg paid a 178MB re-pull. Three legs, three pulls, every run.
+          #
+          # The `until=168h` filter there does not save it. That filter reads the image's
+          # CREATED timestamp, not when it was pulled, and this image was built months ago,
+          # so it matches the age filter immediately. A tag is what actually takes an image
+          # out of `docker image prune`'s scope. Verified in run #68: "Unable to find image
+          # ... locally" in all three legs, and "untagged: aquasec/trivy@sha256:cffe..." in
+          # each prune.
+          #
+          # The digest is still the pin — it is enforced here, at pull time. `trivy:pinned`
+          # is a local alias for an image whose content was already verified.
+          docker pull "${TRIVY}"
+          docker tag "${TRIVY}" trivy:pinned
           docker run --rm \
             -v /var/run/docker.sock:/var/run/docker.sock \
             -v svcforge-trivy-db:/root/.cache/trivy \
-            aquasec/trivy:0.72.0@sha256:cffe3f5161a47a6823fbd23d985795b3ed72a4c806da4c4df16266c02accdd6f \
+            trivy:pinned \
             image \
               --severity HIGH,CRITICAL \
               --ignore-unfixed \
@@ -439,6 +484,44 @@ jobs:
           docker buildx imagetools inspect "${IMAGE}:${GITHUB_SHA}" \
             --format '{{.Manifest.Digest}}'
 
+      - name: reclaim dind disk
+        # dind's /var/lib/docker is a hostPath on node0 (see oci-k8s addons role), so
+        # nothing reclaims it automatically — kubelet's image GC does not manage a nested
+        # daemon's store. Left alone it grows every run until node0 hits disk pressure and
+        # starts evicting pods, which looks like a cluster problem rather than a CI one.
+        #
+        # `always()`: a failed build still leaves layers behind, and that is exactly when
+        # the disk is most likely to be the reason it failed.
+        #
+        # Deliberately narrow. `docker image prune` WITHOUT -a removes dangling images
+        # only; with -a it would delete the act runner image, which no container references
+        # between jobs, and buy back a 1.6GB re-pull on the very next run.
+        #
+        # Named volumes are never pruned here: that is where the trivy vuln DB lives.
+        #
+        # NO age filter on the image prune, and that is the whole point of this comment.
+        # `--filter until=168h` reads an image's CREATED time, so it got both cases exactly
+        # backwards: it deleted trivy every leg (a released tool image is always older than
+        # any window — run #68, 178MB re-pulled three times) while protecting the dangling
+        # build layers it was added to remove (they are minutes old). Measured on node0
+        # afterwards: 21 dangling images, 5.96GB, 19 of them created inside 25 hours, none
+        # of them reclaimable while the filter was there. Tagging trivy is what protects
+        # trivy; nothing needs to protect a dangling layer from the current build, because
+        # `always()` runs this after that build has already been pushed.
+        #
+        # The buildx cache keeps its age filter: a week is recent enough that `--cache-from`
+        # still hits on normal traffic, and that cache really does grow by age.
+        #
+        # The act runner image survives only because this step runs inside an act container,
+        # so the image is in use exactly while the prune runs. That is luck, not design; if
+        # it ever starts disappearing, tag it the same way.
+        if: always()
+        run: |
+          docker image prune -f
+          docker buildx prune -af --filter until=168h
+          echo "--- dind disk after prune ---"
+          docker system df
+
   # --- stage 11: bump the chart's digests. CI's last act. ------------------------------
   bump:
     runs-on: ubuntu-latest

ARCHITECTURE.md

@@ -46,7 +46,7 @@ flowchart LR
     WORKER -->|"claim<br/>SKIP LOCKED"| PG
     WORKER -->|"helm upgrade --install"| K8S
     RECON -->|"drift, leases,<br/>TTL, versions"| PG
-    RECON -->|"helm list"| K8S
+    RECON -->|"list releases"| K8S
 
     classDef truth fill:#2d4a22,stroke:#5a8f3d,color:#fff
     classDef derived fill:#4a3222,stroke:#8f6a3d,color:#fff
@@ -191,12 +191,12 @@ stateDiagram-v2
     ready --> deleting: DELETE, or TTL expired
     deleting --> deleted: helm uninstall succeeded
 
-    requested --> failed: attempts exhausted
+    requested --> failed: provision attempts exhausted
-    provisioning --> failed: attempts exhausted
+    provisioning --> failed: provision attempts exhausted
     ready --> failed: drift — the release vanished
     failed --> provisioning: retry
     failed --> deleting: give up, tear it down
-    deleting --> failed: attempts exhausted
+    deleting --> deleting: deprovision retried, never failed
 
     deleted --> [*]: terminal
 ```
@@ -205,19 +205,31 @@ stateDiagram-v2
 chain of `if`s. `deleted` maps to an **empty frozenset** rather than being absent, so
 "terminal" is stated rather than implied by a missing key.
 
-**The state machine is enforced in SQL too.** `TaskRepo.fail` writes `instances.state`
+**Dead-lettering the task does not fail the instance, except for provision.** When a task
-directly, so it derives its guard from the same `LEGAL` table:
+exhausts its retries `TaskRepo.fail` marks the *task* `failed` for every kind. It moves the
+*instance* to `failed` only for `provision`, because that is the only kind where a dead
+letter means the instance is broken. For the others the instance is still healthy and
+something else owns its recovery:
+
+| kind | instance state on dead-letter | why |
+|------|-------------------------------|-----|
+| provision | `failed` | it never came up; a human re-provisions |
+| deprovision | stays `deleting` | so `due_for_deprovision` re-enqueues it; `failed` would strand it and leak the release |
+| upgrade | stays `ready` | `helm --atomic` rolled back; it still serves the old version |
+| verify | stays `ready` | `handle_verify` already halted the rollout |
+
+The provision write is still guarded by the state machine, derived from the same `LEGAL`
+table rather than restated:
 
 ```python
 _CAN_FAIL = tuple(s.value for s, allowed in LEGAL.items() if InstanceState.FAILED in allowed)
 ...
+if kind == 'provision':
     UPDATE instances SET state='failed' WHERE id=%s AND state = ANY(%s)
 ```
 
-Without that, a deprovision exhausting its retries against an already-`deleted` instance
+The `SvcforgeTaskDeadLettered` alert fires for every kind, so leaving the instance alone
-would resurrect it into `failed` — a transition `transition()` explicitly forbids,
+loses no operator visibility.
-performed by raw SQL that never asked it. A state machine only one layer respects is
-decoration.
 
 ---
 
@@ -278,7 +290,7 @@ One replica. Four checks. Every 60 seconds.
 
 ```mermaid
 flowchart LR
-    TICK(("tick<br/>every 60s")) --> D["<b>drift</b><br/>helm list vs DB"]
+    TICK(("tick<br/>every 60s")) --> D["<b>drift</b><br/>live releases vs DB"]
     TICK --> L["<b>lease expiry</b><br/>running + locked_at old"]
     TICK --> T["<b>TTL</b><br/>ready + expires_at passed"]
     TICK --> V["<b>version drift</b><br/>chart_version ≠ catalog"]

README.md

@@ -78,6 +78,24 @@ uv run pytest -q -m slow
 uv run python -m scripts.redis_budget            # projects month-end burn, exits 1 if over
 ```
 
+## Using the API
+
+**[USER_GUIDE.md](USER_GUIDE.md)** is the guide for callers: auth, the catalog, every
+endpoint with curl, the lifecycle, the error table, the CLI.
+
+The API also documents itself — FastAPI generates OpenAPI from the same models and routes
+it serves, so the spec cannot drift the way a hand-written one does.
+
+| What | Where |
+|---|---|
+| Swagger UI (try requests in the browser) | `https://svcforge.oci-oci.duckdns.org/docs` |
+| ReDoc (nicer to read) | `https://svcforge.oci-oci.duckdns.org/redoc` |
+| Raw spec, for generating clients | `https://svcforge.oci-oci.duckdns.org/openapi.json` |
+
+Locally, `uv run uvicorn services.api.main:app --factory` then <http://127.0.0.1:8000/docs>.
+`tests/integration/test_api.py` pins the description, the tags and the bearer security
+scheme, so the docs fail CI if they rot.
+
 ## Where things live
 
 | Module | Teaches | Read here |

RUNBOOK.md

@@ -118,13 +118,176 @@ kubectl -n gitea exec gitea-actions-runner-0 -c dind -- docker pull \
   ghcr.io/catthehacker/ubuntu:act-24.04@sha256:c710431fbad9eb3bcb102d04e5ff74fbd0ce6e383f78afebfb3770a1a817fdf9
 ```
 
-The durable fix is to stop the runner restarting. Its `/data` PVC is ReadWriteOnce, so
+The durable fix is a persistent image store, which the runner now has: `/var/lib/docker`
-every reschedule hits `Multi-Attach error` and the pod sits in Init until Longhorn detaches
+is a hostPath on node0 (see `oci-k8s/.../addons/tasks/main.yml`), so the act image survives
-from the old node. It is pinned to node2 in `oci-k8s/.../addons/tasks/main.yml` for exactly
+a restart and is not re-pulled. The runner is pinned to **node0**, not node2 — node2 is a
-that reason. A dedicated PVC for the image cache would survive restarts outright, but on
+single-core control-plane node whose pod network was measured 21x slower under its own
-this cluster that volume faulted and blocked the runner, so it is deliberately not used.
+load, which starved every clone and pull. Its `/data` PVC is NFS ReadWriteMany, so a
+reschedule attaches immediately with no `Multi-Attach` wait. Entries 9 and 10 cover the
+caches and the node move in full.
 
-### 6. Verify the whole loop, not just the green checkmarks
+### 6. Stopping a run, and reading a restarted runner correctly
+
+Restarting the act_runner StatefulSet does not reliably orphan its in-flight jobs. Both
+outcomes have been observed on this cluster:
+
+- run #14 was left with two jobs `in_progress` and nothing behind them, still stuck at
+  15m45s, so `ZOMBIE_TASK_TIMEOUT` is not a rescue you can wait for
+- run #16 had its remaining jobs re-dispatched to the new pod and carried on normally
+
+So `in_progress` after a restart is ambiguous on its own. Check whether the runner is
+actually working before concluding anything, or you will diagnose a healthy run as a zombie:
+
+```bash
+kubectl -n gitea logs gitea-actions-runner-0 -c runner --tail=4
+```
+
+Recent `NewParallelExecutor` lines mean it is executing, not stuck. At `capacity: 1` a later
+run sitting in `waiting` behind a live one is correct, not a block.
+
+**Gitea 1.26 has no cancel endpoint at all** — the full swagger contains no path matching
+`cancel`. The red "Cancel workflow run" button in the web UI is a CSRF-protected web route,
+so an API token cannot drive it, and cancelling from a script is simply not available.
+
+`DELETE .../actions/runs/{index}` exists and takes the **run index** (`14`), not the database
+id (`51`). It cleared the stuck #14, but returned 204 against the live #16 without stopping
+it and then 404 on retry. Treat it as a way to remove a finished run, not a cancel.
+
+To stop a running job: click Cancel in the UI.
+
+### 7. Gitea postgres: `Input/output error`, and when scale 0/1 is not enough
+
+`gitea-postgresql-0` CrashLoopBackOff with:
+
+    mkdir: cannot create directory '/bitnami/postgresql/data': Input/output error
+
+and the Gitea API returning 500, so every CI run dies at checkout with
+`Failed to connect to gitea-http:3000`. The "Initializing PostgreSQL database" line above
+that error is alarming and is not what it looks like: the data is fine, the mount is broken,
+so the container sees an empty directory.
+
+The documented recovery — scale to 0, wait for `detached`, scale back to 1 — was **not
+enough** here. The volume came back `detached/faulted` and simply refused to attach, so the
+pod sat in ContainerCreating. `auto-salvage: true` does not help: salvage happens during
+attach, and a faulted volume never gets that far, so it cannot rescue itself.
+
+What the volume was actually saying:
+
+```bash
+V=$(kubectl -n gitea get pvc -o jsonpath='{.items[?(@.metadata.name=="data-gitea-postgresql-0")].spec.volumeName}')
+kubectl -n longhorn-system get volume $V -o jsonpath='{.status.state}/{.status.robustness}'   # detached/faulted
+kubectl -n longhorn-system get replicas.longhorn.io -o json \
+  | jq -r '.items[]|select(.spec.volumeName=="'$V'")|[.metadata.name,.spec.failedAt]|@tsv'
+```
+
+The replica carries a `failedAt` timestamp, and that alone is what keeps the volume faulted.
+Clearing it is the salvage:
+
+```bash
+kubectl -n longhorn-system patch replicas.longhorn.io <replica-name> --type merge \
+  -p '{"spec":{"failedAt":"","lastFailedAt":""}}'
+```
+
+The volume went `attached/healthy` and postgres reached 1/1 within 40 seconds, with the repo,
+its size and the whole CI run history intact.
+
+**Check the backups before patching anything**, because this cluster runs Longhorn at one
+replica — there is no second copy to fall back on, only the nightly backup:
+
+```bash
+kubectl -n longhorn-system get backups.longhorn.io -o json \
+  | jq -r '.items[]|select(.status.volumeName=="'$V'")|[.status.backupCreatedAt,.status.state]|@tsv'
+```
+
+Salvage reuses the replica exactly as it was when it failed, so Postgres may do crash
+recovery on start. If it cannot, restore the most recent Completed backup instead.
+
+### 8. OPEN: dind is killed by its own liveness probe
+
+Unresolved as of 2026-07-20. Recorded because it probably explains build failures that were
+diagnosed as something else.
+
+The runner sits at `Init:1/2` and its dind sidecar accumulates restarts:
+
+```
+Liveness probe failed: command timed out:
+  "/usr/bin/test -S /var/run/docker.sock" timed out after 1s   (x27 over 156m)
+Killing: Init container dind failed liveness probe
+```
+
+The probe is hardcoded at `timeoutSeconds: 1`, `failureThreshold: 3`, `periodSeconds: 10`.
+`test -S` only asks whether a socket exists. When that cannot finish inside a second, the
+node is starved rather than dind being unhealthy, and kubelet kills a working daemon.
+
+**Why this matters beyond the runner restarting.** Image builds failed with:
+
+    ERROR: failed to solve: DeadlineExceeded: no active session for <id>
+
+which was attributed to CPU starvation alone and addressed by dropping the runner's
+`capacity` to 1. Starvation is real, but the mechanism is more likely that kubelet killed
+dind mid-build and the buildkit session died with it. Lowering capacity reduced the load
+that trips the probe, which is consistent with run #17 passing — it treated the cause of
+the trigger, not the trigger. Runs #18-#21 then failed anyway.
+
+Treat this as a strong hypothesis, not a settled one. Confirming it means correlating the
+kill timestamps against the failed builds:
+
+```bash
+kubectl -n gitea describe pod gitea-actions-runner-0 | grep -A10 Events:
+kubectl -n gitea get pod gitea-actions-runner-0 \
+  -o jsonpath='{.status.initContainerStatuses[?(@.name=="dind")].lastState.terminated}'
+```
+
+**The chart exposes no probe knobs** — `helm show values gitea-charts/actions` has no match
+for `probe`. So this cannot be fixed the way `capacity` was, and a `kubectl patch` is
+reverted by the next Ansible run. Same shape as the longhorn-csi-plugin probe problem.
+
+The candidate fix is a Kyverno mutating policy authored in Ansible, relaxing the probe to
+roughly `timeoutSeconds: 5` and `failureThreshold: 6`. This cluster already mutates
+workloads that way — see `force-best-effort-cpu`, which rewrites every CPU request to 0 —
+so the precedent and the tooling are both in place.
+
+### 9. The runner's three ephemeral caches, and the boot cascade they cause
+
+Three separate caches on this runner were container-layer only, each found the same way —
+something was slow, and the cause was a cache that had never survived a restart:
+
+| cache | path | fixed by |
+|---|---|---|
+| dind image store | `/var/lib/docker` | hostPath `/var/lib/gitea-dind` |
+| trivy vulnerability DB | `/root/.cache/trivy` in dind | named docker volume |
+| act's action clones | `/root/.cache/act` | hostPath `/var/lib/gitea-act-cache` |
+
+act clones each action with **full history**, not shallow: 66.7MB/538 commits for
+`astral-sh/setup-uv`, 24.4MB/222 commits for `actions/checkout`, and this workflow uses
+five. After a restart that made `Set up job` an 11-minute step in which the job container
+sat idle running `sleep` while the runner cloned GitHub. If a job looks hung in setup, check
+the job container before blaming the network:
+
+```bash
+C=$(kubectl -n gitea exec gitea-actions-runner-0 -c dind -- docker ps -q | head -1)
+kubectl -n gitea exec gitea-actions-runner-0 -c dind -- docker exec "$C" ps -eo pid,etime,comm
+```
+
+Only `sleep` means the work is in the runner, not the job.
+
+**The cascade this creates.** Giving dind a persistent image store made it boot slowly,
+because dockerd scans that store on startup — measured at 38s, 2m13s, and over 5 minutes
+depending on node load. Two independent timeouts then fire:
+
+1. dind's startup probe. Widened to 5 minutes by the Kyverno policy in oci-k8s, and a cold
+   boot has still exceeded it.
+2. **The runner container's own `Docker wait timeout of 5m0s`**, which is internal to the
+   runner image and not configurable from the chart. When dind is late, the runner exits 1
+   and restarts — and that restart kills whatever job was running, which surfaces as every
+   step in the job failing at once with no error in the log, right after a green
+   `Set up job`.
+
+The pair self-heals: the second dind boot is fast because the store is warm, and the runner
+comes up behind it. The cost is roughly ten minutes of thrash after any runner restart, and
+one lost CI run. Restart the runner deliberately, not casually.
+
+### 10. Verify the whole loop, not just the green checkmarks
 
 ```bash
 # the digest CI pushed
@@ -319,3 +482,171 @@ remembering.
 
 **Escalate** if size is growing with `tasks` already pruned — that means `instances` is
 growing, i.e. tenants are real, i.e. the free tier is the wrong tier.
+
+---
+
+## Deploy stuck: ArgoCD says Synced at an old commit
+
+**Symptom:** CI is green and the bump commit is on `master`, but the running pods are on the
+previous digest. `kubectl -n argocd get application svcforge` says **`Synced`** — at a
+revision several commits behind. Nothing looks broken, which is what makes this expensive.
+
+**Diagnose.** Compare what ArgoCD thinks it synced against what `master` actually is:
+
+```bash
+kubectl -n argocd get application svcforge \
+  -o jsonpath='{.status.sync.revision}{"  reconciledAt="}{.status.reconciledAt}{"\n"}'
+git -C ~/workspace/svcforge-reference log --oneline origin/master -1
+```
+
+**`reconciledAt` alone does not tell you.** ArgoCD writes that field only when the computed
+status *changes*, so on a cluster where everything is Synced and nothing is deploying it can
+sit still while the controller is fine. It is evidence only when paired with a
+`sync.revision` that is *behind `master`* — which is exactly the case you are in if you are
+reading this section.
+
+**One metric answers it, and only one.** The controller observes
+`argocd_app_reconcile_count` once per completed app reconciliation. Flat means it is doing
+nothing:
+
+```bash
+kubectl -n monitoring exec prometheus-kube-prometheus-stack-prometheus-0 -c prometheus -- \
+  wget -qO- --post-data='query=sum(increase(argocd_app_reconcile_count[10m]))' \
+  http://localhost:9090/api/v1/query
+```
+
+**Two metrics that look like they answer it and do not.** Both were tried on 2026-07-22
+against a controller that had reconciled nothing for 82 minutes, and both read healthy:
+
+| metric | reading at the time | why it lies |
+|---|---|---|
+| `argocd_redis_request_total` | 45 reads / 15m, climbing | something in the process still touches the cache when nothing is reconciling — it measures "the pod is running", which `up` already covers |
+| `workqueue_unfinished_work_seconds{name="app_reconciliation_queue"}` | 0 | it only counts work already *in* the queue, and the queue is empty. Nothing is stuck; nothing is being enqueued |
+
+Keep the second one anyway — it catches a genuinely stuck queue item, which is a different
+failure. Just never read a zero from it as "healthy".
+
+**What is actually wrong: the periodic git poll does not run.** Read the controller's own
+metrics on an idle cluster:
+
+```bash
+kubectl -n monitoring exec prometheus-kube-prometheus-stack-prometheus-0 -c prometheus -- \
+  wget -qO- http://argocd-application-controller-metrics.argocd.svc:8082/metrics \
+  | grep -E '^workqueue_(depth|adds_total|longest_running_processor_seconds)\{controller="app_reconciliation_queue"'
+```
+
+Measured 2026-07-22, across two separate controller pods, on a fully healthy API server:
+
+```
+03:37:03  controller starts, refreshes all 3 apps   adds_total = 3
+03:38:31  adds_total = 3
+03:41:33  adds_total = 3
+03:44:34  adds_total = 3
+03:47:35  adds_total = 3      <- expiry is 2m0s, jitter 60s
+```
+
+`workqueue_depth 0`, `longest_running_processor_seconds 0`, `adds_total` frozen. Nothing is
+*blocked* — nothing is being **enqueued**. The controller logs its own schedule at startup
+(`appResyncPeriod=2m0s, appResyncJitter=1m0s`) and `argocd-cm` carries the matching
+`timeout.reconciliation: 120s`, so the setting is read and then never acted on.
+
+Refreshes still happen from two other paths, which is what makes this so easy to
+misread as working:
+
+| path | fires when | observed |
+|---|---|---|
+| startup | controller (re)starts | 3 apps refreshed within ~2s of ready |
+| cluster events | a watched resource changes | 68 adds during one svcforge rollout, then flat the moment the cluster went quiet |
+| periodic poll | every 2m ± 60s | **never** |
+
+**The consequence is the thing to take away: a commit that changes only the repo is never
+noticed.** Every "auto-sync" observed on 2026-07-22 happened within seconds of a controller
+restart, i.e. it was the startup refresh, not the poll. Do not read a successful deploy
+straight after a restart as evidence that polling works.
+
+**A webhook now covers for it** (added 2026-07-22, `oci-k8s` `--tags argocd,gitea`). Gitea
+POSTs every push to `https://argocd.oci-oci.duckdns.org/api/webhook`, so a commit refreshes
+ArgoCD in under a second instead of waiting for a poll that never comes.
+
+It is registered as Gitea's **`gogs`** type, which looks wrong and is not: ArgoCD's webhook
+handler dispatches on the `X-Gogs-Event` header and ships no Gitea parser. Gitea forked from
+Gogs and still emits that wire format on request. The shared secret lives in
+`argocd-secret` as `webhook.gogs.secret` and in Ansible as `argocd_webhook_secret`; both
+sides must match or every delivery fails signature validation *silently*, which looks
+identical to having no webhook at all.
+
+Check a delivery when a push does not deploy:
+
+```bash
+# Gitea's own record of the last attempt, including the response ArgoCD gave
+curl -s -u "$USER:$PASS" \
+  https://gitea.oci-oci.duckdns.org/api/v1/repos/gitea_admin/svcforge/hooks | jq '.[].id'
+
+# ArgoCD's side
+kubectl -n argocd logs deploy/argocd-server --tail=200 | grep -i webhook
+```
+
+`Unknown webhook event` means the hook type is wrong (it must be `gogs`). A 400 on signature
+means the secrets have drifted — re-run `03_install_addons.yml --tags argocd,gitea`, which
+rewrites both ends from the same variable.
+
+Manual nudge, still valid if the webhook is ever down:
+
+```bash
+kubectl -n argocd annotate application svcforge argocd.argoproj.io/refresh=normal --overwrite
+```
+
+Without Prometheus, fall back to the logs — this works and is what found the 2026-07-22
+wedge before the metrics were checked:
+
+```bash
+# Healthy: a few hundred lines an hour. Stalled: exactly 6 — the 10-minute memory heartbeat.
+kubectl -n argocd logs statefulset/argocd-application-controller --tail=8000 \
+  | grep -oE 'time="[0-9-]+T[0-9]{2}' | sort | uniq -c | tail
+```
+
+A flat `Goroutines=NNN` across hours in those heartbeat lines means blocked goroutines, not
+an idle controller.
+
+**Cause seen here (2026-07-21).** Not ArgoCD config — `timeout.reconciliation` was 120s the
+whole time. The controller's server-side dry-run applies go through the cluster's admission
+webhooks, and Kyverno's mutate webhook was `failurePolicy: Fail`. Kyverno restarts under
+this cluster's memory pressure, and each restart is a window where that webhook is
+unreachable, so the applies blocked and the controller wedged for **11 hours** — reconciling
+zero apps while still reporting `Synced`. Fixed in `oci-k8s` by setting `failurePolicy:
+Ignore` on both ClusterPolicies; see the comment there.
+
+**Unstick it now:**
+
+```bash
+# 1. Force a re-poll. If the revision advances, polling was the only problem.
+kubectl -n argocd annotate application svcforge argocd.argoproj.io/refresh=hard --overwrite
+
+# 2. If it does not advance within ~60s, the controller is wedged. Restart it —
+#    ArgoCD holds no state of its own; everything is in the cluster and in git.
+kubectl -n argocd rollout restart statefulset/argocd-application-controller
+kubectl -n argocd rollout status statefulset/argocd-application-controller --timeout=180s
+```
+
+**Then check the actual chain, because `Synced` is not the same as `deployed`:**
+
+```bash
+git show origin/master:deploy/chart/values.yaml | grep -A2 -E '^\s+(api|worker|reconciler):'
+kubectl -n svcforge get pods \
+  -o jsonpath='{range .items[*]}{.spec.containers[0].image}{"\n"}{end}' | sort -u
+```
+
+The digests must match. If the sync stalls with a Job stuck `Complete` but never deleted,
+it is holding `argocd.argoproj.io/hook-finalizer` — see below.
+
+**Related: the migrate Job deadlock.** A PreSync hook Job that finished but keeps the
+finalizer blocks the sync forever:
+
+```bash
+kubectl -n svcforge get job svcforge-migrate -o jsonpath='{.metadata.finalizers}{"\n"}'
+kubectl -n svcforge patch job svcforge-migrate --type=merge -p '{"metadata":{"finalizers":null}}'
+```
+
+The Application goes `Synced` within seconds of the patch.
+
+<!-- ci: exercising the runner CPU limit and the webhook end to end, 2026-07-22 -->

USER_GUIDE.md

@@ -0,0 +1,267 @@
+# svcforge — user guide
+
+Ask for a managed service, get one. svcforge provisions Elasticsearch, Redis, Postgres and
+a couple of tiny test services into Kubernetes, one namespace per team, and keeps them
+matching what the database says they should be.
+
+**Base URL:** `https://svcforge.oci-oci.duckdns.org`
+
+| | |
+|---|---|
+| Swagger UI (send requests from the browser) | [`/docs`](https://svcforge.oci-oci.duckdns.org/docs) |
+| ReDoc (nicer to read) | [`/redoc`](https://svcforge.oci-oci.duckdns.org/redoc) |
+| Raw OpenAPI spec | [`/openapi.json`](https://svcforge.oci-oci.duckdns.org/openapi.json) |
+
+The spec is generated from the same models and routes the server runs, so it cannot drift
+from the implementation. Generate a client from it rather than hand-rolling one.
+
+---
+
+## The three things that will surprise you
+
+1. **Writes are asynchronous.** `POST` and `DELETE` return **202 Accepted**. Nothing is
+   provisioned when you get the response — you have a row and a queued task. Poll
+   `GET /v1/instances/{id}` and watch `state`.
+2. **Authorisation is a WHERE clause.** Another team's instance returns **404**, not 403.
+   The API never confirms that an id you cannot access exists.
+3. **Every non-2xx body is `{"code", "message"}`** — including the 404s and 405s raised by
+   the framework itself. Never branch on the body's shape.
+
+---
+
+## Getting a token
+
+Every `/v1` route needs `Authorization: Bearer <jwt>`. The token is verified against the
+configured JWKS (RS256 only), and its **`team` claim** decides which instances you see.
+`aud`, `iss` and `exp` are all required and all checked.
+
+> **The public deployment currently issues no tokens.** `SVCFORGE_JWKS_URL` points at
+> `https://auth.oci-oci.duckdns.org/realms/svcforge/...`, and no identity provider is
+> deployed there — the hostname resolves to the ingress, which answers with its default
+> self-signed certificate. The API logs `JWKS warm-up failed` at startup and every `/v1`
+> request returns `401`. Unauthenticated routes (`/healthz`, `/readyz`, `/metrics`,
+> `/docs`, `/openapi.json`) work normally. To make the live API usable, either deploy an
+> OIDC provider at that realm URL or repoint `auth.jwksUrl` / `auth.issuer` in
+> `deploy/chart/values.yaml` at one that exists.
+
+Any OIDC provider works. With Keycloak, the client-credentials flow is:
+
+```bash
+TOKEN=$(curl -s -X POST \
+  https://auth.example.com/realms/svcforge/protocol/openid-connect/token \
+  -d grant_type=client_credentials \
+  -d client_id=svcforge-cli \
+  -d client_secret="$CLIENT_SECRET" | jq -r .access_token)
+```
+
+The provider must put a `team` claim in the token (a Keycloak protocol mapper, or the
+equivalent) and set `aud: svcforge`. A token without a non-empty string `team` is a 401.
+
+### Running it locally instead
+
+The fastest way to actually drive the API is to run it yourself with auth off:
+
+```bash
+export SVCFORGE_PG_DSN="postgresql://svcforge:svcforge@127.0.0.1:5432/svcforge"
+export SVCFORGE_AUTH_DISABLED=true          # refused unless SVCFORGE_ENVIRONMENT=local
+uv run uvicorn services.api.main:app --factory
+```
+
+Every request is then team `platform` and no header is needed. `check_production()` refuses
+this flag whenever `SVCFORGE_ENVIRONMENT` is anything but `local`, so it cannot escape a
+laptop.
+
+---
+
+## The catalog
+
+`service_type` and `size` must both exist in the catalog. An unknown `service_type` is a
+**404**; a real service type with an unknown size is a **422** that lists the valid sizes.
+
+| `service_type` | `size` | Memory request | Notes |
+|---|---|---|---|
+| `elasticsearch` | `small`, `medium` | 1Gi / 4Gi per replica | 1 or 3 replicas |
+| `redis` | `small`, `medium` | 256Mi / 1Gi | 1 or 3 replicas |
+| `postgres` | `small`, `medium` | 512Mi / 2Gi | 1 or 2 replicas |
+| `podinfo` | `small`, `medium` | 16Mi / 32Mi | tiny, for exercising the platform |
+| `nginx` | `small`, `medium` | 32Mi / 64Mi | tiny, for exercising the platform |
+
+Use `podinfo` or `nginx` to exercise the control plane: they are single small pods and fit
+on a node with no room for a real Elasticsearch. Both are pulled straight from an OCI
+registry. The three larger entries reference a `bitnamilegacy/` chart repo that the worker
+image does not currently configure, so they will fail at provision time until it is added.
+
+---
+
+## Endpoints
+
+### `POST /v1/instances` → 202
+
+```bash
+curl -X POST https://svcforge.oci-oci.duckdns.org/v1/instances \
+  -H "Authorization: Bearer $TOKEN" \
+  -H 'Content-Type: application/json' \
+  -d '{"service_type": "podinfo", "size": "small", "ttl_days": 7}'
+```
+
+```json
+{
+  "id": "0f8b7d3e-1c2a-4f5b-9e6d-7a8b9c0d1e2f",
+  "state": "requested",
+  "service_type": "podinfo",
+  "size": "small",
+  "endpoint": null,
+  "chart_version": "6.7.1",
+  "error": null
+}
+```
+
+The response carries a `Location` header pointing at the instance. `ttl_days` (1–30,
+optional) deletes the instance automatically; omit it for no expiry. Unknown body fields
+are rejected with a 422 rather than ignored.
+
+### `GET /v1/instances/{id}` → 200
+
+The polling endpoint. Repeat until `state` is `ready` or `failed`.
+
+```bash
+curl -H "Authorization: Bearer $TOKEN" \
+  https://svcforge.oci-oci.duckdns.org/v1/instances/$ID
+```
+
+### `GET /v1/instances` → 200
+
+Your team's instances, newest first. `?limit=` accepts 1–200 and defaults to 50. No
+endpoint here returns "all rows".
+
+### `DELETE /v1/instances/{id}` → 202
+
+```bash
+curl -X DELETE -H "Authorization: Bearer $TOKEN" \
+  https://svcforge.oci-oci.duckdns.org/v1/instances/$ID
+```
+
+Moves the instance to `deleting` and queues the teardown; the helm uninstall has not
+happened when this returns. Deleting something already `deleting` or `deleted` is a **409**.
+
+### Unauthenticated
+
+`GET /healthz` (liveness, no I/O) · `GET /readyz` (readiness, checks Postgres only) ·
+`GET /metrics` (Prometheus exposition).
+
+---
+
+## The lifecycle
+
+```
+requested ──> provisioning ──> ready ──> deleting ──> deleted
+     │              │            
+     └──────────────┴──> failed
+```
+
+| State | What it means |
+|---|---|
+| `requested` | The row exists and a provision task is queued. |
+| `provisioning` | A worker is running `helm upgrade --install`. |
+| `ready` | The release is up. **Only this state carries a usable `endpoint`.** |
+| `failed` | The provision exhausted its retries. `error` says why. |
+| `deleting` | Teardown queued or running. |
+| `deleted` | Terminal. |
+
+`endpoint` is in-cluster DNS —
+`http://<release>.tenant-<team>.svc.cluster.local` — reachable from inside the cluster, not
+from your laptop.
+
+A few things happen without you asking:
+
+- **Drift repair.** If a `ready` instance's helm release disappears, the reconciler notices
+  within ~60s and re-provisions it. You may see `ready → failed → provisioning → ready`.
+- **TTL.** An instance past `expires_at` is torn down automatically.
+- **Upgrades.** When the catalog pins a newer chart version, instances are upgraded inside
+  their maintenance window, one at a time. Entries marked `security: true` skip the window.
+
+A `failed` instance is not retried automatically — it needs a human.
+
+---
+
+## Errors
+
+Every non-2xx body is the same shape. `code` is stable and meant for machines; `message` is
+for humans and must not be parsed.
+
+```json
+{"code": "unknown_service_type", "message": "no such service_type: mongodb"}
+```
+
+| Status | `code` | Cause |
+|---|---|---|
+| 401 | `unauthorized` | Missing, expired, malformed or unverifiable token. Never says which. |
+| 404 | `unknown_service_type` | Not in the catalog. |
+| 404 | `not_found` | No such instance — **or it belongs to another team**. |
+| 409 | `illegal_transition` | e.g. deleting something already deleted. |
+| 409 | `conflict` | The row changed between the read and the write. Retry. |
+| 422 | `unknown_size` | The message lists the sizes that exist. |
+| 422 | `validation_error` | Malformed body: bad type, extra field, `ttl_days` out of range. |
+| 429 | `rate_limited` | Over the per-team budget. Honour `Retry-After`. |
+| 503 | `not_ready` | `/readyz` only: Postgres is unreachable. |
+
+**Rate limiting** is per team, 60 requests/minute by default, and it fails *open* — if the
+limiter's Redis is down you are unmetered rather than refused. A 429 always means a real,
+counted overage.
+
+---
+
+## The CLI
+
+`services/cli/` is a thin API client. It holds a URL and a token and never touches the
+database.
+
+```bash
+export SVCFORGE_API_URL=https://svcforge.oci-oci.duckdns.org
+export SVCFORGE_API_TOKEN="$TOKEN"
+
+svcforge create podinfo --size small --ttl 7d --wait
+svcforge list --state ready
+svcforge status <instance-id>
+svcforge delete <instance-id> --yes
+```
+
+`--wait` polls until the instance reaches `ready` or `failed`.
+
+---
+
+## Generating a client
+
+```bash
+curl -s https://svcforge.oci-oci.duckdns.org/openapi.json > openapi.json
+openapi-generator-cli generate -i openapi.json -g python -o ./client
+```
+
+The error models are declared on every route, so a generated client gets typed 401/404/409/
+422 bodies rather than guessing. `tests/integration/test_api.py` pins the description, the
+tags and the bearer security scheme, so these docs fail CI if they rot.
+
+---
+
+## Polling, end to end
+
+```bash
+ID=$(curl -s -X POST https://svcforge.oci-oci.duckdns.org/v1/instances \
+  -H "Authorization: Bearer $TOKEN" -H 'Content-Type: application/json' \
+  -d '{"service_type":"podinfo","size":"small","ttl_days":1}' | jq -r .id)
+
+while :; do
+  BODY=$(curl -s -H "Authorization: Bearer $TOKEN" \
+    https://svcforge.oci-oci.duckdns.org/v1/instances/$ID)
+  STATE=$(jq -r .state <<<"$BODY")
+  echo "$STATE"
+  case "$STATE" in
+    ready)  jq -r .endpoint <<<"$BODY"; break ;;
+    failed) jq -r .error    <<<"$BODY"; exit 1 ;;
+  esac
+  sleep 5
+done
+```
+
+Poll every few seconds, not every few milliseconds. A provision is a helm install against a
+StatefulSet; `podinfo` takes seconds, Elasticsearch takes minutes.

catalog.yaml

@@ -3,11 +3,33 @@
 #
 # security: true bypasses every tenant's maintenance window for this entry. Set it for a
 # CVE with a public exploit; leave it false and the bump waits for 03:00 Sunday.
+#
+# Every chart is addressed as `oci://`, which is not cosmetic: an OCI chart is pulled by
+# reference, with no `helm repo add` first. The three entries below previously named
+# `bitnamilegacy/<chart>`, a classic repo alias that nothing in the worker image
+# configures — so they could never resolve at provision time. OCI is the form that works
+# from a bare container.
+#
+# All of them go through mirror.gcr.io rather than registry-1.docker.io. Docker Hub
+# rate-limits anonymous pulls per source IP and every node here shares one NAT address, so
+# a busy afternoon becomes `toomanyrequests` on an unrelated deploy. mirror.gcr.io is a
+# pull-through cache with no such limit, verified digest-for-digest identical.
+#
+# The chart pull is only half of it. A bitnami chart defaults its own images to
+# `registry-1.docker.io/bitnami/<name>`, so the pods would still go to Docker Hub even
+# though the chart did not. `values:` on an entry is the fix: anything there is passed to
+# helm underneath the size's replicas and resources, so `global.imageRegistry` moves the
+# image pull too. `podinfo` needs none of this — its chart already points at ghcr.io.
 services:
   elasticsearch:
-    chart: bitnamilegacy/elasticsearch
+    chart: oci://mirror.gcr.io/bitnamicharts/elasticsearch
-    chart_version: 21.3.15
+    chart_version: 22.1.6
     security: false
+    # Sends the chart's own image pulls through the mirror as well, so nothing in this
+    # entry touches a rate-limited registry. Merged under the size below.
+    values:
+      global:
+        imageRegistry: mirror.gcr.io
     sizes:
       small:
         replicas: 1
@@ -29,9 +51,14 @@ services:
             memory: 8Gi
 
   redis:
-    chart: bitnamilegacy/redis
+    chart: oci://mirror.gcr.io/bitnamicharts/redis
-    chart_version: 20.6.2
+    chart_version: 27.0.15
     security: false
+    # Sends the chart's own image pulls through the mirror as well, so nothing in this
+    # entry touches a rate-limited registry. Merged under the size below.
+    values:
+      global:
+        imageRegistry: mirror.gcr.io
     sizes:
       small:
         replicas: 1
@@ -53,9 +80,14 @@ services:
             memory: 2Gi
 
   postgres:
-    chart: bitnamilegacy/postgresql
+    chart: oci://mirror.gcr.io/bitnamicharts/postgresql
-    chart_version: 16.4.5
+    chart_version: 18.8.0
     security: false
+    # Sends the chart's own image pulls through the mirror as well, so nothing in this
+    # entry touches a rate-limited registry. Merged under the size below.
+    values:
+      global:
+        imageRegistry: mirror.gcr.io
     sizes:
       small:
         replicas: 1
@@ -75,3 +107,70 @@ services:
           limits:
             cpu: "2"
             memory: 4Gi
+
+  # --- Small services, for exercising the control plane on a cluster with no room ---------
+  #
+  # The three entries above are real products and size accordingly: one `elasticsearch`
+  # small asks for 1Gi, and a medium asks for 4Gi across three replicas. On a test cluster
+  # that is a request that never schedules, so provisioning them proves nothing about
+  # svcforge and everything about the node.
+  #
+  # These two exist to exercise the actual loop — claim, helm install, CAS to ready, drift,
+  # TTL, deprovision — in seconds and in tens of megabytes.
+
+  podinfo:
+    # A single small Go binary with no dependencies, no PVC and a fast image pull. The e2e
+    # test provisions exactly this for the same reason.
+    chart: oci://ghcr.io/stefanprodan/charts/podinfo
+    chart_version: 6.14.0
+    security: false
+    sizes:
+      small:
+        replicas: 1
+        resources:
+          requests:
+            cpu: 10m
+            memory: 16Mi
+          limits:
+            cpu: 50m
+            memory: 32Mi
+      medium:
+        replicas: 2
+        resources:
+          requests:
+            cpu: 25m
+            memory: 32Mi
+          limits:
+            cpu: 100m
+            memory: 64Mi
+
+  nginx:
+    # A recognisable web server, still small. Useful when the thing being demonstrated needs
+    # to look like a service someone would actually ask for.
+    chart: oci://mirror.gcr.io/bitnamicharts/nginx
+    chart_version: 25.0.14
+    security: false
+    # Sends the chart's own image pulls through the mirror as well, so nothing in this
+    # entry touches a rate-limited registry. Merged under the size below.
+    values:
+      global:
+        imageRegistry: mirror.gcr.io
+    sizes:
+      small:
+        replicas: 1
+        resources:
+          requests:
+            cpu: 10m
+            memory: 32Mi
+          limits:
+            cpu: 100m
+            memory: 64Mi
+      medium:
+        replicas: 2
+        resources:
+          requests:
+            cpu: 25m
+            memory: 64Mi
+          limits:
+            cpu: 200m
+            memory: 128Mi

deploy/argocd/app.yaml

@@ -67,3 +67,23 @@ spec:
       kind: Job
       jsonPointers:
         - /spec/template/metadata/labels
+    # This cluster runs a Kyverno ClusterPolicy, `force-best-effort-cpu`, whose rule
+    # set-cpu-request-to-zero rewrites every container's CPU request to "0" at admission.
+    # It is deliberate and predates this app by well over a year: the nodes are
+    # oversubscribed, and making pods BestEffort on CPU is how everything gets scheduled.
+    #
+    # The chart asks for 50m and the cluster writes 0, so without this the Deployments sit
+    # permanently OutOfSync while being perfectly Healthy — the failure mode where a
+    # dashboard is always yellow, everyone learns to ignore it, and it stops meaning
+    # anything the day it goes yellow for a real reason.
+    #
+    # The chart deliberately keeps its real request rather than capitulating to 0. What the
+    # chart asks for is the honest intent; what the cluster does with it is the cluster's
+    # business, and a reader of the repo should see the former.
+    #
+    # jqPathExpressions, not jsonPointers: a pointer would have to name a container index,
+    # and this has to hold for every container in every one of the three Deployments.
+    - group: apps
+      kind: Deployment
+      jqPathExpressions:
+        - .spec.template.spec.containers[].resources.requests.cpu

deploy/chart/templates/migrate-job.yaml

@@ -13,6 +13,37 @@ after a failure — the one time you actually want it — and clears it on the n
 Deliberately no terminationGracePeriodSeconds: 60 here. Three Deployments carry it; a
 migration is not one of them.
 */}}
+{{/*
+The hook needs its own ServiceAccount, and it has to be a hook itself.
+
+It used to run as the api ServiceAccount, which is an ordinary chart resource. Hooks are
+created before the release's ordinary manifests, so on a first install that account does
+not exist yet and the Job never starts:
+
+    Error creating: pods "svcforge-migrate-" is forbidden: error looking up service
+    account svcforge/svcforge-api: serviceaccount "svcforge-api" not found
+
+This is not an ArgoCD quirk. `helm install` orders hooks the same way, so it failed
+identically on both paths. It survived review because the chart was only ever checked with
+`helm template` and `helm install --dry-run=server`, and neither creates a Job — the pod is
+what fails, so nothing short of a real install can catch it.
+
+Weight -10 so it is created before the Job at -5. Deliberately bound to no Role: the
+migration talks to Postgres and needs nothing from the Kubernetes API.
+*/}}
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: {{ include "svcforge.fullname" . }}-migrate
+  labels:
+    {{- include "svcforge.labels" . | nindent 4 }}
+    app.kubernetes.io/component: migrate
+  annotations:
+    "helm.sh/hook": pre-install,pre-upgrade
+    "helm.sh/hook-weight": "-10"
+    "helm.sh/hook-delete-policy": before-hook-creation
+automountServiceAccountToken: false
+---
 apiVersion: batch/v1
 kind: Job
 metadata:
@@ -36,7 +67,7 @@ spec:
         app.kubernetes.io/component: migrate
     spec:
       restartPolicy: Never
-      serviceAccountName: {{ include "svcforge.serviceAccountName" (dict "ctx" $ "component" "api") }}
+      serviceAccountName: {{ include "svcforge.fullname" . }}-migrate
       {{- with .Values.image.pullSecrets }}
       imagePullSecrets:
         {{- toYaml . | nindent 8 }}

deploy/chart/values.yaml

@@ -18,13 +18,13 @@ image:
   # deploy, which is the intended failure mode. Never hand-edit these.
   api:
     repo: gitea.oci-oci.duckdns.org/gitea_admin/svcforge-api
-    digest: sha256:ca2eb7cae9aac4d388b1c6cd8f44b402f271704a40585801092a1ab09d7b6608
+    digest: sha256:51c9b6d09dc9f3861a18ed4116f03ce24717a34cb2f985fbcf2a33e96d96cae8
   worker:
     repo: gitea.oci-oci.duckdns.org/gitea_admin/svcforge-worker
-    digest: sha256:f62c71d65a9fb78953d8005e1bca79ce760995f25b9725ab91d1a23b63224bf2
+    digest: sha256:5307e1e23a815dcc40b290c27b37b0847c12c654a84aa80643821789c95763d9
   reconciler:
     repo: gitea.oci-oci.duckdns.org/gitea_admin/svcforge-reconciler
-    digest: sha256:5a592ecef4df1c9a454652079777d537eefc975b158bbcc928e111f363eb0b4a
+    digest: sha256:bcf7c309c74bb59572c745f6a34e71cdbef39f3881aa779ee182bd67a63186be
 api:
   replicas: 2
   # One process per pod. Module 7 took the "scale with replicas" fix over
@@ -81,9 +81,30 @@ otel:
 log:
   level: info
 # The DSNs are pulled from Vault by external-secrets into a Secret the pods envFrom.
-# No DSN is ever a chart value, a ConfigMap key, or a CI variable.
+# No DSN is ever a chart value, a ConfigMap key, or a CI variable. That invariant holds
+# either way here — what changes below is only who creates the Secret.
+#
+# DISABLED ON THIS CLUSTER, AND THIS IS A DEVIATION, NOT THE DESIGN.
+#
+# The block below describes a ClusterSecretStore named `vault` with HashiCorp-style
+# key/property refs. This cluster has `oci-vault` instead: OCI Vault via InstancePrincipal,
+# whose provider addresses a secret by NAME and takes a JSON property, so these remoteRefs
+# do not translate as written. There are also no ExternalSecrets anywhere on the cluster
+# yet, so nothing has ever exercised this path.
+#
+# With this false, `svcforge.secretName` still resolves through targetName, so the
+# deployments and the migrate hook read a Secret called `svcforge-secrets` that was created
+# out of band:
+#
+#   kubectl -n svcforge create secret generic svcforge-secrets \
+#     --from-env-file=~/.config/svcforge/secrets.env
+#
+# ArgoCD does not manage that Secret, so prune and selfHeal cannot touch it — which is also
+# why it is invisible in git, and the one part of this deployment you cannot read from the
+# repo. Restoring the intended design means adding oci_vault_secret resources to
+# oci-k8s/infra/vault.tf and repointing secretStoreRef at oci-vault.
 externalSecret:
-  enabled: true
+  enabled: false
   secretStoreRef:
     name: vault
     kind: ClusterSecretStore

libs/svcforge_core/svcforge_core/adapters/clock.py

@@ -1,18 +1,15 @@
 """Time, as a dependency.
 
-The centrepiece of the Day-2 module, and it is nine lines. `datetime.now()` called from
+`datetime.now()` inside domain logic is an untestable global read: a maintenance-window
-inside domain logic is an untestable global read: a maintenance-window test that wants
+test wanting "03:00 next Sunday" must sleep until Sunday or monkeypatch a stdlib symbol.
-"03:00 next Sunday" would have to either sleep until Sunday or monkeypatch a stdlib symbol
+Passing a Clock makes it a `FakeClock(start=...)` and an `advance()`.
-and hope nothing else in the process noticed. Passing a Clock makes the same test a
-`FakeClock(start=...)` and an `advance()`.
 
-Aware UTC, always. A naive datetime is a bug that survives every test on a UTC CI box and
+Aware UTC, always. `datetime.utcnow()` returns a naive value that survives every test on a
-detonates the first time it meets a tenant in Asia/Ho_Chi_Minh: `datetime.utcnow()` returns
+UTC CI box, then raises TypeError against a `timestamptz` from Postgres — or compares wrong
-a naive value, and comparing it to a `timestamptz` from Postgres raises TypeError, or worse,
+after someone "fixes" it with `.replace(tzinfo=...)`.
-silently compares wrong after somebody "fixes" it with a `.replace(tzinfo=...)`.
 
-The fake lives in `tests/fakes.py`, not here: shipping test doubles in the production
+The fake lives in `tests/fakes.py`: test doubles shipped in the production package end up
-package is how they end up imported by production code.
+imported by production code.
 """
 
 from __future__ import annotations

libs/svcforge_core/svcforge_core/adapters/helm.py

@@ -1,19 +1,16 @@
 """Driving helm from asyncio, with timeouts that actually kill helm.
 
-The whole module exists for `_run`. Everything above it is argv construction.
+The module exists for `_run`; everything above it is argv construction. Four things go
-
+wrong when an event loop spawns a process, and all four are handled here:
-Four things go wrong when you spawn a process from an event loop, and all four are
-handled here rather than in the caller:
 
 1. `subprocess.run` blocks the loop. Use `create_subprocess_exec`.
-2. `stdout=PIPE` with `proc.wait()` and nobody draining deadlocks at ~64 KB of output —
+2. `stdout=PIPE` with nobody draining deadlocks at ~64 KB — one `helm --debug` install.
-   `helm --debug` clears that in one install. Use `communicate()`.
+   Use `communicate()`.
-3. `asyncio.wait_for` cancels the *coroutine*. The process does not know it was waited on:
+3. `asyncio.wait_for` cancels the *coroutine*; helm keeps running and keeps mutating the
-   helm keeps running and keeps mutating the cluster. The timeout has to kill it.
+   cluster. The timeout has to kill it.
-4. `proc.kill()` signals the direct child. `helm` forks; its children reparent to init and
+4. `proc.kill()` signals the direct child, and helm's children reparent and survive. Only
-   survive. Only `killpg` gets the whole tree, and only if the group exists — which needs
+   `killpg` gets the tree, and only with `start_new_session=True` at spawn time — setsid
-   `start_new_session=True` **at spawn time**, because setsid can only run in the window
+   can only run between fork and exec.
-   between fork and exec.
 """
 
 from __future__ import annotations
@@ -21,36 +18,57 @@ from __future__ import annotations
 import asyncio
 import json
 import os
-import shutil
 import signal
-import tempfile
 from collections.abc import Sequence
 from pathlib import Path
 from typing import Any, Protocol
 
-import yaml
+import httpx
 from pydantic import BaseModel, ConfigDict, Field
 
+from svcforge_core.adapters.tempyaml import yaml_tempfile
 from svcforge_core.domain.models import CatalogEntry
 from svcforge_core.errors import SvcforgeError
 
-# How long the process group gets to honour SIGTERM before SIGKILL. Helm traps SIGTERM
+# Grace for SIGTERM before SIGKILL. Helm traps SIGTERM and tries to leave the release
-# and tries to leave the release in a coherent state; give it a moment to do so.
+# coherent; give it a moment.
 _TERM_GRACE_S = 5.0
 
-# `_run` is the backstop, not the primary timeout: helm gets its own `--timeout` so that
+# `_run` is the backstop, not the primary timeout: helm gets its own `--timeout` so
-# `--atomic` can roll back cleanly. `_run` only fires when helm itself is wedged, so its
+# `--atomic` can roll back cleanly. `_run` fires only when helm itself is wedged.
-# deadline sits this far past helm's.
 _RUN_TIMEOUT_MARGIN_S = 30
 
 _STDERR_TAIL_BYTES = 2048
 
+# The label every svcforge release carries — written by install(), read by list_releases().
+# It is the only thing that tells svcforge's releases from the rest of the cluster's.
+# Changing one value without the other empties the reconciler's view, which reads as
+# "no drift" rather than as an error.
+MANAGED_BY_LABEL = "app.kubernetes.io/managed-by"
+MANAGED_BY_VALUE = "svcforge"
+
+# Where the kubelet mounts the pod's ServiceAccount. Their presence is also how this module
+# decides it is in-cluster: in-cluster takes the fast release read, everything else shells
+# out to helm.
+_SA_DIR = Path("/var/run/secrets/kubernetes.io/serviceaccount")
+_SA_TOKEN = _SA_DIR / "token"
+_SA_CA = _SA_DIR / "ca.crt"
+
+# helm's own label on every release secret it writes.
+_HELM_OWNER_LABEL = "owner=helm"
+
+# The states `helm list` shows by default. In the selector so superseded revisions never
+# leave the API server — 96 release secrets here, 71 of them superseded.
+_LIVE_STATUSES = "status in (deployed,failed,pending-install,pending-upgrade,pending-rollback)"
+
+_API_TIMEOUT_S = 15.0
+
 
 class HelmError(SvcforgeError, RuntimeError):
     """Non-zero exit. str(self) is the stderr tail that lands in instances.error.
 
     RuntimeError stays in the MRO so callers written against it keep catching; SvcforgeError
-    comes first so `except SvcforgeError` can separate a modelled failure from a stray bug.
+    comes first so `except SvcforgeError` separates a modelled failure from a stray bug.
     """
 
 
@@ -80,9 +98,9 @@ class Provisioner(Protocol):
 def _tail(raw: bytes, tail_bytes: int) -> str:
     """The last `tail_bytes` of a stream, as text.
 
-    Truncation happens here, at the adapter boundary, and nowhere else. A helm failure can
+    Truncation happens here, at the adapter boundary, and nowhere else: a helm failure can
-    emit megabytes; `instances.error` is a text column read by humans. Slice the bytes, not
+    emit megabytes and `instances.error` is read by humans. Slice the bytes and decode with
-    the decoded string, then decode with `replace` — the cut can land mid-codepoint.
+    `replace` — the cut can land mid-codepoint.
     """
     return raw[-tail_bytes:].decode("utf-8", errors="replace").strip()
 
@@ -90,8 +108,8 @@ def _tail(raw: bytes, tail_bytes: int) -> str:
 def _signal_group(proc: asyncio.subprocess.Process, sig: int) -> None:
     """Signal the process's whole group. No-op if it has already exited.
 
-    `os.getpgid` rather than `proc.pid`: with `start_new_session=True` they are equal, but
+    `os.getpgid` rather than `proc.pid`: `start_new_session=True` makes them equal, but that
-    that equality is an implementation detail, and asking the kernel costs nothing.
+    equality is an implementation detail and asking the kernel costs nothing.
     """
     if proc.returncode is not None:
         return
@@ -154,8 +172,8 @@ class HelmProvisioner:
         self, *, helm_bin: str = "helm", kubeconfig: Path | None = None, timeout_s: int = 600
     ) -> None:
         """kubeconfig=None means the ambient config: $KUBECONFIG, ~/.kube/config, or the
-        in-cluster service account when svcforge runs as a pod. Keyword-only so that the
+        in-cluster service account. Keyword-only so the three can never be swapped by
-        three arguments can never be swapped by position at a call site.
+        position at a call site.
         """
         self._helm_bin = helm_bin
         self._kubeconfig = kubeconfig
@@ -174,10 +192,10 @@ class HelmProvisioner:
     async def _run_helm(self, argv: Sequence[str]) -> str:
         """`_run`, with the timeout path translated to this adapter's declared error type.
 
-        `_run` raises a bare TimeoutError so that the process-group test can assert on it
+        `_run` raises a bare TimeoutError so the process-group test can assert on it, but
-        directly, but every public method here is documented as raising HelmError; a wedged
+        every public method here is documented as raising HelmError. A wedged helm arriving
-        helm arriving as TimeoutError sails straight past a caller's `except HelmError` and
+        as TimeoutError sails past a caller's `except HelmError` and fails the task as an
-        fails the task as an unmodelled crash. Translate once, at the public boundary.
+        unmodelled crash, so translate once, at the public boundary.
         """
         try:
             return await _run(argv, timeout_s=self._run_timeout_s)
@@ -186,12 +204,11 @@ class HelmProvisioner:
 
     async def install(self, release: str, ns: str, entry: CatalogEntry, values: dict[str, Any]) -> None:
         """helm upgrade --install --wait --timeout. Idempotent by construction."""
-        # `upgrade --install` is why this is idempotent: a retried task after a crash mid-provision
+        # `upgrade --install`: a task retried after a crash mid-provision converges on the
-        # converges on the same release instead of erroring with "release already exists".
+        # same release instead of erroring with "release already exists". `--wait` is why
-        # `--wait` is why `ready` in the DB means ready — it returns when the pods are up.
+        # `ready` in the DB means ready. `--atomic` rolls back a failed upgrade and doubles
-        # `--atomic` rolls back a failed upgrade; it doubles the worst case, which is what
+        # the worst case, which is what the two timeouts are sized around.
-        # `_RUN_TIMEOUT_MARGIN_S` and helm's own `--timeout` are sized around.
+        with yaml_tempfile(values, prefix="svcforge-values-", name="values.yaml") as path:
-        with _values_file(values) as path:
             argv = self._base_argv(
                 "upgrade",
                 "--install",
@@ -199,13 +216,15 @@ class HelmProvisioner:
                 entry.chart,
                 "--namespace",
                 ns,
-                # `--namespace X` does not create X. Every tenant's first provision targets
+                # `--namespace X` does not create X, and every tenant's first provision
-                # a namespace that does not exist yet, and helm fails with "namespaces not
+                # targets one that does not exist yet. helm creates it here rather than a
-                # found". helm creates it here rather than a separate `kubectl apply` step,
+                # `kubectl apply` step, which keeps kubectl out of the worker image — one
-                # which keeps kubectl out of the worker image entirely — one fewer binary,
+                # fewer binary and one fewer set of vendored Go CVEs. Idempotent.
-                # and one fewer set of vendored Go CVEs to track. Idempotent: existing
-                # namespaces are left alone.
                 "--create-namespace",
+                # Stamps MANAGED_BY_LABEL, which is what lets list_releases() ask for
+                # svcforge's releases and nobody else's.
+                "--labels",
+                f"{MANAGED_BY_LABEL}={MANAGED_BY_VALUE}",
                 "--version",
                 entry.chart_version,
                 "--values",
@@ -232,8 +251,36 @@ class HelmProvisioner:
         await self._run_helm(argv)
 
     async def list_releases(self) -> list[ReleaseInfo]:
-        """Every release helm knows about, in every namespace. The reconciler's view of reality."""
+        """Every release svcforge provisioned, in every namespace. The reconciler's reality.
-        argv = self._base_argv("list", "--all-namespaces", "--output", "json")
+
+        Scoped by label for correctness: the reconciler diffs this against the database in
+        both directions, and `live - known` is reported as `drift.orphan_release` at ERROR.
+        Unscoped, `live` is every release in the cluster, so argocd, longhorn, gitea and
+        cert-manager are all reported as orphans on every sweep.
+
+        In-cluster this reads the release secrets off the API server instead of shelling
+        out — 21ms and 31KB against helm's 4392ms, because helm applies `--selector` only
+        after fetching and decompressing every release secret in the cluster. That cost was
+        not theoretical: with the CPU request mutated to 0 by a cluster policy, helm's list
+        took over 330s and timed out on every tick, and a check that never completes reports
+        no drift. Out of cluster there is no ServiceAccount, so it falls back to helm and the
+        e2e suite keeps working. See `_list_releases_via_api`.
+
+        Releases provisioned before the label existed do not match, so the first sweep sees
+        them as missing and re-provisions. That is safe — provisioning is `upgrade --install`
+        against a deterministic release name — and the re-provision applies the label.
+        """
+        via_api = await self._list_releases_via_api()
+        if via_api is not None:
+            return via_api
+        argv = self._base_argv(
+            "list",
+            "--all-namespaces",
+            "--selector",
+            f"{MANAGED_BY_LABEL}={MANAGED_BY_VALUE}",
+            "--output",
+            "json",
+        )
         raw = await self._run_helm(argv)
         try:
             parsed: Any = json.loads(raw or "[]")
@@ -243,28 +290,97 @@ class HelmProvisioner:
             raise HelmError(f"helm list returned {type(parsed).__name__}, expected a list")
         return [ReleaseInfo.model_validate(row) for row in parsed]
 
+    async def _list_releases_via_api(self) -> list[ReleaseInfo] | None:
+        """Release names and namespaces read straight off helm's release secrets.
 
-class _ValuesFile:
+        None when there is no in-cluster ServiceAccount, which is the caller's signal to
-    """Context manager yielding a path to a values.yaml written from a dict.
+        fall back to helm.
 
-    A file, not `--set`: `--set` has its own escaping grammar (commas, dots, backslashes) and
+        helm gunzips every release payload to build its table. The only fields either caller
-    values carry tenant-shaped strings. Serialising YAML sidesteps the grammar entirely.
+        reads are name and namespace, and both live in the secret's labels and metadata, so
+        nothing has to be decompressed. Three details carry the correctness:
+
+        * `PartialObjectMetadataList` in the Accept header asks for metadata only. Without
+          it the response carries every release's gzipped manifest — megabytes fetched to be
+          thrown away, which is the cost this method exists to avoid.
+        * The status selector drops superseded revisions server-side (96 secrets here, 25
+          live). The states kept are the ones `helm list` shows, so a failed release still
+          counts as existing — it does, and calling it missing would re-provision on top.
+        * helm writes one secret per revision, so a release can appear several times. The
+          newest `version` label wins; without that, any caller counting releases over-counts.
+
+        `chart` comes back empty because the chart name lives only in the compressed payload.
+        The field stays so the helm fallback, which does populate it, returns the same shape.
         """
+        try:
+            token = _SA_TOKEN.read_text(encoding="utf-8").strip()
+        except OSError:
+            return None
+        # The CA is checked here rather than left to httpx, which loads it eagerly at client
+        # construction and raises OSError — not in the except below. A half-mounted
+        # ServiceAccount would crash the tick as a bare bug instead of falling back. A
+        # complete ServiceAccount is the in-cluster signal; a missing CA means "not
+        # in-cluster" exactly as a missing token does.
+        if not token or not os.access(_SA_CA, os.R_OK):
+            return None
+        host, port = (
+            os.environ.get("KUBERNETES_SERVICE_HOST"),
+            os.environ.get("KUBERNETES_SERVICE_PORT_HTTPS", "443"),
+        )
+        if not host:
+            return None
 
-    def __init__(self, values: dict[str, Any]) -> None:
+        selector = f"{_HELM_OWNER_LABEL},{MANAGED_BY_LABEL}={MANAGED_BY_VALUE},{_LIVE_STATUSES}"
-        self._values = values
+        try:
-        self._dir: str | None = None
+            async with httpx.AsyncClient(verify=str(_SA_CA), timeout=_API_TIMEOUT_S) as client:
+                # No `limit`, and that is load-bearing: the apiserver only returns a
+                # `metadata.continue` token when the client sets one, so the single read
+                # below is the complete set. Adding `limit` without looping on `continue`
+                # would truncate silently, and the reconciler would read the missing
+                # releases as orphans to delete or as vanished releases to re-provision.
+                resp = await client.get(
+                    f"https://{host}:{port}/api/v1/secrets",
+                    params={"labelSelector": selector},
+                    headers={
+                        "authorization": f"Bearer {token}",
+                        "accept": ("application/json;as=PartialObjectMetadataList;g=meta.k8s.io;v=v1"),
+                    },
+                )
+                resp.raise_for_status()
+                # `or []`, not `.get("items", [])`. Kubernetes serialises an empty list as
+                # `"items": null`, so the key is present and the default never fires. This
+                # shipped and failed on the first tick that matched no releases.
+                items: Any = resp.json().get("items") or []
+        except (httpx.HTTPError, json.JSONDecodeError) as exc:
+            # Raise, do not fall back to helm. The fallback is for "there is no
+            # ServiceAccount", a fact about the environment known before any request goes
+            # out. Here the API server was reachable and something went wrong, and retrying
+            # through helm would swap a visible error for the 330s timeout this method
+            # exists to remove. The tick logs check.failed and tries again in 60s.
+            raise HelmError(f"listing release secrets failed: {exc}") from exc
 
-    def __enter__(self) -> Path:
+        newest: dict[tuple[str, str], tuple[int, ReleaseInfo]] = {}
-        self._dir = tempfile.mkdtemp(prefix="svcforge-values-")
+        for item in items:
-        path = Path(self._dir) / "values.yaml"
+            meta = item.get("metadata") or {}
-        path.write_text(yaml.safe_dump(self._values, default_flow_style=False), encoding="utf-8")
+            labels = meta.get("labels") or {}
-        return path
+            name, namespace = labels.get("name"), meta.get("namespace")
-
+            if not name or not namespace:
-    def __exit__(self, *exc: object) -> None:
+                continue  # not a release secret this method understands; leave it alone
-        if self._dir is not None:
+            try:
-            shutil.rmtree(self._dir, ignore_errors=True)
+                revision = int(labels.get("version", 0))
-
+            except ValueError:
-
+                revision = 0
-def _values_file(values: dict[str, Any]) -> _ValuesFile:
+            key = (name, namespace)
-    return _ValuesFile(values)
+            if key in newest and newest[key][0] >= revision:
+                continue
+            newest[key] = (
+                revision,
+                ReleaseInfo(
+                    name=name,
+                    namespace=namespace,
+                    chart="",
+                    status=labels.get("status", ""),
+                    revision=max(revision, 0),
+                ),
+            )
+        return [info for _, info in newest.values()]

libs/svcforge_core/svcforge_core/adapters/k8s.py

@@ -13,14 +13,11 @@ from __future__ import annotations
 import base64
 import binascii
 import json
-import shutil
-import tempfile
 from pathlib import Path
 from typing import Any
 
-import yaml
+from svcforge_core.adapters.helm import MANAGED_BY_LABEL, MANAGED_BY_VALUE, HelmError, _run
-
+from svcforge_core.adapters.tempyaml import yaml_tempfile
-from svcforge_core.adapters.helm import HelmError, _run
 from svcforge_core.errors import SvcforgeError
 
 _KUBECTL_TIMEOUT_S = 60
@@ -71,10 +68,10 @@ class KubectlClient:
             "kind": "Namespace",
             "metadata": {
                 "name": ns,
-                "labels": {"app.kubernetes.io/managed-by": "svcforge", **(labels or {})},
+                "labels": {MANAGED_BY_LABEL: MANAGED_BY_VALUE, **(labels or {})},
             },
         }
-        with _manifest_file(manifest) as path:
+        with yaml_tempfile(manifest, prefix="svcforge-manifest-", name="manifest.yaml") as path:
             await self._kubectl("apply", "--filename", str(path))
 
     async def read_secret(self, ns: str, name: str) -> dict[str, str]:
@@ -122,25 +119,3 @@ class KubectlClient:
             # timeout path does not come through HelmError. Without this clause a wedged
             # kubectl surfaces as TimeoutError past a caller written to `except K8sError`.
             raise K8sError(f"kubectl {args[0] if args else ''} timed out after {self._timeout_s}s") from exc
-
-
-class _ManifestFile:
-    """A temp file holding one YAML manifest, removed on exit."""
-
-    def __init__(self, manifest: dict[str, Any]) -> None:
-        self._manifest = manifest
-        self._dir: str | None = None
-
-    def __enter__(self) -> Path:
-        self._dir = tempfile.mkdtemp(prefix="svcforge-manifest-")
-        path = Path(self._dir) / "manifest.yaml"
-        path.write_text(yaml.safe_dump(self._manifest, default_flow_style=False), encoding="utf-8")
-        return path
-
-    def __exit__(self, *exc: object) -> None:
-        if self._dir is not None:
-            shutil.rmtree(self._dir, ignore_errors=True)
-
-
-def _manifest_file(manifest: dict[str, Any]) -> _ManifestFile:
-    return _ManifestFile(manifest)

libs/svcforge_core/svcforge_core/adapters/notify.py

@@ -1,12 +1,11 @@
 """Telling someone a provision finished, or didn't.
 
-Best-effort by construction: `send` never raises. A notifier that can fail a task is a
+Best-effort by construction: `send` never raises. A notifier that can fail a task lets a
-notifier that lets a Slack outage roll back a successful provision. The instance is ready;
+Slack outage roll back a successful provision — the instance is ready and the DB says so,
-the DB says so; failing the task would re-run helm for nothing. Delivery failures are
+and failing the task would re-run helm for nothing. Delivery failures are logged and dropped.
-logged and dropped on the floor, which is the correct amount of ceremony for a webhook.
 
-Two implementations, so the Protocol earns its place: LogNotifier (the default, and what
+Two implementations, so the Protocol earns its place: LogNotifier (the default) and
-tests and local dev get) and WebhookNotifier (the one that leaves the process).
+WebhookNotifier (the one that leaves the process).
 """
 
 from __future__ import annotations
@@ -47,18 +46,14 @@ class LogNotifier:
     """Writes the event to the log. The default: structured logs are already shipped somewhere."""
 
     async def send(self, event: str, message: str, fields: dict[str, str] | None = None) -> None:
-        # structlog kwargs, NOT logging's `extra=`. obs bridges stdlib records through
+        # structlog kwargs, NOT logging's `extra=`: ProcessorFormatter builds the event dict
-        # ProcessorFormatter, which builds the event dict from `record.msg` alone — every
+        # from `record.msg` alone, so `extra=` keys are dropped and this used to emit a bare
-        # key passed via `extra=` is dropped on the floor, so the default notifier used to
+        # {"event": "notify"} with the payload gone.
-        # emit a bare {"event": "notify"} with the payload gone.
         #
-        # Fields are splatted rather than nested under "fields" so each one is its own
+        # Fields are splatted rather than nested so each is its own queryable key in Loki.
-        # queryable key in Loki. `detail`, not `message`: `message` is a reserved LogRecord
+        # `detail`, not `message` — a reserved LogRecord attribute the bridge raises on. And
-        # attribute and the stdlib bridge raises KeyError on it.
+        # `notify_event`, not `event` — structlog's first positional parameter is named
-        #
+        # `event`, so passing it as a kwarg is a TypeError at the call.
-        # `notify_event`, not `event`: structlog's first positional parameter IS named
-        # `event` (it becomes the rendered line's "event" key, here the literal "notify"),
-        # so passing event= alongside it is a TypeError at the call, not a rename.
         log = obs.get_logger(__name__)
         log.info("notify", notify_event=event, detail=message, **_safe_fields(fields))
 
@@ -77,8 +72,8 @@ class WebhookNotifier:
         self._timeout_s = timeout_s
         self._owns_client = client is None
         # Eager, not lazy. `AsyncClient()` does no I/O, so laziness bought nothing and cost a
-        # race: two concurrent `send`s could both see None, both construct a client, and the
+        # race: two concurrent `send`s both see None, both construct a client, and the loser's
-        # loser's connection pool would leak because only one of them survived the assignment.
+        # connection pool leaks because only one survives the assignment.
         self._client = client if client is not None else httpx.AsyncClient(timeout=self._timeout_s)
 
     async def send(self, event: str, message: str, fields: dict[str, str] | None = None) -> None:
@@ -87,10 +82,9 @@ class WebhookNotifier:
             resp = await self._client.post(self._url, json=payload, timeout=self._timeout_s)
             resp.raise_for_status()
         except Exception as exc:  # the bare `except Exception` IS the specification here
-            # `send` must not raise; that is the contract in the module docstring, and it is
+            # "send must not raise" is not satisfiable by catching httpx.HTTPError alone:
-            # not satisfiable by catching httpx.HTTPError alone. `httpx.InvalidURL` is not an
+            # `httpx.InvalidURL` is not a subclass, and posting on an aclose()d client raises
-            # HTTPError subclass, and posting on an already-aclose()d client raises
+            # RuntimeError — so a typo'd webhook URL would fail a task whose helm work
-            # RuntimeError — so a typo'd webhook URL would fail a task whose helm work has
             # already succeeded. exc_info so the traceback survives the swallowing.
             obs.get_logger(__name__).warning(
                 "notify webhook failed", notify_event=event, error=str(exc), exc_info=exc
@@ -99,9 +93,9 @@ class WebhookNotifier:
     async def aclose(self) -> None:
         """Close the client, if we made it. Call at process shutdown, next to the pool's close.
 
-        The client reference is kept rather than cleared: a `send` that races shutdown now
+        The reference is kept rather than cleared: a `send` racing shutdown then raises
-        raises RuntimeError on a closed client, and `send` swallows and logs that like any
+        RuntimeError on a closed client and is swallowed like any other delivery failure,
-        other delivery failure instead of resurrecting a pool nobody will close.
+        instead of resurrecting a pool nobody will close.
         """
         if self._owns_client:
             await self._client.aclose()

libs/svcforge_core/svcforge_core/adapters/redis.py

@@ -1,45 +1,34 @@
 """Redis: derived state only. Never the truth, never the queue.
 
-Everything in here is a shortcut past Postgres, and every one of them is optional. Postgres
+Everything here is an optional shortcut past Postgres, which holds the instances, the
-holds the instances, the tasks, the leases and the `release_name` UNIQUE constraint. Redis
+tasks, the leases and the `release_name` UNIQUE constraint. Redis holds a counter, a claim
-holds a counter, a claim marker and a copy — all of it rebuildable by doing nothing and
+marker and a copy — all rebuildable by waiting for a TTL.
-waiting for a TTL.
 
-That framing decides the error handling, and the error handling is the module. Each class
+That decides the error handling, and the error handling is the module. Each class catches
-below catches `RedisError` and returns a *safe* answer rather than raising:
+`RedisError` and returns a safe answer rather than raising:
 
 | Path        | Redis is down          | Why                                              |
-|-------------|--------------------------|--------------------------------------------------|
+|-------------|------------------------|--------------------------------------------------|
 | Cache       | miss -> read Postgres  | It was an optimisation. Nobody notices.          |
-| Rate limit  | **allow**                | An internal platform that refuses every request  |
+| Rate limit  | **allow**              | Briefly unmetered beats refusing every request.  |
-|             |                          | because the limiter is sick is worse than one    |
+| Idempotency | fall through to the DB | `instances.release_name` UNIQUE is the guarantee.|
-|             |                          | that is briefly unmetered.                       |
-| Idempotency | fall through to the DB   | `instances.release_name` is UNIQUE. That is the  |
-|             |                          | real guarantee; this is the fast path.           |
 
-Consequently nothing here raises out to a caller, and `/readyz` stays Postgres-only. A
+Nothing here raises out to a caller and `/readyz` stays Postgres-only, so a Redis outage
-Redis outage must not make a single pod unready — that would convert "the cache is down"
+never makes a pod unready.
-into "the platform is down", which is the exact inversion this module exists to prevent.
 
-**The budget is a design constraint.** Upstash free tier:
+**The budget is a design constraint.** Upstash free tier is 500,000 commands/month =
+16,129/day = 0.19/second sustained. One worker polling every five seconds spends the entire
+budget producing nothing, so the rule is structural: Redis lives on the request path only,
+never in a poll or control loop. It is also why the limiter is a Lua script —
+`GET`/`INCR`/`EXPIRE` is three billed commands and a race, one `EVALSHA` is one and atomic.
+A pipeline batches round trips but still bills N.
 
-    500,000 commands / month = 16,129 / day = 11 / minute = 0.19 / second, sustained
+Every key gets a TTL. 256 MB with no expiry is a leak that ends by evicting what mattered.
-
-One worker polling Redis every five seconds spends 518,400/month: the entire budget,
-producing nothing. So the rule is structural — **Redis lives on the request path only**,
-where volume is bounded by the number of humans with an API token, and never inside a poll
-or control loop. That is also why the limiter is a Lua script: `GET`/`INCR`/`EXPIRE` is
-three billed commands and a race; one `EVALSHA` is one billed command and atomic. A
-pipeline would not help — it batches round trips but still bills N.
-
-Every key gets a TTL. 256 MB with no expiry is a slow leak that ends by evicting the keys
-you cared about.
 """
 
 from __future__ import annotations
 
 import asyncio
-import logging
 import math
 from dataclasses import dataclass
 from datetime import UTC, datetime
@@ -51,6 +40,7 @@ from pydantic import ValidationError
 from redis.asyncio import Redis
 from redis.exceptions import RedisError
 
+from svcforge_core import obs
 from svcforge_core.adapters.clock import Clock, SystemClock
 from svcforge_core.domain.models import Instance
 
@@ -59,19 +49,18 @@ if TYPE_CHECKING:
 
     from svcforge_core.settings import Settings
 
-_log = logging.getLogger(__name__)
+# structlog via obs, not stdlib logging: the stdlib bridge builds the event dict from the
+# record message alone and drops `extra=`. A bound logger takes fields as kwargs and keeps
+# them. Bound per instance in __init__, after obs.setup() runs, never at import time.
 
 # --- The budget metric ------------------------------------------------------------------
 #
-# The counter that `scripts/redis_budget.py` projects month-end burn from. It counts
+# What `scripts/redis_budget.py` projects month-end burn from. It counts commands *sent*,
-# commands we *send*, incremented next to each call, because the number that matters is
+# incremented next to each call, because the billed number is what matters — a cache miss
-# the one Upstash bills — not the number of times a method was called. A cache miss calls
+# spends one command on `get()` and another on the `put()` after it.
-# `get()` once and spends one command; a `put()` after it spends another.
 #
-# The dangerous failure this makes visible: when Redis is down, every path degrades
+# This is the only warning available: when Redis is down every path degrades silently and
-# silently and correctly, so nothing pages. Nothing fails until the month rolls over and
+# correctly, so nothing pages until the month rolls over and every call starts erroring.
-# every Redis call starts erroring at once. A counter you can extrapolate from is the only
-# warning you get.
 
 REDIS_COMMANDS = Counter(
     "svcforge_redis_commands_total",
@@ -85,26 +74,24 @@ REDIS_ERRORS = Counter(
     ["op"],
 )
 
-# A hung Redis must not hang the request path. Without these, a TCP connection that is
+# A hung Redis must not hang the request path: an open but unanswered TCP connection blocks
-# open but unanswered blocks the handler until the client gives up — which turns "Redis is
+# the handler until the client gives up, turning "Redis is slow" into "the API is down".
-# slow" into "the API is down", the same inversion the fail-open policy prevents. Upstash
+# Upstash steady-state RTT is ~2.4 ms, so two seconds is already pathological.
-# steady-state RTT is ~2.4 ms; two seconds is already pathological.
 _SOCKET_TIMEOUT_S = 2.0
 _CONNECT_TIMEOUT_S = 2.0
 
-# Errors that mean "Redis did not answer". Every public method below turns these into a
+# "Redis did not answer" — every public method turns these into a safe default. `OSError`
-# safe default. `OSError` because a DNS failure at connect time need not arrive wrapped,
+# because a DNS failure at connect time need not arrive wrapped, `TimeoutError` because the
-# `TimeoutError` because the socket timeouts above raise it.
+# socket timeouts above raise it.
 _REDIS_DOWN = (RedisError, OSError, asyncio.TimeoutError)
 
 
 def _as_text(value: bytes | str) -> str:
     """`decode_responses=True` already did this; redis-py's annotations do not know it.
 
-    The client is configured for text, so the `bytes` branch is unreachable in this
+    The `bytes` branch is unreachable in this process. It stays rather than becoming a
-    process. It stays because the type says it is reachable, and a `cast` here would hide
+    `cast` so that a client built without `decode_responses` gets a working value instead
-    the day someone builds a client without `decode_responses` and gets a `UUID(b'...')`
+    of a `UUID(b'...')` TypeError three frames away.
-    TypeError from three frames away instead of a value that just works.
     """
     return value.decode() if isinstance(value, bytes) else value
 
@@ -112,21 +99,14 @@ def _as_text(value: bytes | str) -> str:
 def make_redis(settings: Settings) -> Redis | None:
     """One client per process, opened in lifespan next to the psycopg pool, closed on exit.
 
-    `None` when no DSN is configured, and that is a supported way to run: every consumer
+    `None` when no DSN is configured, which is a supported way to run: every consumer is
-    below is optional by construction, so "no Redis" and "Redis is down" take the same
+    optional, so "no Redis" and "Redis is down" take the same path. The return type is
-    code path. The signature is `Redis | None` rather than `Redis` precisely so that
+    `Redis | None` so "unconfigured" cannot be faked with a client pointed at nothing.
-    "unconfigured" cannot be faked with a client pointed at nothing.
 
-    Two settings are not negotiable:
+    `decode_responses=True` is not optional — without it every read is `bytes` and the
-
+    traceback is an `AttributeError` several frames from the cause. Neither is `rediss://`:
-    `decode_responses=True` — the first bug everyone hits. Without it every read is
+    Upstash rejects plaintext, and the ~56 ms handshake against a ~2.4 ms steady-state RTT
-    `bytes` and the traceback is `AttributeError: 'bytes' object has no attribute
+    is the whole argument for one pooled client per process.
-    'encode'`, several frames away from the cause.
-
-    `rediss://` (TLS) — Upstash rejects plaintext. The handshake is ~56 ms against a
-    ~2.4 ms steady-state RTT, which is the whole argument for one pooled client per
-    process: a client per request pays the handshake every time and turns a cache into a
-    latency regression.
     """
     if settings.redis_dsn is None:
         return None
@@ -140,15 +120,14 @@ def make_redis(settings: Settings) -> Redis | None:
 
 # --- Rate limiting ----------------------------------------------------------------------
 
-# One INCR; EXPIRE only when the counter is new. The `== 1` test is the entire trick: set
+# One INCR; EXPIRE only when the counter is new. The `== 1` test is the trick: set the TTL
-# the TTL unconditionally and every request slides the window forward, so a caller at
+# unconditionally and every request slides the window forward, so a caller at steady load is
-# steady load is never reset and the "window" is a sliding refusal that never lets up.
+# never reset and the window becomes a refusal that never lets up.
 #
-# KEYS and ARGV arrive as 1-based tables — Lua indexes from 1, and `ARGV[0]` is silently
+# KEYS and ARGV are 1-based — `ARGV[0]` is silently nil rather than an error, which reads as
-# nil rather than an error, which reads as "the limit is nil" and compares false forever.
+# "the limit is nil" and compares false forever.
 #
-# Everything derivable in Python is derived in Python: `reset_at` comes from the window
+# `reset_at` is derived in Python from the window number, so there is no TTL round trip.
-# number the caller already computed, so there is no TTL round trip. One command, total.
 _RATE_LIMIT_LUA = """
 local n = redis.call('INCR', KEYS[1])
 if n == 1 then
@@ -175,6 +154,10 @@ class RateLimitResult:
     limit: int
     remaining: int
     reset_at: datetime
+    # From the same injected clock as reset_at. The two must share a clock or retry_after_s
+    # (their difference) is meaningless under a FakeClock and drifts by the request latency
+    # in production.
+    checked_at: datetime
     degraded: bool = False
 
     @property
@@ -184,7 +167,7 @@ class RateLimitResult:
         Rounded up and floored at one: `Retry-After: 0` invites an immediate retry into
         the same closed window, which is a busy loop with extra steps.
         """
-        delta = (self.reset_at - datetime.now(UTC)).total_seconds()
+        delta = (self.reset_at - self.checked_at).total_seconds()
         return max(1, math.ceil(delta))
 
 
@@ -199,16 +182,14 @@ class RateLimiterProto(Protocol):
 class RateLimiter:
     """Fixed-window limiter. One EVALSHA per check. Fails OPEN.
 
-    Fixed window, not a token bucket or a sliding log, because the window boundary is the
+    A fixed window's only error is the boundary, where a caller can spend 2x the limit. The
-    only thing a fixed window gets wrong and the cost of getting it wrong is that a caller
+    sliding log that fixes it costs four billed commands and an unbounded key. The limit is
-    can spend 2x the limit across a boundary. A sliding log is a sorted set, an
+    a courtesy; the security control is the JWT.
-    `ZREMRANGEBYSCORE`, an `ZADD` and a `ZCARD` — four billed commands and unbounded key
-    size — to fix a burst nobody is paying for. The limit is a courtesy, not a security
-    control; the security control is the JWT.
     """
 
     def __init__(self, r: Redis, limit: int, window_s: int, *, clock: Clock | None = None) -> None:
         """`clock` is injectable so the window boundary is testable without sleeping."""
+        self._log = obs.get_logger(__name__)
         if limit < 1:
             raise ValueError("limit must be >= 1")
         if window_s < 1:
@@ -216,10 +197,9 @@ class RateLimiter:
         self._limit = limit
         self._window_s = window_s
         self._clock = clock or SystemClock()
-        # register_script() is local: it hashes the source and returns a callable. No round
+        # register_script() is local — it hashes the source and returns a callable, with no
-        # trip here, and none wasted at import. The first call sends EVALSHA; redis-py
+        # round trip. The first call sends EVALSHA; redis-py catches NOSCRIPT and replays it
-        # catches NOSCRIPT and replays it as EVAL, which is why a restarted Redis costs one
+        # as EVAL, so a restarted Redis costs one extra command rather than an outage.
-        # extra command once rather than an outage.
         self._script: AsyncScript = r.register_script(_RATE_LIMIT_LUA)
 
     def _window(self) -> tuple[int, datetime]:
@@ -233,9 +213,8 @@ class RateLimiter:
     async def check(self, team: str) -> RateLimitResult:
         """Count one request against `team`. Never raises.
 
-        On any Redis error: allow, log loudly, count it. The metric is the point — a
+        On any Redis error: allow, log loudly, count it. The metric is the point — a limiter
-        limiter that fails open silently is indistinguishable from no limiter at all, and
+        that fails open silently is indistinguishable from no limiter at all.
-        you find out which one you shipped during the incident.
         """
         window, reset_at = self._window()
         key = f"rl:{team}:{window}"
@@ -244,16 +223,17 @@ class RateLimiter:
             allowed, remaining = await self._script(keys=[key], args=[self._limit, self._window_s])
         except _REDIS_DOWN:
             REDIS_ERRORS.labels(op="ratelimit").inc()
-            _log.warning(
+            self._log.warning(
                 "rate limiter degraded: redis unavailable, failing OPEN",
                 exc_info=True,
-                extra={"team": team},
+                team=team,
             )
             return RateLimitResult(
                 allowed=True,
                 limit=self._limit,
                 remaining=self._limit,
                 reset_at=reset_at,
+                checked_at=self._clock.now(),
                 degraded=True,
             )
         return RateLimitResult(
@@ -261,6 +241,7 @@ class RateLimiter:
             limit=self._limit,
             remaining=int(remaining),
             reset_at=reset_at,
+            checked_at=self._clock.now(),
         )
 
 
@@ -278,22 +259,20 @@ class IdempotencyStoreProto(Protocol):
 class IdempotencyStore:
     """`SET NX EX`. Maps an `Idempotency-Key` to the instance UUID it created.
 
-    Claimed BEFORE the DB transaction, so the marker exists before the row it names. The
+    Claimed BEFORE the DB transaction: claim after the commit and a crash in between leaves
-    inversion matters: claim after the commit and a crash in between leaves a created
+    a created instance with no marker, so the client's retry creates a second one. Claiming
-    instance with no marker, and the client's retry creates a second one.
+    first has the opposite hole — a marker naming an instance that never committed, and the
+    retry is told "already done" about nothing — and that is the better hole, because the
+    client polls the id, gets a 404 and retries with a fresh key.
 
-    Claiming first has its own hole — a crash after the claim and before the commit leaves
+    Neither hole is load-bearing. `instances.release_name` is UNIQUE and deterministic from
-    a marker pointing at an instance that never existed, and the retry is told "already
+    (team, service_type, id); that constraint is the guarantee and this only saves a round
-    done" about nothing. It is the better hole: the client polls the id, gets a 404, and
+    trip.
-    retries with a fresh key. The alternative loses money to a duplicate Elasticsearch.
-
-    And neither hole is load-bearing, because `instances.release_name` is UNIQUE and
-    deterministic from (team, service_type, id). **That constraint is the guarantee.** This
-    class only saves the round trip to find out.
     """
 
     def __init__(self, r: Redis, ttl_s: int = 86400) -> None:
         """A day is the window a client might reasonably retry in; then the key is garbage."""
+        self._log = obs.get_logger(__name__)
         if ttl_s < 1:
             raise ValueError("ttl_s must be >= 1")
         self._r = r
@@ -302,13 +281,11 @@ class IdempotencyStore:
     async def claim(self, key: str, instance_id: UUID) -> UUID | None:
         """Try to bind `key` to `instance_id`. Never raises.
 
-        `None` from the happy path means "you won, go create it". `None` from a Redis
+        `None` means "you won, go create it", and a Redis failure returns the same thing —
-        failure means the same thing — the caller creates, and the UNIQUE constraint
+        the caller creates and the UNIQUE constraint catches a real duplicate. Degrading to
-        catches an actual duplicate. Degrading to "create it" is safe *only* because that
+        "create it" is safe only because that constraint exists.
-        constraint exists; without it this would have to fail closed.
 
-        One command when we win, which is the common case and the one the budget is sized
+        One command when we win, two when we lose: the loser pays a GET, and losers are rare.
-        for. Two when we lose: the loser pays a GET, and losers are rare by definition.
         """
         redis_key = f"idem:{key}"
         try:
@@ -320,20 +297,20 @@ class IdempotencyStore:
             existing = await self._r.get(redis_key)
         except _REDIS_DOWN:
             REDIS_ERRORS.labels(op="idempotency").inc()
-            _log.warning(
+            self._log.warning(
                 "idempotency degraded: redis unavailable, falling through to the DB constraint",
                 exc_info=True,
             )
             return None
 
         if existing is None:
-            # The key expired between the SET and the GET. Vanishingly rare, and the honest
+            # The key expired between the SET and the GET. The honest answer is "no winner
-            # answer is "no winner recorded" — let the caller create and let Postgres decide.
+            # recorded" — let the caller create and let Postgres decide.
             return None
         try:
             return UUID(_as_text(existing))
         except ValueError:
-            _log.warning("idempotency key holds a non-UUID value; ignoring it")
+            self._log.warning("idempotency key holds a non-UUID value; ignoring it")
             return None
 
 
@@ -359,18 +336,17 @@ class InstanceCacheProto(Protocol):
 class InstanceCache:
     """Cache-aside for `GET /v1/instances/{id}`. TTL 30s.
 
-    Hit costs one command, miss costs two (the GET, then the SET after Postgres answers).
+    A hit costs one command and a miss two, so ~1 per read at any useful hit rate — which is
-    That is ~1 per read at any useful hit rate, which is what keeps a read-heavy poller
+    what keeps a read-heavy poller inside the budget.
-    inside the budget.
 
-    The TTL is short on purpose and is the actual correctness argument. `invalidate()` on
+    The short TTL is the correctness argument. `invalidate()` on every state transition is
-    every state transition is the fast path, not the guarantee: the worker can crash
+    the fast path, not the guarantee: a worker can crash between the UPDATE and the DEL, and
-    between the UPDATE and the DEL, and then the cache is wrong. Thirty seconds bounds how
+    30 seconds bounds how wrong the cache gets. Trusting invalidation and raising the TTL to
-    wrong. Trusting the invalidation instead — and raising the TTL to an hour — is how a
+    an hour is how a deleted instance stays `ready` in the API for an hour.
-    deleted instance stays `ready` in the API for an hour.
     """
 
     def __init__(self, r: Redis, ttl_s: int = 30) -> None:
+        self._log = obs.get_logger(__name__)
         if ttl_s < 1:
             raise ValueError("ttl_s must be >= 1")
         self._r = r
@@ -383,24 +359,24 @@ class InstanceCache:
     async def get(self, instance_id: UUID) -> Instance | None:
         """One GET. A miss, a Redis outage and a corrupt entry are all the same answer.
 
-        Which is the point: the caller writes `cache.get() or repo.get()` and has no branch
+        The caller writes `cache.get() or repo.get()` and has no branch for "Redis is
-        for "Redis is broken", because there is nothing different to do about it.
+        broken", because there is nothing different to do about it.
         """
         try:
             REDIS_COMMANDS.labels(op="cache_get").inc()
             raw = await self._r.get(self._key(instance_id))
         except _REDIS_DOWN:
             REDIS_ERRORS.labels(op="cache_get").inc()
-            _log.warning("cache read degraded: redis unavailable, falling through to Postgres")
+            self._log.warning("cache read degraded: redis unavailable, falling through to Postgres")
             return None
         if raw is None:
             return None
         try:
             return Instance.model_validate_json(raw)
         except ValidationError:
-            # A model change deployed over a warm cache. Treat it as a miss and let the TTL
+            # A model change deployed over a warm cache. A miss, not an error — the TTL
-            # take the old shape out. Not an error: the truth is in Postgres either way.
+            # takes the old shape out and the truth is in Postgres either way.
-            _log.info("cache entry failed validation; treating as a miss")
+            self._log.info("cache entry failed validation; treating as a miss")
             return None
 
     async def put(self, inst: Instance) -> None:
@@ -410,17 +386,17 @@ class InstanceCache:
             await self._r.set(self._key(inst.id), inst.model_dump_json(), ex=self._ttl_s)
         except _REDIS_DOWN:
             REDIS_ERRORS.labels(op="cache_put").inc()
-            _log.warning("cache write degraded: redis unavailable")
+            self._log.warning("cache write degraded: redis unavailable")
 
     async def invalidate(self, instance_id: UUID) -> None:
         """One DEL. Called by the worker inside the code path that writes the state.
 
-        Inside that path, not after it and not from a subscriber: an invalidation that can
+        Inside that path, not after it and not from a subscriber: an invalidation an early
-        be skipped by an early return is an invalidation that will be.
+        return can skip is an invalidation that will be skipped.
         """
         try:
             REDIS_COMMANDS.labels(op="cache_del").inc()
             await self._r.delete(self._key(instance_id))
         except _REDIS_DOWN:
             REDIS_ERRORS.labels(op="cache_del").inc()
-            _log.warning("cache invalidate degraded: redis unavailable; entry expires within the TTL")
+            self._log.warning("cache invalidate degraded: redis unavailable; entry expires within the TTL")

libs/svcforge_core/svcforge_core/adapters/tempyaml.py

@@ -0,0 +1,34 @@
+"""One temp YAML file, written from a dict and removed on exit.
+
+helm and kubectl both take their input as a file rather than on the command line: `--set`
+and inline manifests each have their own escaping grammar, and tenant-shaped values would
+have to be escaped into it. Serialising YAML to a file sidesteps the grammar entirely. Both
+adapters needed the same throwaway-file dance, so it lives here once.
+"""
+
+from __future__ import annotations
+
+import shutil
+import tempfile
+from collections.abc import Iterator
+from contextlib import contextmanager
+from pathlib import Path
+from typing import Any
+
+import yaml
+
+
+@contextmanager
+def yaml_tempfile(payload: dict[str, Any], *, prefix: str, name: str) -> Iterator[Path]:
+    """Yield a path to `name` inside a fresh temp dir, holding `payload` as YAML.
+
+    The whole dir is removed on exit, `ignore_errors` so a cleanup race never masks the real
+    error from the block.
+    """
+    tmpdir = tempfile.mkdtemp(prefix=prefix)
+    try:
+        path = Path(tmpdir) / name
+        path.write_text(yaml.safe_dump(payload, default_flow_style=False), encoding="utf-8")
+        yield path
+    finally:
+        shutil.rmtree(tmpdir, ignore_errors=True)

libs/svcforge_core/svcforge_core/domain/catalog.py

@@ -10,9 +10,10 @@ import yaml
 from pydantic import ValidationError
 
 from svcforge_core.domain.models import CatalogEntry
+from svcforge_core.errors import SvcforgeError
 
 
-class CatalogError(Exception):
+class CatalogError(SvcforgeError):
     """A catalog file could not be parsed or validated.
 
     `key` names the offending service type, or None when the failure is file-level.

libs/svcforge_core/svcforge_core/domain/models.py

@@ -55,6 +55,16 @@ class CatalogEntry(BaseModel):
     # Bypass tenant maintenance windows for this entry's upgrades. Defaults False: a
     # normal version bump waits for 03:00 Sunday; a CVE with a public exploit does not.
     security: bool = False
+    # Chart values that apply to every size of this entry, merged UNDER the size's own
+    # replicas and resources. This is where a chart's own knobs go — `global.imageRegistry`
+    # to keep image pulls off a rate-limited registry, a storageClass, a disabled subchart.
+    # Without it the only expressible values are replicas and resources, and anything else
+    # a chart needs is a code change, which is the line between a platform and a script.
+    #
+    # Operator-supplied, never tenant-supplied: the catalog is a file only the platform team
+    # edits. A tenant reaching this dict would be handing arbitrary helm values — image
+    # references, securityContext, hostPath mounts — straight to the cluster.
+    values: dict[str, Any] = Field(default_factory=dict)
 
 
 class Instance(BaseModel):

libs/svcforge_core/svcforge_core/domain/states.py

@@ -3,6 +3,8 @@
 from enum import StrEnum
 from typing import Final
 
+from svcforge_core.errors import SvcforgeError
+
 
 class InstanceState(StrEnum):
     """Lifecycle of a provisioned service instance."""
@@ -15,7 +17,7 @@ class InstanceState(StrEnum):
     FAILED = "failed"
 
 
-class IllegalTransition(Exception):
+class IllegalTransition(SvcforgeError):
     """Raised by transition() when cur -> nxt is not in LEGAL."""
 
 

libs/svcforge_core/svcforge_core/domain/windows.py

@@ -19,11 +19,13 @@ from zoneinfo import ZoneInfo, ZoneInfoNotFoundError
 
 from croniter import croniter
 
+from svcforge_core.errors import SvcforgeError
+
 CRON_FIELDS = 5
 SEPARATOR = "|"
 
 
-class BadWindow(ValueError):
+class BadWindow(SvcforgeError, ValueError):
     """A maintenance window spec is not a 5-field cron plus a known IANA zone."""
 
 

libs/svcforge_core/svcforge_core/errors.py

@@ -1,13 +1,11 @@
 """The one base class every svcforge-raised exception shares.
 
-Without it, a caller that wants "the cluster failed" has to write `except Exception`, which
+Without it, "the cluster failed" has to be caught as `except Exception`, which also swallows
-also swallows the `AttributeError` from a typo three frames down. The two are not the same
+the `AttributeError` from a typo three frames down. One is retried and the other is a bug
-incident: one is retried, the other is a bug that must reach the dead-letter loudly. A single
+that must dead-letter loudly; a single root makes that expressible in one clause.
-root makes that distinction expressible in one clause.
 
-Subclasses keep their existing stdlib base as well (`HelmError(SvcforgeError, RuntimeError)`),
+Subclasses keep their stdlib base too (`HelmError(SvcforgeError, RuntimeError)`), so code
-so code already written against `except RuntimeError` keeps working. The MRO order matters:
+written against `except RuntimeError` keeps working. `SvcforgeError` comes first in the MRO.
-`SvcforgeError` first, so the svcforge-specific class is the more derived one.
 """
 
 from __future__ import annotations

libs/svcforge_core/svcforge_core/obs.py

@@ -1,33 +1,27 @@
 """Logs, traces, metrics. One setup() call, made once, before anything else.
 
-Three libraries, one module, because the three are one decision. A log line without the
+Three libraries in one module because they are one decision: a log line without its trace
-trace id it belongs to is a log line you cannot join to anything; a span without the
+id joins to nothing, and a span without the `instance_id` cannot be searched for. Wiring
-`instance_id` the request is about is a span you cannot search for. They are wired here
+them together here stops a service configuring two of the three and shipping.
-together so that no service can configure two of the three and ship.
 
-The three things that make this module worth reading:
+Three things worth knowing:
 
-1. **Context does not cross a queue.** A trace is a chain of parent/child span contexts
+1. **Context does not cross a queue.** `POST /v1/instances` inserts a row and returns; the
-   passed in-process or over a wire header. `POST /v1/instances` inserts a row and
+   worker picks it up ninety seconds later in another pod, with no ambient context. So:
-   returns; the worker picks that row up ninety seconds later in a different pod. Nothing
+   `inject_traceparent()` at enqueue, a `traceparent` column, `context_from_traceparent()`
-   carries the context across — unless we carry it ourselves. So: `inject_traceparent()`
+   at claim. Two disconnected traces in Tempo is the symptom of skipping this.
-   at enqueue, a `traceparent` column, `context_from_traceparent()` at claim. Two
-   disconnected traces in Tempo is the symptom of skipping this.
 
-2. **Histogram buckets are a domain decision.** prometheus_client's defaults top out at
+2. **Histogram buckets are a domain decision.** prometheus_client's defaults were chosen
-   10 seconds because they were chosen for HTTP handlers. A provision is `helm --wait` on
+   for HTTP handlers and top out at 10s; a provision is `helm --wait` on a StatefulSet, so
-   a StatefulSet: minutes. With the defaults every observation lands in `+Inf`,
+   every observation lands in `+Inf` and the p95 is interpolated inside a bucket spanning
-   `histogram_quantile` interpolates inside a bucket that spans 10s→infinity, and the p95
+   10s→infinity. The buckets below are sized for what is measured.
-   it prints is a number with no relationship to reality. The buckets below are sized for
-   what is being measured.
 
-3. **One process per pod.** prometheus_client keeps its registry in process memory. Run
+3. **One process per pod.** prometheus_client keeps its registry in process memory, so
-   `uvicorn --workers 4` and Prometheus scrapes whichever of the four children the socket
+   `uvicorn --workers 4` has Prometheus scraping whichever child the socket hands it and
-   happens to hand it, so counters appear to jump backwards. There are two fixes:
+   counters appear to jump backwards. The alternative fix — `PROMETHEUS_MULTIPROC_DIR` and
-   `PROMETHEUS_MULTIPROC_DIR` + `MultiProcessCollector` (a shared mmap directory, a
+   `MultiProcessCollector` — costs a shared mmap directory, a gauge-mode decision at every
-   gauge-mode decision at every call site, and dead files to garbage-collect after every
+   call site, and dead files to collect after every crash. This repo scales with replicas
-   crash), or one process per pod and scale with replicas. This repo takes the second.
+   instead; nothing here reads that variable.
-   `PROMETHEUS_MULTIPROC_DIR` is deliberately not set, and nothing here reads it.
 """
 
 from __future__ import annotations
@@ -52,9 +46,8 @@ if TYPE_CHECKING:
 # --- Metrics ----------------------------------------------------------------------------
 #
 # Module level, created exactly once at import. A second registration of the same name
-# against the default registry raises ValueError, which is a feature: it turns "two modules
+# raises ValueError, which turns "two modules each defined their own copy" into a startup
-# each defined their own copy of this counter" into an ImportError at startup instead of a
+# failure instead of a metric that silently reports half the truth.
-# metric that silently reports half the truth.
 
 TASKS_CLAIMED = Counter(
     "svcforge_tasks_claimed_total",
@@ -79,9 +72,8 @@ TASKS_DEAD_LETTERED = Counter(
 PROVISION_TIME = Histogram(
     "svcforge_provision_duration_seconds",
     "Wall time of a provision task, claim to terminal report.",
-    # NOT the defaults. See the module docstring: the defaults end at 10s and a provision
+    # Not the defaults — see the module docstring. The top finite bucket is 1800 because
-    # takes minutes. The top finite bucket is 1800 because helm's own --timeout is 600 and
+    # helm's own --timeout is 600, so a provision past thirty minutes belongs in +Inf.
-    # a provision past thirty minutes is broken and belongs in +Inf.
     buckets=(10, 30, 60, 120, 300, 600, 1800, float("inf")),
 )
 
@@ -123,9 +115,9 @@ def _add_trace_ids(
 ) -> structlog.typing.EventDict:
     """Stamp the active trace/span id onto the line, if there is one.
 
-    This is the join key. Without it, "find the logs for this trace" is a full-text search
+    The join key: without it, "find the logs for this trace" is a full-text search over a
-    over a time window and a guess; with it, it is one query. Hex-formatted to the widths
+    time window and a guess. Hex-formatted to the W3C widths, so a value pasted from Tempo
-    the W3C spec uses, so the value pasted from Tempo matches the value in Loki.
+    matches the value in Loki.
     """
     span = trace.get_current_span()
     ctx = span.get_span_context()
@@ -138,10 +130,9 @@ def _add_trace_ids(
 def setup(service_name: str, settings: Settings) -> None:
     """Configure structlog, the tracer provider, and the metric registry. Idempotent.
 
-    Called once from each service's entrypoint, before anything else — "before anything
+    Called once from each service's entrypoint, before anything else: a logger bound before
-    else" because any logger bound before this runs keeps the default configuration
+    this runs keeps the default configuration (`cache_logger_on_first_use`), so a
-    (`cache_logger_on_first_use`), and a module-level `log = structlog.get_logger()` in an
+    module-level `log = structlog.get_logger()` prints unstructured text forever.
-    import that lands first will print unstructured text forever.
     """
     global _configured  # process-wide config is process-wide state
     if _configured:
@@ -155,9 +146,9 @@ def setup(service_name: str, settings: Settings) -> None:
 def _setup_logging(service_name: str, settings: Settings) -> None:
     """structlog + stdlib logging, both rendering JSON to stdout through one handler.
 
-    The stdlib half is not optional. `psycopg`, `httpx`, `uvicorn` and the OTEL SDK all log
+    The stdlib half is not optional: `psycopg`, `httpx`, `uvicorn` and the OTEL SDK all log
-    through `logging`; without the ProcessorFormatter bridge below, their lines arrive as
+    through `logging`, and without the ProcessorFormatter bridge their lines arrive as bare
-    bare text on the same stdout and every one of them is a parse failure in the collector.
+    text on the same stdout — a parse failure each in the collector.
     """
     level = getattr(logging, settings.log_level.upper(), logging.INFO)
 
@@ -200,14 +191,13 @@ def _setup_logging(service_name: str, settings: Settings) -> None:
 
     root = logging.getLogger()
     # Replace rather than append: basicConfig may already have run, and two handlers means
-    # two copies of every line. stdout only — a container writes logs to stdout and the
+    # two copies of every line. stdout only — a log file inside a pod dies with the pod.
-    # collector tails them from there. A log file inside a pod is deleted with the pod.
     root.handlers = [handler]
     root.setLevel(level)
 
-    # Remembered so bind_task_context can restore it after clearing. Without this, every
+    # Remembered so bind_task_context can restore it after clearing. Without it every line
-    # log line emitted inside a task loses `service`, and those are exactly the lines you
+    # emitted inside a task loses `service`, which is what tells worker output from
-    # filter on when you are trying to tell worker output from reconciler output.
+    # reconciler output.
     global _service_name
     _service_name = service_name
     structlog.contextvars.bind_contextvars(service=service_name)
@@ -216,10 +206,9 @@ def _setup_logging(service_name: str, settings: Settings) -> None:
 def _setup_tracing(service_name: str, settings: Settings) -> None:
     """Set the global tracer provider, exporting over OTLP when an endpoint is configured.
 
-    Skipped entirely when something already set a provider: the API runs under
+    Skipped when something already set one: the API runs under `opentelemetry-instrument`,
-    `opentelemetry-instrument`, whose auto-instrumentation installs one before our
+    which installs a provider before `main()` is reached. Overwriting it drops the FastAPI
-    `main()` is reached. Overwriting it drops the FastAPI and psycopg instrumentation's
+    and psycopg spans on the floor, and the SDK only logs a warning.
-    spans on the floor, and the SDK only logs a warning about it.
     """
     if isinstance(trace.get_tracer_provider(), TracerProvider):
         return
@@ -230,7 +219,7 @@ def _setup_tracing(service_name: str, settings: Settings) -> None:
         exporter = _otlp_exporter(settings.otel_endpoint)
         if exporter is not None:
             # Batch, not Simple: SimpleSpanProcessor exports inline on span end, so every
-            # helm span would block on a network round trip to the collector.
+            # helm span would block on a round trip to the collector.
             provider.add_span_processor(BatchSpanProcessor(exporter))
 
     trace.set_tracer_provider(provider)
@@ -239,9 +228,9 @@ def _setup_tracing(service_name: str, settings: Settings) -> None:
 def _otlp_exporter(endpoint: str) -> Any | None:  # noqa: ANN401 - one of two exporter classes
     """The OTLP exporter, if the optional exporter package is installed.
 
-    Optional on purpose. In the cluster the API runs under `opentelemetry-instrument`,
+    Optional on purpose: in the cluster the API runs under `opentelemetry-instrument`, which
-    which brings its own exporter and configures it from `OTEL_EXPORTER_OTLP_*`. Making it
+    brings its own exporter configured from `OTEL_EXPORTER_OTLP_*`. As a hard dependency of
-    a hard dependency of the shared library would mean every unit test imports gRPC.
+    the shared library it would make every unit test import gRPC.
     """
     try:
         from opentelemetry.exporter.otlp.proto.grpc.trace_exporter import OTLPSpanExporter
@@ -263,8 +252,8 @@ def get_logger(name: str) -> structlog.stdlib.BoundLogger:
 def tracer() -> trace.Tracer:
     """The svcforge tracer. Manual spans wrap helm calls, and nothing else.
 
-    Everything else is auto-instrumented (FastAPI, psycopg). A hand-rolled span around a
+    FastAPI and psycopg are auto-instrumented, and a hand-rolled span around something the
-    function that the SDK already wraps is a duplicated span and a maintenance cost.
+    SDK already wraps is a duplicate to maintain.
     """
     return trace.get_tracer(_TRACER_NAME)
 
@@ -285,17 +274,15 @@ def start_metrics_server(port: int) -> None:
 def bind_task_context(instance_id: UUID, task_id: int, team: str) -> None:
     """Bind the three keys every log line in a task must carry. Called at claim time.
 
-    `clear_contextvars()` first, and this is the whole reason the function exists rather
+    `clear_contextvars()` first, which is why this is a function rather than three
-    than three `bind_contextvars` calls at the call site. A worker coroutine reuses its
+    `bind_contextvars` calls at the call site: a worker coroutine reuses its context across
-    context across loop iterations; without the clear, task 41's `instance_id` is still
+    iterations, so without the clear, task 41's `instance_id` is still bound when task 42
-    bound when task 42 starts logging, and the log for the incident you are debugging
+    logs and the incident names the wrong tenant. Contextvars are per-task in asyncio, so
-    names the wrong tenant. Contextvars are per-task in asyncio, which makes this safe
+    two handlers under the concurrency semaphore do not see each other's.
-    under the concurrency semaphore: two handlers running at once do not see each other's.
     """
     structlog.contextvars.clear_contextvars()
     structlog.contextvars.bind_contextvars(
-        # `service` is re-bound because the clear above took it with it. It is set once in
+        # Re-bound because the indiscriminate clear above took it; it is not per-task.
-        # setup() and is not per-task, but clear_contextvars() is indiscriminate.
         service=_service_name,
         instance_id=str(instance_id),
         task_id=task_id,
@@ -307,8 +294,7 @@ def inject_traceparent() -> str | None:
     """Serialise the active span context to a W3C traceparent, for the tasks row.
 
     None when there is no recording span — a task enqueued by the reconciler's own tick has
-    no inbound request to be part of. Nullable column, nullable return: an untraced task is
+    no inbound request to belong to. Nullable column, nullable return.
-    normal, not an error.
     """
     carrier: dict[str, str] = {}
     _propagator.inject(carrier)
@@ -318,9 +304,9 @@ def inject_traceparent() -> str | None:
 def context_from_traceparent(traceparent: str | None) -> Context:
     """Inverse of inject_traceparent. Used at claim to parent the worker span to the API's.
 
-    An empty Context for None or for a malformed value — `extract` does not raise on a
+    An empty Context for None or for a malformed value: `extract` does not raise on an
-    traceparent that fails to parse, it returns the carrier's context unchanged, and the
+    unparseable traceparent, it returns the carrier's context unchanged and the span starts
-    resulting span starts a new trace. A bad header must never fail a provision.
+    a new trace. A bad header must never fail a provision.
     """
     if not traceparent:
         return Context()

libs/svcforge_core/svcforge_core/repo/db.py

@@ -12,11 +12,15 @@ from psycopg import AsyncConnection
 from psycopg.rows import dict_row
 from psycopg_pool import AsyncConnectionPool
 
-# The pool hands out dict-row connections because of `row_factory=dict_row` below. Say so
+# The cap on error text written to `instances.error` and `tasks.last_error`. A helm failure
-# in the type system too, or every `row["attempts"]` in this codebase is a mypy error
+# can emit megabytes and these columns are read by humans. Defined once so the two call
-# against a bare `AsyncConnectionPool`, which resolves to tuple rows. The runtime was
+# paths that feed the same columns agree.
-# always right; without these aliases the annotations quietly disagree with it, and the
+ERROR_MAX_CHARS = 2000
-# fix people reach for is `# type: ignore`, which throws away the checking entirely.
+
+# The pool hands out dict-row connections because of `row_factory=dict_row` below, and the
+# type system has to say so: against a bare `AsyncConnectionPool`, which resolves to tuple
+# rows, every `row["attempts"]` is a mypy error. The reach-for fix is `# type: ignore`,
+# which throws away the checking entirely.
 type DictRow = dict[str, Any]
 type DictConnection = AsyncConnection[DictRow]
 type DictPool = AsyncConnectionPool[DictConnection]
@@ -25,28 +29,25 @@ type DictPool = AsyncConnectionPool[DictConnection]
 def make_pool(dsn: str, min_size: int = 1, max_size: int = 5) -> DictPool:
     """Construct the pool. Does NOT open it — the caller owns open/close.
 
-    `open=False` is deliberate: the constructor does zero I/O, so building a pool at
+    `open=False` because the constructor does zero I/O: a pool built at import time and
-    import time and never opening it fails later as a PoolTimeout at first use, far
+    never opened fails later as a PoolTimeout at first use, far from the cause. The caller
-    from the cause. The caller (a FastAPI lifespan, a worker main) opens and closes it.
+    (a FastAPI lifespan, a worker main) opens and closes it.
 
-    kwargs are per-connection:
+    Two per-connection kwargs matter:
 
-    * `prepare_threshold=None` — REQUIRED through pgbouncer in transaction mode.
+    * `prepare_threshold=None` — REQUIRED through pgbouncer in transaction mode. psycopg3
-      psycopg3 auto-prepares a statement after it sees it 5 times. pgbouncer may hand
+      auto-prepares a statement after five executions, and pgbouncer may hand the sixth to a
-      the next execution to a different backend, which has never heard of that prepared
+      backend that has never heard of it. Symptom: `prepared statement "_pg3_0" does not
-      statement. Symptom: everything works for exactly five calls, then
+      exist`, intermittent, only under concurrency, never in a unit test.
-      `prepared statement "_pg3_0" does not exist` — intermittent, only under
+    * `row_factory=dict_row` — rows arrive as dicts, so `Instance.model_validate(row)` works
-      concurrency, never in a unit test.
+      without unpacking tuples by position.
-
-    * `row_factory=dict_row` — rows arrive as dicts, so `Instance.model_validate(row)`
-      works directly instead of unpacking tuples by position.
 
     Also gone on 6543: LISTEN/NOTIFY, session-level SET, cross-statement advisory locks.
-    `SELECT ... FOR UPDATE SKIP LOCKED` inside one transaction is unaffected — which is
+    `SELECT ... FOR UPDATE SKIP LOCKED` inside one transaction is unaffected, which is why
-    exactly why the queue is built on it. Use the session pooler (5432) for migrations.
+    the queue is built on it. Migrations use the session pooler (5432).
 
     max_size is a database-capacity decision, not a throughput knob: the free tier has a
-    small connection budget, and replicas multiply this number.
+    small connection budget and replicas multiply this number.
     """
     return AsyncConnectionPool(
         conninfo=dsn,

libs/svcforge_core/svcforge_core/repo/instances.py

@@ -20,7 +20,7 @@ from svcforge_core.domain.models import Instance
 from svcforge_core.domain.states import InstanceState
 from svcforge_core.repo.db import DictPool
 
-_COLUMNS = """id, team, service_type, size, state, namespace, release_name, chart_version,
+INSTANCE_COLUMNS = """id, team, service_type, size, state, namespace, release_name, chart_version,
               endpoint, error, expires_at, created_at, updated_at"""
 
 
@@ -57,7 +57,7 @@ class InstanceRepo:
                                            release_name, chart_version, endpoint, error, expires_at)
                     values (%(id)s, %(team)s, %(service_type)s, %(size)s, %(state)s, %(namespace)s,
                             %(release_name)s, %(chart_version)s, %(endpoint)s, %(error)s, %(expires_at)s)
-                    returning {_COLUMNS}""",  # noqa: S608 - _COLUMNS is a module constant, not input
+                    returning {INSTANCE_COLUMNS}""",  # noqa: S608 - INSTANCE_COLUMNS is a module constant, not input
                 {
                     "id": inst.id,
                     "team": inst.team,
@@ -80,7 +80,7 @@ class InstanceRepo:
         """Fetch one instance owned by `team`. None if it does not exist OR is not theirs."""
         async with self._pool.connection() as conn, conn.cursor() as cur:
             await cur.execute(
-                f"select {_COLUMNS} from instances where id = %s and team = %s",  # noqa: S608
+                f"select {INSTANCE_COLUMNS} from instances where id = %s and team = %s",  # noqa: S608
                 (id, team),
             )
             row = await cur.fetchone()
@@ -90,7 +90,7 @@ class InstanceRepo:
         """The team's instances, newest first."""
         async with self._pool.connection() as conn, conn.cursor() as cur:
             await cur.execute(
-                f"""select {_COLUMNS} from instances
+                f"""select {INSTANCE_COLUMNS} from instances
                     where team = %s order by created_at desc limit %s""",  # noqa: S608
                 (team, limit),
             )
@@ -157,7 +157,7 @@ class InstanceRepo:
         # and would otherwise type these rows as tuples.
         async with self._pool.connection() as conn, conn.cursor(row_factory=dict_row) as cur:
             await cur.execute(
-                f"""select {_COLUMNS}, maintenance_window from instances
+                f"""select {INSTANCE_COLUMNS}, maintenance_window from instances
                      where state = 'ready'
                        and service_type = %(service_type)s
                        and chart_version <> %(catalog_version)s
@@ -166,7 +166,7 @@ class InstanceRepo:
                               where cv.service_type = %(service_type)s
                                 and cv.rollout_state = 'halted')
                      order by team = %(own_team)s desc, created_at
-                     limit %(max_in_flight)s""",  # noqa: S608 - _COLUMNS is a module constant, not input
+                     limit %(max_in_flight)s""",  # noqa: S608 - INSTANCE_COLUMNS is a module constant, not input
                 {
                     "service_type": service_type,
                     "catalog_version": catalog_version,

libs/svcforge_core/svcforge_core/repo/reconcile.py

@@ -1,17 +1,14 @@
 """The reconciler's SQL.
 
-Why this file exists rather than the queries living in `services/reconciler/main.py`: the
+Its own module rather than queries in `services/reconciler/main.py` (transport knows no
-layer rule says transport knows nothing about SQL, and the reconciler is transport — a CLI
+SQL) and rather than more methods on `InstanceRepo`/`TaskRepo`, because everything here is
-entrypoint. It gets its own repo module rather than growing `InstanceRepo` and `TaskRepo`
+a *sweep*: it reads rows nobody asked about and writes an instance state and a task row in
-because everything here is a *sweep*: it reads rows nobody asked about and it writes an
+one transaction. `InstanceRepo.update_state` owns its own connection by design, so the
-instance state and a task row in the same transaction. `InstanceRepo.update_state` owns its
+reconciler cannot get that atomicity without reaching around the repo.
-own connection by design, so the reconciler cannot get atomicity from it without reaching
-around the repo — which is the thing the layer rule exists to prevent.
 
-The recurring shape below is: lock the row, re-check the condition under the lock, act.
+The recurring shape is: lock the row, re-check the condition under the lock, act. The
-The re-check is not paranoia about concurrency — the reconciler is a singleton. It is what
+re-check makes the sweep idempotent against *itself* — the reconciler is a singleton, but a
-makes the sweep idempotent against *itself*: a tick that crashes after the insert and
+tick that crashes before its commit must leave nothing behind for the next one to double.
-before the commit must leave nothing behind, and the next tick must not double-enqueue.
 """
 
 from __future__ import annotations
@@ -25,15 +22,12 @@ from psycopg import AsyncCursor
 from svcforge_core.domain.models import Instance, TaskKind, TaskState
 from svcforge_core.domain.states import InstanceState, transition
 from svcforge_core.obs import inject_traceparent
-from svcforge_core.repo.db import DictPool
+from svcforge_core.repo.db import ERROR_MAX_CHARS, DictPool
+from svcforge_core.repo.instances import INSTANCE_COLUMNS
 
-_COLUMNS = """id, team, service_type, size, state, namespace, release_name, chart_version,
+# What "already outstanding" means to the idempotency guard on every enqueue below.
-              endpoint, error, expires_at, created_at, updated_at"""
+# 'done'/'failed' are not outstanding: a deprovision that exhausted its attempts must be
-
+# re-enqueueable, or a transient cluster outage strands the instance permanently.
-# A task nobody will ever run again. The idempotency guard on every enqueue below asks
-# "is one already outstanding?", and 'done'/'failed' are not outstanding: a failed
-# deprovision that exhausted its attempts must be re-enqueueable by the next sweep, or a
-# transient cluster outage would permanently strand the instance.
 _UNFINISHED = (TaskState.QUEUED.value, TaskState.RUNNING.value)
 
 
@@ -48,9 +42,8 @@ class ReconcileRepo:
     async def queue_depth(self) -> int:
         """Tasks waiting to be claimed.
 
-        Counts every `queued` row, not just the runnable ones (`run_after <= now()`). The
+        Every `queued` row, not just the runnable ones. The alert is `deriv(...) > 0` — "the
-        alert on this gauge is `deriv(...) > 0` — "the backlog is growing" — and a backlog
+        backlog is growing" — and tasks parked on backoff are part of that backlog.
-        of tasks parked on backoff is exactly the backlog you want to see growing.
         """
         async with self._pool.connection() as conn, conn.cursor() as cur:
             await cur.execute("select count(*) as n from tasks where state = %s", (TaskState.QUEUED.value,))
@@ -70,7 +63,7 @@ class ReconcileRepo:
         """Every instance the DB believes is running. The drift check's expectation."""
         async with self._pool.connection() as conn, conn.cursor() as cur:
             await cur.execute(
-                f"select {_COLUMNS} from instances where state = %s",  # noqa: S608 - module constant
+                f"select {INSTANCE_COLUMNS} from instances where state = %s",  # noqa: S608 - module constant
                 (InstanceState.READY.value,),
             )
             rows = await cur.fetchall()
@@ -79,9 +72,9 @@ class ReconcileRepo:
     async def known_releases(self) -> set[tuple[str, str]]:
         """(release_name, namespace) for every instance row, in any state.
 
-        Any state, deliberately. An instance that is still `requested` has no release yet,
+        Any state, deliberately. A `requested` instance has no release yet, but a worker may
-        but a worker may be installing it *right now* — treating it as unknown would
+        be installing it right now, and treating it as unknown reports a healthy in-flight
-        report a healthy in-flight provision as an orphan on every tick.
+        provision as an orphan.
         """
         async with self._pool.connection() as conn, conn.cursor() as cur:
             await cur.execute("select release_name, namespace from instances")
@@ -93,20 +86,16 @@ class ReconcileRepo:
 
         None if the row moved, or if a provision is already outstanding.
 
-        The two-hop state change is the interesting part:
+        The state change takes two hops. `LEGAL` has no `ready -> provisioning` edge — the
+        tenant-visible lifecycle leaves `ready` only through `deleting` or `failed`, and
+        drift is a failure — so it goes `ready -> failed -> provisioning`, both edges legal
+        and asserted below rather than assumed. It has to land in `provisioning`, not
+        `failed`: `handle_provision` finishes with a `provisioning -> ready` CAS, and given a
+        `failed` row helm runs, the CAS matches nothing, and the instance sits in `failed`
+        forever with a healthy release behind it.
 
-        * `LEGAL` has no `ready -> provisioning` edge. The tenant-visible lifecycle only
+        Both hops and the insert are one transaction, so the intermediate `failed` is never
-          leaves `ready` through `deleting` or `failed`, and drift is a failure — the
+        observable and a crash mid-sweep leaves nothing half-done.
-          service the tenant is paying for is gone. So: `ready -> failed -> provisioning`,
-          both edges legal, asserted below by the domain function rather than assumed.
-        * The row must land in `provisioning`, not `failed`, before the worker sees the
-          task. `handle_provision` CASes `requested -> provisioning` best-effort and then
-          CASes `provisioning -> ready` for real; hand it a `failed` row and helm runs, the
-          final CAS matches nothing, and the instance sits in `failed` forever with a
-          healthy release behind it.
-
-        Both hops and the insert are one transaction, so the row is never observable in the
-        intermediate `failed` state and a crash mid-sweep leaves nothing half-done.
         """
         async with self._pool.connection() as conn:
             async with conn.transaction(), conn.cursor() as cur:
@@ -119,14 +108,14 @@ class ReconcileRepo:
                 if await _has_unfinished(cur, instance_id, TaskKind.PROVISION):
                     return None
 
-                # Assert the path through the state machine instead of trusting the SQL.
+                # Assert the path through the state machine instead of trusting the SQL: an
-                # If someone edits LEGAL, this raises here rather than corrupting rows.
+                # edit to LEGAL raises here rather than corrupting rows.
                 failed = transition(InstanceState.READY, InstanceState.FAILED)
                 provisioning = transition(failed, InstanceState.PROVISIONING)
 
                 await cur.execute(
                     "update instances set state = %s, error = %s, updated_at = now() where id = %s",
-                    (provisioning.value, reason[-2000:], instance_id),
+                    (provisioning.value, reason[-ERROR_MAX_CHARS:], instance_id),
                 )
                 return await _insert_task(cur, instance_id, TaskKind.PROVISION)
 
@@ -137,21 +126,19 @@ class ReconcileRepo:
 
         Two populations, one query:
 
-        * `ready` and past `expires_at` — the TTL sweep proper. The whole reason a
+        * `ready` past `expires_at` — the TTL sweep, which is what stops a throwaway
-          throwaway Elasticsearch does not become a permanent line on the cloud bill.
+          Elasticsearch becoming a permanent line on the cloud bill.
-        * `deleting` with nothing to do the deleting — the API CASes to `deleting` and then
+        * `deleting` with nothing doing the deleting — the API CASes to `deleting` and
           enqueues in a second statement, and a crash between the two leaves exactly this.
-          That ordering is chosen *because* this sweep exists; the other order would leave
+          That order is chosen because this sweep exists; the reverse would leave a
-          a deprovision task pointing at a `ready` instance, and a worker would tear down a
+          deprovision task on a `ready` instance and tear down a live service.
-          live service nobody asked to delete.
 
-        Note the parentheses around the OR. Without them, `and not exists (...)` binds to
+        Note the parentheses around the OR: without them `and not exists (...)` binds to the
-        the second branch alone and the query re-enqueues a deprovision for every deleting
+        second branch alone and every deleting instance is re-enqueued on every tick.
-        instance on every tick, forever.
         """
         async with self._pool.connection() as conn, conn.cursor() as cur:
             await cur.execute(
-                f"""select {_COLUMNS} from instances i
+                f"""select {INSTANCE_COLUMNS} from instances i
                      where ((i.state = %(ready)s and i.expires_at < now()) or i.state = %(deleting)s)
                        and not exists (
                              select 1 from tasks t
@@ -172,10 +159,10 @@ class ReconcileRepo:
     async def enqueue_deprovision(self, instance_id: UUID) -> int | None:
         """CAS to `deleting` if needed, and enqueue the task. One transaction. None if moot.
 
-        The instance must be in `deleting` before the worker claims the task, for the same
+        The instance must reach `deleting` before the worker claims the task, for the same
-        reason as `enqueue_reprovision`: `handle_deprovision` finishes with a
+        reason as `enqueue_reprovision`: `handle_deprovision` ends with a `deleting ->
-        `deleting -> deleted` CAS, and a `ready` row would make helm uninstall the release
+        deleted` CAS, and on a `ready` row helm uninstalls the release while the DB keeps
-        and the DB keep advertising an endpoint that no longer resolves.
+        advertising an endpoint that no longer resolves.
         """
         async with self._pool.connection() as conn:
             async with conn.transaction(), conn.cursor() as cur:
@@ -210,15 +197,13 @@ class ReconcileRepo:
         """Enqueue an upgrade unless one is already outstanding. None if it is.
 
         The guard is what keeps the fleet at `max_in_flight`. The work list is a query over
-        `chart_version`, and that column is only written *after* helm reports success — so
+        `chart_version`, which is written only after helm reports success, so an instance
-        an instance stays on the work list for the entire duration of its own upgrade, and
+        stays on the list for the whole duration of its own upgrade and for the hours it
-        for the hours it spends parked waiting for its 03:00 window. Without this check the
+        spends parked waiting for its 03:00 window. Without the check, `max_in_flight=1`
-        sweep enqueues one more upgrade for the same instance every 60 seconds, and
+        becomes sixty tasks an hour against one release.
-        `max_in_flight=1` becomes sixty tasks an hour against one release.
 
-        `verify` counts as outstanding too: an upgrade whose verify has not reported is an
+        `verify` counts as outstanding too: re-enqueueing an upgrade whose verify has not
-        upgrade still in progress, and re-enqueueing it would race the probe that decides
+        reported would race the probe that decides whether the rollout halts.
-        whether the whole rollout halts.
         """
         async with self._pool.connection() as conn:
             async with conn.transaction(), conn.cursor() as cur:
@@ -234,9 +219,8 @@ async def _has_unfinished(
 ) -> bool:
     """Is a task of any of these kinds queued or running for this instance?
 
-    Takes the caller's cursor on purpose: the answer is only true for as long as the
+    Takes the caller's cursor: the answer holds only for the asking transaction, and a
-    transaction that asked, and checking on a separate connection would be a check against
+    separate connection would check a different snapshot than the insert that follows.
-    a different snapshot than the insert that follows it.
     """
     await cur.execute(
         """select 1 from tasks
@@ -260,10 +244,9 @@ async def _insert_task(
     """Insert one task in the caller's transaction, carrying the current trace context.
 
     `traceparent` is written here rather than left to `TaskRepo.enqueue` because these rows
-    are inserted inside a transaction the reconciler owns. Nothing propagates a trace
+    go in inside a transaction the reconciler owns, and nothing propagates a trace through a
-    through a table on its own — see `obs.inject_traceparent`. It is null when the sweep is
+    table on its own (see `obs.inject_traceparent`). Null when the sweep is not itself inside
-    not itself inside a span, which is fine and expected: a nullable column for an untraced
+    a span, which is normal.
-    task.
     """
     await cur.execute(
         """insert into tasks (instance_id, kind, run_after, traceparent)

libs/svcforge_core/svcforge_core/repo/tasks.py

@@ -1,11 +1,8 @@
-"""The queue.
+"""The queue: a Postgres table, not Redis.
 
-The queue is a Postgres table, not Redis. The reason is one sentence: a task and the
+A task and the instance state it describes must commit atomically. Split across two stores
-instance state it describes must commit atomically. Split them across two stores and you
+that is a distributed commit problem with no winning move — the process can die between the
-own a distributed commit problem that has no winning move — the process can die between
+two writes, and whichever went first is the one that lies. Everything here follows from that.
-the two writes, and whichever you write first is the one that lies.
-
-Everything else here follows from that.
 """
 
 from __future__ import annotations
@@ -20,42 +17,32 @@ from svcforge_core.domain.backoff import next_attempt_at
 from svcforge_core.domain.models import Task, TaskKind
 from svcforge_core.domain.states import LEGAL, InstanceState
 from svcforge_core.obs import TASKS_DEAD_LETTERED, inject_traceparent
-from svcforge_core.repo.db import DictPool
+from svcforge_core.repo.db import ERROR_MAX_CHARS, DictPool
 
-# Which states may legally become `failed`, derived from the domain's own table rather
+# Which states may legally become `failed`, derived from the domain's table rather than
-# than restated here. Without this guard the UPDATE below would happily move a `deleted`
+# restated here. Without this guard the UPDATE below would move a `deleted` instance to
-# instance to `failed` — a transition domain.transition() explicitly forbids, performed
+# `failed` — a transition domain.transition() forbids, performed by SQL that never asks it.
-# by raw SQL that never asks it. The state machine has to be the same one everywhere, or
-# it is decoration.
 _CAN_FAIL: Final[tuple[str, ...]] = tuple(
     state.value for state, allowed in LEGAL.items() if InstanceState.FAILED in allowed
 )
 
 # The claim query. Do not "simplify" this into two statements.
 #
-# Postgres has no `UPDATE ... LIMIT`, so the row is chosen by a subquery. That subquery
+# Postgres has no `UPDATE ... LIMIT`, so a subquery picks the row. It takes a row lock
-# takes a row lock (`for update`) and steps over rows other workers already hold
+# (`for update`) and steps over rows other workers hold (`skip locked`) instead of blocking
-# (`skip locked`) instead of blocking behind them — which is what makes N workers scale
+# behind them, which is what lets N workers scale instead of queueing behind the oldest
-# instead of queueing single-file behind the oldest task.
+# task. Select-then-update as two statements leaves a gap where a second worker reads the
+# same id and both provision — small enough to miss in testing and hit in production.
 #
-# The whole thing is ONE statement on purpose. Select-then-update as two statements
+# The `with claimed as (...)` wrapper changes nothing about the locking: the UPDATE and its
-# leaves a gap in which a second worker reads the same id, and both provision. The gap
+# subquery are still one statement, and a data-modifying CTE runs exactly once. The outer
-# is small, which means you will not hit it in testing and will hit it in production.
+# SELECT only joins `instances.team` onto the claimed row so the worker can bind `team` to
+# its log context without a second round trip.
 #
-# The `with claimed as (...)` wrapper is the ONLY addition to the canonical form, and it
+# LEFT join, not inner. The CTE's UPDATE has already taken effect when the outer select
-# changes nothing about the locking: the UPDATE and its `for update skip locked` subquery
+# runs, so an inner join matching nothing returns no row — `claim()` would report "queue
-# are still one statement, executed once. The outer SELECT only joins `instances.team`
+# empty" for a task it just marked `running`, stranding it until the lease expires and
-# onto the row that was already claimed, so the worker can bind `team` to its log context
+# burning an attempt. `Task.team` is already `str | None`.
-# before it has loaded anything. A data-modifying CTE runs exactly once and cannot claim
-# twice. The alternative — a second SELECT for the team — would be a second round trip per
-# task to fetch a column the database already had in hand.
-#
-# LEFT join, not inner. The UPDATE inside the CTE has already taken effect by the time the
-# outer select runs, so an inner join that matches nothing would return no row — and
-# `claim()` would report "queue empty" for a task it had just marked `running`, stranding
-# it until the lease expires and silently burning an attempt. The FK cascade makes that
-# nearly impossible in practice; "nearly" is not a reason to leave a silent failure in the
-# one query the whole system depends on. `Task.team` is already `str | None`.
 _CLAIM_SQL = """
 with claimed as (
   update tasks set state='running', attempts=attempts+1, locked_by=%(worker)s, locked_at=now()
@@ -88,16 +75,14 @@ class TaskRepo:
     ) -> Task:
         """Insert a task inside the CALLER's transaction.
 
-        Takes `conn` so the API can insert the instance and enqueue its provision task in
+        Takes `conn` so the API can insert the instance and enqueue its provision task
-        one transaction. Rolling back must lose both, or you get an orphan task pointing
+        together. A rollback must lose both, or an orphan task points at an instance that
-        at an instance that was never committed.
+        was never committed.
 
-        The `traceparent` is captured here, at enqueue time, because this is the last
+        `traceparent` is captured here because this is the last moment the caller's span
-        moment the caller's span context still exists. Trace context does NOT survive a
+        context exists. Trace context does not survive a queue on its own — the worker picks
-        queue on its own: the worker picks the row up in a different process, minutes
+        the row up in another process minutes later — so writing the W3C traceparent onto
-        later, with no ambient context. Writing the W3C traceparent onto the row is the
+        the row is what lets it re-parent its span to the POST that caused it.
-        thread that lets the worker re-parent its span to the POST that caused it — the
-        difference between one trace spanning API → queue → helm and two unrelated ones.
         """
         async with conn.cursor() as cur:
             await cur.execute(
@@ -118,10 +103,8 @@ class TaskRepo:
     ) -> int:
         """Enqueue in its own transaction, returning the new task id.
 
-        For callers with nothing to commit alongside it — the reconciler, tests. The
+        For callers with nothing to commit alongside it — the reconciler, tests. A separate
-        module specs disagree about enqueue's shape (Module 2 passes a conn, Module 4
+        method rather than an optional `conn`, which would hide the transaction question.
-        does not); rather than making `conn` optional and quietly hiding the transaction
-        question, both callers get an honest method name.
         """
         async with self._pool.connection() as conn:
             task = await self.enqueue(conn, instance_id, kind, run_after)
@@ -151,12 +134,11 @@ class TaskRepo:
         and recording what it accomplished belong in one transaction, or a crash between
         them leaves a task marked done whose work never landed.
 
-        `and state='running' and locked_by=%s` is not defensive padding — without it this
+        `and state='running' and locked_by=%s` is a lost-update guard with a real trigger. A
-        is a lost-update bug with a real trigger. A worker that hangs past `lease_seconds`
+        worker that hangs past `lease_seconds` has its task requeued and re-claimed; when it
-        has its task requeued by the reconciler and re-claimed by someone else. When the
+        returns, an unconditional UPDATE marks the task `done` while the new owner is still
-        hung worker finally returns, an unconditional UPDATE here marks the task `done`
+        running it. The loser gets False and treats it as "someone else owns this", not an
-        while the new owner is still running it, and its work goes unaccounted for. The
+        error.
-        loser gets False and must treat it as "someone else owns this now", not an error.
         """
         sql = "update tasks set state='done', locked_by=null where id=%s and state='running' and locked_by=%s"
         if conn is not None:
@@ -170,17 +152,16 @@ class TaskRepo:
     async def fail(self, task_id: int, err: str, worker_id: str, max_attempts: int = 5) -> bool:
         """Retry with backoff, or give up. False if this worker no longer owns the task.
 
-        Under max_attempts: back to 'queued' with run_after pushed out by exponential
+        Under max_attempts: back to 'queued', run_after pushed out by exponential backoff
-        backoff with full jitter. Jitter matters — a cluster-wide outage fails every task
+        with full jitter. Jitter matters — a cluster-wide outage fails every task at once,
-        at once, and without it every worker retries in the same instant, forever.
+        and without it every worker retries in the same instant, forever.
 
-        At max_attempts: 'failed', and the error is copied onto the instance so the tenant
+        At max_attempts: 'failed', a dead-letter state rather than an infinite retry, and
-        can see it. A dead-letter state, not an infinite retry: a task that cannot succeed
+        for a provision the error is copied onto the instance so the tenant can see it.
-        must stop and become someone's problem.
 
-        The ownership check in the SELECT is the same lost-lease guard as `complete`, and
+        The ownership check is the same lost-lease guard as `complete`, and matters more
-        it matters more here: a stale worker reporting failure would push a task the new
+        here: a stale worker reporting failure would push a task the new owner is running
-        owner is actively running back to `queued`, letting a *third* worker claim it.
+        back to `queued`, letting a third worker claim it.
         """
         now = datetime.now(UTC)
         async with self._pool.connection() as conn:
@@ -193,8 +174,8 @@ class TaskRepo:
                 )
                 row = await cur.fetchone()
                 if row is None:
-                    # Either the task is gone, or the lease was stolen. Both mean: not ours
+                    # Task gone, or the lease was stolen. Either way it is not ours to
-                    # to report on. Writing anything here would corrupt the new owner's run.
+                    # report on, and writing here would corrupt the new owner's run.
                     return False
                 attempts = int(row["attempts"])
                 instance_id = row["instance_id"]
@@ -205,7 +186,7 @@ class TaskRepo:
                               set state='queued', locked_by=null, locked_at=null,
                                   last_error=%s, run_after=%s
                             where id = %s""",
-                        (err[-2000:], next_attempt_at(attempts - 1, now=now), task_id),
+                        (err[-ERROR_MAX_CHARS:], next_attempt_at(attempts - 1, now=now), task_id),
                     )
                     return True
 
@@ -213,15 +194,27 @@ class TaskRepo:
                     """update tasks
                           set state='failed', locked_by=null, locked_at=null, last_error=%s
                         where id = %s""",
-                    (err[-2000:], task_id),
+                    (err[-ERROR_MAX_CHARS:], task_id),
                 )
-                # `state = any(%s)` keeps this honest: a deprovision that exhausts its
+                # Dead-lettering the task is right for every kind; moving the INSTANCE to
-                # retries against an already-deleted instance records nothing rather than
+                # `failed` is right only for provision, where nothing but a human recovers
-                # resurrecting it into `failed`.
+                # it. For the other three the instance is still healthy and something else
+                # owns recovery:
+                #   deprovision — stays `deleting`, which is what lets due_for_deprovision
+                #                 re-enqueue it. `failed` drops it out of that query and
+                #                 leaks the release forever.
+                #   upgrade     — helm --atomic rolled back, so it is `ready` on the previous
+                #                 version. check_version_drift retries next window; `failed`
+                #                 would drop it off the upgrade work-list.
+                #   verify      — handle_verify already halted the rollout; drift
+                #                 re-provisions if the release vanished.
+                # The dead-letter metric and its alert cover all four, so leaving the
+                # instance alone loses no visibility.
+                if row["kind"] == TaskKind.PROVISION.value:
                     await cur.execute(
                         """update instances set error=%s, state=%s, updated_at=now()
                             where id=%s and state = any(%s)""",
-                    (err[-2000:], InstanceState.FAILED.value, instance_id, list(_CAN_FAIL)),
+                        (err[-ERROR_MAX_CHARS:], InstanceState.FAILED.value, instance_id, list(_CAN_FAIL)),
                     )
                 # Counted here, not in the worker: this is the only place that knows the
                 # difference between "attempt 2 of 5 failed" and "this task is done trying".
@@ -231,10 +224,9 @@ class TaskRepo:
     async def reset_expired_leases(self, lease_seconds: int) -> int:
         """Return tasks whose worker died back to the queue. Called by the reconciler.
 
-        No distributed lock survives a power cut. A worker that is SIGKILLed leaves
+        No distributed lock survives a power cut. A SIGKILLed worker leaves `state='running'`
-        `state='running'` and `locked_by` set with nobody running it, and that row would
+        and `locked_by` set with nobody running it, and that row sits there forever. The
-        sit there forever. The lease is the only thing that recovers it, which is why
+        lease is the only thing that recovers it, which is why `locked_at` exists.
-        `locked_at` exists.
         """
         async with self._pool.connection() as conn, conn.cursor() as cur:
             await cur.execute(

libs/svcforge_core/svcforge_core/runtime.py

@@ -0,0 +1,35 @@
+"""Shared asyncio scaffolding for the long-lived services.
+
+The worker and the reconciler are both a loop that runs until SIGTERM. They wake immediately
+on shutdown rather than sleeping through it, and they install the same loop-safe signal
+handlers. Both lived in each service before; keeping one copy means the shutdown behaviour
+cannot drift between them.
+"""
+
+from __future__ import annotations
+
+import asyncio
+import contextlib
+import signal
+
+
+async def sleep_or_stop(stop: asyncio.Event, seconds: float) -> None:
+    """Sleep for `seconds`, but return the instant `stop` is set.
+
+    `await asyncio.sleep(seconds)` would make every SIGTERM cost up to `seconds` of
+    Kubernetes waiting on terminationGracePeriod for nothing.
+    """
+    with contextlib.suppress(TimeoutError):
+        await asyncio.wait_for(stop.wait(), timeout=seconds)
+
+
+def install_stop_signals(stop: asyncio.Event) -> None:
+    """Set `stop` on SIGTERM and SIGINT, loop-safely.
+
+    add_signal_handler, not signal.signal: the latter runs at an arbitrary bytecode boundary
+    on whatever thread the C-level handler lands on, and the loop does not notice until its
+    next timer fires — up to a full sleep interval away.
+    """
+    loop = asyncio.get_running_loop()
+    for sig in (signal.SIGTERM, signal.SIGINT):
+        loop.add_signal_handler(sig, stop.set)

libs/svcforge_core/svcforge_core/settings.py

@@ -67,12 +67,8 @@ class Settings(BaseSettings):
     kubectl_bin: str = "kubectl"
     helm_timeout_s: float = Field(default=300.0, gt=0)
 
-    # --- CLI ----------------------------------------------------------------------
+    # The CLI's own settings live in `services/cli/main.py`, not here: this model requires
-    # The CLI is an API client and nothing more. It gets a URL and a token; it does not
+    # SVCFORGE_PG_DSN, and the CLI is an API client that must never hold one.
-    # get a DSN, because the moment a human can reach the database directly, someone will
-    # "just fix one row" and the state machine stops being true.
-    api_url: str = "http://localhost:8000"
-    api_token: str | None = None
 
     # --- Observability ------------------------------------------------------------
     log_level: str = "info"
@@ -83,6 +79,15 @@ class Settings(BaseSettings):
     # just for /metrics. 9000 matches the chart's PodMonitor; change both or neither.
     metrics_port: int = Field(default=9000, ge=1, le=65535)
 
+    @property
+    def runtime_dsn(self) -> str:
+        """The transaction-pooler DSN the services open their pool against, as a string.
+
+        A property so the three entrypoints do not each pick between `str(pg_dsn)` and
+        `pg_dsn.unicode_string()`, which had drifted across the services.
+        """
+        return str(self.pg_dsn)
+
     @property
     def migration_dsn(self) -> str:
         """Migrations need a session-mode connection; fall back to the runtime DSN locally."""
@@ -91,13 +96,11 @@ class Settings(BaseSettings):
     def check_production(self) -> None:
         """Refuse the dev escape hatches outside local development. Call at startup.
 
-        This is a no-op unless `SVCFORGE_ENVIRONMENT` says otherwise, which is what makes
+        A no-op unless `SVCFORGE_ENVIRONMENT` says otherwise, which is what makes it safe to
-        it safe to call unconditionally from every entrypoint — and calling it
+        call unconditionally — and unconditionally is the point. A version invoked only from
-        unconditionally is the point. The previous version could only be invoked from a
+        a branch that already knew it was production never ran at all, and
-        branch that already knew it was production, so no such branch was ever written and
+        `SVCFORGE_AUTH_DISABLED=true` in prod would silently serve every unauthenticated
-        the check never ran: `SVCFORGE_AUTH_DISABLED=true` in prod would have started the
+        request as team `platform`.
-        API with JWT verification off, serving every unauthenticated request as team
-        `platform`, silently.
         """
         if self.environment == "local":
             return

services/api/Dockerfile

@@ -7,7 +7,7 @@
 # Two syncs, not one: deps change rarely and our own code changes every commit, so the
 # expensive layer (third-party wheels) must land before the cheap one (our source).
 
-FROM python:3.14-slim@sha256:cea0e6040540fb2b965b6e7fb5ffa00871e632eef63719f0ea54bca189ce14a6 AS builder
+FROM mirror.gcr.io/library/python:3.14-slim@sha256:cea0e6040540fb2b965b6e7fb5ffa00871e632eef63719f0ea54bca189ce14a6 AS builder
 
 COPY --from=ghcr.io/astral-sh/uv:0.11.29@sha256:eb2843a1e56fd9e30c7276ce1a52cba86e64c7b385f5e3279a0e08e02dd058fc /uv /usr/local/bin/uv
 
@@ -44,7 +44,7 @@ p = svcforge_core.__file__; \
 sys.exit(0) if "site-packages" in p else sys.exit("not a wheel install: " + p)'
 
 # --- runtime --------------------------------------------------------------------------
-FROM python:3.14-slim@sha256:cea0e6040540fb2b965b6e7fb5ffa00871e632eef63719f0ea54bca189ce14a6
+FROM mirror.gcr.io/library/python:3.14-slim@sha256:cea0e6040540fb2b965b6e7fb5ffa00871e632eef63719f0ea54bca189ce14a6
 
 ARG BUILD_SHA=unknown
 LABEL org.opencontainers.image.title="svcforge-api" \

services/api/deps.py

@@ -1,8 +1,8 @@
 """Dependency injection: how a handler gets a pool, a repo, a catalog, and a team.
 
 Everything expensive — the pool, the JWKS client, the parsed catalog — is built once in
-`lifespan` and parked on `app.state`. These functions only hand it out. A `Depends` that
+`lifespan` and parked on `app.state`; these functions only hand it out. A `Depends` that
-does I/O per request is a `Depends` that does that I/O on every request forever.
+does I/O per request does that I/O on every request forever.
 """
 
 from __future__ import annotations
@@ -22,10 +22,9 @@ from svcforge_core.repo.instances import InstanceRepo
 from svcforge_core.repo.tasks import TaskRepo
 from svcforge_core.settings import Settings
 
-# The algorithm allow-list is the whole point of naming algorithms explicitly.
+# The algorithm allow-list is not configuration. Without it, `jwt.decode` accepts whatever
-# `jwt.decode(..., algorithms=...)` without it accepts whatever the *token* claims in its
+# the *token* claims in its own header — including `none`, and including HS256 verified
-# own header — including `none`, and including HS256 verified with the RSA public key as
+# with the RSA public key as an HMAC secret. Both are forgery.
-# an HMAC secret. Both are forgery. The list is not configuration.
 ALLOWED_ALGORITHMS = ["RS256"]
 
 # What `auth_disabled` returns. Settings.check_production() refuses that flag in prod.
@@ -42,9 +41,9 @@ _bearer = HTTPBearer(auto_error=False)
 def _unauthorized() -> HTTPException:
     """One shape for every auth failure.
 
-    Expired, wrong issuer, wrong audience, bad signature, malformed, no header: all the
+    Expired, wrong issuer, wrong audience, bad signature, malformed, no header: the same 401
-    same 401 with the same body. Telling a caller *which* one turns the endpoint into an
+    with the same body. Naming which one turns the endpoint into an oracle a forger can tune
-    oracle they can tune a forgery against.
+    against.
     """
     return HTTPException(
         status_code=status.HTTP_401_UNAUTHORIZED,
@@ -68,8 +67,8 @@ async def get_pool(request: Request) -> DictPool:
 def get_catalog(request: Request) -> dict[str, CatalogEntry]:
     """The catalog, parsed once at startup.
 
-    Read from disk per request and a mid-flight edit to catalog.yaml changes the answer
+    Read per request, a mid-flight edit to catalog.yaml would change the answer between two
-    between two requests of the same deploy. Load it at startup; a change is a restart.
+    requests of the same deploy. A catalog change is a restart.
     """
     catalog: dict[str, CatalogEntry] = request.app.state.catalog
     return catalog
@@ -102,16 +101,14 @@ async def get_current_team(
 
     jwks_client: PyJWKClient | None = getattr(request.app.state, "jwks_client", None)
     if jwks_client is None:
-        # Auth is on but there is no key source. Fail closed. Answering 500 here would be
+        # Auth is on but there is no key source. Fail closed. A 500 would be honest about
-        # honest about the cause and would also let a misconfigured deploy be told apart
+        # the cause and would also let a forger tell a misconfigured deploy from a bad token.
-        # from a bad token; 401 is the same answer a forger gets.
         raise _unauthorized()
 
     try:
-        # PyJWKClient keeps its own TTL cache, so this is a dict lookup on the hot path.
+        # PyJWKClient keeps a TTL cache, so this is a dict lookup on the hot path and only
-        # It is only blocking on a cache MISS (key rotation) — hence to_thread, which
+        # blocks on a miss (key rotation) — hence to_thread, a thread hop a few times a day
-        # costs a thread hop we take a handful of times a day rather than an event loop
+        # rather than an event loop stalled on someone else's HTTP call.
-        # stalled on someone else's HTTP call once per rotation.
         signing_key = await _signing_key(jwks_client, creds.credentials)
         claims: dict[str, Any] = jwt.decode(
             creds.credentials,
@@ -139,10 +136,9 @@ async def get_current_team(
 async def _signing_key(client: PyJWKClient, token: str) -> jwt.PyJWK:
     """Fetch the signing key off the event loop.
 
-    PyJWKClient.get_signing_key_from_jwt() does a synchronous urlopen on a cache miss.
+    `get_signing_key_from_jwt()` does a synchronous urlopen on a cache miss. Called directly
-    Called directly from `async def`, that blocks the loop — every other in-flight request
+    from `async def` it blocks the loop: every other in-flight request stops until the IdP
-    on this worker stops until the identity provider answers, and if it hangs, so does the
+    answers, and if the IdP hangs so does the pod, with /readyz still saying it is fine.
-    pod, and /readyz keeps saying it is fine.
     """
     return await asyncio.to_thread(client.get_signing_key_from_jwt, token)
 
@@ -159,11 +155,10 @@ async def rate_limit(
 ) -> None:
     """Per-team rate limiting. One Redis command per check, and it fails OPEN.
 
-    Failing open is the entire policy. Redis holds derived state; losing it must degrade
+    Redis holds derived state, so losing it must degrade the platform rather than stop it: a
-    the platform, never stop it. A limiter that fails closed converts a cache outage into
+    limiter that fails closed turns a cache outage into a total outage, a worse incident
-    a total outage, which is a strictly worse incident than the burst it was protecting
+    than the burst it was guarding against. `RateLimiter.check` swallows its own errors and
-    against — so `RateLimiter.check` swallows its own errors and returns `allowed=True`.
+    returns `allowed=True`, so the 429 below only comes from a real, counted overage.
-    The 429 below therefore only ever comes from a real, counted overage.
     """
     limiter = get_rate_limiter(request)
     if limiter is None:

services/api/main.py

@@ -1,7 +1,7 @@
 """The app factory and its lifespan.
 
-`create_app(settings)` is a factory, not a module-level `app = FastAPI()`, for one reason:
+`create_app(settings)` is a factory rather than a module-level `app = FastAPI()` because a
-a test needs an app pointed at a throwaway Postgres, and an import-time app reads the real
+test needs an app pointed at a throwaway Postgres, and an import-time app reads the real
 environment at import time — before any fixture can say otherwise.
 """
 
@@ -11,9 +11,11 @@ import asyncio
 from collections.abc import AsyncIterator
 from contextlib import asynccontextmanager
 
-from fastapi import FastAPI, HTTPException, Request
+from fastapi import FastAPI, Request
+from fastapi.exceptions import RequestValidationError
 from fastapi.responses import JSONResponse
 from jwt import PyJWKClient
+from starlette.exceptions import HTTPException
 
 from services.api.models import ErrorBody
 from services.api.routes import health, instances
@@ -30,45 +32,43 @@ log = obs.get_logger("svcforge.api")
 async def lifespan(app: FastAPI) -> AsyncIterator[None]:
     """Open the pool, yield, close the pool.
 
-    A lifespan context, not the deprecated startup/shutdown event decorators: those cannot
+    A lifespan context, not the deprecated startup/shutdown decorators: those cannot express
-    express "this resource lives for exactly as long as the app", and give you no place to
+    "this resource lives exactly as long as the app" and leave no place to put teardown next
-    put the teardown next to the setup. Closing the pool matters — an unclosed pool means
+    to setup. Closing matters — an unclosed pool leaves connections open server-side after
-    connections linger server-side after SIGTERM, and on a pooled Postgres with a small
+    SIGTERM, and on a pooled Postgres with a small budget a few rolling deploys exhaust it.
-    connection budget a few rolling deploys exhaust it.
 
-    (The old decorator's name is spelled nowhere in this package on purpose: CI greps for
+    (The old decorator's name is spelled nowhere here on purpose: CI greps for the literal
-    the literal string, and a comment quoting it fails the gate just as loudly as a call.)
+    string, so a comment quoting it fails the gate as loudly as a call would.)
     """
     settings: Settings = app.state.settings
 
     app.state.catalog = load_catalog(settings.catalog_path)
 
-    # Redis is optional by construction. `make_redis` returns None when no DSN is set, and
+    # Redis is optional by construction: `make_redis` returns None when no DSN is set and
-    # every consumer treats None as "skip" — so a deployment without Redis loses rate
+    # every consumer treats None as "skip", so a deployment without Redis loses rate
-    # limiting and keeps everything else. Built here rather than per request because a
+    # limiting and keeps everything else. Built once here, not per request.
-    # connection pool per request is a connection pool per request.
     redis = make_redis(settings)
     app.state.redis = redis
     app.state.rate_limiter = (
         RateLimiter(redis, limit=settings.rate_limit_per_minute, window_s=60) if redis is not None else None
     )
 
-    pool = make_pool(str(settings.pg_dsn), settings.pool_min_size, settings.pool_max_size)
+    pool = make_pool(settings.runtime_dsn, settings.pool_min_size, settings.pool_max_size)
     # wait=True fails NOW, loudly, if the DSN is wrong — instead of at the first request,
     # as a PoolTimeout, in front of a user.
     await pool.open(wait=True)
     app.state.pool = pool
 
-    # The pool is open from here on, so everything below is inside the try: an exception
+    # The pool is open from here, so everything below is inside the try: an exception in
-    # in JWKS setup must still close it, or a crash-looping pod leaks a connection per
+    # JWKS setup must still close it, or a crash-looping pod leaks a connection per restart
-    # restart until the database refuses new ones.
+    # until the database refuses new ones.
     try:
         if settings.jwks_url and not settings.auth_disabled:
             client = PyJWKClient(settings.jwks_url, cache_keys=True, lifespan=300)
             app.state.jwks_client = client
-            # Warm the cache off the loop so the first authenticated request does not pay
+            # Warm the cache off the loop so the first authenticated request does not pay a
-            # a blocking urlopen. Best-effort: a slow identity provider must not stop the
+            # blocking urlopen. Best-effort: a slow IdP must not stop the pod from starting,
-            # pod from starting — a cache miss later just costs one to_thread hop.
+            # and a miss later costs one to_thread hop.
             try:
                 await asyncio.to_thread(client.get_signing_keys)
             except Exception:  # deliberate catch-all: startup must not hinge on the IdP being up
@@ -83,17 +83,77 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]:
             await redis.aclose()
 
 
+# --------------------------------------------------------------------------- API docs
+
+# What the generated schema cannot express. Kept next to create_app because /docs is what
+# someone integrating reads, and they do not have this repo. USER_GUIDE.md is the longer form.
+API_DESCRIPTION = """
+Provision managed service instances into Kubernetes. The catalog offers Elasticsearch,
+Redis and Postgres, plus two deliberately tiny entries — `podinfo` and `nginx` — for
+exercising the platform where there is no room for the real thing.
+
+## Authentication
+
+Every `/v1` route needs a bearer JWT: `Authorization: Bearer <token>`. The token is
+verified against the configured JWKS (RS256), and its `team` claim decides which instances
+you can see. **Authorisation is a WHERE clause** — asking for another team's instance
+returns `404`, not `403`, so the API never confirms that an id you cannot access exists.
+
+## Writes are asynchronous
+
+`POST` and `DELETE` return **202 Accepted**, not 201/204. They enqueue work and return
+immediately; nothing is provisioned yet when you get the response. Poll
+`GET /v1/instances/{id}` and watch `state`.
+
+## Instance lifecycle
+
+    requested -> provisioning -> ready
+                                  |
+                                  v
+                              deleting -> deleted
+
+`failed` is reachable from `requested` and `provisioning` when a provision exhausts its
+retries. A `ready` instance whose release vanished is re-provisioned automatically by the
+reconciler, so `ready` is the only state that carries a usable `endpoint`.
+
+## Errors
+
+Every non-2xx body is the same shape — `{"code": ..., "message": ...}` — including the
+404s and 405s raised by the framework itself. `code` is stable and meant for machines;
+`message` is for humans.
+"""
+
+OPENAPI_TAGS = [
+    {
+        "name": "instances",
+        "description": "Create, inspect and delete service instances. All writes are 202 + poll.",
+    },
+    {
+        "name": "ops",
+        "description": (
+            "Liveness, readiness and Prometheus metrics. Unauthenticated, and not part of "
+            "the tenant API surface."
+        ),
+    },
+]
+
+
 async def _http_exception_handler(request: Request, exc: Exception) -> JSONResponse:
     """Render HTTPException bodies as ErrorBody, so every error has one shape.
 
-    Handlers raise `detail={"code": ..., "message": ...}`; FastAPI's default would nest
+    Handlers raise `detail={"code": ..., "message": ...}`, which FastAPI's default would
-    that under `{"detail": {...}}`. Plain-string details (raised by FastAPI itself, e.g.
+    nest under `{"detail": {...}}`. Plain-string details (a framework 405, say) are wrapped
-    a 405) are wrapped so clients never have to branch on the body's type.
+    so clients never branch on the body's type.
+
+    Registered on starlette's HTTPException, not fastapi's. The FastAPI class is a subclass
+    and Starlette matches handlers by walking `type(exc).__mro__`, so a handler keyed on the
+    subclass never fires for a framework-raised 404 or 405. Keying on the parent catches
+    both, and the branch below renders each into ErrorBody.
     """
     assert isinstance(exc, HTTPException)  # noqa: S101 - registered only for HTTPException
-    # Widened to object deliberately. Starlette types `detail` as str, but FastAPI passes
+    # Widened to object deliberately: Starlette types `detail` as str, but FastAPI passes
-    # through whatever a handler raised — our handlers raise dicts. Narrowing off the
+    # through whatever a handler raised, and ours raise dicts. Narrowing off the declared
-    # declared type would make mypy call the dict branch unreachable and delete it.
+    # type would let mypy call the dict branch unreachable and delete it.
     detail: object = exc.detail
     if isinstance(detail, dict) and "code" in detail and "message" in detail:
         body = ErrorBody(code=str(detail["code"]), message=str(detail["message"]))
@@ -102,26 +162,44 @@ async def _http_exception_handler(request: Request, exc: Exception) -> JSONRespo
     return JSONResponse(status_code=exc.status_code, content=body.model_dump(), headers=exc.headers)
 
 
+async def _validation_exception_handler(request: Request, exc: Exception) -> JSONResponse:
+    """Render request-validation failures as ErrorBody too.
+
+    A forbidden extra field, a bad type or an out-of-range ttl_days raises
+    RequestValidationError, which the handler above never sees. Without this, FastAPI's
+    default `{"detail": [...]}` is a second 422 shape alongside the handlers' ErrorBody.
+    """
+    assert isinstance(exc, RequestValidationError)  # noqa: S101 - registered only for this
+    return JSONResponse(
+        status_code=422,
+        content=ErrorBody(code="validation_error", message=str(exc.errors())).model_dump(),
+    )
+
+
 def create_app(settings: Settings | None = None) -> FastAPI:
     """App factory: lifespan, routers, exception handler, /metrics."""
     settings = settings or load_settings()
 
-    # FIRST, before any router is built and before any logger is bound. Without this the
+    # FIRST, before any router is built and any logger is bound. Without it the API is the
-    # API is the one service of three that never configures structlog: its lines go out
+    # one service of three that never configures structlog, and its lines go out through
-    # through logging.lastResort as bare text on stderr with no service, no trace_id and
+    # logging.lastResort as bare text on stderr — no service, no trace_id, no JSON envelope.
-    # no JSON envelope — a parse failure in the collector, and unattributable in Loki.
     # `settings.log_json` was silently inert here for the same reason.
     obs.setup("svcforge-api", settings)
 
     # Refuse the dev escape hatches when SVCFORGE_ENVIRONMENT says this is not a laptop.
-    # Called unconditionally and early: a check that only runs from a branch someone
+    # Unconditional and early: a check that runs only from a branch someone remembered to
-    # remembered to write is a check that does not run.
+    # write is a check that does not run.
     settings.check_production()
 
+    # The description is the API's documentation, rendered as markdown at /docs. It is the
+    # only place a caller without this repo learns the two things the schema cannot say:
+    # every write is asynchronous, and the lifecycle is a state machine they have to poll.
     app = FastAPI(
         title="svcforge",
         version="0.1.0",
         summary="X-as-a-Service control plane",
+        description=API_DESCRIPTION,
+        openapi_tags=OPENAPI_TAGS,
         lifespan=lifespan,
     )
     app.state.settings = settings
@@ -132,6 +210,7 @@ def create_app(settings: Settings | None = None) -> FastAPI:
     app.include_router(instances.router)
 
     app.add_exception_handler(HTTPException, _http_exception_handler)
+    app.add_exception_handler(RequestValidationError, _validation_exception_handler)
     return app
 
 
@@ -140,7 +219,6 @@ def app() -> FastAPI:
     return create_app()
 
 
-# There is deliberately no `if __name__ == "__main__"` here. `services/api/__main__.py` is
+# No `if __name__ == "__main__"` here on purpose. `services/api/__main__.py` is the single
-# the single entrypoint, and the image's ENTRYPOINT uses it. A second one in this module
+# entrypoint and the image's ENTRYPOINT uses it. A second one in this module drifted from
-# drifted from it — different log_level, different access_log — so `python -m services.api`
+# it — different log_level, different access_log — so the same app started two ways.
-# and `python services/api/main.py` started the same app two different ways.

services/api/models.py

@@ -1,9 +1,8 @@
-"""Wire types.
+"""Wire types, deliberately not the domain models.
 
-These are deliberately NOT the domain models. `Instance` carries `team`, `namespace` and
+`Instance` carries `team`, `namespace` and `release_name` — placement details a tenant has
-`release_name` — placement details a tenant has no business seeing and no business
+no business seeing or setting. The response model is the allow-list that keeps them off the
-setting. The response model is the allow-list that keeps them off the wire, which is why
+wire, which is why it is written by hand instead of derived from `Instance`.
-it is written out by hand instead of derived from `Instance`.
 """
 
 from __future__ import annotations
@@ -19,34 +18,75 @@ class CreateInstanceRequest(BaseModel):
     """What a tenant may ask for.
 
     `service_type` and `size` are plain strings, not enums: the catalog is data loaded at
-    runtime, so baking its keys into a type would mean a redeploy to add a service type,
+    runtime, so baking its keys into a type would mean a redeploy to add a service type and
-    and a 422 (schema) where the spec wants a 404 (unknown resource). They are validated
+    a 422 where the spec wants a 404. The handler validates them against the catalog.
-    against the catalog in the handler.
     """
 
-    model_config = ConfigDict(extra="forbid")
+    model_config = ConfigDict(
+        extra="forbid",
+        json_schema_extra={"examples": [{"service_type": "redis", "size": "small", "ttl_days": 7}]},
+    )
 
-    service_type: str = Field(min_length=1)
+    service_type: str = Field(
-    size: str
+        min_length=1,
-    ttl_days: int | None = Field(default=None, ge=1, le=30)
+        description=(
+            "A service type in the catalog, e.g. `elasticsearch`, `redis`, `postgres`, "
+            "`podinfo`, `nginx`. "
+            "Unknown values return 404."
+        ),
+    )
+    size: str = Field(
+        description=(
+            "A size the catalog defines for that service type, e.g. `small`. Unknown values return 422."
+        ),
+    )
+    ttl_days: int | None = Field(
+        default=None,
+        ge=1,
+        le=30,
+        description="Delete the instance automatically after this many days. Omit for no expiry.",
+    )
 
 
 class InstanceResponse(BaseModel):
     """What a tenant gets back. A subset of Instance, on purpose."""
 
-    model_config = ConfigDict(from_attributes=True)
+    model_config = ConfigDict(
+        from_attributes=True,
+        json_schema_extra={
+            "examples": [
+                {
+                    "id": "0f8b7d3e-1c2a-4f5b-9e6d-7a8b9c0d1e2f",
+                    "state": "ready",
+                    "service_type": "redis",
+                    "size": "small",
+                    "endpoint": "http://acme-redis-0f8b7d3e.tenant-acme.svc.cluster.local",
+                    "chart_version": "20.6.2",
+                    "error": None,
+                }
+            ]
+        },
+    )
 
-    id: UUID
+    id: UUID = Field(description="Poll `GET /v1/instances/{id}` with this to watch the state change.")
-    state: InstanceState
+    state: InstanceState = Field(description="Lifecycle state. Only `ready` carries a usable endpoint.")
     service_type: str
     size: str
-    endpoint: str | None
+    endpoint: str | None = Field(description="In-cluster DNS name. Null until the instance is `ready`.")
-    chart_version: str
+    chart_version: str = Field(
-    error: str | None
+        description="The chart version actually deployed, written only after helm succeeds."
+    )
+    error: str | None = Field(description="Why the last attempt failed. Null unless `state` is `failed`.")
 
 
 class ErrorBody(BaseModel):
     """Every non-2xx body. `code` is for machines, `message` is for humans."""
 
-    code: str
+    model_config = ConfigDict(
-    message: str
+        json_schema_extra={
+            "examples": [{"code": "unknown_service_type", "message": "no such service_type: mongodb"}]
+        }
+    )
+
+    code: str = Field(description="Stable machine-readable identifier for the failure.")
+    message: str = Field(description="Human-readable detail. Do not parse this.")

services/api/routes/health.py

@@ -3,11 +3,11 @@
 The distinction between the first two is the difference between a 30-second blip and a
 fleet-wide outage:
 
-* `/healthz` (liveness) answers "is this process wedged?" A failure here gets the
+* `/healthz` (liveness) answers "is this process wedged?" A failure here KILLS the
-  container KILLED. It must therefore touch NOTHING external. Wire it to the DB and a
+  container, so it must touch nothing external. Wired to the DB, a 20-second Postgres
-  20-second Postgres failover restarts every pod at once; they come back, find the DB
+  failover restarts every pod at once; they come back, find the DB still down, and
-  still down, and CrashLoopBackOff with exponential restart delays — so the fleet is now
+  CrashLoopBackOff with exponential delays — the fleet stays down for minutes after the
-  down for minutes after the database recovered.
+  database recovered.
 * `/readyz` (readiness) answers "should this pod get traffic?" A failure here only removes
   it from the Service endpoints. It is allowed to check dependencies, and it recovers by
   itself the moment the check passes.
@@ -26,10 +26,10 @@ from services.api.models import ErrorBody
 
 router = APIRouter(tags=["ops"])
 
-# No PROMETHEUS_MULTIPROC_DIR here, deliberately: it exists for prefork servers where each
+# No PROMETHEUS_MULTIPROC_DIR, deliberately: it exists for prefork servers where each
-# worker process holds a slice of the counters. One uvicorn process per container means
+# process holds a slice of the counters. One uvicorn process per container makes the
-# the default in-process registry is already correct, and multiproc mode would add a
+# in-process registry correct, and multiproc mode would add a shared temp dir, a cleanup
-# shared temp dir, a cleanup obligation, and a class of stale-file bugs for nothing.
+# obligation, and a class of stale-file bugs for nothing.
 
 
 @router.get("/healthz", status_code=status.HTTP_200_OK)
@@ -45,10 +45,9 @@ async def healthz() -> dict[str, str]:
 async def readyz(pool: PoolDep) -> dict[str, str]:
     """Readiness. Postgres only.
 
-    Postgres-only is the rule, and Redis is the temptation. Redis holds derived state —
+    Redis is the temptation and stays out: it holds derived state that degrades gracefully,
-    rate-limit buckets, caches — and everything degrades gracefully without it. Put it in
+    so checking it here would let an Upstash hiccup mark every pod unready, empty the
-    this check and an Upstash hiccup marks every pod unready, Kubernetes empties the
+    Service, and turn a cache outage into a total API outage.
-    Service, and a cache outage becomes a total API outage.
     """
     try:
         async with pool.connection() as conn, conn.cursor() as cur:
@@ -68,14 +67,13 @@ async def readyz(pool: PoolDep) -> dict[str, str]:
 async def metrics(request: Request) -> Response:
     """The Prometheus scrape endpoint.
 
-    A route rather than `app.mount("/metrics", make_asgi_app())`, for two reasons. A
+    A route rather than `app.mount("/metrics", make_asgi_app())`: a Starlette `Mount`
-    Starlette `Mount` compiles to `^/metrics(?P<path>/.*)$`, which does not match a bare
+    compiles to `^/metrics(?P<path>/.*)$`, which does not match the bare `/metrics` every
-    `/metrics` — the exact URL every scrape config uses — and a `Mount` is invisible to
+    scrape config uses, and a Mount is invisible to OpenAPI.
-    OpenAPI, while the deliverable asks for `/metrics` in `openapi.json`.
 
-    The encoding is still prometheus_client's: `choose_encoder` reads the Accept header and
+    The encoding stays prometheus_client's — `choose_encoder` reads Accept and picks the
-    picks the exposition format (Prometheus text vs OpenMetrics) with its matching content
+    exposition format with its matching content type. Hand-rolling it serves text/plain a
-    type. Hand-rolling either is how you end up serving text/plain that a scraper rejects.
+    scraper rejects.
     """
     encoder, content_type = choose_encoder(request.headers.get("Accept", ""))
     return Response(content=encoder(REGISTRY), media_type=content_type)

services/api/routes/instances.py

@@ -1,12 +1,12 @@
 """The tenant-facing API.
 
-Two rules run through every handler here:
+Two rules run through every handler:
 
-* **AuthZ is the WHERE clause.** No handler ever compares `inst.team` to the caller's
+* **AuthZ is the WHERE clause.** No handler compares `inst.team` to the caller's team,
-  team, because the repo never returns another team's row to compare. A wrong-team id is
+  because the repo never returns another team's row to compare. A wrong-team id is a 404;
-  a 404. 403 would confirm the id exists, which is the leak.
+  403 would confirm the id exists.
-* **The instance and its task commit together.** A committed instance with no task is an
+* **The instance and its task commit together.** A committed instance with no task never
-  instance that never provisions and that nothing will ever retry.
+  provisions and nothing retries it.
 """
 
 from __future__ import annotations
@@ -30,9 +30,9 @@ from services.api.models import CreateInstanceRequest, ErrorBody, InstanceRespon
 from svcforge_core.domain.models import CatalogEntry, Instance, TaskKind
 from svcforge_core.domain.states import IllegalTransition, InstanceState, transition
 
-# Declared on the router so every error shape lands in openapi.json under ErrorBody.
+# Declared on the router so every error shape lands in openapi.json under ErrorBody. The
-# The exception handler already renders this at runtime; without declaring it, generated
+# exception handler already renders these at runtime; undeclared, a generated client sees
-# clients see the contract for 2xx only and invent their own guess for the rest.
+# the contract for 2xx only and guesses the rest.
 ERROR_RESPONSES: dict[int | str, dict[str, Any]] = {
     401: {"model": ErrorBody, "description": "Missing or invalid credentials"},
     404: {"model": ErrorBody, "description": "No such instance, or not this team's"},
@@ -46,14 +46,12 @@ router = APIRouter(prefix="/v1/instances", tags=["instances"], responses=ERROR_R
 def release_name_for(team: str, service_type: str, instance_id: UUID) -> str:
     """The helm release name. Deterministic, and `unique` in the schema.
 
-    This is the idempotency anchor. A worker that dies after `helm install` but before it
+    The idempotency anchor: a worker that dies after `helm install` but before marking the
-    marks the task done will retry, compute the same name, and `helm upgrade --install`
+    task done retries, computes the same name, and upgrades the same release instead of
-    onto the same release instead of creating a second one. Derive it from anything that
+    creating a second one. Derive it from anything not already durable — a timestamp, a
-    is not already durable — a timestamp, a random suffix, the retry count — and a retry
+    random suffix, the retry count — and a retry provisions a duplicate.
-    provisions a duplicate.
 
-    Truncated to the uuid's first 8 chars to stay inside the 53-char limit helm imposes
+    Truncated to the uuid's first 8 chars to stay inside helm's 53-char release-name limit.
-    on release names (Kubernetes label values, minus room for chart-generated suffixes).
     """
     return f"{team}-{service_type}-{str(instance_id)[:8]}"
 
@@ -66,9 +64,8 @@ def namespace_for(team: str) -> str:
 def _resolve(catalog: dict[str, CatalogEntry], service_type: str, size: str) -> CatalogEntry:
     """Look up service_type + size, or raise the right 4xx.
 
-    The two failures are different HTTP problems and the spec asks for different codes:
+    Two different HTTP problems: an unknown service_type is a resource that does not exist
-    an unknown service_type is a resource that does not exist (404); an unknown size for a
+    (404), an unknown size for a real one is a body understood and unprocessable (422).
-    real service_type is a body the server understood and cannot process (422).
     """
     entry = catalog.get(service_type)
     if entry is None:
@@ -107,9 +104,9 @@ async def create_instance(
 ) -> Instance:
     """Accept a provisioning request. 202, never 201.
 
-    Nothing is provisioned when this returns. The row exists and a task is queued; a
+    Nothing is provisioned when this returns: the row exists and a task is queued, and a
-    worker will do the work seconds or minutes from now. 201 Created would be a lie about
+    worker does the work seconds or minutes later. 201 Created would be a lie about a
-    a resource that does not exist yet, and clients would stop polling.
+    resource that does not exist yet, and clients would stop polling.
     """
     entry = _resolve(catalog, body.service_type, body.size)
 
@@ -123,9 +120,9 @@ async def create_instance(
         state=InstanceState.REQUESTED,
         namespace=namespace_for(team),
         release_name=release_name_for(team, body.service_type, instance_id),
-        # Pinned from the catalog AT CREATION TIME, not read from the catalog later.
+        # Pinned at creation time, not read from the catalog later. The column records what
-        # This column records what is actually deployed; bumping catalog.yaml must show up
+        # is deployed, so bumping catalog.yaml shows up as drift the reconciler can see
-        # as drift the reconciler can see, not silently rewrite history.
+        # rather than silently rewriting history.
         chart_version=entry.chart_version,
         expires_at=now + timedelta(days=body.ttl_days) if body.ttl_days is not None else None,
         created_at=now,
@@ -181,12 +178,11 @@ async def delete_instance(
 ) -> Instance:
     """state -> deleting, enqueue deprovision. 202: the helm uninstall has not happened yet.
 
-    Ordering note. `InstanceRepo.update_state` owns its own connection, so the CAS and the
+    `InstanceRepo.update_state` owns its own connection, so the CAS and the enqueue cannot
-    enqueue cannot share one transaction without reaching around the repo. Given two
+    share a transaction without reaching around the repo. Given two statements, the order is
-    statements, the order is chosen for its failure mode: CAS first, enqueue second. A
+    chosen for its failure mode: a crash between CAS and enqueue leaves an instance in
-    crash in between leaves an instance in `deleting` with no task, which the reconciler's
+    `deleting` with no task, which the reconciler's sweep re-enqueues. The reverse would
-    sweep re-enqueues. The other order leaves a deprovision task pointing at a `ready`
+    leave a deprovision task on a `ready` instance and tear down a live service.
-    instance, and a worker would tear down a live service nobody asked to delete.
     """
     inst = await instances.get(instance_id, team)
     if inst is None:

services/cli/main.py

@@ -1,9 +1,9 @@
 """svcforge — the control plane client.
 
-This talks to the API over HTTP and never touches the database. That restraint is the
+Talks to the API over HTTP and never touches the database. If the CLI could write to
-whole design: if the CLI could write to Postgres, every invariant the API enforces
+Postgres, every invariant the API enforces — the state machine, the one-transaction create,
-(the state machine, the one-transaction create, AuthZ in the WHERE clause) would have a
+AuthZ in the WHERE clause — would have a back door, and the first 3am incident would go
-back door, and the first 3am incident would go through it.
+through it. `ClientSettings` below is what keeps that true in practice.
 """
 
 from __future__ import annotations
@@ -16,9 +16,9 @@ from typing import Annotated, Any
 
 import httpx
 import typer
+from pydantic_settings import BaseSettings, SettingsConfigDict
 
 from svcforge_core.domain.states import InstanceState
-from svcforge_core.settings import load_settings
 
 app = typer.Typer(help="svcforge control plane client", no_args_is_help=True)
 
@@ -31,6 +31,9 @@ class ServiceType(StrEnum):
     ELASTICSEARCH = "elasticsearch"
     REDIS = "redis"
     POSTGRES = "postgres"
+    # Small enough to provision on a cluster with no spare memory; see catalog.yaml.
+    PODINFO = "podinfo"
+    NGINX = "nginx"
 
 
 class Size(StrEnum):
@@ -38,8 +41,24 @@ class Size(StrEnum):
     MEDIUM = "medium"
 
 
+class ClientSettings(BaseSettings):
+    """The two values the CLI needs, and nothing else.
+
+    Its own model rather than `svcforge_core.settings.Settings`, which requires
+    `SVCFORGE_PG_DSN`: loading that here would refuse to run the CLI without a database URL
+    it then never opens, on a laptop that has no reason to hold one.
+    """
+
+    model_config = SettingsConfigDict(
+        env_prefix="SVCFORGE_", env_file=".env", env_file_encoding="utf-8", extra="ignore", frozen=True
+    )
+
+    api_url: str = "http://localhost:8000"
+    api_token: str | None = None
+
+
 def _client() -> httpx.Client:
-    settings = load_settings()
+    settings = ClientSettings()
     headers = {"authorization": f"Bearer {settings.api_token}"} if settings.api_token else {}
     return httpx.Client(base_url=settings.api_url, headers=headers, timeout=10.0)
 

services/reconciler/Dockerfile

@@ -1,4 +1,4 @@
-# syntax=docker/dockerfile:1.10
+# syntax=mirror.gcr.io/docker/dockerfile:1.10@sha256:865e5dd094beca432e8c0a1d5e1c465db5f998dca4e439981029b3b81fb39ed5
 #
 # svcforge reconciler. Build from the REPO ROOT:
 #     docker buildx build -f services/reconciler/Dockerfile -t svcforge/reconciler:dev .
@@ -7,7 +7,7 @@
 # writes: the four checks enqueue tasks, they do not provision. Orphans are logged, never
 # deleted.
 
-FROM python:3.14-slim@sha256:cea0e6040540fb2b965b6e7fb5ffa00871e632eef63719f0ea54bca189ce14a6 AS builder
+FROM mirror.gcr.io/library/python:3.14-slim@sha256:cea0e6040540fb2b965b6e7fb5ffa00871e632eef63719f0ea54bca189ce14a6 AS builder
 
 COPY --from=ghcr.io/astral-sh/uv:0.11.29@sha256:eb2843a1e56fd9e30c7276ce1a52cba86e64c7b385f5e3279a0e08e02dd058fc /uv /usr/local/bin/uv
 
@@ -30,7 +30,7 @@ p = svcforge_core.__file__; \
 sys.exit(0) if "site-packages" in p else sys.exit("not a wheel install: " + p)'
 
 # --- runtime --------------------------------------------------------------------------
-FROM python:3.14-slim@sha256:cea0e6040540fb2b965b6e7fb5ffa00871e632eef63719f0ea54bca189ce14a6
+FROM mirror.gcr.io/library/python:3.14-slim@sha256:cea0e6040540fb2b965b6e7fb5ffa00871e632eef63719f0ea54bca189ce14a6
 
 ARG BUILD_SHA=unknown
 LABEL org.opencontainers.image.title="svcforge-reconciler" \
@@ -44,7 +44,7 @@ COPY --from=builder --chown=10001:10001 /app /app
 # helm 3.21.3, not 3.16.2 — see services/worker/Dockerfile. 3.16.2 is a Go 1.22.9 build
 # carrying CRITICAL CVE-2025-68121 (crypto/tls) and CVE-2026-33186 (grpc) and HIGH
 # CVE-2026-35469 (spdystream). Kept on 3.x on purpose: helm 4 is a breaking change.
-COPY --from=alpine/helm:3.21.3@sha256:35da09ba0716fc7c3cd63b6b31ee380a9c7662e95f29ab0e4ae962420afd315b /usr/bin/helm /usr/local/bin/helm
+COPY --from=mirror.gcr.io/alpine/helm:3.21.3@sha256:35da09ba0716fc7c3cd63b6b31ee380a9c7662e95f29ab0e4ae962420afd315b /usr/bin/helm /usr/local/bin/helm
 
 ENV PATH="/app/.venv/bin:$PATH" \
     PYTHONUNBUFFERED=1 \

services/reconciler/main.py

@@ -1,36 +1,33 @@
 """The control loop.
 
-Every other service in svcforge is edge-triggered: a tenant POSTs, a row appears, a worker
+Every other service here is edge-triggered: a tenant POSTs, a row appears, a worker claims
-claims it. Edge-triggered systems are correct exactly as long as nothing is ever missed —
+it. That is correct only as long as nothing is missed, and things are missed — a worker
-and things are missed. A worker is SIGKILLed holding a lease. An operator runs
+SIGKILLed holding a lease, an operator running `helm uninstall` by hand, a pod dying
-`helm uninstall` by hand. A pod dies between the CAS and the enqueue. Nobody sends an event
+between the CAS and the enqueue. Nothing sends an event for any of it, because the thing
-for any of that, because the thing that would have sent it is the thing that died.
+that would have sent it is the thing that died.
 
 So: level-triggered. Every 60 seconds, compare the world to the database and enqueue what
-is missing. The four checks below do not know or care what went wrong, or whether anything
+is missing. The four checks below do not know what went wrong, or whether anything did;
-did; they are the same code on the happy path and after an outage. That property is the
+they are the same code on the happy path and after an outage. That is why each is written
-entire reason this service exists, and it is why each check is written as a *query for
+as a query for work rather than a reaction to an event.
-work*, never as a reaction to an event.
 
 Three rules hold the design together:
 
-* **Singleton.** `replicas: 1`, `strategy: Recreate` in the chart. Two reconcilers
+* **Singleton.** `replicas: 1`, `strategy: Recreate`. Two reconcilers double-enqueue drift
-  double-enqueue drift and race on TTL. There is no leader election here on purpose: the
+  and race on TTL. No leader election on purpose — the right lease for that lives in
-  correct lease for that lives in Postgres next to the data, not in a Redis lock, and
+  Postgres next to the data, and until there is a second replica to elect between, an
-  until there is a second replica to elect between, an election is a subsystem that can
+  election is a subsystem that can only fail. The `SvcforgeReconcilerStale` alert notices
-  only fail. One pod, and the `SvcforgeReconcilerStale` alert is what notices it is gone.
+  when the one pod is gone.
-* **Each check is independent.** One failing check must not skip the other three. A helm
+* **Each check is independent.** A helm binary that cannot reach the API server must not
-  binary that cannot reach the API server must not stop TTLs from expiring.
+  stop TTLs from expiring.
-* **Enqueue, never act.** The reconciler diagnoses; workers treat. It writes task rows and
+* **Enqueue, never act.** The reconciler diagnoses and workers treat: it writes task rows
-  instance states, and never calls `helm install`. The one exception is reading — the drift
+  and instance states and never calls `helm install`. Reading is the exception, since
-  check runs `helm list`, because seeing reality is the job.
+  seeing reality is the job.
 """
 
 from __future__ import annotations
 
 import asyncio
-import contextlib
-import signal
 from collections.abc import Awaitable, Callable
 from dataclasses import dataclass
 
@@ -55,21 +52,19 @@ from svcforge_core.repo.db import DictPool, make_pool
 from svcforge_core.repo.instances import InstanceRepo
 from svcforge_core.repo.reconcile import ReconcileRepo
 from svcforge_core.repo.tasks import TaskRepo
+from svcforge_core.runtime import install_stop_signals, sleep_or_stop
 from svcforge_core.settings import Settings, load_settings
 
 log = get_logger("svcforge.reconciler")
 
-# The chart's PodMonitor scrapes the port named `metrics` on 9000. Keep them in step.
-DEFAULT_METRICS_PORT = 9000
-
 
 @dataclass(frozen=True)
 class ReconcilerDeps:
     """Everything a check is allowed to touch. Built once in main(), passed down.
 
-    Same shape as `WorkerDeps` for the same reason: the checks take `deps` instead of
+    Same shape as `WorkerDeps` and for the same reason: checks take `deps` instead of
-    reaching for globals, so the integration tests below run every check against a real
+    reaching for globals, so the integration tests run every check against a real Postgres
-    Postgres and a `FakeProvisioner` without a cluster anywhere in sight.
+    and a `FakeProvisioner` with no cluster in sight.
     """
 
     pool: DictPool
@@ -92,23 +87,19 @@ class ReconcilerDeps:
 
 
 async def check_drift(deps: ReconcilerDeps) -> None:
-    """`helm list -A -o json` versus what the database believes.
+    """The live helm releases versus what the database believes.
 
-    This is the only check that looks outside Postgres, and the only one that can catch the
+    The only check that looks outside Postgres, and the only one that catches someone
-    failure nothing else can: someone ran `helm uninstall` by hand, or a node was drained
+    running `helm uninstall` by hand or a drained node whose release never came back — where
-    and the release never came back. The DB still says `ready` and still hands the tenant an
+    the DB still says `ready` and still hands the tenant an endpoint resolving to nothing.
-    endpoint that resolves to nothing.
 
-    Two directions, two very different answers:
+    Two directions, two different answers:
 
-    * **Release gone, DB says `ready`** -> re-enqueue provision. Safe, because provisioning
+    * **Release gone, DB says `ready`** -> re-enqueue provision. Safe because provisioning
-      is `helm upgrade --install` against a deterministic release name: converging on
+      is `helm upgrade --install` against a deterministic release name.
-      desired state, not a blind re-install.
+    * **Release exists, DB knows nothing** -> log at error and stop. **Never delete in v1.**
-    * **Release exists, DB knows nothing** -> log at error with release and namespace, and
+      "The DB knows nothing" is one query against one database, and the release might belong
-      stop. **Never delete in v1.** The reconciler's view of "the DB knows nothing" is one
+      to another team, another tool, or a half-finished migration. A human decides.
-      query against one database; the release might belong to another team, another tool,
-      or a migration half-finished. Deleting on that evidence is how an automated system
-      takes down production faster than any human could. A human reads the log and decides.
     """
     with tracer().start_as_current_span("helm.list"):
         releases = await deps.provisioner.list_releases()
@@ -136,31 +127,30 @@ async def check_drift(deps: ReconcilerDeps) -> None:
                 {"instance_id": str(inst.id), "team": inst.team},
             )
         except Exception:
-            # The task is already committed; the notification is a courtesy. A webhook
+            # The task is committed; the notification is a courtesy. A webhook timing out
-            # timing out must not abandon the rest of the sweep — the instances after this
+            # must not abandon the rest of the sweep — the instances after this one have the
-            # one in the loop have the same problem and nobody else is coming to find them.
+            # same problem and nobody else is coming to find them.
             log.exception("notify.failed", instance_id=str(inst.id))
 
     known = await deps.reconcile.known_releases()
     for name, namespace in sorted(live - known):
-        # error, not warning: this is a resource nobody is billing for and nobody owns.
+        # error, not warning: a resource nobody owns and nobody is billing for. It repeats
-        # It will sit here every 60s until a human deletes it or adopts it. That is the
+        # every 60s until a human deletes or adopts it, which is the intended pressure.
-        # intended pressure.
         log.error("drift.orphan_release", release=name, namespace=namespace, action="none (v1 never deletes)")
 
 
 async def check_lease_expiry(deps: ReconcilerDeps) -> None:
     """Tasks whose worker died -> back to `queued`.
 
-    A lease. No lock survives a power cut: a worker SIGKILLed mid-provision
+    A lease, not a lock: no lock survives a power cut. A worker SIGKILLed mid-provision
-    leaves `state='running'` with `locked_by` set and nobody running it, and no amount of
+    leaves `state='running'` with `locked_by` set and nobody running it, and cleanup code in
-    cleanup code in the worker helps, because the worker is the part that died. `locked_at`
+    the worker cannot help because the worker is what died. `locked_at` plus a timeout is
-    plus a timeout is the only thing that recovers the row, which is why `locked_at` exists.
+    the only thing that recovers the row.
 
-    The 5-minute default must exceed the longest a healthy task can hold a lease, or the
+    The 5-minute default must exceed the longest a healthy task can hold a lease, or a
-    reconciler hands a still-running provision to a second worker. Handlers are idempotent,
+    still-running provision is handed to a second worker. Handlers are idempotent so that is
-    so that is survivable, though it still costs a duplicated helm run — which is why
+    survivable, but it costs a duplicated helm run — hence `lease_seconds` > helm's
-    `lease_seconds` sits above helm's `--timeout`.
+    `--timeout`.
     """
     freed = await deps.tasks.reset_expired_leases(deps.settings.lease_seconds)
     if freed:
@@ -170,14 +160,13 @@ async def check_lease_expiry(deps: ReconcilerDeps) -> None:
 async def check_ttl(deps: ReconcilerDeps) -> None:
     """Expired instances -> `deleting`, plus a deprovision task.
 
-    The line item that stops a demo cluster from becoming a permanent cloud bill. Also the
+    What stops a demo cluster becoming a permanent cloud bill, and the sweep the API's
-    sweep the API's DELETE route depends on: it CASes to `deleting` and enqueues in two
+    DELETE route depends on: DELETE CASes and enqueues in two statements, and a crash
-    statements, and a crash in between lands here on the next tick.
+    between them lands here on the next tick.
 
-    Idempotent by construction — the work list excludes anything that already has a queued
+    Idempotent by construction — the work list excludes anything with a queued or running
-    or running deprovision, and the CAS and the insert share one transaction. Without that
+    deprovision, and the CAS and insert share one transaction. Without that, a deprovision
-    guard, a deprovision that takes longer than 60 seconds gets a second task on the next
+    taking longer than 60 seconds collects a new task every tick.
-    tick, and a third on the tick after.
     """
     for inst in await deps.reconcile.due_for_deprovision():
         task_id = await deps.reconcile.enqueue_deprovision(inst.id)
@@ -196,20 +185,18 @@ async def check_ttl(deps: ReconcilerDeps) -> None:
 async def check_version_drift(deps: ReconcilerDeps) -> None:
     """The day-2 rollout: the work-list query, one service type at a time.
 
-    Everything that makes this safe is somewhere else, which is the point:
+    Everything that makes it safe is somewhere else, which is the point:
 
     * `list_upgradable` limits to `max_in_flight` and returns nothing while
       `rollout_state='halted'`, so a bad chart stops after one tenant.
-    * `schedule_upgrade_at` turns the tenant's maintenance window into a `run_after`; the
+    * `schedule_upgrade_at` turns the maintenance window into a `run_after` and the queue
-      queue does the waiting, in `where run_after <= now()`. There is no scheduler here and
+      does the waiting, in `where run_after <= now()`. No scheduler here, and there must not
-      there must not be one — a task parked in Postgres until 03:00 Sunday survives a
+      be one: a task parked in Postgres until 03:00 Sunday survives a restart, a timer does
-      reconciler restart, and an in-memory timer does not.
+      not.
-    * `security: true` in the catalog bypasses the window. A CVE with a public exploit does
+    * `security: true` in the catalog bypasses the window.
-      not wait until Sunday.
 
-    A bad window spec is this instance's problem, not the fleet's: log it and move to the
+    A bad window spec is one instance's problem: log it and move on. Failing the check would
-    next one. Failing the whole check would let one tenant's typo freeze everyone's
+    let one tenant's typo freeze everyone's security rollout.
-    security rollout.
     """
     now = deps.clock.now()
 
@@ -264,23 +251,20 @@ CHECKS: dict[str, Callable[[ReconcilerDeps], Awaitable[None]]] = {
 async def tick(deps: ReconcilerDeps) -> None:
     """One pass: all four checks, then the gauges, then the heartbeat.
 
-    Checks first, gauges second: `svcforge_queue_depth` is read straight after the checks
+    Checks first, gauges second, so `svcforge_queue_depth` reports what this tick left
-    that add to the queue, so the value scraped is the value the tick left behind rather
+    behind rather than what preceded its own work.
-    than one from before its own work.
 
-    The heartbeat is set unconditionally, and that is deliberate. It answers "is the loop
+    The heartbeat is set unconditionally. It answers "is the loop running", not "is
-    running", not "is everything fine" — the checks have their own alerts. Gating it on
+    everything fine" — the checks have their own alerts. Gating it on success would make
-    success would make `SvcforgeReconcilerStale` fire for a helm blip and mean two things
+    `SvcforgeReconcilerStale` fire for a helm blip and mean two things at once, and an alert
-    at once, and an alert that means two things gets muted.
+    that means two things gets muted.
 
-    The whole tick runs inside one span, which is a considered exception to "manual spans go
+    The whole tick runs in one span, a considered exception to "manual spans wrap helm calls
-    around helm calls only". That rule exists so the API does not hand-roll spans that
+    only". That rule keeps the API from hand-rolling spans `opentelemetry-instrument`
-    `opentelemetry-instrument` already creates for it. Nothing auto-instruments the
+    already makes; nothing auto-instruments the reconciler, so without this it emits no
-    reconciler: without a span here it emits no traces at all, and — because
+    traces at all and — since `inject_traceparent` serialises the *active* context — every
-    `inject_traceparent` serialises the *active* context — every task it enqueues would be
+    task it enqueues would carry a null `traceparent` and be unjoinable to the tick that
-    written with a null `traceparent` and be unjoinable to the tick that decided to create
+    created it.
-    it. One span per tick is what makes "why was this instance re-provisioned at 03:00?" a
-    question the traces can answer.
     """
     with tracer().start_as_current_span("reconciler.tick"):
         await _run_checks(deps)
@@ -295,12 +279,11 @@ async def _run_checks(deps: ReconcilerDeps) -> None:
         try:
             await check(deps)
         except Exception:  # the tick is the error boundary
-            # The swallow is the design. These four checks share nothing but a database
+            # The swallow is the design. The four checks share nothing but a database
-            # handle, and the value of a level-triggered loop is that it keeps running: an
+            # handle, and a level-triggered loop is only worth having if it keeps running:
-            # unreachable cluster must not stop TTLs from expiring, and one tenant's broken
+            # an unreachable cluster must not stop TTLs expiring, and one tenant's broken
             # window spec must not stop drift detection. This means "this check achieved
-            # nothing for 60 seconds", which the log says out loud. It never means "the
+            # nothing for 60 seconds", never "the reconciler stops".
-            # reconciler stops".
             log.exception("check.failed", check=name)
 
     try:
@@ -312,23 +295,17 @@ async def _run_checks(deps: ReconcilerDeps) -> None:
         log.exception("gauges.failed")
 
 
-async def _sleep_or_stop(stop: asyncio.Event, seconds: float) -> None:
-    """Sleep, but wake immediately on SIGTERM. A 60s nap must not cost 60s of shutdown."""
-    with contextlib.suppress(TimeoutError):
-        await asyncio.wait_for(stop.wait(), timeout=seconds)
-
-
 async def run_reconciler(deps: ReconcilerDeps, stop: asyncio.Event) -> None:
     """Tick, sleep, repeat, until told to stop.
 
-    Tick first, then sleep: a pod that has just been restarted should reconcile now, not in
+    Tick first, then sleep: a just-restarted pod should reconcile now, not in sixty seconds.
-    sixty seconds. Fixed interval rather than a fixed period — a tick that overruns simply
+    Fixed interval rather than fixed period, so a tick that overruns delays the next one
-    delays the next one, instead of stacking a second tick on top of the first, which for a
+    instead of stacking a second on top — which for a singleton is exactly the concurrent
-    singleton would be exactly the concurrent reconciler `replicas: 1` exists to prevent.
+    reconciler `replicas: 1` exists to prevent.
     """
     while not stop.is_set():
         await tick(deps)
-        await _sleep_or_stop(stop, deps.settings.reconcile_interval_s)
+        await sleep_or_stop(stop, deps.settings.reconcile_interval_s)
 
 
 def build_deps(
@@ -353,32 +330,27 @@ def build_deps(
     )
 
 
-async def _amain(once: bool, metrics_port: int, own_team: str, max_in_flight: int) -> None:
+async def _amain(once: bool, own_team: str, max_in_flight: int) -> None:
     settings = load_settings()
     setup("svcforge-reconciler", settings)
 
-    pool = make_pool(settings.pg_dsn.unicode_string(), settings.pool_min_size, settings.pool_max_size)
+    pool = make_pool(settings.runtime_dsn, settings.pool_min_size, settings.pool_max_size)
     await pool.open(wait=True)
     deps = build_deps(pool, settings, own_team, max_in_flight)
 
     try:
         if once:
-            # One pass and exit: the acceptance path, and how you drive a reconcile by hand
+            # One pass and exit: the acceptance path, and how to drive a reconcile by hand.
-            # from a shell. No metrics server — nothing would ever scrape it.
+            # No metrics server — nothing would ever scrape it.
             await tick(deps)
             return
 
-        start_metrics_server(metrics_port)
+        # settings.metrics_port, like the worker. SVCFORGE_METRICS_PORT overrides it through
+        # pydantic rather than a second CLI option, so the port has one definition.
+        start_metrics_server(settings.metrics_port)
 
         stop = asyncio.Event()
-        loop = asyncio.get_running_loop()
+        install_stop_signals(stop)
-        for sig in (signal.SIGTERM, signal.SIGINT):
-            # add_signal_handler, NOT signal.signal. signal.signal fires the handler at an
-            # arbitrary bytecode boundary on the main thread and the loop does not notice
-            # until its next timer — which here is up to a full 60s tick away. This one is
-            # scheduled as an ordinary loop callback, so the `stop.wait()` above returns
-            # immediately.
-            loop.add_signal_handler(sig, stop.set)
 
         await run_reconciler(deps, stop)
     finally:
@@ -391,9 +363,6 @@ app = typer.Typer(add_completion=False, help="svcforge reconciler: the control l
 @app.command()
 def main(
     once: bool = typer.Option(False, "--once", help="Run one tick and exit."),
-    metrics_port: int = typer.Option(
-        DEFAULT_METRICS_PORT, envvar="SVCFORGE_METRICS_PORT", help="Port for /metrics."
-    ),
     own_team: str = typer.Option(
         "platform", envvar="SVCFORGE_OWN_TEAM", help="Team whose instances upgrade first."
     ),
@@ -403,7 +372,7 @@ def main(
 ) -> None:
     """Run the reconciler."""
     # One asyncio.run, at the top, never nested. Everything below it is already async.
-    asyncio.run(_amain(once, metrics_port, own_team, max_in_flight))
+    asyncio.run(_amain(once, own_team, max_in_flight))
 
 
 if __name__ == "__main__":

services/worker/Dockerfile

@@ -1,4 +1,4 @@
-# syntax=docker/dockerfile:1.10
+# syntax=mirror.gcr.io/docker/dockerfile:1.10@sha256:865e5dd094beca432e8c0a1d5e1c465db5f998dca4e439981029b3b81fb39ed5
 #
 # svcforge worker. Build from the REPO ROOT:
 #     docker buildx build -f services/worker/Dockerfile -t svcforge/worker:dev .
@@ -7,7 +7,7 @@
 # binaries. They are copied from pinned images rather than curl'd, so the version is a
 # reviewable line in a Dockerfile instead of a network call at build time.
 
-FROM python:3.14-slim@sha256:cea0e6040540fb2b965b6e7fb5ffa00871e632eef63719f0ea54bca189ce14a6 AS builder
+FROM mirror.gcr.io/library/python:3.14-slim@sha256:cea0e6040540fb2b965b6e7fb5ffa00871e632eef63719f0ea54bca189ce14a6 AS builder
 
 COPY --from=ghcr.io/astral-sh/uv:0.11.29@sha256:eb2843a1e56fd9e30c7276ce1a52cba86e64c7b385f5e3279a0e08e02dd058fc /uv /usr/local/bin/uv
 
@@ -30,7 +30,7 @@ p = svcforge_core.__file__; \
 sys.exit(0) if "site-packages" in p else sys.exit("not a wheel install: " + p)'
 
 # --- runtime --------------------------------------------------------------------------
-FROM python:3.14-slim@sha256:cea0e6040540fb2b965b6e7fb5ffa00871e632eef63719f0ea54bca189ce14a6
+FROM mirror.gcr.io/library/python:3.14-slim@sha256:cea0e6040540fb2b965b6e7fb5ffa00871e632eef63719f0ea54bca189ce14a6
 
 ARG BUILD_SHA=unknown
 LABEL org.opencontainers.image.title="svcforge-worker" \
@@ -45,7 +45,7 @@ COPY --from=builder --chown=10001:10001 /app /app
 # CVE-2025-68121 (crypto/tls) and CVE-2026-33186 (grpc), plus HIGH CVE-2026-35469
 # (spdystream, fixed in 0.5.1) — trivy fails the build on them and is right to.
 # Deliberately 3.x: helm 4 is a breaking change and is not a CVE fix.
-COPY --from=alpine/helm:3.21.3@sha256:35da09ba0716fc7c3cd63b6b31ee380a9c7662e95f29ab0e4ae962420afd315b /usr/bin/helm /usr/local/bin/helm
+COPY --from=mirror.gcr.io/alpine/helm:3.21.3@sha256:35da09ba0716fc7c3cd63b6b31ee380a9c7662e95f29ab0e4ae962420afd315b /usr/bin/helm /usr/local/bin/helm
 
 ENV PATH="/app/.venv/bin:$PATH" \
     PYTHONUNBUFFERED=1 \

services/worker/handlers.py

@@ -1,13 +1,12 @@
 """Task handlers.
 
-Every handler here obeys one rule: running it twice must equal running it once.
+Every handler obeys one rule: running it twice must equal running it once.
 
-A worker can be SIGKILLed after helm has installed the release but
+A worker can be SIGKILLed after helm installed the release but before the DB row says so;
-before the DB row says so; the lease expires; another worker claims the same task and runs
+the lease expires, another worker claims the same task, and this function runs again. A
-this function again. If the handler is not idempotent, the tenant gets two Elasticsearches
+handler that is not idempotent gives the tenant two Elasticsearches and you a bill.
-and you get a bill. Idempotency is what makes the crash safe, and it is bought in two
+Idempotency is bought in two places: a deterministic `release_name`, and adapters that
-places: a deterministic `release_name`, and adapters that state desired state
+state desired state (`helm upgrade --install`) instead of issuing imperative commands.
-(`helm upgrade --install`) instead of issuing imperative commands.
 """
 
 from __future__ import annotations
@@ -18,18 +17,21 @@ from typing import Any
 from services.worker.deps import WorkerDeps
 from svcforge_core.domain.models import CatalogEntry, Instance, Task, TaskKind
 from svcforge_core.domain.states import InstanceState
+from svcforge_core.errors import SvcforgeError
+from svcforge_core.obs import get_logger
+from svcforge_core.repo.instances import INSTANCE_COLUMNS
+
+log = get_logger("svcforge.worker")
 
 
-class HandlerError(RuntimeError):
+class HandlerError(SvcforgeError, RuntimeError):
     """A task failed in a way worth retrying. The message lands in tasks.last_error."""
 
 
 async def _load_instance(task: Task, deps: WorkerDeps) -> Instance:
     async with deps.pool.connection() as conn, conn.cursor() as cur:
         await cur.execute(
-            """select id, team, service_type, size, state, namespace, release_name,
+            f"select {INSTANCE_COLUMNS} from instances where id = %s",  # noqa: S608 - module constant
-                      chart_version, endpoint, error, expires_at, created_at, updated_at
-                 from instances where id = %s""",
             (task.instance_id,),
         )
         row = await cur.fetchone()
@@ -38,12 +40,35 @@ async def _load_instance(task: Task, deps: WorkerDeps) -> Instance:
     return Instance.model_validate(row)
 
 
+def _deep_merge(base: dict[str, Any], override: dict[str, Any]) -> dict[str, Any]:
+    """`override` wins, except where both sides hold a dict — then merge those too.
+
+    Shallow `base | override` would be wrong the moment two layers touch different keys of
+    the same nested map: `{"global": {"imageRegistry": ...}}` overridden by
+    `{"global": {"storageClass": ...}}` silently drops the registry, and the pod pulls from
+    somewhere nobody chose.
+    """
+    out = dict(base)
+    for key, value in override.items():
+        current = out.get(key)
+        if isinstance(current, dict) and isinstance(value, dict):
+            out[key] = _deep_merge(current, value)
+        else:
+            out[key] = value
+    return out
+
+
 def _values_for(inst: Instance, entry: CatalogEntry) -> dict[str, Any]:
-    """Turn a catalog size into helm values."""
+    """Catalog values, with the requested size's replicas and resources on top.
+
+    Size last, deliberately. An entry that sets `replicaCount` in its own `values:` would
+    otherwise beat the size the tenant actually asked for, and every size would deploy the
+    same shape.
+    """
     size = entry.sizes.get(inst.size)
     if size is None:
         raise HandlerError(f"size {inst.size!r} not in catalog for {inst.service_type!r}")
-    return {"replicaCount": size.replicas, "resources": size.resources}
+    return _deep_merge(entry.values, {"replicaCount": size.replicas, "resources": size.resources})
 
 
 async def handle_provision(task: Task, deps: WorkerDeps) -> None:
@@ -75,11 +100,18 @@ async def handle_provision(task: Task, deps: WorkerDeps) -> None:
         inst.id, InstanceState.PROVISIONING, InstanceState.READY, endpoint=endpoint
     )
     if ok:
+        try:
             await deps.notifier.send(
                 "instance.ready",
                 f"instance {inst.id} is ready at {endpoint}",
                 {"instance_id": str(inst.id), "team": inst.team, "service_type": inst.service_type},
             )
+        except Exception:
+            # The provision succeeded and the row is READY; the notification is a courtesy.
+            # Propagating a webhook timeout would fail the task, and the retry would hit the
+            # READY early-return and drop the notification anyway — so a flaky notifier
+            # would turn every provision into a "failed" task.
+            log.exception("notify.failed", instance_id=str(inst.id))
 
 
 async def handle_deprovision(task: Task, deps: WorkerDeps) -> None:
@@ -93,10 +125,9 @@ async def handle_deprovision(task: Task, deps: WorkerDeps) -> None:
     # swallows not-found, because the desired state — no release — is already true.
     await deps.provisioner.uninstall(release=inst.release_name, ns=inst.namespace)
 
-    # Raise rather than ignore the CAS result. Swallowing it means: the release is gone,
+    # Raise rather than ignore the CAS result. Swallowing it leaves the release gone, the
-    # the row keeps `state=ready` and its now-dangling endpoint, the task is marked done,
+    # row on `state=ready` with a dangling endpoint, the task marked done — and 60 seconds
-    # and 60 seconds later the reconciler's drift check re-provisions the thing the tenant
+    # later the drift check re-provisions the thing the tenant asked to delete.
-    # asked to delete. Failing loudly turns a silent ping-pong into one visible error.
     if not await deps.instances.update_state(inst.id, InstanceState.DELETING, InstanceState.DELETED):
         raise HandlerError(
             f"instance {inst.id} was {inst.state.value}, expected {InstanceState.DELETING.value}"
@@ -135,10 +166,9 @@ async def handle_upgrade(task: Task, deps: WorkerDeps) -> None:
 async def handle_verify(task: Task, deps: WorkerDeps) -> None:
     """Post-upgrade health probe. On failure, halt the whole rollout for this service type.
 
-    One column decides whether the fleet keeps rolling. The work-list query returns nothing
+    The work-list query returns nothing while `rollout_state='halted'`, so a bad chart stops
-    while `rollout_state='halted'`, so a bad chart stops after the first tenant instead of
+    after the first tenant instead of all of them. Clearing it is a deliberate SQL statement:
-    after all of them. You clear it with SQL, deliberately: an automatic un-halt would just
+    an automatic un-halt would resume breaking things.
-    resume breaking things.
     """
     inst = await _load_instance(task, deps)
     releases = {r.name for r in await deps.provisioner.list_releases()}
@@ -146,10 +176,9 @@ async def handle_verify(task: Task, deps: WorkerDeps) -> None:
     if inst.release_name in releases:
         return
 
-    # `returning` + a `where` on the update half tells us whether THIS call was the one
+    # `returning` plus a `where` on the update half says whether THIS call halted the
-    # that halted the rollout. The halt itself is idempotent; the page is not. Without the
+    # rollout. The halt is idempotent; the page is not. Without the distinction, a verify
-    # distinction, a verify that fails its full retry budget sends five identical
+    # that burns its full retry budget sends five identical notifications for one incident.
-    # notifications for one incident, spread across the backoff curve.
     async with deps.pool.connection() as conn, conn.cursor() as cur:
         await cur.execute(
             """insert into catalog_versions (service_type, rollout_state)

services/worker/main.py

@@ -1,17 +1,14 @@
 """The claim loop.
 
-Poll every 5 seconds. Claim while a semaphore slot is free. Run the handler. Report.
+Poll every 5 seconds. Claim while a semaphore slot is free. Run the handler. Report. The
-That is the whole design, and the restraint is the point: LISTEN/NOTIFY would shave the
+poll is not a placeholder for something better: LISTEN/NOTIFY would shave latency, but it
-latency, is fire-and-forget so it can never replace the poll anyway, is strictly extra
+is fire-and-forget so it can never replace the poll, and it does not exist on pgbouncer's
-code, and does not exist on pgbouncer's transaction pooler. The poll is not a placeholder
+transaction pooler.
-for something better.
 """
 
 from __future__ import annotations
 
 import asyncio
-import contextlib
-import signal
 import time
 from collections.abc import Awaitable
 
@@ -28,35 +25,25 @@ from svcforge_core.domain.models import Task, TaskKind
 from svcforge_core.repo.db import make_pool
 from svcforge_core.repo.instances import InstanceRepo
 from svcforge_core.repo.tasks import TaskRepo
+from svcforge_core.runtime import install_stop_signals, sleep_or_stop
 from svcforge_core.settings import Settings, load_settings
 
 log = obs.get_logger("svcforge.worker")
 
 
-async def _sleep_or_stop(stop: asyncio.Event, seconds: float) -> None:
-    """Sleep, but wake immediately on shutdown.
-
-    `await asyncio.sleep(5)` would make every SIGTERM cost up to five seconds of
-    Kubernetes waiting on terminationGracePeriod for no reason.
-    """
-    with contextlib.suppress(TimeoutError):
-        await asyncio.wait_for(stop.wait(), timeout=seconds)
-
-
 async def _report(coro: Awaitable[bool], task_id: int, what: str) -> None:
     """Run a terminal report, and never let its failure escape.
 
-    Reporting is the one thing that must not kill the worker. `_run_one` runs inside a
+    `_run_one` runs inside a TaskGroup, which cancels every sibling the moment one child
-    TaskGroup, and a TaskGroup cancels every sibling the moment one child raises — so a
+    raises — so a DB blip during `tasks.fail()` would abort every other in-flight provision
-    DB blip during `tasks.fail()` would abort every other in-flight provision on this pod,
+    on this pod. The task itself is safe either way: it stays `running` and the lease sweep
-    not just this one. The task itself is safe either way: it stays `running` and the
+    returns it to the queue. Losing the report costs one lease interval; losing the siblings
-    reconciler's lease sweep returns it to the queue. Losing the report costs one lease
+    costs their work.
-    interval; losing the siblings costs their work.
     """
     try:
         if not await coro:
-            # The lease was stolen while we were working: another worker owns this task
+            # The lease was stolen while we were working: another worker owns this task now
-            # now and is mid-run. Reporting is theirs to do, not ours.
+            # and is mid-run. Reporting is theirs, not ours.
             log.warning("lease lost before report; another worker owns this task", task_id=task_id)
     except Exception:
         log.exception("could not report task %s (%s); lease will expire", task_id, what)
@@ -66,10 +53,8 @@ async def _run_one(deps: WorkerDeps, task: Task, sem: asyncio.Semaphore) -> None
     """Run one task to a terminal report. Never lets an exception escape the TaskGroup."""
     worker_id = deps.settings.worker_id
     try:
-        # Every log line from here carries instance_id/task_id/team. Bound once, at claim,
+        # Every log line from here carries instance_id/task_id/team. Bound once at claim
-        # rather than passed down: the alternative is threading three arguments through
+        # rather than threaded through every function that might log.
-        # every function that might log, and the first one anyone forgets is the one you
-        # need at 3am.
         obs.bind_task_context(task.instance_id, task.id, team=task.team or "unknown")
         log.info("task claimed", kind=task.kind.value, attempt=task.attempts)
         obs.TASKS_CLAIMED.labels(kind=task.kind.value).inc()
@@ -83,9 +68,9 @@ async def _run_one(deps: WorkerDeps, task: Task, sem: asyncio.Semaphore) -> None
             )
             return
 
-        # Re-parent to the span that enqueued this task. Without the stored traceparent
+        # Re-parent to the span that enqueued this task. Without the stored traceparent the
-        # the worker's span starts a brand-new trace, and the POST that caused the work
+        # worker's span starts a new trace, putting the POST that caused the work in a
-        # is in a different trace to the helm call that did it.
+        # different trace from the helm call that did it.
         ctx = obs.context_from_traceparent(task.traceparent)
         started = time.monotonic()
         with obs.tracer().start_as_current_span(
@@ -119,10 +104,9 @@ async def _run_one(deps: WorkerDeps, task: Task, sem: asyncio.Semaphore) -> None
                     "fail",
                 )
             else:
-                # Only provisions go in the provision histogram. The buckets run 10s..1800s
+                # Only provisions go in the provision histogram. Its buckets run 10s..1800s
-                # because they were sized for helm installs; a sub-second `verify` dropped
+                # for helm installs, so a sub-second `verify` in the same series drags the
-                # into the same series drags the p95 down and quietly stops
+                # p95 down and quietly stops SvcforgeProvisionSlow from ever firing.
-                # SvcforgeProvisionSlow from ever firing.
                 if task.kind is TaskKind.PROVISION:
                     obs.PROVISION_TIME.observe(time.monotonic() - started)
                 await _report(deps.tasks.complete(task.id, worker_id), task.id, "complete")
@@ -133,10 +117,9 @@ async def _run_one(deps: WorkerDeps, task: Task, sem: asyncio.Semaphore) -> None
 async def run_worker(deps: WorkerDeps, stop: asyncio.Event) -> None:
     """Claim and run until told to stop, then drain what is in flight.
 
-    Draining is what makes a rolling deploy invisible. Exiting the `async with` block
+    Draining is what makes a rolling deploy invisible: exiting the `async with` awaits every
-    awaits every in-flight handler, so a pod that is being replaced finishes the provision
+    in-flight handler, so a pod being replaced finishes the provision it started instead of
-    it already started instead of abandoning it half-done for the lease to clean up
+    abandoning it for the lease to clean up five minutes later.
-    five minutes later.
     """
     sem = asyncio.Semaphore(deps.settings.worker_concurrency)
     worker_id = deps.settings.worker_id
@@ -154,12 +137,12 @@ async def run_worker(deps: WorkerDeps, stop: asyncio.Event) -> None:
                 # A DB blip must not kill the worker; back off and try again.
                 log.exception("claim failed")
                 sem.release()
-                await _sleep_or_stop(stop, deps.settings.poll_interval_s)
+                await sleep_or_stop(stop, deps.settings.poll_interval_s)
                 continue
 
             if task is None:
                 sem.release()
-                await _sleep_or_stop(stop, deps.settings.poll_interval_s)
+                await sleep_or_stop(stop, deps.settings.poll_interval_s)
                 continue
 
             tg.create_task(_run_one(deps, task, sem))
@@ -169,14 +152,13 @@ async def run_worker(deps: WorkerDeps, stop: asyncio.Event) -> None:
 async def _amain() -> None:
     settings: Settings = load_settings()
 
-    # Before anything else: nothing logged above this line is structured, and the metrics
+    # Before anything else: nothing above this line logs structured, and the metrics the
-    # the SvcforgeTaskFailed / SvcforgeProvisionSlow alerts query do not exist until the
+    # SvcforgeTaskFailed / SvcforgeProvisionSlow alerts query do not exist until it runs.
-    # registry is up.
     obs.setup("svcforge-worker", settings)
     settings.check_production()
     obs.start_metrics_server(settings.metrics_port)
 
-    pool = make_pool(settings.pg_dsn.unicode_string(), settings.pool_min_size, settings.pool_max_size)
+    pool = make_pool(settings.runtime_dsn, settings.pool_min_size, settings.pool_max_size)
     await pool.open(wait=True)
 
     deps = WorkerDeps(
@@ -191,13 +173,7 @@ async def _amain() -> None:
     )
 
     stop = asyncio.Event()
-    loop = asyncio.get_running_loop()
+    install_stop_signals(stop)
-    for sig in (signal.SIGTERM, signal.SIGINT):
-        # add_signal_handler, NOT signal.signal. signal.signal runs the handler at an
-        # arbitrary bytecode boundary on whatever thread the C-level handler lands on,
-        # and the event loop will not notice until its next timer fires. This one is
-        # loop-safe: the callback runs as a normal loop callback.
-        loop.add_signal_handler(sig, stop.set)
 
     try:
         await run_worker(deps, stop)

tests/fakes.py

@@ -145,7 +145,12 @@ class FakeRateLimiter:
             # Fails OPEN, exactly like the real one. A limiter that refused here would make
             # "Redis is down" indistinguishable from "you are over quota".
             return RateLimitResult(
-                allowed=True, limit=self.limit, remaining=self.limit, reset_at=reset_at, degraded=True
+                allowed=True,
+                limit=self.limit,
+                remaining=self.limit,
+                reset_at=reset_at,
+                checked_at=self.clock.now(),
+                degraded=True,
             )
         key = f"rl:{team}:{window}"
         n = self.counts.get(key, 0) + 1
@@ -155,6 +160,7 @@ class FakeRateLimiter:
             limit=self.limit,
             remaining=max(0, self.limit - n),
             reset_at=reset_at,
+            checked_at=self.clock.now(),
         )
 
 

tests/integration/test_api.py

@@ -28,6 +28,7 @@ from fastapi import FastAPI
 
 from services.api.main import create_app
 from services.api.routes.instances import release_name_for
+from svcforge_core.domain.catalog import load_catalog
 from svcforge_core.repo.db import DictPool
 from svcforge_core.settings import Settings
 
@@ -182,8 +183,10 @@ async def test_post_returns_202_and_location(client: httpx.AsyncClient, token: s
     assert resp.headers["location"] == f"/v1/instances/{body['id']}"
     assert body["state"] == "requested"
     assert body["service_type"] == "elasticsearch"
-    # Pinned from catalog.yaml at creation time, not echoed from the request.
+    # Pinned from catalog.yaml at creation time, not echoed from the request. Read from the
-    assert body["chart_version"] == "21.3.15"
+    # catalog rather than hardcoded: the literal made a routine version bump fail here, on a
+    # test whose subject is *where the value comes from*, not what it is.
+    assert body["chart_version"] == load_catalog(CATALOG)["elasticsearch"].chart_version
     assert body["endpoint"] is None
     # The response model is an allow-list: placement details stay off the wire.
     assert "team" not in body and "namespace" not in body and "release_name" not in body
@@ -298,6 +301,46 @@ async def test_ttl_out_of_range_is_422(client: httpx.AsyncClient, token: str) ->
     assert resp.status_code == 422
 
 
+def _is_error_body(payload: object) -> bool:
+    """The uniform error shape: a dict with `code` and `message`, and no default `detail`."""
+    return isinstance(payload, dict) and "code" in payload and "message" in payload
+
+
+async def test_framework_404_uses_the_error_body_shape(client: httpx.AsyncClient) -> None:
+    """A 404 raised by the router, not a handler, must still be ErrorBody.
+
+    Starlette raises its own HTTPException for an unknown route. The exception handler is
+    registered on that parent class precisely so this body is ErrorBody and not FastAPI's
+    default `{"detail": "Not Found"}` — one shape for every error.
+    """
+    resp = await client.get("/v1/no-such-route")
+    assert resp.status_code == 404
+    assert _is_error_body(resp.json()), resp.text
+
+
+async def test_framework_405_uses_the_error_body_shape(client: httpx.AsyncClient) -> None:
+    """A wrong-method 405 comes from the router too, and must be ErrorBody."""
+    resp = await client.delete("/v1/instances")  # collection route has no DELETE
+    assert resp.status_code == 405
+    assert _is_error_body(resp.json()), resp.text
+
+
+async def test_body_validation_422_uses_the_error_body_shape(client: httpx.AsyncClient, token: str) -> None:
+    """A RequestValidationError 422 must match the handler-raised 422 shape.
+
+    A forbidden extra field trips pydantic's `extra="forbid"` and raises
+    RequestValidationError, which the dedicated handler renders as ErrorBody rather than the
+    default `{"detail": [...]}`.
+    """
+    resp = await client.post(
+        "/v1/instances",
+        headers=auth(token),
+        json={"service_type": "redis", "size": "small", "surprise": "field"},
+    )
+    assert resp.status_code == 422
+    assert _is_error_body(resp.json()), resp.text
+
+
 # --------------------------------------------------------------------------- authn
 
 
@@ -594,3 +637,40 @@ async def test_auth_disabled_accepts_an_unauthenticated_request(settings: Settin
         async with httpx.AsyncClient(transport=transport, base_url="http://test") as c:
             resp = await c.get("/v1/instances")
     assert resp.status_code == 200
+
+
+# --------------------------------------------------------------------------- openapi
+
+
+async def test_openapi_documents_the_api_for_a_caller_without_this_repo(
+    client: httpx.AsyncClient,
+) -> None:
+    """/openapi.json is the contract other teams integrate against, so it gets a test.
+
+    Pins the parts a generated schema does not give you for free and that silently rot: the
+    prose description, the tag docs, and the bearer scheme that makes the Authorize button
+    in /docs work. Without the security scheme a caller cannot try a single authenticated
+    route from the UI.
+    """
+    resp = await client.get("/openapi.json")
+    assert resp.status_code == 200
+    spec = resp.json()
+
+    assert spec["info"]["title"] == "svcforge"
+    # The description carries the two things the schema cannot express: writes are async,
+    # and authorisation is a WHERE clause that 404s rather than 403s.
+    description = spec["info"]["description"]
+    assert "202" in description and "404" in description
+
+    assert {t["name"] for t in spec["tags"]} == {"instances", "ops"}
+    assert "HTTPBearer" in spec["components"]["securitySchemes"]
+    assert "/v1/instances" in spec["paths"]
+    # An example payload, so a caller can see a valid body rather than infer one.
+    assert spec["components"]["schemas"]["CreateInstanceRequest"]["examples"]
+
+
+async def test_swagger_and_redoc_are_served(client: httpx.AsyncClient) -> None:
+    """The human-facing docs. Both are on by default; a `docs_url=None` would drop them."""
+    for path in ("/docs", "/redoc"):
+        resp = await client.get(path)
+        assert resp.status_code == 200, path

tests/integration/test_redis.py

@@ -434,7 +434,7 @@ async def test_the_budget_metric_is_exposed_and_labelled_by_op() -> None:
     """`curl -s localhost:8000/metrics | grep svcforge_redis_commands_total`."""
     limiter = FakeRateLimiter(limit=10, window_s=60, clock=FakeClock(start=_T0))
     await limiter.check("acme")  # the fake does not touch the real counter
-    RateLimitResult(allowed=True, limit=10, remaining=9, reset_at=_T0)
+    RateLimitResult(allowed=True, limit=10, remaining=9, reset_at=_T0, checked_at=_T0)
 
     text = generate_latest(REGISTRY).decode()
 

tests/integration/test_tasks.py

@@ -110,6 +110,86 @@ async def test_fail_does_not_resurrect_a_deleted_instance(pool: DictPool) -> Non
     assert inst.error is None
 
 
+async def test_fail_of_deprovision_leaves_the_instance_deleting_to_be_retried(
+    pool: DictPool,
+) -> None:
+    """A dead-lettered deprovision must not strand the instance in `failed`.
+
+    The instance is `deleting`, which is one of the states that CAN legally become `failed`,
+    so the naive blanket UPDATE would move it there. due_for_deprovision only re-selects
+    `ready`(expired) and `deleting`, so `failed` would take the instance out of the recovery
+    sweep and leak the helm release forever. reconcile.py documents that a deprovision which
+    exhausts its retries stays re-enqueueable; this pins that guarantee.
+    """
+    iid = await make_instance(pool, state=InstanceState.DELETING)
+    tasks, instances = TaskRepo(pool), InstanceRepo(pool)
+    tid = await tasks.enqueue_standalone(iid, TaskKind.DEPROVISION)
+
+    claimed = await tasks.claim("w1")
+    assert claimed is not None
+    async with pool.connection() as conn, conn.cursor() as cur:
+        await cur.execute("update tasks set attempts = 5 where id = %s", (tid,))
+
+    assert await tasks.fail(tid, "cluster unreachable", "w1", max_attempts=5) is True
+
+    assert (await _task_row(pool, tid))["state"] == "failed"
+    inst = await instances.get(iid, team="platform")
+    assert inst is not None
+    assert inst.state is InstanceState.DELETING, "a stranded deprovision leaks the release"
+    assert inst.error is None
+
+
+async def test_fail_of_upgrade_leaves_a_working_instance_ready(pool: DictPool) -> None:
+    """A dead-lettered upgrade must not mark a healthy instance `failed`.
+
+    helm --atomic rolls the release back, so after a failed upgrade the instance is still
+    `ready` and serving the previous version. Marking it `failed` mislabels a working
+    service and drops it off the upgrade work-list. check_version_drift retries on the next
+    window; the dead-letter metric is the operator signal.
+    """
+    iid = await make_instance(pool, state=InstanceState.READY)
+    tasks, instances = TaskRepo(pool), InstanceRepo(pool)
+    tid = await tasks.enqueue_standalone(iid, TaskKind.UPGRADE)
+
+    claimed = await tasks.claim("w1")
+    assert claimed is not None
+    async with pool.connection() as conn, conn.cursor() as cur:
+        await cur.execute("update tasks set attempts = 5 where id = %s", (tid,))
+
+    assert await tasks.fail(tid, "upgrade to 1.4.0 kept timing out", "w1", max_attempts=5) is True
+
+    assert (await _task_row(pool, tid))["state"] == "failed"
+    inst = await instances.get(iid, team="platform")
+    assert inst is not None
+    assert inst.state is InstanceState.READY, "a failed upgrade mislabelled a healthy instance"
+    assert inst.error is None
+
+
+async def test_fail_of_verify_leaves_the_instance_ready(pool: DictPool) -> None:
+    """A dead-lettered verify must not mark the instance `failed`.
+
+    handle_verify halts the rollout for the service type; the instance itself is `ready`,
+    and drift re-provisions it if its release vanished. `failed` would take it out of both
+    recovery paths.
+    """
+    iid = await make_instance(pool, state=InstanceState.READY)
+    tasks, instances = TaskRepo(pool), InstanceRepo(pool)
+    tid = await tasks.enqueue_standalone(iid, TaskKind.VERIFY)
+
+    claimed = await tasks.claim("w1")
+    assert claimed is not None
+    async with pool.connection() as conn, conn.cursor() as cur:
+        await cur.execute("update tasks set attempts = 5 where id = %s", (tid,))
+
+    assert await tasks.fail(tid, "release vanished after upgrade", "w1", max_attempts=5) is True
+
+    assert (await _task_row(pool, tid))["state"] == "failed"
+    inst = await instances.get(iid, team="platform")
+    assert inst is not None
+    assert inst.state is InstanceState.READY
+    assert inst.error is None
+
+
 async def test_fail_truncates_error_to_2kb(pool: DictPool) -> None:
     iid = await make_instance(pool)
     repo = TaskRepo(pool)

tests/unit/test_catalog.py

@@ -150,6 +150,60 @@ def test_bare_mapping_without_services_key_is_accepted(tmp_path: Path) -> None:
 def test_repo_catalog_yaml_is_valid() -> None:
     catalog = load_catalog(Path(__file__).parents[2] / "catalog.yaml")
 
-    assert set(catalog) == {"elasticsearch", "redis", "postgres"}
+    assert set(catalog) == {"elasticsearch", "redis", "postgres", "podinfo", "nginx"}
     for entry in catalog.values():
         assert set(entry.sizes) == {"small", "medium"}
+
+
+def test_entry_values_default_to_empty(tmp_path: Path) -> None:
+    """`values:` is optional — an entry that needs no chart knobs says nothing."""
+    body = textwrap.dedent("""
+        services:
+          podinfo:
+            chart: oci://ghcr.io/stefanprodan/charts/podinfo
+            chart_version: "6.14.0"
+            sizes:
+              small: {replicas: 1, resources: {}}
+    """)
+    assert load_catalog(_write(tmp_path, body))["podinfo"].values == {}
+
+
+def test_entry_values_are_parsed(tmp_path: Path) -> None:
+    """Nested values survive the load, which is what `global.imageRegistry` needs."""
+    body = textwrap.dedent("""
+        services:
+          redis:
+            chart: oci://mirror.gcr.io/bitnamicharts/redis
+            chart_version: "27.0.15"
+            values:
+              global:
+                imageRegistry: mirror.gcr.io
+            sizes:
+              small: {replicas: 1, resources: {}}
+    """)
+    assert load_catalog(_write(tmp_path, body))["redis"].values == {
+        "global": {"imageRegistry": "mirror.gcr.io"}
+    }
+
+
+def test_no_catalog_entry_pulls_from_docker_hub() -> None:
+    """Every chart, and every image registry an entry pins, avoids Docker Hub.
+
+    Docker Hub rate-limits anonymous pulls per source IP and the whole cluster shares one
+    NAT address, so a Docker Hub reference here is a provision that fails under load for a
+    reason no log in this repo will explain.
+    """
+    catalog = load_catalog(Path(__file__).parents[2] / "catalog.yaml")
+    banned = ("docker.io", "registry-1.docker.io", "index.docker.io")
+
+    for name, entry in catalog.items():
+        assert not entry.chart.startswith(banned), f"{name}: chart on Docker Hub"
+        assert "docker.io" not in entry.chart, f"{name}: chart on Docker Hub"
+
+        registry = entry.values.get("global", {}).get("imageRegistry")
+        # A bitnami chart defaults its images to Docker Hub, so any entry pointing at one
+        # has to redirect them. podinfo's chart already names ghcr.io and needs nothing.
+        if "bitnamicharts" in entry.chart:
+            assert registry == "mirror.gcr.io", f"{name}: bitnami chart without a registry override"
+        if registry is not None:
+            assert "docker.io" not in registry, f"{name}: imageRegistry on Docker Hub"

tests/unit/test_helm_argv.py

@@ -0,0 +1,358 @@
+"""The helm argv that the reconciler's correctness depends on.
+
+install() writes a label; list_releases() reads it back. Neither is checked by anything at
+runtime — if the two ever disagree, `helm list --selector` matches nothing, the reconciler
+sees an empty cluster, and every ready instance looks like it lost its release. That failure
+is silent and reads as "no drift", so it gets a test rather than a comment.
+
+These assert argv, not behaviour against a real cluster: tests/e2e covers that. The point
+here is that the two sides of the label agree, and that neither drops out under a refactor.
+"""
+
+from __future__ import annotations
+
+from pathlib import Path
+from typing import Any
+
+import httpx
+import pytest
+
+from svcforge_core.adapters import helm
+from svcforge_core.adapters.helm import (
+    MANAGED_BY_LABEL,
+    MANAGED_BY_VALUE,
+    HelmError,
+    HelmProvisioner,
+)
+from svcforge_core.domain.models import CatalogEntry, SizeSpec
+
+ENTRY = CatalogEntry(
+    service_type="redis",
+    chart="oci://example/redis",
+    chart_version="1.2.3",
+    sizes={"small": SizeSpec(replicas=1, resources={})},
+)
+
+
+def _capture(monkeypatch: pytest.MonkeyPatch) -> list[list[str]]:
+    """Record every argv HelmProvisioner would exec, and run none of them."""
+    seen: list[list[str]] = []
+
+    async def fake_run_helm(self: HelmProvisioner, argv: list[str]) -> str:
+        seen.append(list(argv))
+        return "[]"
+
+    monkeypatch.setattr(HelmProvisioner, "_run_helm", fake_run_helm, raising=True)
+    return seen
+
+
+def _pair(argv: list[str], flag: str) -> str | None:
+    """The value following `flag`, or None. Positional, because helm takes `--flag value`."""
+    return argv[argv.index(flag) + 1] if flag in argv else None
+
+
+@pytest.mark.asyncio
+async def test_install_labels_the_release_as_svcforge_managed(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    seen = _capture(monkeypatch)
+    await HelmProvisioner(kubeconfig=Path("/dev/null")).install(
+        "acme-redis", "tenant-acme", ENTRY, {"replicas": 1}
+    )
+
+    assert len(seen) == 1
+    assert _pair(seen[0], "--labels") == f"{MANAGED_BY_LABEL}={MANAGED_BY_VALUE}"
+
+
+@pytest.mark.asyncio
+async def test_list_releases_asks_only_for_svcforge_releases(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    seen = _capture(monkeypatch)
+
+    await HelmProvisioner(kubeconfig=Path("/dev/null")).list_releases()
+
+    assert len(seen) == 1
+    argv = seen[0]
+    assert _pair(argv, "--selector") == f"{MANAGED_BY_LABEL}={MANAGED_BY_VALUE}"
+    # Still every namespace. Tenants get their own, so scoping to one would hide releases
+    # rather than the cluster's; the label is what narrows this, not the namespace.
+    assert "--all-namespaces" in argv
+
+
+@pytest.mark.asyncio
+async def test_the_label_written_is_the_label_read(monkeypatch: pytest.MonkeyPatch) -> None:
+    """The regression that motivated this file.
+
+    Asserted against each other rather than against a literal on both sides, so a rename
+    that updates only one of install/list_releases fails here instead of in production as
+    an empty drift check.
+    """
+    seen = _capture(monkeypatch)
+    prov = HelmProvisioner(kubeconfig=Path("/dev/null"))
+
+    await prov.install("acme-redis", "tenant-acme", ENTRY, {})
+    await prov.list_releases()
+
+    assert _pair(seen[0], "--labels") == _pair(seen[1], "--selector")
+
+
+def _meta(name: str, ns: str, version: str, status: str = "deployed") -> dict[str, object]:
+    """One PartialObjectMetadata item as the API server returns it for a release secret."""
+    return {
+        "metadata": {
+            "name": f"sh.helm.release.v1.{name}.v{version}",
+            "namespace": ns,
+            "labels": {"name": name, "owner": "helm", "status": status, "version": version},
+        }
+    }
+
+
+class _FakeResponse:
+    def __init__(self, items: list[dict[str, object]]) -> None:
+        self._items = items
+
+    def raise_for_status(self) -> None:
+        return None
+
+    def json(self) -> dict[str, object]:
+        return {"items": self._items}
+
+
+def _fake_api(
+    monkeypatch: pytest.MonkeyPatch, tmp_path: Path, items: list[dict[str, object]]
+) -> dict[str, Any]:
+    """Stand in for the in-cluster ServiceAccount and the API call it authenticates.
+
+    The token is a real file at a repointed constant rather than a patched method: the
+    module decides it is in-cluster by whether that file reads, so faking the decision at
+    the filesystem keeps the test honest about what it is exercising.
+    """
+    captured: dict[str, Any] = {}
+
+    token = tmp_path / "token"
+    token.write_text("tok", encoding="utf-8")
+    ca = tmp_path / "ca.crt"
+    ca.write_text("ca", encoding="utf-8")  # a complete SA has both; the readability check needs it
+    monkeypatch.setattr(helm, "_SA_TOKEN", token)
+    monkeypatch.setattr(helm, "_SA_CA", ca)
+    monkeypatch.setenv("KUBERNETES_SERVICE_HOST", "10.96.0.1")
+    monkeypatch.setenv("KUBERNETES_SERVICE_PORT_HTTPS", "443")
+
+    class _Client:
+        def __init__(self, **kw: object) -> None:
+            captured["client_kwargs"] = kw
+
+        async def __aenter__(self) -> _Client:
+            return self
+
+        async def __aexit__(self, *exc: object) -> None:
+            return None
+
+        async def get(self, url: str, **kw: object) -> _FakeResponse:
+            captured["url"] = url
+            captured.update(kw)
+            return _FakeResponse(items)
+
+    monkeypatch.setattr(httpx, "AsyncClient", _Client)
+    return captured
+
+
+@pytest.mark.asyncio
+async def test_api_read_collapses_a_release_to_its_newest_revision(
+    monkeypatch: pytest.MonkeyPatch, tmp_path: Path
+) -> None:
+    """helm writes one secret per revision. Ten revisions is one release, not ten."""
+    _fake_api(
+        monkeypatch,
+        tmp_path,
+        [_meta("acme-redis", "tenant-acme", v) for v in ("1", "2", "10", "9")],
+    )
+
+    out = await HelmProvisioner().list_releases()
+
+    assert [(r.name, r.namespace) for r in out] == [("acme-redis", "tenant-acme")]
+    # 10, not 9: string ordering would pick "9" and quietly report a stale revision.
+    assert out[0].revision == 10
+
+
+@pytest.mark.asyncio
+async def test_api_read_keeps_same_named_releases_in_different_namespaces(
+    monkeypatch: pytest.MonkeyPatch, tmp_path: Path
+) -> None:
+    """Two tenants may both call their instance `redis`. Namespace is part of the identity."""
+    _fake_api(monkeypatch, tmp_path, [_meta("redis", "tenant-a", "1"), _meta("redis", "tenant-b", "1")])
+
+    out = await HelmProvisioner().list_releases()
+
+    assert {(r.name, r.namespace) for r in out} == {("redis", "tenant-a"), ("redis", "tenant-b")}
+
+
+@pytest.mark.asyncio
+async def test_api_read_asks_for_metadata_only_and_scopes_the_selector(
+    monkeypatch: pytest.MonkeyPatch, tmp_path: Path
+) -> None:
+    """The Accept header is the difference between metadata and megabytes of gzipped payload."""
+    cap = _fake_api(monkeypatch, tmp_path, [])
+
+    await HelmProvisioner().list_releases()
+
+    assert "PartialObjectMetadataList" in cap["headers"]["accept"]
+    selector = cap["params"]["labelSelector"]
+    assert "owner=helm" in selector
+    assert f"{MANAGED_BY_LABEL}={MANAGED_BY_VALUE}" in selector
+    assert "superseded" not in selector  # dropped server-side by asking only for live states
+    assert cap["url"].endswith("/api/v1/secrets")
+
+
+@pytest.mark.asyncio
+async def test_api_read_ignores_secrets_that_are_not_releases(
+    monkeypatch: pytest.MonkeyPatch, tmp_path: Path
+) -> None:
+    """A malformed or unrelated secret must not become a phantom release."""
+    _fake_api(
+        monkeypatch,
+        tmp_path,
+        [{"metadata": {"namespace": "x", "labels": {}}}, _meta("real", "tenant-a", "1")],
+    )
+
+    out = await HelmProvisioner().list_releases()
+
+    assert [r.name for r in out] == ["real"]
+
+
+@pytest.mark.asyncio
+async def test_no_service_account_falls_back_to_helm(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
+    """Out of cluster there is nothing to authenticate with, so e2e and laptops keep working."""
+    seen = _capture(monkeypatch)
+
+    monkeypatch.setattr(helm, "_SA_TOKEN", tmp_path / "absent")
+
+    await HelmProvisioner(kubeconfig=Path("/dev/null")).list_releases()
+
+    assert seen and seen[0][:2] == ["helm", "list"]
+
+
+@pytest.mark.asyncio
+async def test_api_read_handles_kubernetes_serialising_empty_as_null(
+    monkeypatch: pytest.MonkeyPatch, tmp_path: Path
+) -> None:
+    """`"items": null`, not `[]`, is what an empty list looks like on the wire.
+
+    The key is present, so `.get("items", [])` returns None and the default never fires.
+    This shipped and failed in production on the first tick that matched no releases:
+    TypeError: 'NoneType' object is not iterable. The earlier tests all passed a real
+    empty list, which is the one shape that cannot catch it.
+    """
+    cap = _fake_api(monkeypatch, tmp_path, [])
+    cap["force_null_items"] = True
+
+    class _NullResponse:
+        def raise_for_status(self) -> None:
+            return None
+
+        def json(self) -> dict[str, object]:
+            return {"apiVersion": "meta.k8s.io/v1", "kind": "PartialObjectMetadataList", "items": None}
+
+    class _NullClient:
+        def __init__(self, **kw: object) -> None:
+            pass
+
+        async def __aenter__(self) -> _NullClient:
+            return self
+
+        async def __aexit__(self, *exc: object) -> None:
+            return None
+
+        async def get(self, url: str, **kw: object) -> _NullResponse:
+            return _NullResponse()
+
+    monkeypatch.setattr(httpx, "AsyncClient", _NullClient)
+
+    assert await HelmProvisioner().list_releases() == []
+
+
+@pytest.mark.asyncio
+async def test_api_read_passes_tls_verify_and_timeout(
+    monkeypatch: pytest.MonkeyPatch, tmp_path: Path
+) -> None:
+    """The API client must verify against the SA CA and carry the read timeout.
+
+    A refactor that dropped `verify` to the default (or None) is a TLS regression the happy
+    path would not reveal, so it is pinned here off the captured client kwargs.
+    """
+    cap = _fake_api(monkeypatch, tmp_path, [])
+
+    await HelmProvisioner().list_releases()
+
+    assert cap["client_kwargs"]["verify"] == str(helm._SA_CA)
+    assert cap["client_kwargs"]["timeout"] == helm._API_TIMEOUT_S
+
+
+@pytest.mark.asyncio
+async def test_api_read_sends_no_limit_param(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
+    """No `limit`, so the apiserver returns the full set and the single read is complete.
+
+    Pins the pagination invariant: adding `limit` without consuming `metadata.continue`
+    would silently truncate the release list.
+    """
+    cap = _fake_api(monkeypatch, tmp_path, [])
+
+    await HelmProvisioner().list_releases()
+
+    assert "limit" not in cap["params"]
+
+
+@pytest.mark.asyncio
+async def test_api_error_raises_helmerror_and_does_not_fall_back(
+    monkeypatch: pytest.MonkeyPatch, tmp_path: Path
+) -> None:
+    """A reachable-but-erroring apiserver raises HelmError; it must not shell out to helm.
+
+    Falling back would swap a visible error for the 330s helm timeout this method exists to
+    remove. The fallback is only for a ServiceAccount that is not present at all.
+    """
+    (tmp_path / "ca.crt").write_text("ca", encoding="utf-8")
+    monkeypatch.setattr(helm, "_SA_TOKEN", tmp_path / "token")
+    (tmp_path / "token").write_text("tok", encoding="utf-8")
+    monkeypatch.setattr(helm, "_SA_CA", tmp_path / "ca.crt")
+    monkeypatch.setenv("KUBERNETES_SERVICE_HOST", "10.96.0.1")
+    seen = _capture(monkeypatch)
+
+    class _ErrClient:
+        def __init__(self, **kw: object) -> None:
+            pass
+
+        async def __aenter__(self) -> _ErrClient:
+            return self
+
+        async def __aexit__(self, *exc: object) -> None:
+            return None
+
+        async def get(self, url: str, **kw: object) -> object:
+            raise httpx.ConnectError("connection reset by peer")
+
+    monkeypatch.setattr(httpx, "AsyncClient", _ErrClient)
+
+    with pytest.raises(HelmError):
+        await HelmProvisioner().list_releases()
+    assert seen == [], "an API error must not fall back to `helm list`"
+
+
+@pytest.mark.asyncio
+async def test_missing_ca_falls_back_to_helm(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
+    """A token without a readable CA is a half-mounted SA: fall back rather than crash.
+
+    httpx loads the CA when the client is built, raising OSError that the API except clause
+    does not catch, so the readability check has to happen before the request. A missing CA
+    means "not in-cluster", the same as a missing token.
+    """
+    monkeypatch.setattr(helm, "_SA_TOKEN", tmp_path / "token")
+    (tmp_path / "token").write_text("tok", encoding="utf-8")
+    monkeypatch.setattr(helm, "_SA_CA", tmp_path / "absent-ca.crt")  # never created
+    monkeypatch.setenv("KUBERNETES_SERVICE_HOST", "10.96.0.1")
+    seen = _capture(monkeypatch)
+
+    await HelmProvisioner(kubeconfig=Path("/dev/null")).list_releases()
+
+    assert seen and seen[0][:2] == ["helm", "list"]

tests/unit/test_values.py

@@ -0,0 +1,102 @@
+"""What reaches `helm --values`: the catalog entry's values, with the size on top."""
+
+from __future__ import annotations
+
+from datetime import UTC, datetime
+from typing import Any
+from uuid import uuid4
+
+import pytest
+
+from services.worker.handlers import HandlerError, _deep_merge, _values_for
+from svcforge_core.domain.models import CatalogEntry, Instance, SizeSpec
+from svcforge_core.domain.states import InstanceState
+
+RESOURCES: dict[str, Any] = {"requests": {"cpu": "10m", "memory": "16Mi"}}
+
+
+def _entry(values: dict[str, Any] | None = None, replicas: int = 1) -> CatalogEntry:
+    return CatalogEntry(
+        service_type="redis",
+        chart="oci://mirror.gcr.io/bitnamicharts/redis",
+        chart_version="27.0.15",
+        sizes={"small": SizeSpec(replicas=replicas, resources=RESOURCES)},
+        values=values or {},
+    )
+
+
+def _instance(size: str = "small") -> Instance:
+    now = datetime.now(UTC)
+    return Instance(
+        id=uuid4(),
+        team="acme",
+        service_type="redis",
+        size=size,
+        state=InstanceState.REQUESTED,
+        namespace="tenant-acme",
+        release_name="acme-redis-0f8b7d3e",
+        chart_version="27.0.15",
+        created_at=now,
+        updated_at=now,
+    )
+
+
+# --------------------------------------------------------------------------- the merge
+
+
+def test_deep_merge_keeps_both_sides_of_a_shared_nested_key() -> None:
+    """The reason this is not `base | override`.
+
+    A shallow merge replaces the whole `global` map and silently drops imageRegistry, so
+    the pod pulls from a registry nobody chose.
+    """
+    merged = _deep_merge(
+        {"global": {"imageRegistry": "mirror.gcr.io"}},
+        {"global": {"storageClass": "longhorn"}},
+    )
+    assert merged == {"global": {"imageRegistry": "mirror.gcr.io", "storageClass": "longhorn"}}
+
+
+def test_deep_merge_override_wins_on_a_scalar() -> None:
+    assert _deep_merge({"a": 1}, {"a": 2}) == {"a": 2}
+
+
+def test_deep_merge_does_not_mutate_its_inputs() -> None:
+    """The catalog is loaded once at startup and shared by every provision.
+
+    Mutating `entry.values` here would leak one instance's size into the next one's values,
+    and the second tenant would get the first tenant's replica count.
+    """
+    base = {"global": {"imageRegistry": "mirror.gcr.io"}}
+    _deep_merge(base, {"global": {"storageClass": "longhorn"}, "replicaCount": 3})
+    assert base == {"global": {"imageRegistry": "mirror.gcr.io"}}
+
+
+# --------------------------------------------------------------------------- what helm gets
+
+
+def test_entry_values_reach_helm() -> None:
+    values = _values_for(_instance(), _entry({"global": {"imageRegistry": "mirror.gcr.io"}}))
+
+    assert values["global"] == {"imageRegistry": "mirror.gcr.io"}
+    assert values["replicaCount"] == 1
+    assert values["resources"] == RESOURCES
+
+
+def test_size_beats_entry_values() -> None:
+    """An entry that sets replicaCount must not override the size the tenant asked for.
+
+    Without this ordering every size deploys the same shape, and `medium` is a lie.
+    """
+    entry = _entry({"replicaCount": 99}, replicas=3)
+
+    assert _values_for(_instance(), entry)["replicaCount"] == 3
+
+
+def test_entry_without_values_is_unchanged() -> None:
+    assert _values_for(_instance(), _entry()) == {"replicaCount": 1, "resources": RESOURCES}
+
+
+def test_unknown_size_raises() -> None:
+    with pytest.raises(HandlerError, match="not in catalog"):
+        _values_for(_instance(size="enormous"), _entry())