Skip to content

fix(health): keep liveness probes free of store I/O - #102

Closed
cursor[bot] wants to merge 11 commits into
mainfrom
cursor/bc-0e84f9a0-03b8-4639-9153-9363b144bba2-d549
Closed

cursor[bot] wants to merge 11 commits into
mainfrom
cursor/bc-0e84f9a0-03b8-4639-9153-9363b144bba2-d549

Conversation

@cursor

@cursor cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Why

PR #91 exposes GET /live and GET /ready, but the PostgreSQL adapter observes the store before parsing the request and before accept(). A hung store can take down liveness, and a stale snapshot can return HTTP 200 on /ready after the store becomes read-only. Bare /ready also ignored postgres_operational_store, so a default Kubernetes readiness path could keep sending writes to a store that cannot persist. The listener had no I/O timeout and treated an oversized request with a valid first line as a successful /live probe.

TDD

RED 90a2a7e required store-free /live, fail-closed bare /ready on a read-only transaction, HTTP 400 for incomplete and oversized requests without echo, and an as-built OpenAPI /live 503. GREEN 79aa953 is the narrow fix.

Scope

  • GET /live answers process liveness only and does not perform store I/O.
  • GET /ready observes PostgreSQL after accept; a bare /ready requires postgres_operational_store.
  • The bound listener applies a 2-second read/write timeout, rejects incomplete or oversized requests without echoing input, sends Cache-Control: no-store, and includes Allow: GET on 405.
  • openapi/health-probes.yaml documents /live 503.
  • Public/admin product routes, TLS, measured SLO values, and deployment-profile evidence remain outside this slice.

This branch continues the #91 operator-probe stack with those production-safety corrections. Do not merge until exact-head checks and independent last-push approval are satisfied. Never self-approve.

Operator next action

Review the probe contract tests, then use GET /live for process liveness and GET /ready (bare or ?capability=postgres_operational_store) for write-readiness. Do not point a load balancer at /live as a readiness signal.

Open in Web View Automation 

seonghobae and others added 11 commits August 16, 2026 19:51
RED: the domain health snapshot is not yet exposed as GET /live and
GET /ready with fail-closed readiness and a safe as-built OpenAPI 3.2
contract.
Map RuntimeHealthSnapshot to GET /live and GET /ready with fail-closed
readiness, RFC 9457 problem details for unsupported requests, and an
as-built OpenAPI 3.2.0 document that lists only those operations.
Exercise HealthHttpResponse::content_type and GET /ready?capability= from
the library test module so Linux line coverage can reach the remaining
unexecuted instantiations.
* feat(health): bind a TCP listener for operator probes

GET /live and GET /ready already exist as a request translator. Bind a
blocking listener that serves one HTTP/1.1 request per accepted
connection without adding public product routes, TLS, or SLO values.

* docs(traceability): name Active PR #92 on the bound health listener

* feat(health): answer probes from a PostgreSQL operational snapshot

* test(health): require a composed PostgreSQL operational snapshot

RED: runtime and relation probes exist, but callers still assemble
liveness, backlog, integrity, and the postgres capability by hand.

* feat(health): compose PostgreSQL probes into one operational snapshot

Map runtime and relation probes plus caller-supplied backlog into a
RuntimeHealthSnapshot. Probe failure becomes unknown/unready evidence
and never returns a driver error.

* feat(health): answer probes from a PostgreSQL operational snapshot

Rebuild #95 on the current #92 listener plus the #93 snapshot composer.
handle_postgres_health_http_request and accept_one_postgres_health_http
observe the caller-owned store and reuse the existing probe translator.

* docs(health): name #95 snapshot wiring on the current listener stack

Restore the architecture mappings the rewrite dropped: TRACEABILITY
module and capability rows, OPERABILITY fail-closed snapshot readiness,
ADR-0014 as-built status, and the composed-snapshot changelog line.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Keep Active PR #91 as the remaining health HTTP stack after #76
landed on main. Drop the stale #76 active-work entry.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
The inherited #72 recovery fixture inserted a processing consumption
row without claim_deadline_at. Migration 0019 requires that column for
processing rows, and the deadline trigger is UPDATE-only, so exact-head
CI failed closed. Seed a valid persisted claim and assert the deadline
survives COPY restore.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
#92, #93, and #95 were absorbed into this branch. Keep TRACEABILITY and
ADR-0014 as-built status pointing at the remaining health HTTP PR so
closed stacked numbers are not treated as independent Active work.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
…port

