fix(reliability): fail closed on artifact ledger persistence errors - #357
fix(reliability): fail closed on artifact ledger persistence errors#357seonghobae wants to merge 10 commits into
Conversation
|
Warning Review limit reached
Next review available in: 47 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 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesArtifactLinkLedger 무결성
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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 `@src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkLedger.java`:
- Around line 78-79: Update ArtifactLinkLedger.appendLine to explicitly force
the file contents to physical storage with FileChannel.force(true) before
returning. Ensure this synchronization completes before in-memory publication at
src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkLedger.java lines 78-79
(issued record), 114-115 (revocation record), and 125-126 (read audit event);
all three sites require no separate direct change if they already publish only
after appendLine returns.
🪄 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: ed8a2f12-b7df-493c-8b08-bd265e0f7643
📒 Files selected for processing (4)
src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkLedger.javasrc/test/java/com/clearfolio/viewer/artifact/ArtifactLinkLedgerAuthorityRebindingTest.javasrc/test/java/com/clearfolio/viewer/artifact/ArtifactLinkLedgerFailClosedTest.javasrc/test/java/com/clearfolio/viewer/artifact/ArtifactLinkServiceTest.java
1 participant
Objective
Prevent configured durable artifact-link/audit persistence failures from publishing contradictory process-local state. Protected main mutates the in-memory issued-link/revocation/read-event structures before appending to the configured ledger, so a failed disk append can throw to the caller while the same process still observes authority/audit state that cannot survive restart.
Test-first state
This Draft starts intentionally RED at exact test-only head
eea4685ab5d7f454f00c3bdbfbef435753b6dc0e. Focused tests make the configured ledger path unwritable after construction and require failed issue, revoke, and read appends to leave the corresponding in-memory state unchanged. Protected main mutates memory first and therefore violates all three invariants.Scope
ArtifactLinkLedgerordering plus focused regression tests only. This does not modify token parsing/signature behavior owned by active PR #276, artifact delivery controllers, credential/OIDC work, filesystem artifact storage, deletion receipts, or canonical documentation. It preserves the optional in-memory-only profile when no ledger path is configured.Acceptance
Observe exact-head RED -> persist-before-publish ordering -> exact-head
mvn -B --no-transfer-progress verify, 100% owned production coverage/public Javadocs -> CI/Security Scan/SAST/fuzz -> current reviews/threads/live-base refetch. Keep Draft until exact-head GREEN evidence exists. Independent write-authorized approval remains a separate protected-merge gate.Summary by CodeRabbit
버그 수정
테스트