Skip to content

feat(lifecycle): integrate tenant-safe administration and durable artifact cleanup - #283

Closed
seonghobae wants to merge 6 commits into
review/durable-deletion-receipt-ledger-v2from
review/durable-artifact-cleanup-worker-v2
Closed

feat(lifecycle): integrate tenant-safe administration and durable artifact cleanup#283
seonghobae wants to merge 6 commits into
review/durable-deletion-receipt-ledger-v2from
review/durable-artifact-cleanup-worker-v2

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Objective

Part of #263, Slice C. Integrate durable artifact cleanup on exact receipt foundation v2. This Draft connects authorized deletion to receipt-first metadata tombstoning, exact-digest cleanup, restart and scheduled recovery, repeat-request idempotency, WebFlux-safe blocking isolation, and a conversion/deletion generation fence. Slice D truthful HTTP status, signed-link revocation, and accessible viewer state remain out of scope.

Exact clean stack

Exact current head is bac871ab0f5c89237e7afdd9ad7f0cc7132a644f on exact foundation #282 head 3396c03a458294175006efd0c9cea35d6911d55c.

Fresh comparison reports:

  • 1 commit ahead and 0 behind;
  • merge base equals the exact foundation head;
  • exactly 24 cleanup-scoped changed files;
  • GitHub reports the Draft mergeable;
  • no workflow, dependency, packaging, SBOM, attribution, version, release, or deployment publication change.

The foundation's strict replay hardening is inherited through clean ancestry rather than copied into this worker slice. Predecessor #279 and pre-reconstruction #283 checks, reviews, comments, and SHAs are not counted.

Bounded implementation

  • ArtifactDeletionCoordinator persists intent before metadata tombstoning, validates exact SHA-256 or the documented absence sentinel, records controlled read/delete/mismatch failures, and replays bounded incomplete work at startup and fixed delay.
  • Per-job lifecycle locks allow unrelated document generations to progress concurrently while preserving same-job ordering.
  • LifecycleFencedArtifactStore rejects every putPdf after a durable receipt exists. An in-flight publication that wins first is snapshotted and deleted; a later publication fails closed.
  • Repeated tenant-scoped and legacy global DELETE requests resume failed receipts or observe completed receipts without recreating work.
  • The administrator DELETE endpoint executes blocking receipt and artifact work through Mono.fromCallable(...).subscribeOn(Schedulers.boundedElastic()), outside the WebFlux event loop.
  • DurableDocumentDeletionService delegates every non-deletion method to the existing conversion service and routes deletion through the durable coordinator.
  • Spring scheduling uses a pool of two threads; cleanup is bounded to 100 receipts per pass with low-cardinality completed, failed, pending, run-count, total-duration, and maximum-duration evidence.
  • Recovery warnings expose only exception class names, never messages, identifiers, tenant data, paths, or document metadata.
  • docs/operations/artifact-deletion-cleanup.md records privacy-safe signals, alert thresholds, investigation, restart, rollback, multi-instance fencing, and release-evidence boundaries.

Deterministic verification contract

Tests cover receipt-before-mutation ordering; tenant concealment and repeated DELETE idempotency; read, delete, checksum, and absence paths; restart replay and configured recovery bounds; global retry after metadata tombstoning; write-after-receipt rejection; in-flight publication/deletion serialization; per-job lock exclusion with started-task latches; bounded-elastic DELETE execution; service delegation; filesystem restart persistence and in-memory volatility; low-cardinality aggregate evidence; SHA-256 provider absence; and defensive construction paths.

Exact-head acceptance state

For exact current head bac871ab0f5c89237e7afdd9ad7f0cc7132a644f:

  • fuzz run 31103589281 completed successfully across all required targets;
  • CodeRabbit commit status is successful, but status-only evidence is not a formal review or counted approval;
  • zero unresolved inline review thread exists;
  • formal exact-current-head CodeRabbit/OpenCode/Noema/Strix reviews are absent;
  • CI, Security Scan, SAST, complete Maven/JaCoCo/Javadoc evidence, and counted independent write-authorized approval are absent and therefore not passing.

PR #279 was closed unmerged as superseded; none of its evidence is transferred. Keep this PR Draft. Parent order remains #270 → reconciled #268#282 → this clean Slice C. Do not resolve #268's upstream incomplete-cleanup finding or claim product-level physical deletion completion until the entire stack is reconciled onto protected main and passes every exact-head protection. Do not weaken tests, infer approval from status-only evidence, bypass protections, or publish a release.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 14 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ad401d79-7be1-459b-b5f5-1540d2b59ec3

📥 Commits

Reviewing files that changed from the base of the PR and between 2d778cd and 0aec056.