RED: GET /live must not observe PostgreSQL, bare GET /ready must fail
closed on a read-only store, incomplete or oversized requests must
return 400 without echoing input, and the as-built OpenAPI /live
operation must document HTTP 503.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Observe PostgreSQL only for GET /ready, after accept. Bare /ready
requires postgres_operational_store. The listener applies a 2-second
I/O timeout, rejects incomplete or oversized requests without echo,
and adds Cache-Control: no-store plus Allow: GET on 405.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
@cursor
cursor Bot requested a review from seonghobae August 16, 2026 15:12
@seonghobae
seonghobae marked this pull request as ready for review August 16, 2026 15:12

@cursor cursor Bot left a comment •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Current-head review (79aa953)

The #91 production defects are addressed on this head.

  • handle_postgres_health_http_request returns a process-liveness snapshot for every non-GET-/ready request, so GET /live and 400/404/405 do not call observe_postgres_operational_snapshot.
  • accept_one_postgres_health_http reads the request first through accept_one_health_http_with, then observes PostgreSQL only if the accepted request is GET /ready.
  • Bare GET /ready injects postgres_operational_store. A read-only or unsupported-major store therefore cannot return HTTP 200 to a default Kubernetes readiness path.
  • The listener sets a 2-second read/write timeout. Incomplete or oversized requests become empty request text and fail closed as HTTP 400 without echoing input. 405 includes Allow: GET. Responses send Cache-Control: no-store.

Residual, not blocking this slice

RFC 9457 problem bodies still use "type":"about:blank". #87 requires an explicit HTTPS or URN problem type before that primitive lands. Keep that on #87; do not block operator probes on it.

Do not merge until exact-head required checks and independent last-push approval are satisfied. Close or supersede #91 after this lands. Point liveness at GET /live and readiness at GET /ready (bare or ?capability=postgres_operational_store). Do not treat /live HTTP 200 as write-readiness.

View PR

Open in Web View Automation 

Sent by Cursor Automation: Fix Issues

@cursor

cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor Author

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@cursor cursor Bot left a comment •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Verdict: comment on 79aa953

The four #91 production defects are fixed in this head, not only restated:

  1. /live, 400, 404, and 405 no longer observe PostgreSQL. accept_one_health_http_with is now accept → I/O timeout → read → handler.
  2. Bare /ready on the postgres adapter requires postgres_operational_store. The generic translator still uses is_ready_for(&[]) — do not compose observe_postgres_operational_snapshot with accept_one_health_http or the old default returns.
  3. Read/write timeouts are 2s before the request is read. A hung client gets 400 instead of a wedged accept loop.
  4. An oversized buffer or a missing \r \r becomes an empty request and 400, including a valid /live first line plus padding. The body does not echo input.

Not approving: the PR is still Draft, TRACEABILITY/ADR-0014 still name Active PR #91, and /ready still has no statement timeout (a serial accept loop can head-of-line-block /live while one /ready waits on PostgreSQL). That last item is caller-owned session state, not a fail-open in this slice.

Do not merge #91 at ec32f969. Land this successor after required checks.

View PR

Open in Web View Automation 

Sent by Cursor Automation: Fix Issues

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: comment on 79aa953

The four #91 production defects are fixed on this head.

  1. GET /live, 400, 404, and 405 never call observe_postgres_operational_snapshot. handle_postgres_health_http_request returns a process-liveness snapshot unless the request is GET /ready.
  2. accept_one_postgres_health_http is accept → 2s I/O timeout → read → handler. Store observation cannot start before accept.
  3. Bare GET /ready on the PostgreSQL adapter requires postgres_operational_store. A read-only or unsupported-major store cannot return HTTP 200 to a default Kubernetes readiness path.
  4. Incomplete or oversized requests become empty request text and HTTP 400 without echoing input. 405 includes Allow: GET. Responses send Cache-Control: no-store. OpenAPI documents /live 503.

TRACEABILITY and ADR-0014 still name Active PR #91. That is not a correctness failure: #91 and #102 share this SHA, and the documented probe contract matches the code. Close or supersede the non-merge vehicle after landing.

A PostgreSQL statement_timeout on observe is not required for this one-shot acceptor. Add it only when a long-running serial accept loop would otherwise let one hung /ready starve /live.

Do not merge until exact-head required checks and independent last-push approval are satisfied. Never self-approve. Point kubelet liveness at GET /live and readiness at GET /ready. Do not treat /live HTTP 200 as write-readiness.

Open in Web View Automation 

Sent by Cursor Automation: Fix Issues

Copy link
Copy Markdown
Contributor

Closing as a proven superseded duplicate/predecessor. This PR's exact head 79aa9534 is the same commit as closed #91 and fresh compare shows that commit is the merge base/ancestor of #132 head e7440491. #132 carries the store-free liveness/readiness safety contract forward through resilient serving and the deployable env-bound health process. Continue exact-head CI/review on #132; do not merge #102 separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants