Skip to content

fix(health): redact public readiness diagnostics - #70

Closed
seonghobae wants to merge 113 commits into
mainfrom
agent/redacted-healthz-readiness
Closed

fix(health): redact public readiness diagnostics#70
seonghobae wants to merge 113 commits into
mainfrom
agent/redacted-healthz-readiness

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Purpose

Harden the standalone /healthz trust boundary without removing trusted local operator diagnostics. The public endpoint redacts diagnostic detail and arbitrary component identities, rejects malformed readiness evidence without truth coercion, bounds PostgreSQL health work, suppresses the stdlib/Python Server fingerprint, serves bounded concurrent probes with finite read lifetime, makes broad listener exposure explicit, validates listener input before socket construction, and removes shell interpretation from the bundled container readiness command path.

Bounded implementation

  • check_health() retains detailed local/operator diagnostics and requires exactly one observation for every required component.
  • Database is_ready values are accepted only when their exact Python type is bool; malformed values are never truth-coerced.
  • public_health_report() exposes only top-level ready plus component and is_ready for the fixed required-component allow-list; diagnostic detail and unknown component identities remain local.
  • Malformed or contradictory public readiness fails closed to HTTP 503.
  • PostgreSQL readiness applies a parameterized transaction-local 4,000 ms statement_timeout before pg_llm_batch_health_check().
  • /healthz preserves 200/503 semantics, unrelated paths remain 404, responses use Cache-Control: no-store, and the default stdlib/Python Server fingerprint is omitted.
  • The threaded listener has bounded admission and a finite request-read lifetime; excess connections close before worker/database work and admission slots are released after completion or thread-start failure.
  • Direct serve-healthz invocation defaults to loopback 127.0.0.1; the bundled container opts into 0.0.0.0 explicitly.
  • Listener inputs fail closed before socket creation: host is an exact non-empty string without whitespace or ASCII C0/DEL controls, and port is a non-boolean integer in 1..65535.
  • The bundled component image uses Docker exec-form JSON for both the readiness server and healthcheck at fixed default port 8080; PG_LLM_BATCH_HEALTH_PORT is not shell-expanded through sh -c.
  • ADR 0014, AGENTS, CHANGELOG, focused doctoring, and deterministic regressions retain the exposure/resource/input-validation/no-shell contract.
  • No schema, credential, provider, release, model, or background-writer contract change.

The branch retains test-first RED→GREEN provenance for redaction, malformed/duplicate readiness evidence, exact database booleans, SQL statement bounds, runtime-header suppression, listener concurrency/read bounds, loopback defaults, listener validation, and the no-shell container-command boundary. Predecessor-head evidence does not transfer after a source-head change.

Current exact state

  • Head: ab38b80240fb8705122ee347c421316599175119.
  • Independently resolved protected main: bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10.
  • GitHub reports Draft and mergeable.
  • CI 31361759779: completed / success.
  • Security Scan 31361759700: completed / success.
  • SAST Semgrep 31361759715: completed / success.
  • Formal reviews: none at the latest inspection.
  • Unresolved inline review threads: zero.

Repository CI still predates protected-main exact-source governance #88, so these successful PR-triggered runs are staged integration evidence rather than final source-head acceptance.

Dependency and merge boundary

Keep Draft until #88 reaches protected main and the read-only central review-evidence prerequisite materially changes. Reconcile this branch onto the resulting protected base only when integration correctness requires it, then regenerate final exact-source CI, security, dependency, packaging, provenance, semantic-review, and branch-policy evidence. Do not churn a clean source head solely to retrigger an unchanged external reviewer path.

Merge only with zero valid unresolved findings, live repository policy/protection satisfied, every required exact-current-head gate passing, and current review/approval evidence where live policy actually requires it. The protected code-owner gate remains disabled/on hold for the solo-maintainer state and is not a universal approval requirement. Predecessor, stale-base, queued, pending, cancelled, skipped-required, absent, neutral-required, status-only, author-only, synthetic-only, infrastructure-only, rate-limited, or failed evidence is not acceptance.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 59 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 24df967f-e7a5-484a-8e63-fdd73df3b916

📥 Commits

Reviewing files that changed from the base of the PR and between 7a6a021 and 02d0f8f.