📒 Files selected for processing (24)
  • CHANGELOG.md
  • docs/operations/artifact-deletion-cleanup.md
  • docs/security/2026-08-06-durable-artifact-deletion-receipts.md
  • docs/superpowers/plans/2026-08-06-durable-artifact-cleanup.md
  • docs/superpowers/specs/2026-08-06-durable-artifact-cleanup-design.md
  • src/main/java/com/clearfolio/viewer/ClearfolioViewerApplication.java
  • src/main/java/com/clearfolio/viewer/artifact/LifecycleFencedArtifactStore.java
  • src/main/java/com/clearfolio/viewer/config/ArtifactStoreConfig.java
  • src/main/java/com/clearfolio/viewer/controller/AdminController.java
  • src/main/java/com/clearfolio/viewer/lifecycle/ArtifactDeletionCoordinator.java
  • src/main/java/com/clearfolio/viewer/lifecycle/ArtifactDeletionMetrics.java
  • src/main/java/com/clearfolio/viewer/lifecycle/ArtifactLifecycleLockRegistry.java
  • src/main/java/com/clearfolio/viewer/service/DurableDocumentDeletionService.java
  • src/main/resources/application.yml
  • src/test/java/com/clearfolio/viewer/artifact/LifecycleFencedArtifactStoreTest.java
  • src/test/java/com/clearfolio/viewer/config/ArtifactStoreConfigTest.java
  • src/test/java/com/clearfolio/viewer/controller/AdminControllerTest.java
  • src/test/java/com/clearfolio/viewer/lifecycle/ArtifactDeletionCoordinatorCoverageTest.java
  • src/test/java/com/clearfolio/viewer/lifecycle/ArtifactDeletionCoordinatorTest.java
  • src/test/java/com/clearfolio/viewer/lifecycle/ArtifactDeletionGlobalRetryTest.java
  • src/test/java/com/clearfolio/viewer/lifecycle/ArtifactDeletionRecoveryBoundTest.java
  • src/test/java/com/clearfolio/viewer/lifecycle/ArtifactLifecycleLockRegistryTest.java
  • src/test/java/com/clearfolio/viewer/lifecycle/ArtifactLifecycleSerializationTest.java
  • src/test/java/com/clearfolio/viewer/service/DurableDocumentDeletionServiceTest.java

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@opencode-agent @cwl-noema-review Please independently review exact current head a32a0978cabcaa4b0c4812942cdec8e723e272f0 against exact foundation v2 8c803dafd14c1b6e7cac80b39e5c5a9915fc1cbd. Verify the one-commit/23-file dependency-free Slice C: receipt-before-tombstone ordering, exact digest and absence sentinel, controlled retry evidence, repeat tenant/global DELETE idempotency, bounded startup/scheduled recovery, per-job concurrency, write-after-receipt rejection, in-flight publication/deletion serialization, bounded-elastic WebFlux DELETE, deletion-only primary decorator, standalone source compatibility, privacy-safe aggregate evidence and logs, complete tests/Javadocs, inherited tenant-lifecycle isolation, and absence of dependency/workflow/release drift. Do not approve or claim passing evidence until exact-head checks and every repository protection are satisfied.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

@seonghobae I will review PR #283 against 8c803dafd14c1b6e7cac80b39e5c5a9915fc1cbd at exact head a32a0978cabcaa4b0c4812942cdec8e723e272f0.

I will not approve the PR or claim passing verification until the exact-head checks and required repository protections provide evidence.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@opencode-agent @cwl-noema-review Please independently review exact current head bac871ab0f5c89237e7afdd9ad7f0cc7132a644f against exact foundation 3396c03a458294175006efd0c9cea35d6911d55c. This is now a direct one-commit, zero-behind, 24-file Slice C. Verify receipt-before-tombstone ordering, exact digest and absence sentinel, controlled retry evidence, tenant/global DELETE idempotency, bounded startup/scheduled recovery, per-job concurrency, write-after-receipt rejection, in-flight publication/deletion serialization, bounded-elastic WebFlux DELETE, deletion-only primary decorator, standalone source compatibility, privacy-safe aggregate evidence/logging, operations rollback guidance, complete tests/Javadocs, inherited strict replay and tenant isolation, and absence of dependency/workflow/release drift. Do not approve or claim passing evidence until the exact-head protections are complete.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

@seonghobae I will review PR #283 against foundation 3396c03a458294175006efd0c9cea35d6911d55c at head bac871ab0f5c89237e7afdd9ad7f0cc7132a644f.

I will verify the specified Slice C behavior, test coverage, inherited isolation and replay invariants, and dependency/workflow/release boundaries. I will not approve the PR or report passing protection evidence unless the exact-head checks and all required protections are complete.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Collaborator Author

