feat(api): enforce explicit v1 negotiation contract - #379
Conversation
|
Important Review available on request
Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesAPI 버전 협상
Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: 🔵 Low · up to The new API version-negotiation behavior may be skipped when an earlier request filter ends processing, allowing unsupported versions through or omitting the response version header. The PR is mergeable with explicit owner awareness and follow-up to define ordering and add full-chain coverage. Sequence Diagram(s)sequenceDiagram
participant Client
participant ApiVersionWebFilter
participant WebFilterChain
Client->>ApiVersionWebFilter: API 요청과 버전 헤더 전달
ApiVersionWebFilter->>ApiVersionWebFilter: 버전 헤더 검증
alt 헤더 없음, 빈 값 또는 v1
ApiVersionWebFilter->>WebFilterChain: 요청 전달
WebFilterChain-->>Client: 현재 버전 응답 헤더와 처리 결과
else 지원되지 않는 버전
ApiVersionWebFilter-->>Client: 400 JSON 오류 응답
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
|
@opencode-agent review Review exact head Review read-only; preserve independent reviewer identity. Do not mutate the branch, synthesize author approval, weaken protection, or merge. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
expected='7b98171aff415f05e31df0de37e230f9d39d0758'
actual="$(git rev-parse HEAD)"
printf 'HEAD=%s\nEXPECTED=%s\n' "$actual" "$expected"
if [ "$actual" != "$expected" ]; then
echo 'ERROR: The checked-out revision does not match the requested head.' >&2
exit 2
fi
echo '--- Changed files at exact head ---'
git diff-tree --no-commit-id --name-status -r "$expected"
echo '--- Exact-head diff ---'
git diff "${expected}^" "$expected" -- \
src/main/java/com/clearfolio/viewer/config/ApiVersionWebFilter.java \
src/test/java/com/clearfolio/viewer/config/ApiVersionWebFilterTest.java
echo '--- Filter outline ---'
ast-grep outline src/main/java/com/clearfolio/viewer/config/ApiVersionWebFilter.java \
--items all --view expanded
echo '--- Filter source ---'
cat -n src/main/java/com/clearfolio/viewer/config/ApiVersionWebFilter.java
echo '--- Filter test outline ---'
ast-grep outline src/test/java/com/clearfolio/viewer/config/ApiVersionWebFilterTest.java \
--items all --view expanded
echo '--- Filter test source ---'
cat -n src/test/java/com/clearfolio/viewer/config/ApiVersionWebFilterTest.java
echo '--- Filter registration, ordering, and API path references ---'
rg -n -C 4 --glob '*.java' \
'(ApiVersionWebFilter|WebFilter\b|`@Order`\b|/api/v1|/api/)' src/main/java
echo '--- Exact-head GitHub checks ---'
gh api "repos/ContextualWisdomLab/clearfolio/commits/${expected}/check-runs?per_page=100" \
--jq '.check_runs[] | [.name, .status, .conclusion] | `@tsv`' || true
echo '--- Exact-head GitHub status contexts ---'
gh api "repos/ContextualWisdomLab/clearfolio/commits/${expected}/status" \
--jq '.statuses[] | [.context, .state] | `@tsv`' || trueLength of output: 41270 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo='ContextualWisdomLab/clearfolio'
pr='379'
head='7b98171aff415f05e31df0de37e230f9d39d0758'
echo '--- Pull request metadata ---'
gh pr view "$pr" --repo "$repo" \
--json baseRefName,baseRefOid,headRefName,headRefOid,mergeStateStatus,reviewDecision \
--jq '{baseRefName, baseRefOid, headRefName, headRefOid, mergeStateStatus, reviewDecision}'
base="$(gh pr view "$pr" --repo "$repo" --json baseRefOid --jq '.baseRefOid')"
merge_base="$(git merge-base "$base" "$head")"
printf '%s\n' "--- Merge base ---" "BASE=$base" "MERGE_BASE=$merge_base"
echo '--- Cumulative pull request files ---'
git diff --name-status "$merge_base" "$head"
echo '--- Cumulative pull request diff ---'
git diff --find-renames --find-copies "$merge_base" "$head" -- \
src/main/java/com/clearfolio/viewer/config/ApiVersionWebFilter.java \
src/test/java/com/clearfolio/viewer/config/ApiVersionWebFilterTest.java
echo '--- All request mapping declarations ---'
rg -n -C 2 --glob '*.java' \
'@(?:Get|Post|Put|Patch|Delete|Request)Mapping\(' src/main/java
echo '--- Existing API error codes and response-body construction ---'
rg -n -C 5 --glob '*.java' \
'(errorCode|traceId|supportedVersion|setStatusCode\(|writeWith\(|ErrorResponse|ExceptionHandler|ResponseStatusException)' \
src/main/java
echo '--- Filter and exception-handler ordering declarations ---'
rg -n -C 5 --glob '*.java' \
'(`@Order`\b|Ordered\b|implements WebFilter|WebFilterRegistrationBean|WebExceptionHandler)' \
src/main/java
echo '--- Complete security-header filter for chain behavior ---'
cat -n src/main/java/com/clearfolio/viewer/config/ViewerSecurityHeadersWebFilter.javaLength of output: 110470
I found no blocking issues. The implementation meets the requested behavior:
The cumulative PR diff contains only the new filter and focused tests. The exact-head Maven test, security scans, SAST checks, and listed fuzz checks succeeded.
|
|
Queued @opencode-agent ; Already queued @cwl-noema-review on this exact request for PR #379 at head |
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #379 at head |
6 similar comments
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #379 at head |
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #379 at head |
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #379 at head |
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #379 at head |
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #379 at head |
|
Already queued @cwl-noema-review and @opencode-agent on this exact request for PR #379 at head |
|
Closing as superseded by consolidated current-main Draft #488. #488 preserves this PR's exact runtime version-filter production/test blobs and places negotiation on the same authority line as operation identity, compatibility, provenance, and tagged release acceptance. Previous checks/reviews remain provenance only. |
1 participant
Objective
Advance issue #315 with a bounded HTTP version-negotiation boundary. Existing
/api/v1/**clients remain backward compatible when no version header is sent, while an explicitly unsupported or contradictory requested version fails closed before controller dispatch and every API response identifies the currentv1contract.Exact current-head evidence
961f44a19c4ced87f8062b3ab9842a9a08d05235;7b98171aff415f05e31df0de37e230f9d39d0758;ApiVersionWebFilter.javaand its focused regression only;Version contract
Non-API paths bypass negotiation. Missing or blank version headers preserve v1 compatibility. Explicit
v1is accepted. Every API response advertisesX-Clearfolio-Api-Version: v1. Unsupported or conflicting version requests return a controlled JSON400before downstream dispatch.Scope boundary
Version-negotiation filter only. This does not complete OpenAPI/schema parity, breaking-change detection, generated clients, naruon consumer compatibility, or release schema provenance. Issue #315 remains open for those guarantees.
Merge gate
Keep this exact head unchanged. Auto-merge may act only after live required checks remain terminal-success, zero valid unresolved findings remain, and a qualifying independent non-author approval is attached to this exact head. Any head/base movement requires complete exact-head revalidation; predecessor evidence does not transfer.