📒 Files selected for processing (19)
  • AGENTS.md
  • CHANGELOG.md
  • Dockerfile
  • docs/adr/0014-public-healthz-readiness.md
  • docs/adr/0016-health-request-read-timeout.md
  • docs/doctoring/health-listener-input-validation.md
  • docs/doctoring/health-request-read-timeout.md
  • docs/doctoring/public-healthz-readiness.md
  • pg_llm_batch/cli.py
  • pg_llm_batch/health.py
  • tests/test_health.py
  • tests/test_health_cli_network_boundary.py
  • tests/test_health_concurrency_documentation.py
  • tests/test_health_database_boolean_boundary.py
  • tests/test_health_dsn_authority.py
  • tests/test_health_dsn_authority_documentation.py
  • tests/test_health_listener_validation.py
  • tests/test_health_public_documentation.py
  • tests/test_health_server_concurrency.py
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/redacted-healthz-readiness

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@opencode-agent @cwl-noema-review

Review-only request for exact source head 2706029f5497e51b8bb7099765f8a9432b100d24 against exact protected main tip bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10. Independently verify the /healthz fixed allow-list confidentiality boundary, that local check_health() diagnostics remain intact, transaction-local parameterized PostgreSQL statement_timeout ordering and scope, 200/503/404 compatibility, Cache-Control: no-store, fail-closed behavior for unexpected health-report shapes, 100% production statement/branch and public-docstring evidence, and the absence of schema, credential, release, or generated-artifact changes. Treat current CI as staged synthetic-merge evidence only until PR #69’s exact-source CI governance prerequisite and its read-only central prerequisite integrate. Do not mutate the branch, create repair workflows, mark ready, merge, release, weaken checks, or bypass repository protection.

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10"
head="2706029f5497e51b8bb7099765f8a9432b100d24"

printf '%s\n' '== revisions =='
git rev-parse HEAD
git cat-file -t "$base"
git cat-file -t "$head"
git merge-base --is-ancestor "$base" "$head" && echo "base_is_ancestor=yes" || echo "base_is_ancestor=no"

printf '%s\n' '== changed files and change types =='
git diff --name-status "$base" "$head"

printf '%s\n' '== changed Python symbols =='
git diff --unified=0 "$base" "$head" -- '*.py'

printf '%s\n' '== Python file outlines =='
fd -e py . | xargs -r ast-grep outline --items all --type class,function --view compact

printf '%s\n' '== focused health-related references =='
rg -n -C 4 -g '*.py' \
  'public_health_report|check_health|HEALTH_STATEMENT_TIMEOUT_MILLISECONDS|statement_timeout|set_config|healthz|Cache-Control|no-store' .

printf '%s\n' '== health-related test references =='
rg -n -C 3 -g '*test*.py' -g '*.py' \
  'public_health_report|check_health|statement_timeout|healthz|Cache-Control|no-store|unexpected' .

printf '%s\n' '== schema, credentials, release, and generated artifact indicators in diff =='
git diff --name-only "$base" "$head" | rg -n \
  '(?i)(migration|schema|\.sql$|credential|secret|token|key|version|release|dist/|build/|\.lock$|generated)' \
  || true

Length of output: 14750


🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10"
head="2706029f5497e51b8bb7099765f8a9432b100d24"

printf '%s\n' '== diff whitespace validation =='
git diff --check "$base" "$head"

printf '%s\n' '== health module outline =='
ast-grep outline pg_llm_batch/health.py --items all --view expanded

printf '%s\n' '== health module at exact head =='
git show "$head:pg_llm_batch/health.py" | nl -ba

printf '%s\n' '== health tests at exact head =='
git show "$head:tests/test_health.py" | nl -ba

printf '%s\n' '== malformed report / fail-closed test cases =='
git grep -n -E 'public_health_report|unexpected|malformed|invalid|TypeError|KeyError|fail.closed|fail-closed' "$head" -- \
  ':(glob)**/*.py' ':(glob)**/*.md' || true

printf '%s\n' '== changed documentation at exact head =='
for file in AGENTS.md CHANGELOG.md docs/adr/0014-public-healthz-readiness.md docs/doctoring/public-healthz-readiness.md; do
  printf '\n--- %s ---\n' "$file"
  git show "$head:$file" | nl -ba
done