@coderabbitai @opencode-agent @cwl-noema-review Please perform a fresh formal review of exact clean head bac871ab0f5c89237e7afdd9ad7f0cc7132a644f on exact foundation 3396c03a458294175006efd0c9cea35d6911d55c. The branch is 1 commit ahead, 0 behind, with a 24-file cleanup-only diff. Verify receipt-before-tombstone ordering, exact-digest and absence-sentinel cleanup, restart/scheduled recovery, bounded batches, repeat-request idempotency, WebFlux bounded-elastic isolation, same-job generation fencing, in-flight publication serialization, privacy-safe controlled failures and aggregate metrics, and the operator runbook. Exact-head fuzz 31103589281 succeeded; CI, Security Scan, SAST, Strix, full Maven/JaCoCo/Javadoc evidence, and independent approval remain absent and must not be inferred. Submit current-head actionable findings or a formal review; a separate counted write-authorized independent approval remains mandatory.

@seonghobae seonghobae changed the title feat(lifecycle): integrate durable artifact cleanup worker v2 feat(lifecycle): integrate tenant-safe administration and durable artifact cleanup Aug 6, 2026
@seonghobae
seonghobae changed the base branch from review/durable-deletion-receipt-ledger-v2 to fix/pii-logging-16240128950440010639 August 6, 2026 13:08
@seonghobae
seonghobae changed the base branch from fix/pii-logging-16240128950440010639 to fix/admin-endpoint-auth-clean August 6, 2026 13:09
@seonghobae
seonghobae changed the base branch from fix/admin-endpoint-auth-clean to review/durable-deletion-receipt-ledger-v2 August 6, 2026 13:09

Copy link
Copy Markdown
Collaborator Author

Closing unmerged as superseded. The exact durable-cleanup worker, receipt foundation, tenant-safe administrator ancestry, tests, and documentation are now fast-forward integrated into PR #268 exact head 0aec05655849943887aec232c3107950159c741f. Retaining this dependent Draft would duplicate the same effective tree and fragment exact-head evidence. No previous #283 checks, comments, or review state are transferred; #268 must rerun all CI, security, coverage, fuzz, and review gates on the integrated head.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base='3396c03a458294175006efd0c9cea35d6911d55c'
head='bac871ab0f5c89237e7afdd9ad7f0cc7132a644f'

echo '== exact revisions =='
git rev-parse "$base" "$head"
echo '== ancestry / commit count =='
git merge-base "$base" "$head"
git rev-list --count "${base}..${head}"
echo '== changed files =='
git diff --name-status "$base" "$head"
echo '== stat =='
git diff --stat "$base" "$head"

echo '== Java symbols in changed Java files =='
git diff --name-only "$base" "$head" -- '*.java' | while IFS= read -r f; do
  echo "--- $f"
  ast-grep outline "$f" --items all --view compact || true
done

echo '== changed non-Java text files (first 220 lines each) =='
git diff --name-only "$base" "$head" -- ':!*.java' | while IFS= read -r f; do
  echo "--- $f"
  sed -n '1,220p' "$f"
done

Length of output: 44097


🏁 Script executed:

#!/bin/bash
set -euo pipefail

files=(
  src/main/java/com/clearfolio/viewer/lifecycle/ArtifactDeletionCoordinator.java
  src/main/java/com/clearfolio/viewer/artifact/LifecycleFencedArtifactStore.java
  src/main/java/com/clearfolio/viewer/lifecycle/ArtifactLifecycleLockRegistry.java
  src/main/java/com/clearfolio/viewer/service/DurableDocumentDeletionService.java
  src/main/java/com/clearfolio/viewer/controller/AdminController.java
  src/main/java/com/clearfolio/viewer/config/ArtifactStoreConfig.java
  src/main/java/com/clearfolio/viewer/lifecycle/ArtifactDeletionMetrics.java
  src/main/java/com/clearfolio/viewer/ClearfolioViewerApplication.java
)

for f in "${files[@]}"; do
  echo "===== OUTLINE: $f ====="
  ast-grep outline "$f" --items all --view signatures || true
  echo "===== SOURCE: $f ====="
  cat -n "$f"
done

Length of output: 56560


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Receipt, repository, and artifact symbols =='
rg -n -C 3 --type java \
  '(interface\s+ArtifactDeletionReceiptStore|class\s+.*ArtifactDeletion.*Receipt|record\s+ArtifactDeletionReceipt|interface\s+ConversionJobRepository|class\s+.*ConversionJob.*Repository|interface\s+ArtifactStore|class\s+(FileSystemArtifactStore|InMemoryArtifactStore)|ArtifactDeletionReceiptStore\s+\w+\s*\()' \
  src/main/java

