Skip to content

fix: require JSON media type for orchestrator health - #697

Merged
seonghobae merged 10 commits into
mainfrom
fix/orchestrator-health-media-type-20260913
Sep 13, 2026
Merged

fix: require JSON media type for orchestrator health#697
seonghobae merged 10 commits into
mainfrom
fix/orchestrator-health-media-type-20260913

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Finding

Noema's contextual-orchestrator /healthz verifier bounds response bytes, normalizes locked reader acquisition, uses fatal UTF-8 and duplicate-key JSON admission, and requires the exact { status: "ok", service: "contextual-orchestrator" } identity. It did not validate the successful response media type before accepting those bytes. A reverse proxy, error page, or other non-JSON representation containing the same JSON-shaped text could therefore satisfy identity admission despite not being an application/json health document.

This lane changes only Noema-owned consumption/admission of contextual-orchestrator health evidence. It does not move service/provider/model routing, credentials, outbound authority, quarantine/security authority, release/deployment authority, or foreign domain truth into Noema.

Test-first RED

Test-only exact 27fefd8c23846841660199dcb86775c164715966 added hostile coverage requiring a valid identity document served as text/plain; charset=utf-8 to fail closed with the stable diagnostic contextual-orchestrator health response content-type is not application/json. It also pins acceptance of application/json; charset=utf-8 so ordinary media-type parameters remain valid.

Hosted application CI 34727039322 reproduced the defect after typecheck: release tests failed because the non-JSON representation was accepted as a valid contextual-orchestrator identity.

Causal repair and verification findings

The minimal source repair validates the successful response Content-Type media type before body consumption and preserves the 65,536-byte ceiling, optional caller-supplied deadline with no invented default timeout, fixed retained buffer, best-effort cancellation, reader-lock lifecycle, fatal UTF-8, duplicate-key/JSON admission, and exact orchestrator identity.

Exact 1f5b17eac160beb47735a91938f4fc50daba1491 exposed a separate test-fixture defect in hosted CI 34727236581: 20 existing tests used otherwise-valid health response doubles without a JSON media type, so the strengthened admission correctly failed before those tests reached their intended byte-bound, timeout, identity, or cleanup assertions. The repair updates only those fixtures to satisfy the now-explicit media contract; the production guard is not weakened.

Exact 81c96108c5401b80afc441135a5a40e1545e2df3 then passed all 4,768 tests, but application CI 34727550279 correctly rejected the head because the new async cancellation rejection callback at scripts/lib/orchestrator-gateway.mjs:344 was not executed and owned statement/function coverage fell below 100%. The final test repair makes response.body.cancel() reject asynchronously under a non-JSON response and proves the already-decided media-type failure remains authoritative. It also pins the missing-Content-Type fail-closed path.

Exact-head GREEN

Final exact is 1e0ac2eac6d3172468c842cae957079305c33a1d.

  • application CI 34727701836: SUCCESS
  • reviewer-ci 34727701829: SUCCESS
  • Security Scan 34727701821: SUCCESS
  • patch-validator-image 34727701818: SUCCESS

Inline review threads are empty. No gate weakening, force-push, destructive rebase, self-approval, provider fallback, or foreign authority duplication.

Summary by CodeRabbit

  • 버그 수정

    • 오케스트레이터 상태 확인 응답이 application/json 형식인지 검증합니다.
    • JSON이 아닌 응답은 본문 처리를 중단하고 오류로 처리합니다.
    • application/json에 추가 매개변수가 포함된 형식은 계속 허용됩니다.
  • 테스트

    • 잘못된 미디어 타입, 누락된 헤더, 응답 취소 실패 등 관련 시나리오를 추가로 검증합니다.

@coderabbitai

coderabbitai Bot commented Sep 13, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 1fa57e73-b18c-40ed-bd69-5c920c38c24a

📥 Commits

Reviewing files that changed from the base of the PR and between fae9dc3 and 1e0ac2e.

📒 Files selected for processing (8)
  • scripts/lib/orchestrator-gateway.mjs
  • test/orchestrator-gateway-body-timeout.test.ts
  • test/orchestrator-gateway-bounded-healthz.test.ts
  • test/orchestrator-gateway-contract.test.ts
  • test/orchestrator-gateway-json-integrity.test.ts
  • test/orchestrator-gateway-residual-coverage.test.ts
  • test/orchestrator-gateway-secret-source.test.ts
  • test/orchestrator-gateway-stream-bound.test.ts

📝 Walkthrough

Walkthrough

verifyOrchestratorHealthz가 성공 응답의 Content-Typeapplication/json으로 제한합니다. 비JSON 응답은 본문 스트림을 취소한 뒤 오류를 발생시킵니다. 관련 mock과 검증 테스트가 갱신되었습니다.

Changes

오케스트레이터 healthz Content-Type 검증

Layer / File(s) Summary
healthz Content-Type 검사
scripts/lib/orchestrator-gateway.mjs
JSDoc이 JSON 응답 조건을 설명합니다. verifyOrchestratorHealthz는 상태 확인 후 Content-Type을 검사하고, 비JSON 응답의 본문 스트림을 취소한 뒤 지정된 오류를 발생시킵니다.
Content-Type 무결성 테스트
test/orchestrator-gateway-json-integrity.test.ts
비JSON 미디어 타입, Content-Type 누락, 비동기 취소 실패를 검증합니다. application/json에 파라미터가 포함된 경우를 허용하는지 검증합니다.
기존 테스트 응답 mock 조정
test/orchestrator-gateway-body-timeout.test.ts, test/orchestrator-gateway-bounded-healthz.test.ts, test/orchestrator-gateway-contract.test.ts, test/orchestrator-gateway-residual-coverage.test.ts, test/orchestrator-gateway-secret-source.test.ts, test/orchestrator-gateway-stream-bound.test.ts
기존 성공 응답 mock과 헤더 mock이 application/json Content-Type을 제공하도록 조정되었습니다. Content-Length 및 스트림 경계 검증은 기존 조건을 유지합니다.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: claude

✨ 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 fix/orchestrator-health-media-type-20260913

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.

@seonghobae
seonghobae marked this pull request as ready for review September 13, 2026 00:22

@seonghobae seonghobae 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.

Exact-head review at 1e0ac2eac6d3172468c842cae957079305c33a1d: the media-type admission is confined to Noema's contextual-orchestrator health consumer; prior byte/deadline/UTF-8/duplicate-key/reader-cleanup invariants remain intact. Hosted RED reproduced the original acceptance defect; subsequent fixture and coverage findings were repaired without weakening the production guard. All four exact-head required lanes are terminal SUCCESS and there are no inline review threads. This is a COMMENT-only review, not self-approval.

@seonghobae
seonghobae merged commit 80b2343 into main Sep 13, 2026
17 of 18 checks passed
@seonghobae
seonghobae deleted the fix/orchestrator-health-media-type-20260913 branch September 13, 2026 00:23
seonghobae added a commit that referenced this pull request Sep 13, 2026
…alth-media-authority-20260913

docs: converge protected #697 health media authority
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