Add readiness pre-drain before graceful shutdown

[dkijania]

The production-readiness PR set still leaves rolling deploys with an ownership gap across readiness and shutdown behavior.\n\nCurrent split:\n- #187 adds /readiness, but does not flip readiness false during shutdown.\n- #188 closes the HTTP listener on SIGTERM and drains in-flight work, but traffic can still be routed until endpoints are removed.\n- #196 adds a Kubernetes preStop sleep in the reference manifest, but the app itself has no pre-drain readiness state and non-Kubernetes deployments have no equivalent default.\n\nDesired behavior:\n- On shutdown intent, mark readiness false before closing the listener.\n- Keep accepting/draining already-routed in-flight requests during a short pre-drain window.\n- Document the required preStop / terminationGracePeriodSeconds relationship for Kubernetes and the equivalent behavior for other supervisors.\n- Add regression coverage for readiness returning 503 after shutdown starts while liveness remains process-only.\n\nThis should prevent ordinary rolling deploys from resetting in-flight requests and make the behavior owned by the app, not only by the reference manifests.

[SanabriaRusso]

Confirming this issue is accurate against the current heads of all three PRs, and adding a concrete implementation sketch so a dev agent can pick it up without re-deriving the design.

Re-verified state (heads 2496d84c / 848af58c / 6f89b0fd):

• P1: Add readiness probe distinct from liveness (#169) #187 src/server/readiness.ts:59-73 — the plugin's only input is the DB ping. There is no shutdown state, so /readiness returns 200 right up until the listener closes.
• P1: Graceful shutdown — drain, flush traces, uncaught handlers (#170) #188 src/index.ts:46-52 — closeServer calls server.close() then server.closeIdleConnections() immediately on SIGTERM. No pre-drain.
• P2: Reference deployment manifests — k8s + prod Compose (#179) #196 deploy/kubernetes.yaml:90-95 — does now add preStop: exec ["/bin/sh","-c","sleep 15"] with terminationGracePeriodSeconds: 45, so the Kubernetes half is covered.

So the remaining gap is exactly as described: the in-process half. Non-Kubernetes supervisors (the Compose reference in #196, systemd, Nomad) have no equivalent, and even on Kubernetes the app cannot signal "stop routing to me" independently of the manifest's fixed sleep.

---

Suggested implementation

1. A tiny shared shutdown-state module — deliberately separate from readiness.ts so graceful-shutdown.ts does not have to import the plugin:

// src/server/shutdown-state.ts
let shuttingDown = false;

/** True once shutdown intent has been signalled (SIGTERM/SIGINT/crash). */
export function isShuttingDown(): boolean {
  return shuttingDown;
}

/** Idempotent. Called by the signal handler before the listener closes. */
export function beginShutdown(): void {
  shuttingDown = true;
}

2. Check it first in the readiness plugin. In useReadiness's onRequest, ahead of the ping — a draining instance is not-ready regardless of DB health, and skipping the ping avoids a pointless 2 s wait during shutdown:

async onRequest({ url, endResponse, fetchAPI }) {
  if (url.pathname.replace(/\/$/, '') !== READINESS_PATH) return;

  // Draining: report not-ready without pinging. The DB may be perfectly
  // healthy; we are refusing new traffic, not reporting an outage.
  const ready = isShuttingDown()
    ? false
    : await pingWithin(db, timeoutMs);

  endResponse(new fetchAPI.Response(
    JSON.stringify({ status: ready ? 'ready' : 'not ready' }),
    { status: ready ? 200 : 503, headers: { 'content-type': 'application/json', 'cache-control': 'no-store' } }
  ));
}

3. Flip it, then wait, then close in src/index.ts's shutdown sequence. The wait is what gives the orchestrator time to observe the 503 and pull the endpoint:

const PRE_DRAIN_MS = Number(process.env.PRE_DRAIN_MS) || 5000;

const shutdown = createGracefulShutdown({
  timeoutMs: SHUTDOWN_TIMEOUT_MS,
  closers: [
    ['pre-drain', async () => {
      beginShutdown();                 // /readiness starts answering 503
      await setTimeout(PRE_DRAIN_MS);  // node:timers/promises
    }],
    ['server', closeServer],
    ['traces', ...],
    ['pg pool', ...],
  ],
});

Ordering it as the first closer means it is already inside #188's bounded-teardown machinery — the outer forceTimer covers it, so a misconfigured PRE_DRAIN_MS cannot hang shutdown indefinitely.

Important: SHUTDOWN_TIMEOUT_MS must exceed PRE_DRAIN_MS plus the real drain time, or the force-exit fires mid-drain. With the suggested defaults that is 5 s + drain against a 20 s budget, which is fine. Worth a validation check in #193's validateConfig once both have landed:

PRE_DRAIN_MS must be < SHUTDOWN_TIMEOUT_MS

4. Liveness must stay untouched. /healthcheck is Yoga's built-in and must keep returning 200 throughout the drain — if it starts failing, the kubelet restarts the pod instead of letting it drain. This is the single most important invariant in this change, and it is worth an explicit test rather than a comment.

5. Docs and manifests.

• docs/getting-started.md: document PRE_DRAIN_MS and state the relationship preStop sleep >= PRE_DRAIN_MS and terminationGracePeriodSeconds > PRE_DRAIN_MS + SHUTDOWN_TIMEOUT_MS.
• P2: Reference deployment manifests — k8s + prod Compose (#179) #196's Compose reference needs stop_grace_period (Docker's default is 10 s, below the 20 s SHUTDOWN_TIMEOUT_MS) — this is a separate note already left on that PR.
• With the app-level pre-drain in place, P2: Reference deployment manifests — k8s + prod Compose (#179) #196's preStop: sleep 15 can be reduced or dropped, since the app now owns the behaviour. Suggest keeping a short preStop anyway as belt-and-braces for the endpoint-propagation delay, which is genuinely a cluster property rather than an app one.

---

Suggested test coverage

test('readiness reports 503 once shutdown has begun, without pinging', async () => {
  let pings = 0;
  const db = { ping: async () => { pings++; return true; } };
  // ... build server with useReadiness(db) ...
  assert.strictEqual((await fetch('/readiness')).status, 200);
  beginShutdown();
  const res = await fetch('/readiness');
  assert.strictEqual(res.status, 503);
  assert.strictEqual(JSON.parse(await res.text()).status, 'not ready');
  assert.strictEqual(pings, 1, 'must not ping the DB while draining');
});

test('liveness stays 200 throughout the drain', async () => {
  beginShutdown();
  assert.strictEqual((await fetch('/healthcheck')).status, 200);
});

test('in-flight requests still complete after shutdown begins', async () => {
  // a slow resolver started before beginShutdown() must resolve normally
});

The third one is the point of the whole exercise and is the easiest to forget.

---

Ownership

This is best done as a single PR touching readiness.ts, index.ts, and the docs, after #187 and #188 have both merged — splitting it across the two would leave an intermediate state where the flag exists but nothing sets it. See #211 for the merge order.

I would not expand #187 or #188 to cover it; both are already correct and approved, and #188's bounded-teardown machinery is exactly the right foundation to build this on.