fix(server): stop webhook ingestion failing silently (#1501)

Bounds every webhook stream by disk as well as by age, so a stream can no longer fill the host volume and wedge the broker. Puts the ingestion, consumer and review health checks into the readiness group they were written for, so an ingestion outage stops reporting healthy. Counts webhooks actually lost rather than how full a stream is, configures the broker to carry what the receiver admits, and expires message-queue consumers after 30 days with nothing connected.

Removes NATS_JS_MAX_FILE in favour of NATS_JS_MAX_FILE_BYTES; see MIGRATION.md.

Author: FelixTJDietrich

.changeset/reap-disposable-nats-consumers.md

@@ -2,4 +2,6 @@
 "hephaestus": minor
 ---
 
-Deployments that are deleted rather than shut down can now hand their NATS consumers back automatically. Set `HEPHAESTUS_INTEGRATION_CONSUMER_INACTIVE_THRESHOLD` (for example `72h`) on any stack that shares a NATS server but is disposable — a pull request preview, a throwaway test environment — and its consumers are removed once nothing has been bound for that long, instead of accumulating on the shared stream one generation per deleted stack. Left unset, which is the default and what a long-lived deployment wants, consumers keep their position across restarts exactly as before.
+A deployment's message-queue consumers no longer outlive the deployment itself. Consumers were kept forever, and nothing else removes one, so every stack that shared a broker and was deleted rather than shut down — a pull request preview, a throwaway test environment — left its consumers, and backlogs that would never drain, behind for good. They now expire on their own once nothing has been connected to them for long enough.
+
+**Operators:** `HEPHAESTUS_INTEGRATION_CONSUMER_INACTIVE_THRESHOLD` defaults to `30d`, where it previously meant never. It measures time with nothing connected, not time without traffic — a running deployment resets it continuously even while its queues are silent, so no restart, deploy or incident reaches it. Set something far shorter, such as `72h`, on any disposable stack that shares a broker. If your deployment may be offline for longer than the threshold and must resume exactly where it left off, set `0s` to switch expiry off. Values between `0s` and `1h` are now rejected at startup rather than quietly expiring a consumer across an ordinary restart.

.changeset/webhook-ingestion-says-when-it-stops.md

@@ -0,0 +1,9 @@
+---
+"hephaestus": minor
+---
+
+An ingestion outage is no longer silent. The readiness check reported only that the process had started: the checks that know whether webhooks can be received, whether the message consumer is connected and whether reviews can run were meant to be part of it and never were. A deployment whose message broker had stopped accepting writes kept answering healthy while it dropped every delivery from GitHub, GitLab, Slack and Outline. Readiness now reports all of them, which is what makes an alert on it possible.
+
+The server also counts webhooks that were lost rather than how close a stream is to being full: if a message is deleted before the consumer that needed it has read it, that is recorded, named, and logged as an error — the one thing nobody can recover from afterwards.
+
+**Operators:** readiness now fails while the message broker is unreachable, which on a container that also serves the app takes it out of load-balancer rotation until the broker recovers. If you treated readiness as a liveness signal, it now reports operational dependencies too. Alert on `webhook.stream.unacknowledged.deletions` — any increase is webhook data that is gone for good — and on `webhook.stream.poll.age` beside it, because a check that cannot reach the broker reports no loss and no loss the same way.

.changeset/webhook-streams-cannot-fill-the-disk.md

@@ -0,0 +1,11 @@
+---
+"hephaestus": minor
+---
+
+Webhook message streams can no longer fill the disk and take ingestion down with them. They were bounded only by a message count, which says nothing about storage: one deployment's GitHub stream reached 32.3 GB at exactly its cap, filled the host, stopped the broker writing, and dropped every inbound webhook until the broker was restarted by hand.
+
+Each stream now states both of its bounds: how long a delivery is kept at most, and a disk ceiling under that. Which one you actually get depends on your traffic — at low volume the time limit is delivered in full, at high volume the disk ceiling recycles the stream sooner — and the server reports the answer for your deployment as the age of the oldest message it still holds. It refuses to start if the streams together are allowed more than the broker's own budget. Lowering a limit also takes effect on a stream that already exists, instead of being reported and ignored; a change that would delete messages already stored is held back and logged with exactly what it would cost until you allow it, and a change that would leave a stream with no limit at all is held back regardless.
+
+Deliveries larger than the broker will carry are no longer accepted and then lost at publish: the broker is configured to take everything the receiver admits, and the receiver says so loudly if the two disagree.
+
+**Operators:** `NATS_JS_MAX_FILE` is **removed** and nothing reads it any more. Replace it with `NATS_JS_MAX_FILE_BYTES`, in bytes — a deployment that leaves the old variable set silently drops to the new 16 GiB default instead of the 50 GB it had. Set it below the free space on the broker's volume, and keep the per-stream ceilings totalling under it or the server will not start. The 180-day retention limit is unchanged, but a busy GitHub stream now recycles on disk well before that; `HEPHAESTUS_WEBHOOK_STREAM_MAX_BYTES` and the new `HEPHAESTUS_WEBHOOK_STREAM_MAX_BYTES_GITHUB` set that ceiling. A stream already larger than its new ceiling stays as it is and logs what bounding it would delete, until you set `HEPHAESTUS_WEBHOOK_STREAM_ALLOW_DESTRUCTIVE_LIMIT_UPDATES=true` once. See MIGRATION.md.

.github/workflows/ci-quality-gates.yml

@@ -113,7 +113,7 @@ jobs:
           pnpm install --frozen-lockfile
 
           ISSUES_FOUND=()
-          FMT_OK=true; BIOME_OK=true; PIN_OK=true; TESTS_OK=true; TYPES_OK=true; SCRIPT_TYPES_OK=true; CONTRACTS_OK=true
+          FMT_OK=true; BIOME_OK=true; PIN_OK=true; TESTS_OK=true; TYPES_OK=true; SCRIPT_TYPES_OK=true; CONTRACTS_OK=true; ENV_OK=true
 
           pnpm run format:java:check || { FMT_OK=false; ISSUES_FOUND+=("Java formatting failed. Run: pnpm run format:java"); }
           # Runs before the Biome step so a version skew is reported as a version skew rather than
@@ -136,6 +136,10 @@ jobs:
           pnpm run typecheck:scripts \
             || { SCRIPT_TYPES_OK=false; ISSUES_FOUND+=("scripts/ TypeScript errors. Run: pnpm run typecheck:scripts"); }
           pnpm run check:contracts || { CONTRACTS_OK=false; ISSUES_FOUND+=("Artifact-source contract validation failed. Run: pnpm run check:contracts"); }
+          # Reads application.yml against the compose files, both behind this job's path filter. It
+          # was reachable only from the pre-push hook, which --no-verify skips, so the gate written
+          # for a defect that reached production could not fail a required build.
+          pnpm run check:env || { ENV_OK=false; ISSUES_FOUND+=("A setting does not reach a container that can read it. Run: pnpm run check:env"); }
 
           if [ ${#ISSUES_FOUND[@]} -gt 0 ]; then
             echo "### App Server Quality" >> $GITHUB_STEP_SUMMARY
@@ -148,6 +152,7 @@ jobs:
             [[ "$TYPES_OK" == "false" ]] && echo "| Agent typecheck | :x: Failed | \`pnpm run typecheck:agents\` |" >> $GITHUB_STEP_SUMMARY
             [[ "$SCRIPT_TYPES_OK" == "false" ]] && echo "| scripts/ typecheck | :x: Failed | \`pnpm run typecheck:scripts\` |" >> $GITHUB_STEP_SUMMARY
             [[ "$CONTRACTS_OK" == "false" ]] && echo "| Artifact-source contracts | :x: Failed | \`pnpm run check:contracts\` |" >> $GITHUB_STEP_SUMMARY
+            [[ "$ENV_OK" == "false" ]] && echo "| Env delivery by runtime role | :x: Failed | \`pnpm run check:env\` |" >> $GITHUB_STEP_SUMMARY
             for issue in "${ISSUES_FOUND[@]}"; do echo "::error::$issue"; done
             exit 1
           fi
@@ -164,7 +169,8 @@ jobs:
           BIOME_OK=true
           TYPES_OK=true
           # The webapp-scoped tree gates run here as well as in the pre-push hook, because a hook is
-          # skippable and a merge is not. check:env stays out: it reads server files this leg omits.
+          # skippable and a merge is not. check:env runs on the App Server leg, which has the server
+          # and compose files it reads.
           COMPONENTS_OK=true
           STORIES_OK=true
 

.github/workflows/cicd.yml

@@ -67,6 +67,10 @@ jobs:
               - 'pnpm-lock.yaml'
               # check:biome-pin compares this file's $schema against the pinned version.
               - 'webapp/biome.json'
+              # check:env compares application.yml against the deployment that has to deliver it, so
+              # a change to either side has to be able to trigger the job that runs it.
+              - 'docker/compose.app.yaml'
+              - 'docker/compose.core.yaml'
             agent-images:
               - 'docker/agents/**'
             docs:

.gitignore

@@ -5,7 +5,9 @@
 .ai/
 #  and can be added to the global gitignore or merged into this file.  For a more nuclear
 application-live-local.yml
+application-live-local.yaml
 application-local.yml
+# Spring reads both extensions, so a .yaml typo is a config file that works and is tracked.
 application-local.yaml
 **/atlassian-ide-plugin.xml
 # auto-import.

MIGRATION.md

@@ -122,6 +122,128 @@ A preview now derives its agent image from its own commit, and CI publishes one
 when the pull request changed the agent tree or a workflow. Previously such a preview silently used
 the last release's image. Set `HEPHAESTUS_AGENT_IMAGE_REFERENCE` in the preview's `.env` to the agent
 image you want it to exercise — `docker/preview/.env.example` shows the line.
+#### 🔴 `NATS_JS_MAX_FILE` is gone, and webhook streams now have a disk bound
+
+**Affected**: every deployment.
+
+**Do this before upgrading:**
+
+1. **If you set `NATS_JS_MAX_FILE`, replace it with `NATS_JS_MAX_FILE_BYTES`, in bytes.** The old
+   variable is no longer read by anything. Nothing warns you: a deployment that had `100G` silently
+   drops to the new 16 GiB default. `50G` becomes `NATS_JS_MAX_FILE_BYTES=53687091200`.
+2. **Check your current stream sizes** with `nats stream report`. A stream already larger than its
+   new ceiling is left exactly as it is and logs an error on every start until you decide — see
+   below.
+3. **Keep the per-stream ceilings totalling under `NATS_JS_MAX_FILE_BYTES`**, or the receiver refuses
+   to start.
+
+Everything else in this entry is context.
+
+---
+
+Webhook streams were bounded only by message count, which says nothing about disk: on one deployment
+2,000,000 GitHub deliveries came to 32.3 GB, filled the host, stopped the broker writing, and every
+inbound webhook was dropped until the broker was restarted by hand.
+
+**Two bounds now, not three.** `HEPHAESTUS_WEBHOOK_STREAM_MAX_AGE` stays at 180 days: it is the
+*ceiling*, the longest a delivery is kept if disk allows. `HEPHAESTUS_WEBHOOK_STREAM_MAX_BYTES`
+(1 GiB per stream) and `HEPHAESTUS_WEBHOOK_STREAM_MAX_BYTES_GITHUB` (10 GiB) are the *floor* under
+it, and on a busy deployment they are what actually decides retention. Both are true; which one binds
+is a function of your volume, and the server now publishes the answer as
+`webhook.stream.oldest.message.age{stream}`, in seconds, so you can read your own effective retention
+rather than infer it. The message-count bound is gone: a count describes neither disk nor time.
+
+The byte ceiling is sized against what a shed message costs, not against the age ceiling. The nightly
+reconciliation sync re-fetches the last `MONITORING_TIMEFRAME` days from the provider API, so a
+webhook shed inside that window is recoverable by other means and one shed outside it is recoverable
+by nothing. If you raise `MONITORING_TIMEFRAME`, raise the byte ceilings with it.
+
+`NATS_JS_MAX_FILE_BYTES` sets the broker's own budget and the application's from one value, and the
+server refuses to start if the per-stream bounds sum above it:
+
+```
+Webhook stream bounds total 21474836480 bytes, over the 17179869184-byte broker storage budget
+```
+
+Set it below the free space on the broker's volume. This is the difference between the broker filling
+its own budget — where it refuses new messages and recovers by itself — and filling the filesystem,
+where it cannot write its own metadata and stays wedged even after space is freed.
+
+**A stream that already exceeds its new bound is left exactly as it is.** Bounding it deletes the
+excess the moment it applies, so startup withholds the change and logs what it would cost:
+
+```
+Stream github limit change withheld because it would delete stored messages:
+[maxBytes -1 -> 10737418240 (32300000000 bytes stored, 21562581760 would be deleted)] —
+set hephaestus.webhook.stream.allow-destructive-limit-updates=true to apply it
+Stream github bound removal withheld because it would leave the stream unbounded:
+[maxMessages 2000000 -> -1 (no byte bound is in force to replace it)] —
+get hephaestus.webhook.stream.max-bytes[-by-stream] applied first
+```
+
+Decide the bound first — raise `HEPHAESTUS_WEBHOOK_STREAM_MAX_BYTES_GITHUB` if the size the log
+reports is legitimate for your traffic, keeping the total under `NATS_JS_MAX_FILE_BYTES`. Then set
+`HEPHAESTUS_WEBHOOK_STREAM_ALLOW_DESTRUCTIVE_LIMIT_UPDATES=true` for one start-up to apply it, and
+unset it again. Until you do, the stream keeps the message-count cap it already has: the count cap is
+only released once a byte ceiling is in force to replace it, so an upgrade cannot leave a stream with
+no limit at all.
+
+Streams that already fit inside the bound are bounded automatically, with nothing deleted. Subjects,
+retention mode, storage and discard policy are never rewritten by a deployment. A stream whose shape
+has drifted from what this deployment expects is left entirely alone and logged — repair it with
+`nats stream edit` before the bound can be applied.
+
+Full metric roster and the wedged-broker procedure:
+[Webhook ingestion operations](https://ls1intum.github.io/Hephaestus/admin/webhook-ingestion-operations).
+
+
+#### 🟡 Readiness now answers for more than "the process started"
+
+**Affected**: every deployment that monitors `/actuator/health/readiness`, and in particular any that
+runs webhook receiving in the same container as the application — a single-container install, or the
+preview stacks.
+
+`/actuator/health/readiness` was the stock probe group on every container: the process's own
+availability state and nothing else. The group meant to add the message-consumer, practice-review and
+webhook checks was written under a property path Spring does not bind, so it was silently ignored, and
+a container whose broker had stopped accepting writes answered 200 throughout. Those checks are now in
+the probe. Expect readiness to follow broker availability, and on a combined-role container expect a
+broker outage to take it out of load-balancer rotation until the broker returns.
+
+**Migration**: none, but re-read what you alert on. If your dashboards treated readiness as a liveness
+signal, it now reports operational dependencies as well, which is the point.
+
+#### 🟡 Message-queue consumers now expire after 30 days with nothing connected
+
+**Affected**: deployments that may be offline for more than 30 consecutive days and must resume
+exactly where they left off. Everything else needs no action.
+
+`HEPHAESTUS_INTEGRATION_CONSUMER_INACTIVE_THRESHOLD` previously defaulted to *never expire*. Nothing
+else removes a consumer, so any stack that shared a broker and was deleted rather than shut down — a
+preview, a test environment — left its consumers and their undrained backlogs on that broker
+permanently, one generation per deleted stack.
+
+**What it measures is connection, not traffic.** A running deployment holds standing requests against
+its consumers, so it resets the clock continuously even while its queues are completely silent. Only a
+deployment that no longer exists ages out, which is why 30 days is safe: no restart, deploy or incident
+reaches it.
+
+**Migration**: nothing to do, unless your deployment can be down for more than 30 days and must not
+skip what arrived meanwhile. In that case set `HEPHAESTUS_INTEGRATION_CONSUMER_INACTIVE_THRESHOLD=0s`
+before upgrading, which switches expiry off exactly as before. A consumer that does expire is recreated
+pointing at new messages only, so it skips anything that arrived while the deployment was gone.
+
+`0s` preserves the consumer's *position*, not the *messages*. Those expire on the stream's own
+retention, independently, so a deployment offline past the stream's byte or age ceiling comes back to
+a cursor pointing at messages the stream no longer holds — the loss counter will say so on the first
+poll. Switching expiry off buys you the stream's retention window, not an unbounded one.
+
+Values between `0s` and `1h` are now rejected at startup — a threshold that short expires a consumer
+across an ordinary restart, which is the data loss the setting exists to avoid:
+
+```
+inactive-threshold (PT30M) must be 0 to disable reaping, or at least PT1H
+```
 
 #### 🟡 Reviewer-side practices keep the old wording until you update them
 

docker/.env.example

@@ -98,6 +98,37 @@ HEPHAESTUS_SECURITY_ENCRYPTION_KEY=
 NATS_ENABLED=true
 NATS_DURABLE_CONSUMER_NAME=hephaestus-consumer
 
+# How long a durable consumer may go with nothing CONNECTED to it before JetStream deletes it — not
+# how long it goes without traffic. Set 0s on a deployment that may be offline longer than this and
+# must resume exactly where it left off; between 0s and 1h is rejected at startup.
+HEPHAESTUS_INTEGRATION_CONSUMER_INACTIVE_THRESHOLD=30d
+
+# What the broker may hold for ALL webhook streams together, in bytes. One value sets both the
+# broker's own JetStream budget and the bound the server checks its stream limits against. Keep it
+# below the free space on the nats-data volume. Replaces NATS_JS_MAX_FILE, which was a size string
+# and is no longer read — see MIGRATION.md.
+NATS_JS_MAX_FILE_BYTES=17179869184
+
+# Largest single message the broker will carry. Must be at least what the receiver accepts
+# (hephaestus.webhook.http.max-payload-bytes, 25 MiB) — anything above it is verified, admitted and
+# then lost at publish. NATS' own default is 1MB, which is far below that.
+NATS_MAX_PAYLOAD_BYTES=26214400
+
+# How long a webhook stream keeps a delivery, and the disk ceiling under that. MAX_AGE is the
+# ceiling; MAX_BYTES is the floor under it, and which of the two binds is a function of your volume.
+# Read your own answer off `webhook.stream.oldest.message.age` rather than inferring it. The four
+# streams' ceilings must total under NATS_JS_MAX_FILE_BYTES or the receiver refuses to start.
+# https://ls1intum.github.io/Hephaestus/admin/webhook-ingestion-operations
+HEPHAESTUS_WEBHOOK_STREAM_MAX_AGE=180d
+HEPHAESTUS_WEBHOOK_STREAM_MAX_BYTES=1GB
+HEPHAESTUS_WEBHOOK_STREAM_MAX_BYTES_GITHUB=10GB
+
+# Startup keeps an existing stream's limits in step with configuration, but withholds any change
+# that would delete messages the stream already holds and logs what it would cost. Set true for one
+# deliberate start-up to apply it — for example when first bounding a stream that has already grown
+# past the new limit.
+HEPHAESTUS_WEBHOOK_STREAM_ALLOW_DESTRUCTIVE_LIMIT_UPDATES=false
+
 # -----------------------------------------------------------------------------
 # MONITORING & SYNC
 # -----------------------------------------------------------------------------

docker/compose.app.yaml

@@ -202,6 +202,9 @@ services:
       # Default on: the integration event flow needs NATS in prod (the code-level fallback is off for local dev).
       NATS_ENABLED: ${NATS_ENABLED:-true}
       NATS_DURABLE_CONSUMER_NAME: ${NATS_DURABLE_CONSUMER_NAME:-hephaestus}
+      # How long this deployment's durables may go with nothing bound to them before JetStream
+      # deletes them. See docker/.env.example for what to set it to and when.
+      HEPHAESTUS_INTEGRATION_CONSUMER_INACTIVE_THRESHOLD: ${HEPHAESTUS_INTEGRATION_CONSUMER_INACTIVE_THRESHOLD:-30d}
       MONITORING_TIMEFRAME: ${MONITORING_TIMEFRAME:-7}
       MONITORING_RUN_ON_STARTUP: ${MONITORING_RUN_ON_STARTUP:-true}
       MONITORING_SYNC_CRON: ${MONITORING_SYNC_CRON:-0 0 * * * *}