test(worker): await terminal retry completion before asserting by seonghobae · Pull Request #454 · ContextualWisdomLab/clearfolio

seonghobae · 2026-08-15T10:29:02Z

Objective

Remove a deterministic asynchronous test race that can fail otherwise-correct worker and delivery-receipt PRs. The retry test previously unblocked as soon as the conversion task incremented its attempt counter, but DefaultConversionWorker publishes SUCCEEDED only after that converter call returns and the state store completes its terminal transition.

Exact test-first repair

protected base: eb0a32e87e7470469c8fa0f8c67d9583654bef57;
exact candidate head: b3c0f9cfe218a35e05e0cb8b0dc675e5fa84568f;
changed path: src/test/java/com/clearfolio/viewer/service/DefaultConversionWorkerTest.java only;
predecessor head 23ba9a34e24d62de7974a6e1707dc53f6f69b4ac and one-shot run 31879285024 are historical evidence only and do not satisfy this head's merge gate;
reviewer-requested indentation was repaired by ordinary commit b3c0f9cfe218a35e05e0cb8b0dc675e5fa84568f;
review threads: 1 total, 0 unresolved (the repaired thread is resolved and outdated).

Contract

workerReschedulesWhenRetryAtIsInTheFuture waits until both the converter has executed once and the observed job state is SUCCEEDED. Assertions still require exactly one converter call, one attempt, terminal success, cleared retry time, and the expected converted resource path. Production behavior is unchanged.

Scope boundary

Test synchronization and formatting only. This does not weaken timeouts, add sleeps, retry assertions after failure, modify worker/state-store code, change scheduling, or alter delivery-receipt semantics.

Current exact-head evidence

For b3c0f9cfe218a35e05e0cb8b0dc675e5fa84568f:

mechanically mergeable; open; state preserved as Ready because this exact head was already toggled once in this run before the late Security result arrived;
CI 36932516790: success;
SAST Semgrep 36932516908: success;
fuzz 36932516789: success;
Security Scan 36932516728: failure;
CodeQL PR 36932516797: skipped because it was emitted while the PR was Draft; it is not terminal validation evidence;
independent review: pending; predecessor COMMENTED review does not approve this head.

Security job 110605161686 verified the exact checkout and emitted five fixable jackson-databind findings at pom.xml:151: HIGH CVE-2026-68497, CVE-2026-91776, CVE-2026-91777; MEDIUM CVE-2026-19032, CVE-2026-83557. SARIF upload succeeded. The canonical causal shared-base repair is clearfolio #503 at exact head bf121dffff24e771739f50b9e2cc7961459670a2.

This late terminal RED conflicts with the earlier Ready transition. Concurrency authority forbids toggling the same exact head twice in one run, so the current state is preserved and the race is recorded. HOLD merge until #503 integrates, this branch reconciles non-destructively, and a new exact head receives fresh Checks and review.

Dependency outcome

PR #373 failed only because this shared test observed PROCESSING between converter execution and terminal publication. After this repair reaches protected main, #373 must update to the new exact base and regenerate all checks/reviews; predecessor evidence does not transfer.

Merge gate

HOLD merge. Do not merge this Ready-state head: its exact Security gate is RED. Preserve the branch until clearfolio #503 integrates, then reconcile non-destructively to a new exact head and require fresh CI, Security Scan, SAST Semgrep, fuzz, CodeQL/central terminal evidence, zero valid unresolved findings, and qualifying independent non-author approval. No second same-head state toggle, bypass, Force Push, synthetic evidence, or blind rerun is authorized.

Summary by CodeRabbit

테스트
- 비동기 재시도 테스트가 재시도 횟수뿐 아니라 작업 상태가 성공으로 완료되었는지도 확인하도록 개선되었습니다.

coderabbitai · 2026-08-15T10:29:25Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 850fc5d5-96f9-479b-b252-0141d70dd0bf

📥 Commits

Reviewing files that changed from the base of the PR and between eb0a32e and 23ba9a3.

📒 Files selected for processing (1)

src/test/java/com/clearfolio/viewer/service/DefaultConversionWorkerTest.java

📝 Walkthrough

Walkthrough

비동기 재시도 테스트가 재시도 횟수와 변환 작업의 SUCCEEDED 상태를 함께 확인하도록 변경되었습니다.

Changes

변환 재시도 검증

Layer / File(s)	Summary
재시도 성공 조건 검증 `src/test/java/com/clearfolio/viewer/service/DefaultConversionWorkerTest.java`	재시도 대기 조건에 변환 작업의 `SUCCEEDED` 상태 확인을 추가했습니다.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Merge Risk: ⚪ Minimal · up to 23ba9

This localized test-only change removes an asynchronous assertion race without changing production behavior; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	제목은 비동기 재시도 완료를 확인한 후 어설션하도록 테스트를 수정한 주요 변경 사항을 정확하고 간결하게 설명합니다.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch test/worker-retry-completion-race-20260815

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.

