fix(reliability): keep recovery retry on one current-main clock - #428
fix(reliability): keep recovery retry on one current-main clock#428seonghobae wants to merge 2 commits into
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: 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 |
|
Exact-current-head review request for @opencode-agent review |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base='961f44a19c4ced87f8062b3ab9842a9a08d05235'
head='02d01de14fb7435dccde137ce264539f2c520438'
echo '== Checked-out commit =='
git rev-parse HEAD
echo '== Commit ancestry =='
git merge-base --is-ancestor "$base" "$head" && echo 'base-is-ancestor=yes'
printf 'ahead='; git rev-list --count "${base}..${head}"
printf 'behind='; git rev-list --count "${head}..${base}"
echo '== Changed paths =='
git diff --name-status "$base" "$head"
echo '== Java outlines for changed paths =='
git diff --name-only "$base" "$head" -- '*.java' | while IFS= read -r f; do
echo "--- $f"
ast-grep outline "$f" --items all --type class,method --view compact || true
done
echo '== Exact diff =='
git diff --unified=80 "$base" "$head" -- '*.java'
echo '== Candidate wall-clock calls in production Java =='
rg -n -i -C 3 '(Instant\.now|LocalDateTime\.now|OffsetDateTime\.now|ZonedDateTime\.now|Clock\.system|System\.currentTimeMillis|new[[:space:]]+Date[[:space:]]*\()' \
-g '*.java' -g '!**/test/**' -g '!**/tests/**' .Length of output: 39675 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Relevant type and method outlines =='
ast-grep outline src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java \
--items all --type class,method --view signatures
ast-grep outline src/main/java/com/clearfolio/viewer/model/ConversionJob.java \
--items all --type class,method --view signatures
ast-grep outline src/main/java/com/clearfolio/viewer/service/DefaultConversionWorker.java \
--items all --type class,method --view signatures
echo '== Recovery eligibility implementations and declarations =='
rg -n -C 12 '\bfindRecoverableJobs\s*\(' \
src/main/java src/test/java
echo '== Job state transition and readiness implementations =='
rg -n -C 10 '\b(isReadyForProcessing|markProcessing|markRetryScheduled|scheduleRetry)\s*\(' \
src/main/java/com/clearfolio/viewer/model/ConversionJob.java \
src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java \
src/main/java/com/clearfolio/viewer/repository/ConversionJobStateStore.java
echo '== All recoverPendingJobs call sites =='
rg -n -C 6 '\brecoverPendingJobs\s*\(' -g '*.java' .
echo '== Recovery-focused test source =='
cat -n src/test/java/com/clearfolio/viewer/service/DefaultConversionWorkerRecoveryClockTest.java
echo '== Working tree status =='
git status --shortLength of output: 35320
I verified
The new regression captures the retry timestamp and requires it to equal
You are interacting with an AI system. |
|
@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
base='961f44a19c4ced87f8062b3ab9842a9a08d05235'
head='02d01de14fb7435dccde137ce264539f2c520438'
echo '== commit identity and ancestry =='
printf 'HEAD='; git rev-parse HEAD
git cat-file -e "${head}^{commit}"
git merge-base --is-ancestor "$base" "$head" && echo 'base_is_ancestor=yes'
printf 'ahead='; git rev-list --count "${base}..${head}"
printf 'behind='; git rev-list --count "${head}..${base}"
echo '== changed paths =='
git diff --name-status "$base" "$head"
echo '== changed Java outlines =='
git diff --name-only "$base" "$head" -- '*.java' |
while IFS= read -r file; do
echo "--- ${file}"
ast-grep outline "$file" --items all --type class,method --view signatures || true
done
echo '== exact Java diff =='
git diff --unified=100 "$base" "$head" -- '*.java'
echo '== recovery method and relevant test call sites =='
rg -n -C 14 '\b(recoverPendingJobs|findRecoverableJobs|scheduleRetry)\s*\(' \
-g '*.java' src
echo '== wall-clock calls in conversion-worker recovery context =='
rg -n -C 5 '(Instant\.now\s*\(|Clock\.system|System\.currentTimeMillis\s*\()' \
src/main/java/com/clearfolio/viewer/service/DefaultConversionWorker.java \
src/test/java/com/clearfolio/viewer/service/DefaultConversionWorkerRecoveryClockTest.java \
src/test/java/com/clearfolio/viewer/service/DefaultConversionWorkerTest.java
echo '== status =='
git status --shortLength of output: 99335
I verified that
The remaining wall-clock calls are outside this recovery invariant. They support startup entry, normal processing, failure backoff, or delayed execution.
|
|
Exact-current-head independent review request for The three-file recovery-clock slice is mergeable; CI, Security Scan, SAST Semgrep, and fuzz are terminal-success; unresolved inline findings are zero. One caller-supplied instant now governs stale eligibility and retry scheduling without a hidden second wall-clock read. @opencode-agent review Review this exact head read-only. Do not mutate the branch, synthesize author approval, weaken protection, or merge. Any base update must regenerate all exact-head evidence. |
Rate Limit Exceeded
|
1 participant
Objective
Use one caller-supplied recovery instant for both stale-processing eligibility and retry scheduling so recovery behavior is deterministic and cannot drift across hidden wall-clock reads.
Exact current-head evidence
961f44a19c4ced87f8062b3ab9842a9a08d05235;02d01de14fb7435dccde137ce264539f2c520438;DefaultConversionWorker.javaand two focused worker regressions only;Recovery contract
recoverPendingJobs(now, processingLeaseTimeout)derives stale-processing eligibility and the queued retry timestamp from the same caller-ownednow. The regression fixture derives stale and future timing from that deterministic clock and proves the state store receives the exact recovery instant rather than a secondInstant.now()observation.Scope boundary
Recovery timestamp consistency only. This does not implement durable scheduling, transactional outbox, distributed worker leases, backpressure, cancellation, OpenTelemetry export, or issue #312 completion.
Merge gate
Keep this exact head unchanged. Auto-merge may act only after all live required checks remain successful, zero valid unresolved findings remain, and a qualifying independent non-author approval is attached to this exact head. Protected
mainhas advanced since reconstruction; any required base update must regenerate every exact-head check and review. No predecessor evidence transfers.