echo '== Relevant coordinator tests: method map =='
for f in \
  src/test/java/com/clearfolio/viewer/lifecycle/ArtifactDeletionCoordinatorTest.java \
  src/test/java/com/clearfolio/viewer/lifecycle/ArtifactDeletionCoordinatorCoverageTest.java \
  src/test/java/com/clearfolio/viewer/lifecycle/ArtifactDeletionGlobalRetryTest.java \
  src/test/java/com/clearfolio/viewer/lifecycle/ArtifactDeletionRecoveryBoundTest.java \
  src/test/java/com/clearfolio/viewer/lifecycle/ArtifactLifecycleSerializationTest.java \
  src/test/java/com/clearfolio/viewer/artifact/LifecycleFencedArtifactStoreTest.java \
  src/test/java/com/clearfolio/viewer/controller/AdminControllerTest.java \
  src/test/java/com/clearfolio/viewer/service/DurableDocumentDeletionServiceTest.java; do
  echo "--- $f"
  ast-grep outline "$f" --items all --view signatures || true
done

echo '== Coordinator diff with context =='
git diff --unified=80 3396c03a458294175006efd0c9cea35d6911d55c bac871ab0f5c89237e7afdd9ad7f0cc7132a644f -- \
  src/main/java/com/clearfolio/viewer/lifecycle/ArtifactDeletionCoordinator.java \
  src/main/java/com/clearfolio/viewer/artifact/LifecycleFencedArtifactStore.java \
  src/main/java/com/clearfolio/viewer/config/ArtifactStoreConfig.java \
  src/main/java/com/clearfolio/viewer/controller/AdminController.java

Length of output: 50753


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Receipt and repository contracts/implementations =='
for f in \
  src/main/java/com/clearfolio/viewer/lifecycle/ArtifactDeletionReceiptStore.java \
  src/main/java/com/clearfolio/viewer/lifecycle/ArtifactDeletionReceipt.java \
  src/main/java/com/clearfolio/viewer/lifecycle/ArtifactDeletionLedger.java \
  src/main/java/com/clearfolio/viewer/repository/ConversionJobRepository.java \
  src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java \
  src/main/java/com/clearfolio/viewer/artifact/ArtifactStore.java; do
  echo "--- $f"
  cat -n "$f"
done

echo '== All new lifecycle test methods and relevant assertions =='
for f in \
  src/test/java/com/clearfolio/viewer/lifecycle/ArtifactDeletionCoordinatorTest.java \
  src/test/java/com/clearfolio/viewer/lifecycle/ArtifactDeletionCoordinatorCoverageTest.java \
  src/test/java/com/clearfolio/viewer/lifecycle/ArtifactDeletionGlobalRetryTest.java \
  src/test/java/com/clearfolio/viewer/lifecycle/ArtifactDeletionRecoveryBoundTest.java \
  src/test/java/com/clearfolio/viewer/lifecycle/ArtifactLifecycleSerializationTest.java \
  src/test/java/com/clearfolio/viewer/artifact/LifecycleFencedArtifactStoreTest.java; do
  echo "--- $f"
  rg -n -C 12 \
    '(void\s+\w+\s*\(|ABSENT_ARTIFACT_CHECKSUM|absence|absent|checksum|mismatch|deletePdf|putPdf|retryPendingWork|resumeReceipt|pendingReceipts)' \
    "$f"
done

echo '== Other ArtifactStore injection and direct construction sites =='
rg -n -C 4 --type java \
  '(new\s+(InMemoryArtifactStore|FileSystemArtifactStore|LifecycleFencedArtifactStore)\s*\(|ArtifactStore\s+\w+|\.putPdf\s*\(|\.deletePdf\s*\()' \
  src/main/java src/test/java

Length of output: 202238


@seonghobae Formal source review completed for bac871ab0f5c89237e7afdd9ad7f0cc7132a644f against 3396c03a458294175006efd0c9cea35d6911d55c.

I confirmed the one-commit relationship and 24-file scope. I found no dependency, workflow, packaging, release, or deployment drift.

Actionable finding

  • Missing recovery-run aggregate evidence. ArtifactDeletionMetrics records only completed attempts, failed attempts, and pending receipts. It does not record or expose the required recovery run count, total recovery duration, or maximum recovery duration. See src/main/java/com/clearfolio/viewer/lifecycle/ArtifactDeletionMetrics.java, Line 21 through Line 23. docs/operations/artifact-deletion-cleanup.md documents recoveryBatchRuns(), recoveryBatchTotalDuration(), and recoveryBatchMaximumDuration() as available operational signals, but these methods and their backing measurements do not exist. Measure each retryPendingWork() invocation, update the aggregate values, expose them from ArtifactDeletionMetrics, and add tests for normal and failing recovery passes.

The reviewed source implements the receipt-first tombstone flow, controlled failure states, bounded recovery selection, tenant and global repeat handling, bounded-elastic DELETE execution, same-process lifecycle locking, and post-receipt write rejection.

I did not approve this Draft PR. I did not infer the unavailable CI, Security Scan, SAST, Strix, Maven/JaCoCo/Javadoc, or independent-approval evidence.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@seonghobae seonghobae closed this Aug 6, 2026
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