❤️ Share

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

seonghobae · 2026-08-15T10:50:35Z

@opencode-agent review
@coderabbitai review
@cwl-noema-review review
@strix review

Review exact head 23ba9a34e24d62de7974a6e1707dc53f6f69b4ac only. Exact-head CI, Security Scan, SAST Semgrep, and fuzz are terminal-success; unresolved inline findings are zero. Validate that the asynchronous retry test waits for both converter execution and terminal SUCCEEDED publication without sleeps, production changes, or weakened assertions.

Review read-only; preserve independent reviewer identity. Do not mutate the branch, synthesize author approval, weaken protection, or merge.

coderabbitai · 2026-08-15T10:51:10Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

expected_head='23ba9a34e24d62de7974a6e1707dc53f6f69b4ac'
base='eb0a32e87e7470469c8fa0f8c67d9583654bef57'
repo='ContextualWisdomLab/clearfolio'
pr='454'

echo '== Checked-out commit =='
git rev-parse HEAD
git status --short
echo '== Commit identity =='
git show -s --format='%H%n%P%n%s' "$expected_head"

echo '== Changed paths and diff check =='
git diff --name-status "$base" "$expected_head"
git diff --check "$base" "$expected_head"
echo '== Exact diff =='
git diff --no-ext-diff --unified=80 "$base" "$expected_head" -- src/test/java/com/clearfolio/viewer/service/DefaultConversionWorkerTest.java

echo '== Test file outline =='
ast-grep outline src/test/java/com/clearfolio/viewer/service/DefaultConversionWorkerTest.java --items all --view expanded
echo '== Retry test and synchronization usages =='
rg -n -C 12 -e 'workerReschedulesWhenRetryAtIsInTheFuture' -e '\bawait\b' -e 'Thread\.sleep|TimeUnit\..*sleep' src/test/java/com/clearfolio/viewer/service/DefaultConversionWorkerTest.java

echo '== Pull request metadata =='
gh pr view "$pr" --repo "$repo" --json number,headRefOid,baseRefOid,changedFiles,mergeStateStatus,reviewDecision,statusCheckRollup,reviews
echo '== Review threads =='
gh api graphql -F owner='ContextualWisdomLab' -F name='clearfolio' -F number="$pr" -f query='
query($owner: String!, $name: String!, $number: Int!) {
  repository(owner: $owner, name: $name) {
    pullRequest(number: $number) {
      reviewThreads(first: 100) {
        nodes {
          isResolved
          isOutdated
          path
          line
          comments(first: 20) {
            nodes { author { login } body url }
          }
        }
      }
    }
  }
}'

Length of output: 29071

@seonghobae Review complete for exact head 23ba9a34e24d62de7974a6e1707dc53f6f69b4ac.

Finding: workerReschedulesWhenRetryAtIsInTheFuture does not assert that job.getRetryAt() is cleared after success. The test retains assertions for one converter call, one attempt, SUCCEEDED, and the converted path, but it does not verify the cleared retry time specified for this scenario.

Add:

assertNull(job.getRetryAt());

The new await condition correctly waits for both converter execution and SUCCEEDED publication. The target test has no direct sleep. Only the shared polling helper uses Thread.sleep(25).

The exact-head diff changes one test file only. git diff --check is clean. CI, Security Scan, SAST Semgrep, and the listed fuzz checks are successful. No unresolved review threads exist. Strix Security Scan is still in progress, and coverage-source-tree is queued.

_{You are interacting with an AI system.}

seonghobae · 2026-08-15T11:02:08Z

Exact-current-head independent review request for 23ba9a34e24d62de7974a6e1707dc53f6f69b4ac over protected main eb0a32e87e7470469c8fa0f8c67d9583654bef57.

CI, Security Scan, SAST Semgrep, and fuzz are terminal-success; unresolved inline review threads are zero. The change is test synchronization only and leaves production behavior unchanged.

@opencode-agent review
@coderabbitai review
@cwl-noema-review review
@strix review

Review this exact head read-only. Do not mutate the branch, synthesize author approval, weaken protection, or merge.

coderabbitai · 2026-08-15T11:02:39Z

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='23ba9a34e24d62de7974a6e1707dc53f6f69b4ac'
base='eb0a32e87e7470469c8fa0f8c67d9583654bef57'
repo='ContextualWisdomLab/clearfolio'
pr='454'
file='src/test/java/com/clearfolio/viewer/service/DefaultConversionWorkerTest.java'

echo '== Exact commit =='
git rev-parse HEAD
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$head"
git merge-base --is-ancestor "$base" "$head" && echo 'base-is-ancestor=yes'

echo '== Changed paths and whitespace =='
git diff --name-status "$base" "$head"
git diff --check "$base" "$head"

