feat(operations): separate liveness and readiness probes - #265
feat(operations): separate liveness and readiness probes#265seonghobae wants to merge 67 commits into
Conversation
📝 WalkthroughWalkthrough
Changes가용성 프로브 계약과 구현
Maven verify 품질 게이트
경계 조건 회귀 테스트
공개 API 문서와 기본 생성자
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ProbeClient
participant HealthController
participant ApplicationAvailability
ProbeClient->>HealthController: GET /healthz 또는 /readyz
HealthController->>ApplicationAvailability: 가용성 상태 조회
ApplicationAvailability-->>HealthController: LivenessState 또는 ReadinessState
HealthController-->>ProbeClient: 200/503 응답과 no-store 캐시 정책
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)
Comment |
|
@opencode-agent @cwl-noema-review Please independently inspect exact current head |
|
@coderabbitai review Review exact current head |
|
I will treat CI, Security Scan, SAST, and all three fuzz jobs as required evidence. I will report unresolved blockers. I will not approve or recommend merge unless every required exact-head check succeeds.
|
|
@coderabbitai review Re-review exact current head |
|
@opencode-agent @cwl-noema-review Independently review exact current head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="ContextualWisdomLab/clearfolio"
expected="1c34120e6320c932565aacbdb3878e5d039f7095"
echo "== Revision identity =="
printf 'checked_out='; git rev-parse HEAD
printf 'expected='; printf '%s\n' "$expected"
git status --short
echo "== Exact-head pull request checks and reviews =="
gh pr view 265 --repo "$repo" \
--json number,headRefOid,isDraft,reviewDecision,statusCheckRollup,reviews \
--jq '{
number, headRefOid, isDraft, reviewDecision,
checks: [.statusCheckRollup[] | {name,status,conclusion,detailsUrl}],
reviews: [.reviews[] | {author:.author.login,state,commit:.commit.oid}]
}'
echo "== Candidate files =="
fd -t f . | rg '(^|/)(HealthController|HealthControllerTest|SecurityConfig|.*Security.*Config|test_documentation_contracts|pom|README|CHANGELOG|.*availability-probes|.*threat-model.*)\.(java|py|xml|md)$|(^|/)pom\.xml$' || true
echo "== Availability controller and tests =="
controller="$(fd -t f -e java 'HealthController.java' src | head -n1)"
testfile="$(fd -t f -e java 'HealthControllerTest.java' src | head -n1)"
printf 'controller=%s\ntest=%s\n' "$controller" "$testfile"
test -n "$controller" && sed -n '1,260p' "$controller"
test -n "$testfile" && sed -n '1,360p' "$testfile"
echo "== Probe authorization rules =="
rg -n -C 5 --glob '*.java' '(pathMatchers|authorizeExchange|permitAll|healthz|readyz)' src/main/java
echo "== Maven acceptance gates =="
sed -n '1,360p' pom.xml | rg -n -C 5 '(jacoco|javadoc|doclint|failOnWarnings|verify|check)'
echo "== Operations and security documentation =="
for file in \
"$(fd -t f -e md . docs | rg 'availability-probes\.md$' | head -n1)" \
"$(fd -t f -e md . docs | rg 'threat-model-data-handling\.md$' | head -n1)" \
"$(fd -t f -e py 'test_documentation_contracts.py' scripts | head -n1)"
do
if test -n "$file"; then
echo "--- $file ---"
sed -n '1,340p' "$file"
fi
done
echo "== PR change scope =="
git diff --name-status origin/main...HEADLength of output: 53437
Do not approve or merge until the response-body regression coverage is complete and every required exact-head check succeeds. This automated review is advisory and is not an independent write-authorized approval.
|
|
@opencode-agent @cwl-noema-review @coderabbitai Please review exact current head |
Pull request was converted to draft
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="ContextualWisdomLab/clearfolio"
expected="1c34120e6320c932565aacbdb3878e5d039f7095"
echo "== Revision identity =="
printf 'checked_out='; git rev-parse HEAD
printf 'expected='; printf '%s\n' "$expected"
git status --short
echo "== Exact-head PR checks and unresolved review state =="
gh pr view 265 --repo "$repo" \
--json headRefOid,isDraft,reviewDecision,statusCheckRollup,reviews \
--jq '{
headRefOid, isDraft, reviewDecision,
checks: [.statusCheckRollup[] | {name,status,conclusion,detailsUrl}],
reviews: [.reviews[] | {author:.author.login,state,commit:.commit.oid}]
}'
echo "== Target source/test/documentation files =="
fd -t f -e java 'HealthController.java' src
fd -t f -e java 'HealthControllerTest.java' src
fd -t f -e java . src/main | xargs -r ast-grep outline --items all --type class,method --match 'HealthController|Security|Availability' || true
fd -t f -e md . docs | rg '(availability-probes|threat-model-data-handling)\.md$'
fd -t f -e py 'test_documentation_contracts.py' scripts
echo "== Probe implementation and tests =="
for f in \
"$(fd -t f -e java 'HealthController.java' src | head -n1)" \
"$(fd -t f -e java 'HealthControllerTest.java' src | head -n1)"
do
test -z "$f" || { echo "--- $f ---"; sed -n '1,340p' "$f"; }
done
echo "== Probe authorization configuration =="
rg -n -C 5 --glob '*.java' '(pathMatchers|authorizeExchange|permitAll|healthz|readyz)' src/main/java
echo "== Verify, JaCoCo, and Javadoc gates =="
rg -n -C 8 --glob 'pom.xml' '(jacoco|javadoc|doclint|failOnWarnings|<phase>verify</phase>|<goal>check</goal>)' pom.xml
echo "== Operations, threat-model, and documentation contract =="
for f in \
"$(fd -t f -e md . docs | rg 'availability-probes\.md$' | head -n1)" \
"$(fd -t f -e md . docs | rg 'threat-model-data-handling\.md$' | head -n1)" \
"$(fd -t f -e py 'test_documentation_contracts.py' scripts | head -n1)"
do
test -z "$f" || { echo "--- $f ---"; sed -n '1,360p' "$f"; }
done
echo "== Availability terminology in changed Markdown =="
git diff --name-only origin/main...HEAD -- '*.md' | while IFS= read -r f; do
rg -n -i -C 2 '(healthz|readyz|liveness|readiness|startupProbe|Spring Boot 3\.5\.16|APA)' "$f" || true
doneLength of output: 72140 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="ContextualWisdomLab/clearfolio"
requested="1c34120e6320c932565aacbdb3878e5d039f7095"
current="1645ba381dfa41f5793e0af8d97923f805662bd7"
echo "== Requested commit identity =="
git show -s --format='commit=%H%nparents=%P%nsubject=%s%ncommitter=%cI' "$requested"
echo "== Requested-commit check runs =="
gh api "repos/$repo/commits/$requested/check-runs?per_page=100" \
--jq '[.check_runs[] | {name,status,conclusion,details_url,started_at,completed_at}]'
echo "== Requested-commit combined status =="
gh api "repos/$repo/commits/$requested/status" \
--jq '{state, statuses:[.statuses[] | {context,state,target_url,updated_at}]}'
echo "== Relevant files at requested commit =="
for path in \
src/main/java/com/clearfolio/viewer/controller/HealthController.java \
src/test/java/com/clearfolio/viewer/controller/HealthControllerTest.java \
docs/operations/2026-08-05-availability-probes.md \
docs/security/2026-07-02-threat-model-data-handling.md \
scripts/test_documentation_contracts.py \
pom.xml
do
echo "--- $path @ $requested ---"
git show "$requested:$path" | sed -n '1,360p'
done
echo "== Change from requested commit to current PR head =="
git diff --stat "$requested..$current"
git diff --name-status "$requested..$current"
git diff -- "$requested..$current" \
src/main/java/com/clearfolio/viewer/controller/HealthController.java \
src/test/java/com/clearfolio/viewer/controller/HealthControllerTest.java \
docs/operations/2026-08-05-availability-probes.md \
docs/security/2026-07-02-threat-model-data-handling.md \
scripts/test_documentation_contracts.py \
pom.xml || trueLength of output: 54014
The remaining requested contract is correct at this commit:
The PR has since advanced to You are interacting with an AI system. |
|
@coderabbitai review Re-review exact current head |
|
@opencode-agent @cwl-noema-review Independently review exact current head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="ContextualWisdomLab/clearfolio"
expected="1645ba381dfa41f5793e0af8d97923f805662bd7"
echo "== Commit identity =="
printf 'checked_out='; git rev-parse HEAD
printf 'expected='; printf '%s\n' "$expected"
git show -s --format='subject=%s%ncommitted=%cI' "$expected"
git status --short
echo "== Exact-commit checks and combined statuses =="
gh api "repos/$repo/commits/$expected/check-runs?per_page=100" \
--jq '[.check_runs[] | {name,status,conclusion,details_url,completed_at}]'
gh api "repos/$repo/commits/$expected/status" \
--jq '{state,statuses:[.statuses[] | {context,state,target_url,updated_at}]}'
echo "== Pull request head and unresolved review threads =="
gh pr view 265 --repo "$repo" \
--json headRefOid,isDraft,reviewDecision,statusCheckRollup,reviews \
--jq '{headRefOid,isDraft,reviewDecision,checks:[.statusCheckRollup[] | {name,status,conclusion}],reviews:[.reviews[] | {author:.author.login,state,commit:.commit.oid}]}'
gh api graphql -F owner="ContextualWisdomLab" -F name="clearfolio" -F number=265 -f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes { isResolved comments(first: 1) { nodes { author { login } body path line } } }
}
}
}
}' --jq '[.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved == false)]'
echo "== Implementation, tests, and access control at exact commit =="
for path in \
src/main/java/com/clearfolio/viewer/controller/HealthController.java \
src/test/java/com/clearfolio/viewer/controller/HealthControllerTest.java \
pom.xml \
docs/operations/2026-08-05-availability-probes.md \
docs/security/2026-07-02-threat-model-data-handling.md \
scripts/test_documentation_contracts.py
do
echo "--- $path ---"
git show "$expected:$path" | sed -n '1,380p'
done
echo "== Authorization rules =="
git grep -n -C 5 -E '(healthz|readyz|pathMatchers|authorizeExchange|permitAll)' "$expected" -- 'src/main/java/**/*.java' || true
echo "== Changed paths at exact commit =="
git diff-tree --no-commit-id --name-status -r "$expected"Length of output: 62612
The prior test-coverage blocker is addressed. The current implementation is otherwise consistent with the requested contract:
This review is advisory. It is not an independent write-authorized approval.
|
Pull request was converted to draft
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ARCHITECTURE.md (1)
67-72: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win운영 게이트를
verify계약과 일치시키십시오.이 목록은
mvn test와 독립 Javadoc 명령을 게이트로 제시합니다. 그러나 현재 수용 기준은mvn -B --no-transfer-progress verify만 완전한 병합 증거로 허용합니다. 이 목록을verify명령과 그 안의 JaCoCo 및 Javadoc 게이트로 변경하십시오.🤖 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 `@ARCHITECTURE.md` around lines 67 - 72, Update the build and test gates section in ARCHITECTURE.md to identify `mvn -B --no-transfer-progress verify` as the single complete merge-evidence command, and describe the JaCoCo and JavaDoc gates as checks executed within that verify lifecycle. Remove the standalone `mvn test` and independent Javadoc command entries while retaining the Markdown lint requirement for changed documentation.
🧹 Nitpick comments (1)
src/test/java/com/clearfolio/viewer/controller/ApiExceptionHandlerCoverageTest.java (1)
14-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win차단 범위 안쪽 문자에 대한 검증도 추가하십시오.
현재 테스트는 U+202F가 보존되는 것만 확인합니다. 이 단방향 경계 검증은
sanitizeForLog가 전혀 치환하지 않도록 회귀해도 통과합니다. 보안 정제 경로에는 차단 범위 안쪽 문자가 실제로 치환되는지도 확인해야 합니다.DefaultDocumentValidationCoverageTest의 line 86과 동일한 양방향 검증 방식을 적용하십시오.♻️ 제안 변경
- String sanitized = (String) method.invoke(handler, "\u202F"); - - assertEquals("\u202F", sanitized); + assertEquals("\u202F", (String) method.invoke(handler, "\u202F")); + assertEquals("_", (String) method.invoke(handler, "\u202E")); + assertEquals("__", (String) method.invoke(handler, "\r\n"));위 기대값은
ApiExceptionHandler.sanitizeForLog의 치환 문자에 맞추어 조정하십시오.As per coding guidelines: "Tests must exercise real behavior, including failure, security, concurrency, and recovery paths; coverage-only assertions must represent a valid contract."
🤖 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 `@src/test/java/com/clearfolio/viewer/controller/ApiExceptionHandlerCoverageTest.java` around lines 14 - 23, Extend logSanitizationPreservesTheCodePointImmediatelyAfterTheBlockedRange to also pass a code point inside sanitizeForLog’s blocked range and assert that it is replaced with the sanitizer’s configured replacement character, while retaining the existing U+202F preservation assertion. Mirror the two-sided validation used by DefaultDocumentValidationCoverageTest.Source: Coding guidelines
🤖 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 `@src/main/java/com/clearfolio/viewer/api/ApiErrorResponse.java`:
- Around line 10-14: Update ApiExceptionHandler.handleTypeMismatch so failed
UUID and Boolean conversion inputs are not copied directly into
details["value"]; redact them or include them only through an appropriate
allowlist, preserving ApiErrorResponse’s contract that details contain no secret
values.
In
`@src/main/java/com/clearfolio/viewer/exception/UnsupportedDocumentFormatException.java`:
- Line 10: Update the Javadoc for the extension field in
UnsupportedDocumentFormatException to describe it as the blocked extension
supplied to the constructor, replacing the inaccurate “Normalized extension”
wording. Keep the constructor’s behavior of storing the input unchanged.
In
`@src/test/java/com/clearfolio/viewer/service/DefaultDocumentValidationCoverageTest.java`:
- Around line 89-118: Replace the class-private SECURITY_PROVIDERS_LOCK in
src/test/java/com/clearfolio/viewer/service/DefaultDocumentValidationCoverageTest.java:89-118
with the shared test lock, and add
`@ResourceLock`("java.security.Security.providers") to serialize access. Apply the
same shared lock and resource annotation in
src/test/java/com/clearfolio/viewer/controller/ConversionControllerCoverageTest.java:57-92,
extracting the duplicated sha256ProviderPositions and ProviderPosition helpers
into the shared test utility.
- Around line 33-38: Update
rejectsANullPolicySecretSuppliedByAnExternalConfigurationSource and its
JavaDoc/name to reflect the actual ConversionProperties contract, since
getPolicyOverrideSecret() normalizes null to "". Remove the mock-based null
getter scenario or change the test to verify the supported default/normalization
behavior; only add a production configuration path if null is genuinely valid
there.
---
Outside diff comments:
In `@ARCHITECTURE.md`:
- Around line 67-72: Update the build and test gates section in ARCHITECTURE.md
to identify `mvn -B --no-transfer-progress verify` as the single complete
merge-evidence command, and describe the JaCoCo and JavaDoc gates as checks
executed within that verify lifecycle. Remove the standalone `mvn test` and
independent Javadoc command entries while retaining the Markdown lint
requirement for changed documentation.
---
Nitpick comments:
In
`@src/test/java/com/clearfolio/viewer/controller/ApiExceptionHandlerCoverageTest.java`:
- Around line 14-23: Extend
logSanitizationPreservesTheCodePointImmediatelyAfterTheBlockedRange to also pass
a code point inside sanitizeForLog’s blocked range and assert that it is
replaced with the sanitizer’s configured replacement character, while retaining
the existing U+202F preservation assertion. Mirror the two-sided validation used
by DefaultDocumentValidationCoverageTest.
🪄 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: bcc1d0d4-ad1c-497c-8840-34cc9759a24a
📒 Files selected for processing (44)
.github/workflows/ci.ymlARCHITECTURE.mdCHANGELOG.mdCLAUDE.mdREADME.mddocs/architecture.mddocs/engineering/acceptance-criteria.mddocs/operations/2026-08-05-availability-probes.mddocs/security/2026-07-02-threat-model-data-handling.mdpom.xmlscripts/test_documentation_contracts.pysrc/main/java/com/clearfolio/viewer/ClearfolioViewerApplication.javasrc/main/java/com/clearfolio/viewer/api/AdminJobListResponse.javasrc/main/java/com/clearfolio/viewer/api/ApiErrorResponse.javasrc/main/java/com/clearfolio/viewer/api/ConversionJobStatusResponse.javasrc/main/java/com/clearfolio/viewer/api/SubmitConversionResponse.javasrc/main/java/com/clearfolio/viewer/api/ViewerBootstrapResponse.javasrc/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.javasrc/main/java/com/clearfolio/viewer/artifact/ArtifactTokenException.javasrc/main/java/com/clearfolio/viewer/artifact/InMemoryArtifactStore.javasrc/main/java/com/clearfolio/viewer/auth/TenantContext.javasrc/main/java/com/clearfolio/viewer/config/ArtifactStoreConfig.javasrc/main/java/com/clearfolio/viewer/config/ArtifactStoreProperties.javasrc/main/java/com/clearfolio/viewer/config/ConversionExecutorConfig.javasrc/main/java/com/clearfolio/viewer/config/ConversionProperties.javasrc/main/java/com/clearfolio/viewer/config/ViewerSecurityHeadersWebFilter.javasrc/main/java/com/clearfolio/viewer/controller/ApiExceptionHandler.javasrc/main/java/com/clearfolio/viewer/controller/ConversionController.javasrc/main/java/com/clearfolio/viewer/controller/HealthController.javasrc/main/java/com/clearfolio/viewer/controller/ViewerUiController.javasrc/main/java/com/clearfolio/viewer/exception/UnsupportedDocumentFormatException.javasrc/main/java/com/clearfolio/viewer/model/ConversionJobStatus.javasrc/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.javasrc/main/java/com/clearfolio/viewer/service/DefaultConversionWorker.javasrc/main/java/com/clearfolio/viewer/service/DefaultDocumentConversionService.javasrc/test/java/com/clearfolio/viewer/artifact/FileSystemArtifactStoreCoverageTest.javasrc/test/java/com/clearfolio/viewer/config/ConversionPropertiesCoverageTest.javasrc/test/java/com/clearfolio/viewer/controller/ApiExceptionHandlerCoverageTest.javasrc/test/java/com/clearfolio/viewer/controller/ConversionControllerCoverageTest.javasrc/test/java/com/clearfolio/viewer/controller/HealthControllerTest.javasrc/test/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepositoryCoverageTest.javasrc/test/java/com/clearfolio/viewer/service/DefaultDocumentConversionServiceCoverageTest.javasrc/test/java/com/clearfolio/viewer/service/DefaultDocumentValidationCoverageTest.javasrc/test/java/com/clearfolio/viewer/service/DocumentConversionServiceCoverageTest.java
|
@opencode-agent Rebuild this bounded availability slice cleanly on authoritative parent #270 exact head |
1 participant
Buyer-visible operational gap
GET /healthzwas implemented as a static liveness response while README and architecture material described it as readiness. An orchestrator could therefore treat a process-alive signal as permission to route traffic. This bounded slice separates restart eligibility from traffic eligibility without adding a model, network, database, or external-service dependency to liveness.Changes
GET /healthzpayload as200 {"status":"ok"}while sourcing it from Spring BootLivenessState.503 {"status":"broken"}when the application liveness state isBROKEN.GET /readyz, returning200 {"status":"ready"}only forReadinessState.ACCEPTING_TRAFFICand503 {"status":"not_ready"}otherwise.Cache-Control: no-storeto both probes so intermediaries cannot replay stale availability state.ApplicationAvailabilityprovider.mvn -B --no-transfer-progress verify, emit actionable JaCoCo diagnostics on failure, and run warning-free Java 21 public Javadoc validation in the same acceptance lifecycle./healthzliveness and/readyzreadiness roles and protect the terminology with executable documentation contract tests.ConversionPropertiesnormalizes null to its supported blank disabled value.verifymerge-evidence command.CLAUDE.md, andCHANGELOG.md.Test-first and review evidence
The red commit
d39551d104fc918664c3d1aac0e7f2be3d05dbcbcontained only the desired availability-contract tests. CI run30977698977, Maven job92215161701, failed at test compilation because productionHealthControllerdid not yet acceptApplicationAvailability.The minimal production implementation at
25ea732d901bc64da9c17a6cfe9d6999d6c8d6a3made those tests pass. CI run30977769822executed 438 tests with zero failures, errors, or skips and reportedBUILD SUCCESS.When the acceptance lifecycle was strengthened from
mvn testtomvn verify, exact headc067b974db1ce2f720abab658025a05878a743a9correctly failed on one uncovered production line and three uncovered branches. The subsequent changes removed unreachable filename and configuration branches and added executable fail-closed edge tests.CodeRabbit review on
1645ba381dfa41f5793e0af8d97923f805662bd7identified four valid findings: rejected external values were reflected in error payloads, one public field Javadoc overstated normalization, a mock bypassed the real configuration contract, and two tests used independent locks while mutating one JVM-global provider registry. Exact current head8d7c55d94c40689ff28f45c787c77c31fa5e1c98addresses all four findings. Every corresponding review thread is resolved.The intermediate head
72d9e14b7d7526e7fd040d25c3a4166b0ebbf064ran all 452 tests successfully but correctly failed the zero-missed-branch JaCoCo gate because the invalid mock test had been removed while its unreachable production null branch remained. The exact current head removes only that unreachable branch and preserves the supported blank-secret fail-closed behavior.Exact-head evidence
Exact current head
8d7c55d94c40689ff28f45c787c77c31fa5e1c98is mergeable against protectedmainand has successful same-head technical evidence:30988328363: Maven verify and buyer-readiness script jobs succeeded.30988328479: succeeded.30988329094: succeeded.30988328476: succeeded.No queued, pending, cancelled, stale-head, or skipped-required result is counted as passing.
Merge gate and ordering
Keep this pull request draft while authoritative privacy-and-Netty parent #270 remains unmerged. #270 must integrate first; this change must then be reconciled and revalidated against the resulting protected
mainexact head because both slices touch shared production code, documentation, tests, and acceptance infrastructure. After reconciliation, require fresh exact-head CI, Security Scan, SAST, every fuzz matrix job, CodeRabbit, Strix/OpenCode/Noema review, zero unresolved threads, and an independent approving review from a reviewer whose repository permission GitHub counts. Do not bypass branch protection, weaken tests, or treat advisory bot review as counted approval.