printf '%s\n' '== changed files by extension =='
git diff --name-only "$base" "$head" | awk '
  { n=$0; sub(/^.*\//, "", n); if (n !~ /\./) ext="[no extension]"; else { sub(/^.*\./, ".", n); ext=n } count[ext]++ }
  END { for (ext in count) print ext, count[ext] }
' | sort

Length of output: 2233


🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10"
head="2706029f5497e51b8bb7099765f8a9432b100d24"

show_numbered() {
  git show "$head:$1" | awk '{ printf "%6d  %s\n", NR, $0 }'
}

printf '%s\n' '== health module at exact head =='
show_numbered pg_llm_batch/health.py

printf '%s\n' '== health tests at exact head =='
show_numbered tests/test_health.py

printf '%s\n' '== documentation-contract tests at exact head =='
show_numbered tests/test_health_public_documentation.py

printf '%s\n' '== malformed report / fail-closed references =='
git grep -n -E 'public_health_report|unexpected|malformed|invalid|TypeError|KeyError|fail.closed|fail-closed' "$head" -- \
  ':(glob)**/*.py' ':(glob)**/*.md' || true

printf '%s\n' '== changed documentation at exact head =='
for file in AGENTS.md CHANGELOG.md docs/adr/0014-public-healthz-readiness.md docs/doctoring/public-healthz-readiness.md; do
  printf '\n--- %s ---\n' "$file"
  show_numbered "$file"
done

printf '%s\n' '== changed files by extension =='
git diff --name-only "$base" "$head" | awk '
  {
    name=$0
    if (name !~ /\./) extension="[no extension]"
    else { sub(/^.*\./, ".", name); extension=name }
    count[extension]++
  }
  END { for (extension in count) print extension, count[extension] }
' | sort

Length of output: 71028


🏁 Script executed:

#!/bin/bash
set -euo pipefail

head="2706029f5497e51b8bb7099765f8a9432b100d24"
repo="ContextualWisdomLab/pg-llm-batch"

printf '%s\n' '== coverage and test configuration files =='
fd -HI -t f '^(pyproject\.toml|pytest\.ini|setup\.cfg|tox\.ini|\.coveragerc)$' . \
  | sort \
  | while IFS= read -r file; do
      printf '\n--- %s ---\n' "$file"
      awk '{ printf "%6d  %s\n", NR, $0 }' "$file"
    done

printf '%s\n' '== test workflow coverage/docstring references =='
if [ -d .github ]; then
  rg -n -i -C 3 \
    'coverage|branch|docstring|interrogate|pytest|ruff|health_public_documentation' \
    .github
fi

printf '%s\n' '== GitHub check runs on exact source head =='
gh api \
  -H 'Accept: application/vnd.github+json' \
  "/repos/${repo}/commits/${head}/check-runs?per_page=100" \
  --jq '.check_runs[] | [.name, .status, .conclusion, .head_sha, .details_url] | `@tsv`' \
  | sort

printf '%s\n' '== GitHub combined commit status on exact source head =='
gh api \
  -H 'Accept: application/vnd.github+json' \
  "/repos/${repo}/commits/${head}/status" \
  --jq '.state, (.statuses[]? | [.context, .state, .target_url] | `@tsv`)' \
  || true

printf '%s\n' '== pull request head/base metadata =='
gh pr view 70 --repo "$repo" \
  --json number,isDraft,headRefOid,baseRefOid,mergeStateStatus,statusCheckRollup \
  --jq '{number, isDraft, headRefOid, baseRefOid, mergeStateStatus, checks: [.statusCheckRollup[] | {name, status, conclusion, headSha: .commit.oid}]}'

Length of output: 16271


@seonghobae Review completed for source head 2706029f5497e51b8bb7099765f8a9432b100d24 against base bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10.

I found no blocking issue.

Verified items:

  • public_health_report() constructs a new fixed allow-list response. It exposes only ready, component, and is_ready. Extra top-level and component fields do not cross the HTTP boundary.
  • check_health() still returns local diagnostic detail for missing dependencies and database exceptions.
  • set_config('statement_timeout', %s, true) uses a parameterized 4,000 millisecond value. It executes before pg_llm_batch_health_check(). The true scope is transaction-local.
  • /healthz returns 200 when ready and 503 when not ready. / and other unrelated paths return 404.
  • /healthz includes Cache-Control: no-store.
  • The tests cover redaction of secret-like text, internal hostnames, debug fields, and unknown fields. They also cover SQL call ordering, HTTP status behavior, 404, and the cache header.
  • The diff contains only pg_llm_batch/health.py, health tests, and operator documentation. It contains no schema, migration, credential, release/version, lockfile, or generated-artifact changes.
  • The repository configuration requires branch coverage and 100% coverage and public-docstring thresholds. The exposed checks on the exact source head completed successfully, including unit tests, coverage/docstrings/package, container builds, SAST, and security scans.

The fixed allow-list is fail-closed for future diagnostic fields because it constructs the public object from approved fields instead of removing known sensitive fields.

I did not treat the successful check runs as proof of protected integration. PR #69, its central prerequisite, reconciliation onto protected main, and final required exact-source-head gates remain required before merge eligibility.

⚠️ Action not completed

Head commit changed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@opencode-agent @cwl-noema-review

Review-only request for exact current source head d83f28e438f757f36cf14ffb5556f71b5bdc01be against exact protected main tip bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10. Independently verify the fixed allow-list /healthz confidentiality boundary, strict non-coercive malformed-readiness handling, sanitized HTTP 200/503 status derivation, preserved local check_health() diagnostics, parameterized transaction-local PostgreSQL statement_timeout ordering, Cache-Control: no-store, 100% production statement/branch coverage and public-docstring evidence, package/container gates, and absence of schema, credential, release, or generated-artifact changes. CI 31250148154, Security Scan 31250148153, and SAST 31250148159 are successful staged evidence on the current branch; current main still checks out the synthetic merge ref, so do not treat that evidence as final exact-source merge proof. PR #69 and read-only central prerequisite .github#782 remain integration blockers. Do not mutate the branch, create a repair workflow, mark ready, merge, release, weaken checks, or bypass repository protection.

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10"
head="d83f28e438f757f36cf14ffb5556f71b5bdc01be"
repo="ContextualWisdomLab/pg-llm-batch"

echo "== Revision identity =="
printf 'checked_out='; git rev-parse HEAD
printf 'base_type='; git cat-file -t "$base"
printf 'head_type='; git cat-file -t "$head"
git merge-base --is-ancestor "$base" "$head" && echo "base_ancestor_of_head=yes" || echo "base_ancestor_of_head=no"

echo "== Changed files =="
git diff --name-status "$base" "$head"
echo "== Whitespace validation =="
git diff --check "$base" "$head"

echo "== Focused health implementation outline =="
fd -t f -e py . | rg '(^|/)(health|test_health).*\.py$' | while IFS= read -r file; do
  echo "--- $file ---"
  ast-grep outline "$file" --items all --view compact
done

echo "== Exact-head health source and tests =="
for file in pg_llm_batch/health.py tests/test_health.py; do
  if git cat-file -e "$head:$file" 2>/dev/null; then
    echo "--- $file ---"
    git show "$head:$file" | nl -ba
  fi
done

echo "== Relevant exact-head references =="
git grep -n -E \
  'public_health_report|check_health|HEALTH_STATEMENT_TIMEOUT_MILLISECONDS|statement_timeout|set_config|pg_llm_batch_health_check|healthz|Cache-Control|no-store|malformed|unexpected|is_ready|ready' \
  "$head" -- ':(glob)**/*.py' ':(glob)pyproject.toml' ':(glob).github/**' || true

echo "== Coverage, docstring, package, and container configuration =="
fd -HI -t f '^(pyproject\.toml|pytest\.ini|setup\.cfg|tox\.ini|\.coveragerc|Dockerfile.*|docker-compose.*|compose\.ya?ml)$' . \
  | sort \
  | while IFS= read -r file; do
      echo "--- $file ---"
      nl -ba "$file"
    done

echo "== Disallowed change indicators =="
git diff --name-only "$base" "$head" | rg -n -i \
  '(migration|schema|\.sql$|credential|secret|token|key|version|release|dist/|build/|\.lock$|generated)' || true

echo "== Exact-head check runs =="
gh api -H 'Accept: application/vnd.github+json' \
  "/repos/${repo}/commits/${head}/check-runs?per_page=100" \
  --jq '.check_runs[] | [.name, .status, .conclusion, .head_sha, .details_url] | `@tsv`' \
  | sort || true

echo "== PR metadata and check association =="
gh pr view 70 --repo "$repo" \
  --json isDraft,headRefOid,baseRefOid,mergeStateStatus,statusCheckRollup \
  --jq '{isDraft, headRefOid, baseRefOid, mergeStateStatus, checks: [.statusCheckRollup[] | {name, status, conclusion, headSha: .commit.oid}]}' || true

Length of output: 3371


🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10"
head="d83f28e438f757f36cf14ffb5556f71b5bdc01be"
repo="ContextualWisdomLab/pg-llm-batch"

echo "== Focused health implementation outline =="
for file in pg_llm_batch/health.py tests/test_health.py tests/test_health_public_documentation.py; do
  if [ -f "$file" ]; then
    echo "--- $file ---"
    ast-grep outline "$file" --items all --view signatures
  fi
done

echo "== Exact-head health source and tests =="
for file in pg_llm_batch/health.py tests/test_health.py tests/test_health_public_documentation.py; do
  if git cat-file -e "$head:$file" 2>/dev/null; then
    echo "--- $file ---"
    git show "$head:$file" | nl -ba
  fi
done

echo "== Exact-head health diff =="
git diff --find-renames --unified=80 "$base" "$head" -- \
  pg_llm_batch/health.py tests/test_health.py tests/test_health_public_documentation.py

echo "== Relevant exact-head references =="
git grep -n -E \
  'public_health_report|check_health|HEALTH_STATEMENT_TIMEOUT_MILLISECONDS|statement_timeout|set_config|pg_llm_batch_health_check|healthz|Cache-Control|no-store|malformed|unexpected|is_ready|ready' \
  "$head" -- ':(glob)**/*.py' ':(glob)pyproject.toml' ':(glob).github/**' || true

echo "== Changed documentation =="
for file in AGENTS.md CHANGELOG.md docs/adr/0014-public-healthz-readiness.md docs/doctoring/public-healthz-readiness.md; do
  echo "--- $file ---"
  git show "$head:$file" | nl -ba
done

echo "== Coverage, docstring, package, and container configuration =="
fd -HI -t f '^(pyproject\.toml|pytest\.ini|setup\.cfg|tox\.ini|\.coveragerc|Dockerfile.*|docker-compose.*|compose\.ya?ml)$' . \
  | sort \
  | while IFS= read -r file; do
      echo "--- $file ---"
      nl -ba "$file"
    done

echo "== Exact-head check runs =="
gh api -H 'Accept: application/vnd.github+json' \
  "/repos/${repo}/commits/${head}/check-runs?per_page=100" \
  --jq '.check_runs[] | [.name, .status, .conclusion, .head_sha, .details_url] | `@tsv`' \
  | sort || true

echo "== Exact-head combined status and PR check association =="
gh api -H 'Accept: application/vnd.github+json' \
  "/repos/${repo}/commits/${head}/status" \
  --jq '.state, (.statuses[]? | [.context, .state, .target_url] | `@tsv`)' || true
gh pr view 70 --repo "$repo" \
  --json isDraft,headRefOid,baseRefOid,mergeStateStatus,statusCheckRollup \
  --jq '{isDraft, headRefOid, baseRefOid, mergeStateStatus, checks: [.statusCheckRollup[] | {name, status, conclusion, headSha: .commit.oid}]}' || true

echo "== Changed-file category checks =="
git diff --name-only "$base" "$head" | sort
git diff --name-only "$base" "$head" | rg -n -i \
  '(migration|schema|\.sql$|credential|secret|token|key|version|release|dist/|build/|\.lock$|generated)' || true

Length of output: 5335


🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10"
head="d83f28e438f757f36cf14ffb5556f71b5bdc01be"
repo="ContextualWisdomLab/pg-llm-batch"

numbered_head_file() {
  local file="$1"
  echo "--- $file ---"
  git show "$head:$file" | awk '{printf "%5d  %s\n", NR, $0}'
}

echo "== Health implementation and tests at exact head =="
numbered_head_file pg_llm_batch/health.py
numbered_head_file tests/test_health.py
numbered_head_file tests/test_health_public_documentation.py

echo "== Focused exact-head diff =="
git diff --unified=100 "$base" "$head" -- \
  pg_llm_batch/health.py tests/test_health.py tests/test_health_public_documentation.py

echo "== Authoritative changed documentation =="
for file in AGENTS.md CHANGELOG.md docs/adr/0014-public-healthz-readiness.md docs/doctoring/public-healthz-readiness.md; do
  numbered_head_file "$file"
done

echo "== Coverage, docstring, package, and container configuration =="
fd -HI -t f '^(pyproject\.toml|pytest\.ini|setup\.cfg|tox\.ini|\.coveragerc|Dockerfile.*|docker-compose.*|compose\.ya?ml)$' . \
  | sort \
  | while IFS= read -r file; do
      echo "--- $file ---"
      awk '{printf "%5d  %s\n", NR, $0}' "$file"
    done

echo "== Relevant exact-head source references =="
git grep -n -E \
  'public_health_report|check_health|HEALTH_STATEMENT_TIMEOUT_MILLISECONDS|statement_timeout|set_config|pg_llm_batch_health_check|healthz|Cache-Control|no-store|malformed|unexpected|is_ready|ready' \
  "$head" -- ':(glob)**/*.py' ':(glob)pyproject.toml' ':(glob).github/**' || true

echo "== Exact-head check runs =="
gh api -H 'Accept: application/vnd.github+json' \
  "/repos/${repo}/commits/${head}/check-runs?per_page=100" \
  --jq '.check_runs[] | [.name, .status, .conclusion, .head_sha, .details_url] | `@tsv`' \
  | sort || true

echo "== Exact-head combined status =="
gh api -H 'Accept: application/vnd.github+json' \
  "/repos/${repo}/commits/${head}/status" \
  --jq '.state, (.statuses[]? | [.context, .state, .target_url] | `@tsv`)' || true

echo "== Pull request check association =="
gh pr view 70 --repo "$repo" \
  --json isDraft,headRefOid,baseRefOid,mergeStateStatus,statusCheckRollup \
  --jq '{isDraft, headRefOid, baseRefOid, mergeStateStatus, checks: [.statusCheckRollup[] | {name, status, conclusion, headSha: .commit.oid}]}' || true

echo "== Changed-file category checks =="
git diff --name-only "$base" "$head" | sort
git diff --name-only "$base" "$head" | rg -n -i \
  '(migration|schema|\.sql$|credential|secret|token|key|version|release|dist/|build/|\.lock$|generated)' || true

Length of output: 118660


@seonghobae Review completed for source head d83f28e438f757f36cf14ffb5556f71b5bdc01be against protected main tip bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10.

I found no blocking implementation issue.

Verified items:

  • public_health_report() creates a new fixed allow-list object. It exposes only top-level ready and component component and is_ready.
  • The projection drops detail, exception text, debug fields, internal fields, and future unlisted fields.
  • The projection accepts only exact bool, list, dict, and str shapes. Malformed data returns {"ready": false, "components": []}.
  • serve_healthz() selects 200 or 503 from the sanitized projection. It does not use raw-report truthiness.
  • /healthz and /healthz/ retain readiness behavior. / and unrelated paths return 404.
  • /healthz emits Cache-Control: no-store.
  • check_health() retains detailed local diagnostics, including dependency and database exception reasons.
  • The PostgreSQL timeout uses parameterized set_config('statement_timeout', %s, true) with 4,000 milliseconds. It executes before pg_llm_batch_health_check(). The true argument makes the setting transaction-local.
  • The diff contains no schema, migration, credential, release/version, lockfile, or generated-artifact changes.
  • The exact-head checks show success for the Python 3.10, 3.12, and 3.14 unit jobs; coverage/docstrings/lint/package; container builds; Semgrep; Trivy; dependency review; and CodeQL.

The repository configuration enforces branch coverage, fail_under = 100, and 100% public docstrings. The successful coverage/docstrings/package gate supports the stated quality result. The check data available to this review did not include the raw numeric coverage and docstring reports.

Merge evidence remains incomplete. The exact-head strix check is still in_progress. PR #69, .github#782, reconciliation onto the actual protected main, and all final required exact-source-head gates remain blockers. I did not treat synthetic-merge evidence, the combined CodeRabbit status, or unproven branch-protection state as final merge proof.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review exact current head 879b8ef02d3e1b724b738f0f4173a5d3068f3cd0. Focus on the new Docker command-authority boundary: readiness server and healthcheck are exec-form JSON at fixed default port 8080, with no shell expansion of environment-controlled health-port text. Also verify the existing listener-input, public-redaction, resource-bound, and rollback contracts remain intact. Predecessor-head review evidence is historical.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10"
head="879b8ef02d3e1b724b738f0f4173a5d3068f3cd0"
repo="ContextualWisdomLab/pg-llm-batch"

show_head() {
  local file="$1"
  printf '\n--- %s @ %s ---\n' "$file" "$head"
  git show "$head:$file" | awk '{ printf "%5d  %s\n", NR, $0 }'
}

printf '%s\n' '== revision identity =='
printf 'checked_out='; git rev-parse HEAD
printf 'base='; git rev-parse "$base"
printf 'head='; git rev-parse "$head"
git merge-base --is-ancestor "$base" "$head" && echo 'base_ancestor_of_head=yes' || echo 'base_ancestor_of_head=no'

printf '%s\n' '== changed files =='
git diff --name-status "$base" "$head"
printf '%s\n' '== whitespace validation =='
git diff --check "$base" "$head"

printf '%s\n' '== candidate Docker and health files =='
fd -HI -t f . | rg '(^|/)(Dockerfile[^/]*|health\.py|test_health[^/]*\.py|pyproject\.toml|AGENTS\.md|CHANGELOG\.md)$|(^|/)docs/.*health.*\.md$' | sort

printf '%s\n' '== exact-head Dockerfiles and health code =='
while IFS= read -r file; do
  case "$file" in
    *Dockerfile*|pg_llm_batch/health.py|tests/test_health.py|tests/test_health_public_documentation.py|pyproject.toml|AGENTS.md|CHANGELOG.md|docs/adr/0014-public-healthz-readiness.md|docs/doctoring/public-healthz-readiness.md)
      if git cat-file -e "$head:$file" 2>/dev/null; then show_head "$file"; fi
      ;;
  esac
done < <(fd -HI -t f . | sort)

printf '%s\n' '== health module AST outline =='
git show "$head:pg_llm_batch/health.py" > /tmp/health.py
ast-grep outline /tmp/health.py --items all --view expanded

printf '%s\n' '== focused diff =='
git diff --unified=100 "$base" "$head" -- \
  pg_llm_batch/health.py tests/test_health.py tests/test_health_public_documentation.py \
  Dockerfile AGENTS.md CHANGELOG.md docs/adr/0014-public-healthz-readiness.md \
  docs/doctoring/public-healthz-readiness.md

printf '%s\n' '== exact-head Docker, CLI, listener, and health references =='
git grep -n -E \
  'CMD|HEALTHCHECK|ENTRYPOINT|sh -c|PG_LLM_BATCH_HEALTH_PORT|curl|8080|serve-healthz|--host|--port|validate|public_health_report|check_health|REQUIRED_COMPONENTS|statement_timeout|set_config|ThreadingMixIn|BoundedSemaphore|Semaphore|request_timeout|send_response|Cache-Control|no-store' \
  "$head" -- ':(glob)**/*.py' ':(glob)Dockerfile*' ':(glob)pyproject.toml' ':(glob)**/*.md' || true

printf '%s\n' '== all serve-healthz call sites and Docker instructions =='
git grep -n -E 'serve-healthz|^(CMD|ENTRYPOINT|HEALTHCHECK)' \
  "$head" -- ':(glob)**/*' || true

printf '%s\n' '== out-of-scope changed-file indicators =='
git diff --name-only "$base" "$head" | sort
git diff --name-only "$base" "$head" | rg -n -i \
  '(migration|schema|\.sql$|credential|secret|token|key|provider|model|version|release|dist/|build/|\.lock$|generated|writer)' || true

printf '%s\n' '== exact-head check runs and PR metadata =='
gh api -H 'Accept: application/vnd.github+json' \
  "/repos/${repo}/commits/${head}/check-runs?per_page=100" \
  --jq '.check_runs[] | [.name, .status, .conclusion, .head_sha] | `@tsv`' | sort || true
gh pr view 70 --repo "$repo" \
  --json isDraft,headRefOid,baseRefOid,mergeStateStatus,statusCheckRollup \
  --jq '{isDraft,headRefOid,baseRefOid,mergeStateStatus,checks:[.statusCheckRollup[]|{name,status,conclusion,headSha:.commit.oid}]}' || true

Length of output: 5239


🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10"
head="879b8ef02d3e1b724b738f0f4173a5d3068f3cd0"
repo="ContextualWisdomLab/pg-llm-batch"

show_head() {
  local file="$1"
  printf '\n--- %s @ %s ---\n' "$file" "$head"
  git show "$head:$file" | awk '{ printf "%5d  %s\n", NR, $0 }'
}

printf '%s\n' '== exact-head implementation =='
for file in \
  Dockerfile \
  pg_llm_batch/cli.py \
  pg_llm_batch/health.py \
  tests/test_health.py \
  tests/test_health_cli_network_boundary.py \
  tests/test_health_listener_validation.py \
  tests/test_health_server_concurrency.py \
  tests/test_health_public_documentation.py \
  tests/test_health_database_boolean_boundary.py
do
  git cat-file -e "$head:$file"
  show_head "$file"
done

printf '%s\n' '== health AST outline =='
git show "$head:pg_llm_batch/health.py" > /tmp/health.py
ast-grep outline /tmp/health.py --items all --view expanded

printf '%s\n' '== focused diff =='
git diff --unified=100 "$base" "$head" -- \
  Dockerfile pg_llm_batch/cli.py pg_llm_batch/health.py \
  tests/test_health.py tests/test_health_cli_network_boundary.py \
  tests/test_health_listener_validation.py tests/test_health_server_concurrency.py \
  tests/test_health_public_documentation.py tests/test_health_database_boolean_boundary.py

printf '%s\n' '== command authority and listener reference set =='
git grep -n -E \
  '^(CMD|ENTRYPOINT|HEALTHCHECK)|sh -c|PG_LLM_BATCH_HEALTH_PORT|curl|8080|serve-healthz|--host|--port|_validate|validate_|public_health_report|check_health|REQUIRED_COMPONENTS|statement_timeout|set_config|ThreadingMixIn|BoundedSemaphore|Semaphore|request_timeout|send_response|Cache-Control|no-store' \
  "$head" -- ':(glob)**/*.py' ':(glob)Dockerfile*' ':(glob)pyproject.toml' ':(glob)**/*.md' || true

printf '%s\n' '== exact-head checks =='
gh api -H 'Accept: application/vnd.github+json' \
  "/repos/${repo}/commits/${head}/check-runs?per_page=100" \
  --jq '.check_runs[] | [.name, .status, .conclusion, .head_sha] | `@tsv`' | sort || true
gh pr view 70 --repo "$repo" \
  --json isDraft,headRefOid,baseRefOid,mergeStateStatus,statusCheckRollup \
  --jq '{isDraft,headRefOid,baseRefOid,mergeStateStatus,checks:[.statusCheckRollup[]|{name,status,conclusion,headSha:.commit.oid}]}' || true

Length of output: 184747


@seonghobae I found one blocking current-source defect.

pg_llm_batch/health.py, Line 127: public_health_report() calls report.get(...) before it verifies that report is a dict.

If check_health() returns None, a list, or a string, Line 127 raises AttributeError. serve_healthz() does not catch this exception. /healthz then does not return the fixed fail-closed body with HTTP 503.

Validate type(report) is dict before both .get() calls. Return _not_ready_public_health_report() for every other type. Add direct and HTTP-level regressions for None, a list, and a string.

I verified the requested Docker command-authority boundary:

  • Dockerfile Line 43 uses exec-form JSON for HEALTHCHECK.
  • Dockerfile Line 45 uses exec-form JSON for CMD.
  • Both commands use the fixed image port 8080.
  • The Dockerfile has no PG_LLM_BATCH_HEALTH_PORT reference.
  • The readiness command has no sh -c boundary or shell interpolation.
  • The container explicitly supplies --host 0.0.0.0.
  • A deployment that requires another port must override both the executable command and healthcheck.

I also verified the existing source contracts:

  • Direct CLI and library invocation default to 127.0.0.1.
  • Listener host and port validation occurs before socket construction.
  • Listener validation rejects whitespace, C0 controls, DEL, boolean ports, and ports outside 1..65535.
  • The public projection exposes only ready, component, and is_ready for REQUIRED_COMPONENTS.
  • Local diagnostics, exception text, provider fields, and unrecognized component names remain outside the HTTP response.
  • Missing or duplicate required components fail closed.
  • Database readiness accepts only exact bool values.
  • The parameterized transaction-local statement_timeout is set to 4,000 ms before pg_llm_batch_health_check().
  • The listener admits at most 32 concurrent requests, closes excess connections before worker work, and releases slots after completion or thread-start failure.
  • The handler sets a five-second request-read timeout.
  • /healthz derives 200 or 503 from the sanitized projection. Other paths return 404.
  • /healthz sends Cache-Control: no-store.
  • The custom send_response() retains Date and omits the default Server fingerprint.
  • The diff contains no schema, credential, provider, model, release, generated-artifact, or background-writer change.

The exact-head checks are successful for unit tests, coverage/docstrings/lint/package, container builds, and security scans. strix remains in progress. These checks do not change the source defect above.

You are interacting with an AI system.

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.

1 participant