echo '== Target file diff =='
git diff --no-ext-diff --unified=100 "$base" "$head" -- "$file"

echo '== Target file structure =='
ast-grep outline "$file" --items all --view expanded

echo '== Retry test, await predicates, assertions, and sleep references =='
rg -n -C 15 \
  -e 'workerReschedulesWhenRetryAtIsInTheFuture' \
  -e '\bawait\b' \
  -e 'assert(Equals|Null|That)' \
  -e 'Thread\.sleep|TimeUnit\..*sleep' \
  "$file"

echo '== PR metadata and check status =='
gh pr view "$pr" --repo "$repo" \
  --json number,headRefOid,baseRefOid,changedFiles,mergeStateStatus,reviewDecision,statusCheckRollup \
  --jq '{number,headRefOid,baseRefOid,changedFiles,mergeStateStatus,reviewDecision,statusCheckRollup:[.statusCheckRollup[]|{name,conclusion,status}]}'

echo '== Review thread counts =='
gh api graphql -F owner='ContextualWisdomLab' -F name='clearfolio' -F number="$pr" -f query='
query($owner: String!, $name: String!, $number: Int!) {
  repository(owner: $owner, name: $name) {
    pullRequest(number: $number) {
      reviewThreads(first: 100) {
        nodes { isResolved isOutdated path line }
      }
    }
  }
}' --jq '
  [.data.repository.pullRequest.reviewThreads.nodes[]] as $t |
  {
    total: ($t|length),
    unresolved: ([$t[]|select(.isResolved|not)]|length),
    unresolvedCurrent: ([$t[]|select((.isResolved|not) and (.isOutdated|not))]|length),
    unresolvedCurrentThreads: [$t[]|select((.isResolved|not) and (.isOutdated|not))|{path,line}]
  }'

Length of output: 52176

@seonghobae Review complete for exact head 23ba9a34e24d62de7974a6e1707dc53f6f69b4ac.

Finding — src/test/java/com/clearfolio/viewer/service/DefaultConversionWorkerTest.java, Line 403: workerReschedulesWhenRetryAtIsInTheFuture does not assert that success clears retryAt.

Add this assertion after the success-state assertion:

assertNull(job.getRetryAt());

The new await predicate correctly waits for both converter execution and terminal SUCCEEDED publication. The target test has no direct sleep. The shared polling helper uses Thread.sleep(25).

The diff changes one test file only. git diff --check is clean. The listed required checks are successful. No unresolved review threads exist. opencode-review is still queued.

⚠️ Action not completed

Already reviewed.

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 · 2026-08-15T12:07:18Z

Exact-current-head independent review request for 23ba9a34e24d62de7974a6e1707dc53f6f69b4ac over protected main eb0a32e87e7470469c8fa0f8c67d9583654bef57.

Freshly rechecked: CI, Security Scan, SAST Semgrep, and fuzz are terminal-success; mergeable is true; unresolved inline review threads are zero. This is a one-file test-synchronization repair with no production-code change. Review this exact head only; predecessor evidence does not transfer.

@coderabbitai review
@opencode-agent review
@cwl-noema-review review
@strix review

coderabbitai · 2026-08-15T12:07:53Z

Rate Limit Exceeded

@seonghobae have exceeded the limit for the number of chat messages per hour. Please wait 26 minutes and 15 seconds before sending another message.

cursor

Review: race wait is correct; indent before merge

Exact head 23ba9a34 changes one wait in workerReschedulesWhenRetryAtIsInTheFuture. Waiting for both attempts.get() >= 1 and ConversionJobStatus.SUCCEEDED matches the following assertions and closes the window where the converter has started but the job object is not terminal yet. Hosted checks on this SHA are already green.

The await(...) call itself is smashed to the left. Restore the surrounding indentation so this stays a one-line contract change. Do not expand the PR. After that indent-only follow-up, this is mergeable as a test-only race repair.

This is a source review of the one-file diff. CodeRabbit CLI cannot authenticate in this environment.

_{Sent by Cursor Automation: Fix Issues}

opencode-agent · 2026-08-19T11:13:57Z

Queued @opencode-agent ; Already queued @cwl-noema-review on this exact request for PR #454 at head 23ba9a34e24d62de7974a6e1707dc53f6f69b4ac. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

opencode-agent · 2026-08-19T11:14:00Z

Queued @opencode-agent ; Already queued @cwl-noema-review on this exact request for PR #454 at head 23ba9a34e24d62de7974a6e1707dc53f6f69b4ac. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

opencode-agent · 2026-08-19T11:14:02Z

Queued @opencode-agent ; Already queued @cwl-noema-review on this exact request for PR #454 at head 23ba9a34e24d62de7974a6e1707dc53f6f69b4ac. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

opencode-agent · 2026-08-20T03:31:08Z