fix(api): restack health method contract on current main - #208
Merged
Conversation
📝 WalkthroughWalkthrough
Changes/health 메서드 계약
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/health-method-contract.test.ts`:
- Around line 20-23: Update the /health contract tests to assert
success-response trace_id, x-trace-id, x-latency-ms, cache-control, and
x-content-type-options headers, comparing trace_id in the body with the
corresponding header. Extend the 405 response assertions to verify its
trace_id/header relationship and x-latency-ms, while preserving the existing
body assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1b0ca443-1f9f-4790-bf97-a674d9a27eab
📒 Files selected for processing (2)
src/index.tstest/health-method-contract.test.ts
Comment on lines
+20
to
+23
| expect(await response.json()).toMatchObject({ | ||
| ok: true, | ||
| data: { name: "noema" }, | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
/health의 성공 및 오류 프로토콜 헤더를 모두 테스트하세요.
Line 20-23은 성공 응답의 trace_id, x-trace-id, x-latency-ms, cache-control, x-content-type-options를 확인하지 않습니다. Line 40-51은 405 응답의 x-latency-ms를 확인하지 않습니다. 이 계약이 제거되어도 현재 테스트는 통과할 수 있습니다.
성공 및 405 응답에서 본문의 trace_id와 헤더를 비교하고, 두 응답의 x-latency-ms를 검증하세요.
제안 수정
expect(response.status).toBe(200);
- expect(await response.json()).toMatchObject({
+ expect(response.headers.get("cache-control")).toBe("no-store");
+ expect(response.headers.get("x-content-type-options")).toBe("nosniff");
+ const traceId = response.headers.get("x-trace-id");
+ expect(traceId).toEqual(expect.any(String));
+ expect(response.headers.get("x-latency-ms")).toEqual(expect.any(String));
+ const body = await response.json() as Record<string, unknown>;
+ expect(body).toMatchObject({
ok: true,
data: { name: "noema" },
});
+ expect(body.trace_id).toBe(traceId);
...
const traceId = response.headers.get("x-trace-id");
expect(traceId).toEqual(expect.any(String));
+ expect(response.headers.get("x-latency-ms")).toEqual(expect.any(String));As per coding guidelines, test/**/*.ts must preserve assertions covering protocol contracts.
Also applies to: 40-51
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/health-method-contract.test.ts` around lines 20 - 23, Update the /health
contract tests to assert success-response trace_id, x-trace-id, x-latency-ms,
cache-control, and x-content-type-options headers, comparing trace_id in the
body with the corresponding header. Extend the 405 response assertions to verify
its trace_id/header relationship and x-latency-ms, while preserving the existing
body assertions.
Source: Coding guidelines
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fresh protected-main successor for historical #100 on exact base
1a8a77d67f61a2c61e88974be9206a91d19acdf4. No predecessor checks, reviews, scanner output, model judgment, or production evidence transfers.RCA -> RED -> GREEN
Protected
src/index.tsreturned a liveness success for/healthsolely by pathname and did not validate the request method. The intended public liveness contract exposes onlyGET /health; unsupported methods must not become false 200 liveness evidence.Exact RED head
8f13b389d7ebcef88ee33bba96281a1defa643e5added only the executable contract: GET remains successful, while HEAD/POST/PUT/PATCH/DELETE/OPTIONS require 405,Allow: GET, no-store/security headers, and trace consistency. Application CI run31564064301, job94012184630, checked out that exact head and failed inrelease verify; exact checkout, toolchain and install steps had already succeeded.Exact GREEN head
c4a92e3e2d00edb88612b3fd9c2a0b3d520d32a7applies only the narrow production remedy: reject non-GET/healthbefore liveness success through the existingApiError/standard error path. The pre-repair protectedsrc/index.tsblob on current main was byte-identical to historical #100's base blob (283527e4989b4f069673ec7f8dabc2e75998e685), so the production result reuses the previously reviewed exact replacement blob7093e381ea87cd29940d4e7ecb9517f5e2a5b443without importing any stale tree or predecessor evidence./ready,/exchange, rate limiting, OIDC/GitHub credential handling and all other runtime behavior remain unchanged.Exact-head technical proof
For unchanged GREEN head
c4a92e3e2d00edb88612b3fd9c2a0b3d520d32a7on unchanged live protected base1a8a77d67f61a2c61e88974be9206a91d19acdf4:ci31564230581: terminal success; exact checkout andrelease verifysucceeded;reviewer-ci31564230580: terminal success;Security Scan31564230600: terminal success;The repository release-verification contract retains the exact 100% owned-production coverage gate and package/security checks; successful
release verifyis current-head technical evidence, not production/release/acquisition evidence.Governance / authority boundary
Current observable Noema governance is organization ruleset
18794436, requiring central.github/workflows/security-scan.yml@refs/heads/mainon the default branch, with no bypass actors and no pull-request approval rule. Therefore no nonexistent independent approval is invented as a current merge blocker; checks, formal reviews, scanner/model evidence and production evidence remain separate.Historical #100 remains open until this current-main successor integrates and the two-file contract is proven protected truth. This change does not publish a release, deploy production, establish production KPI, choose an outbound license, or create acquisition readiness.
Summary by CodeRabbit
버그 수정
/health엔드포인트가GET요청만 허용하도록 변경되었습니다.405오류와Allow: GET안내가 제공됩니다.테스트