fix(retry): claim due provider writeback retries with SKIP LOCKED

[seonghobae]

Current authority — 2026-09-11

• protected root: develop@042b0c70531b229af3acbd0421a2f23098d848b3
• canonical dependency-security parent: #1623@17a7618eda2b212b691f08fa936e042b34258fc9
• duplicate-claim / per-item-lock lineage: 75ffe70d... → 7b8071fa... / c9fc20b... → 5ddb10cf...
• reviewed fake-session due-time repair: 050e5f00430c31fbae5b6fe031105e7af2c01e8f → cf1e6220bcc5ad7f2927d7f0fb86b155dd91a0b3
• invalid retry-policy RED/fix: efd526ad707e7098b4f453a92e414c585e1b0905 → 18c825dbb17905d0f7e2ed54fdbf36f03f1b76a1
• poll-cadence/backoff coupling RED: df673f778a6963c4a5cb7d129d850ec7d7d868d3
• causal cadence/backoff separation: 088652d57d4dcd9a17bcd9008b8c72fcb9152902
• exact head / code-current doctoring: 23b2cae3073df5edba9323abe01c7fd00cb03c0f
• fresh compare over fix(deps): patch frontend audit security floors #1623: ahead 21 / behind 0 / exactly six effective files
• lifecycle: Draft / source repaired / exact-head hosted execution and post-last-push independent approval absent / prerequisite integration absent / do not merge

Current finding and causal repair

Fresh review found that ProviderWritebackRetryWorker represented two different time policies with one field. interval_seconds controls how often the worker polls for due work, but _sync() also passed that value as retry_delay_seconds to process_due_provider_writeback_retries().

That coupling made an operational scheduling choice silently rewrite the retry domain policy. With the existing defaults, an initial transient failure is scheduled with the service's 300-second delay, but a later failure processed by the long-running worker used its 60-second polling interval as the backoff base. Tuning worker polling latency therefore also changed provider retry pressure without an explicit retry-policy decision.

Source-order RED df673f7... adds a focused worker-policy test. It constructs a worker with interval_seconds=7 and retry_delay_seconds=300, executes the real _sync() boundary with controlled session/process doubles, and requires the process function to receive those values independently. The predecessor constructor cannot express that contract. The same RED requires negative worker retry delay to fail closed while preserving explicit retry_delay_seconds=0 semantics.

Production fix 088652d... adds a distinct retry_delay_seconds worker parameter with the existing service default of 300 seconds, validates only its semantic lower bound, stores it separately, and passes it through _sync(). interval_seconds remains solely the polling cadence consumed by _run_loop(). No arbitrary maximum delay, attempt, batch, token, or timeout value is introduced.

docs/doctoring/provider-writeback-retry-policy-cadence.md records the problem, RED/fix lineage, rejected alternatives, acceptance boundary, and Python 3.14.7 asyncio.sleep() authority. The earlier concurrency doctoring remains unchanged and still owns the PostgreSQL claim/lock contract.

Retained concurrency contract

The worker claims one due row at a time with FOR UPDATE SKIP LOCKED, performs provider I/O while holding only that row's lock, persists the resulting terminal/rescheduled state, commits, and only then claims another row. batch_limit bounds per-item claim cycles in one pass. Real PostgreSQL acceptance verifies both duplicate-claim prevention and that a blocked first dispatch does not lock later independent due rows.

The corrected fake-session regression preserves production's next_retry_at <= current_time predicate, so a rescheduled future row cannot be spuriously reselected in the same pass. Configuration boundaries still require interval_seconds > 0, batch_limit > 0, max_attempts > 0, and retry_delay_seconds >= 0.

This PR still does not claim exactly-once remote side effects. A process failure after the provider accepts the write but before the local transaction commits remains a provider-idempotency / conditional-write or durable-lease recovery boundary.

Evidence boundary

Fresh lookup for exact 23b2cae... returns zero PR-triggered workflow runs on the stacked feature base. Therefore the new cadence/backoff regression and production fix, the prior retry unit regressions, and the real PostgreSQL two-session contracts have no current hosted product receipt. Predecessor receipts are historical only.

This remains a .github#2073 stacked-verification blocker. Do not manufacture evidence through temporary develop retargeting, dummy/no-op commits, copied workflows, synthetic statuses, or stale receipt transfer.

No formal post-last-push APPROVED review currently binds to 23b2cae.... Request fresh independent review against exact 17a7618... → 23b2cae... and keep Draft until executable evidence, zero valid current-head findings/threads, qualifying approval, and #1623 prerequisite integration are all present.

Standards traceability

• docs/doctoring/provider-writeback-retry-claim.md — PostgreSQL claim/locking, per-item transaction scope, fake-session due-time fidelity, retry configuration validity, crash/idempotency limits.
• docs/doctoring/provider-writeback-retry-policy-cadence.md — polling cadence and retry-backoff separation.

Primary authority remains PostgreSQL 18 Explicit Locking / SELECT locking-clause documentation plus Python Software Foundation (2026), Coroutines and tasks — Python 3.14.7 documentation: Sleeping.

Merge boundary

Keep Draft. Merge is prohibited until the exact current base/head has trustworthy executable product/PostgreSQL evidence, current-head independent approval, zero unresolved valid findings, and the canonical dependency-security prerequisite is integrated or otherwise satisfied through the canonical stacked-verification path. No self-approval, force-push, destructive rebase, dummy requeue, copied central workflow, provider/model workaround, stale evidence transfer, or gate weakening.

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

✨ 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 autoresearch/provider-retry-claim-1086

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[seonghobae]

Infrastructure evidence for the shared required-check failures at head 53094e9: trivy-fs job 103096736615 fails after git call failed ... fatal: bad object 546f5873336cbc5bb321ed339ef1ebdc1990adc5; CodeQL compatibility job 103096123471 exits because the dispatch workflow reports that the scan was dispatched and will rerun the exact failed job after its terminal verdict. The same CodeQL/trivy failures are present on unrelated draft PRs #1653, #1655, and #1656, so this is not evidence of a defect in the provider retry delta. Local focused verification remains green (8 tests, strict deprecation warnings).

[seonghobae]

@coderabbitai review

Fresh exact-range review requested for canonical base 17a7618eda2b212b691f08fa936e042b34258fc9 → head 1252505dd6926b88b8bce72c9377fddfdf8dc8f1. Please verify the FOR UPDATE SKIP LOCKED concurrency contract, the real-PostgreSQL two-session acceptance test, cleanup/isolation, and the explicit limitation that this prevents concurrent claims but does not claim exactly-once recovery after a remote side effect. Pre-retarget receipts are predecessor evidence only.

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="17a7618eda2b212b691f08fa936e042b34258fc9"
head="1252505dd6926b88b8bce72c9377fddfdf8dc8f1"

echo "== Exact commit presence and merge-base =="
git cat-file -e "${base}^{commit}"
git cat-file -e "${head}^{commit}"
git merge-base "$base" "$head"
git rev-list --left-right --count "${base}...${head}"

echo "== Changed files in requested range =="
git diff --name-status "$base" "$head"

echo "== Full exact-range diff =="
git diff --find-renames --find-copies --no-ext-diff "$base" "$head"

echo "== Changed-file outlines =="
while IFS= read -r file; do
  case "$file" in
    *.py)
      echo "--- $file"
      ast-grep outline "$file" --items all || true
      ;;
  esac
done < <(git diff --name-only "$base" "$head")

echo "== Production symbol definitions and calls =="
rg -n -C 5 --glob '*.py' \
  '\b(process_due_provider_writeback_retries|_due_retry_query)\s*\(' .

echo "== PostgreSQL fixture and retry model references =="
rg -n -C 5 --glob '*.py' \
  '\b(postgresql|postgres|PostgreSQL|provider_writeback|ProviderWriteback|writeback_retry)\b' \
  .

Length of output: 13339

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Retry service structure and implementation =="
ast-grep outline backend/services/provider_writeback_retry_service.py --items all
nl -ba backend/services/provider_writeback_retry_service.py | sed -n '1,280p'

echo "== Retry item model definition =="
rg -n -C 12 --glob '*.py' \
  'class\s+ProviderWritebackRetryItem\b|ProviderWritebackRetryItem\s*=' \
  backend

echo "== Scheduling and worker call sites =="
rg -n -C 6 --glob '*.py' \
  '\b(schedule_provider_writeback_retry|process_due_provider_writeback_retries)\s*\(' \
  backend

echo "== Test configuration and database URL configuration =="
rg -n -C 8 --glob '*.py' \
  '(DATABASE_URL|create_async_engine|async_sessionmaker|pytest\.fixture|fixture)' \
  backend/tests backend/core 2>/dev/null || true

echo "== Existing PostgreSQL integration-test patterns =="
rg -n -C 10 --glob '*.py' \
  '(engine\.dialect\.name.*postgresql|pytest\.skip\("PostgreSQL|asyncio\.wait_for|FOR UPDATE|with_for_update)' \
  backend/tests

Length of output: 176294

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Production query, scheduling, and transaction lifecycle =="
nl -ba backend/services/provider_writeback_retry_service.py | sed -n '1,270p'

echo "== Retry item model location and definition =="
model_file="$(rg -l --glob '*.py' 'class\s+ProviderWritebackRetryItem\b' backend | head -n 1)"
printf '%s\n' "$model_file"
if [ -n "$model_file" ]; then
  model_line="$(rg -n 'class\s+ProviderWritebackRetryItem\b' "$model_file" | head -n 1 | cut -d: -f1)"
  sed -n "$((model_line-5)),$((model_line+100))p" "$model_file"
fi

echo "== Pytest marker registration and PostgreSQL CI selection =="
rg -n -C 6 --glob 'pytest.ini' --glob 'pyproject.toml' --glob 'tox.ini' --glob 'setup.cfg' \
  'markers|postgres|addopts' .
rg -n -C 8 --glob '*.yml' --glob '*.yaml' \
  '(pytest|backend/tests|postgres)' .github 2>/dev/null || true

echo "== Existing retry-item database setup or cleanup =="
rg -n -C 8 --glob '*.py' \
  '(ProviderWritebackRetryItem|provider_writeback_retry_items|provider_writeback_retry)' \
  backend/tests backend/alembic 2>/dev/null || true

Length of output: 55870

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Workflow inventory and PostgreSQL-related definitions =="
fd -t f -e yml -e yaml . .github/workflows -x sh -c '
  file="$1"
  if rg -q "(postgres|PostgreSQL|DATABASE_URL|alembic|pytest)" "$file"; then
    echo "--- $file"
    nl -ba "$file" | sed -n "1,260p"
  fi
' sh {}

echo "== Backend pytest configuration =="
nl -ba backend/pytest.ini

echo "== Retry migration table definition =="
nl -ba backend/alembic/versions/0002_provider_writeback_retry_queue.py | sed -n '1,110p'

echo "== Exact-head pull request checks and workflow runs =="
gh pr view 1654 --repo ContextualWisdomLab/naruon \
  --json number,state,isDraft,baseRefOid,headRefOid,statusCheckRollup
gh run list --repo ContextualWisdomLab/naruon --commit 1252505dd6926b88b8bce72c9377fddfdf8dc8f1 \
  --limit 30 --json databaseId,name,status,conclusion,event,headSha,workflowName,url

Length of output: 28114

---

@seonghobae Review complete for 17a7618eda2b212b691f08fa936e042b34258fc9...1252505dd6926b88b8bce72c9377fddfdf8dc8f1.

Finding

• P1 — Add a migrated PostgreSQL CI acceptance lane for the concurrency test. backend/tests/test_provider_writeback_retry_postgres.py can skip when PostgreSQL is unavailable. Application CI runs python -m pytest -q without a PostgreSQL service, a DATABASE_URL, or a migration step. No dedicated PostgreSQL workflow exists in .github/workflows. Therefore, the exact-head checks can succeed while the two-session SKIP LOCKED assertion never executes. Add a CI job that starts PostgreSQL, migrates the schema, and runs this test successfully. A local skip must remain non-acceptance evidence.

Verified

• _due_retry_query() uses FOR UPDATE SKIP LOCKED.
• The query preserves due-time ordering and batch_limit.
• process_due_provider_writeback_retries() retains the lock through dispatch_command() and commits after processing.
• The acceptance test uses two independent sessions, keeps the first transaction open, bounds the second query, rolls back both transactions, and deletes only the generated row.
• The doctoring document correctly limits the guarantee to concurrent claims. It does not claim exactly-once recovery after a remote side effect and a process failure.

The current exact-head checks are queued. I did not use predecessor receipts as 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]

@coderabbitai review

현재 exact range 17a7618eda2b212b691f08fa936e042b34258fc9..1252505dd6926b88b8bce72c9377fddfdf8dc8f1만 검토해 주세요. FOR UPDATE SKIP LOCKED queue-claim semantics, 두 독립 PostgreSQL session의 현실 concurrency regression, cleanup scope, 그리고 crash-after-remote-side-effect를 exactly-once로 과장하지 않는 evidence boundary에 집중해 주세요. Pre-retarget predecessor evidence는 current-head approval로 간주하지 않습니다.

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="17a7618eda2b212b691f08fa936e042b34258fc9"
head="1252505dd6926b88b8bce72c9377fddfdf8dc8f1"

echo "== Exact range topology =="
git cat-file -e "${base}^{commit}"
git cat-file -e "${head}^{commit}"
printf 'merge-base: '
git merge-base "$base" "$head"
printf 'left-right count: '
git rev-list --left-right --count "${base}...${head}"

echo "== Exact-range changed files =="
git diff --name-status "$base" "$head"

echo "== Exact-range diff =="
git diff --find-renames --find-copies --no-ext-diff "$base" "$head"

echo "== Python changed-file outlines =="
git diff --name-only "$base" "$head" | while IFS= read -r file; do
  case "$file" in
    *.py)
      echo "--- $file"
      ast-grep outline "$file" --items all || true
      ;;
  esac
done

echo "== Production query and processor definitions =="
rg -n -C 12 --glob '*.py' \
  '(^\s*(async\s+)?def\s+(_due_retry_query|process_due_provider_writeback_retries)\b|\b(_due_retry_query|process_due_provider_writeback_retries)\s*\()' \
  backend

echo "== PostgreSQL acceptance-test and fixture context =="
rg -n -C 12 --glob '*.py' \
  '(ProviderWritebackRetryItem|_due_retry_query|with_for_update|skip_locked|postgresql|PostgreSQL|asyncio\.wait_for)' \
  backend/tests

echo "== Doctoring boundary text =="
fd -t f 'provider-writeback-retry-claim\.md' docs -x sh -c '
  echo "--- $1"
  nl -ba "$1"
' sh {}

Length of output: 183932

---

@seonghobae Review complete for exact range 17a7618eda2b212b691f08fa936e042b34258fc9..1252505dd6926b88b8bce72c9377fddfdf8dc8f1.

Finding

• P1 — Add a PostgreSQL CI acceptance lane for backend/tests/test_provider_writeback_retry_postgres.py. The new test skips if PostgreSQL is unavailable. This exact range does not add or modify a workflow that starts PostgreSQL, prepares the schema, and executes this test. Therefore, ordinary CI can pass while the two-session SKIP LOCKED assertion does not run. Add a migrated PostgreSQL CI job that runs this test against real PostgreSQL. Do not treat a skipped local result as concurrency evidence.

Verified

• backend/services/provider_writeback_retry_service.py::_due_retry_query() selects due pending rows in due-time order, retains batch_limit, and uses FOR UPDATE SKIP LOCKED.
• process_due_provider_writeback_retries() executes the production claim query before dispatch and commits after processing. The transaction retains the claim lock during dispatch.
• backend/tests/test_provider_writeback_retry_postgres.py uses two independent SQLAlchemy sessions. The first session locks the generated due row. The second session executes the same production query under a two-second bound and does not receive that row.
• The test rolls back both worker sessions and deletes only its generated retry_item_uid.
• docs/doctoring/provider-writeback-retry-claim.md correctly limits the guarantee to concurrent claims. It does not claim exactly-once recovery after a remote provider accepts a write and the process fails before the local commit.
• I treated no pre-retarget predecessor receipt as current-head approval evidence.

⚠️ Action not completed

Head commit changed.

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]

@coderabbitai review

현재 exact range 17a7618eda2b212b691f08fa936e042b34258fc9..f345af94f949374c5ee6ce3f48479736d85f4c41만 검토해 주세요. 새 source/test/doc 변경은 PostgreSQL negative LIMIT가 unbounded로 해석될 수 있는 경계를 fail closed하도록 한 58bf1c65... → da0073d5... → f345af94...입니다. FOR UPDATE SKIP LOCKED semantics, real PostgreSQL two-session regression, nonpositive batch-limit rejection, cleanup scope, 그리고 crash-after-side-effect exactly-once 비주장을 함께 확인해 주세요. Predecessor review/check evidence는 current-head approval로 간주하지 않습니다.

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="17a7618eda2b212b691f08fa936e042b34258fc9"
head="f345af94f949374c5ee6ce3f48479736d85f4c41"

echo "== Exact topology =="
git cat-file -e "${base}^{commit}"
git cat-file -e "${head}^{commit}"
printf 'merge-base: '
git merge-base "$base" "$head"
printf 'range count: '
git rev-list --count "${base}..${head}"

echo "== Changed files =="
git diff --name-status "$base" "$head"

echo "== Exact diff =="
git diff --find-renames --find-copies --no-ext-diff "$base" "$head"

echo "== Changed Python outlines =="
git diff --name-only "$base" "$head" | while IFS= read -r file; do
  case "$file" in
    *.py)
      echo "--- $file"
      ast-grep outline "$file" --items all || true
      ;;
  esac
done

echo "== Production retry service =="
nl -ba backend/services/provider_writeback_retry_service.py | sed -n '1,300p'

echo "== Changed retry tests and doctoring =="
for file in \
  backend/tests/test_provider_writeback_retry_service.py \
  backend/tests/test_provider_writeback_retry_postgres.py \
  docs/doctoring/provider-writeback-retry-claim.md
do
  if [ -f "$file" ]; then
    echo "--- $file"
    nl -ba "$file" | sed -n '1,360p'
  fi
done

echo "== Retry model and relevant call sites =="
rg -n -C 8 --glob '*.py' \
  'class\s+ProviderWritebackRetryItem\b|\b(process_due_provider_writeback_retries|_due_retry_query)\s*\(' \
  backend

echo "== Exact-head PR and workflow state =="
gh pr view 1654 --repo ContextualWisdomLab/naruon \
  --json number,state,isDraft,baseRefOid,headRefOid,statusCheckRollup
gh run list --repo ContextualWisdomLab/naruon --commit "$head" --limit 50 \
  --json databaseId,name,status,conclusion,event,headSha,workflowName,url

Length of output: 68422

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="ContextualWisdomLab/naruon"
head="f345af94f949374c5ee6ce3f48479736d85f4c41"

echo "== Canonical .github PR `#2073` =="
gh pr view 2073 --repo "$repo" \
  --json number,state,isDraft,title,url,baseRefName,baseRefOid,headRefName,headRefOid,body,statusCheckRollup

echo "== Workflows that can execute PostgreSQL retry acceptance =="
fd -t f -e yml -e yaml . .github/workflows -x sh -c '
  file="$1"
  if rg -q "(postgres|PostgreSQL|DATABASE_URL|provider_writeback_retry_postgres|alembic)" "$file"; then
    echo "--- $file"
    nl -ba "$file" | sed -n "1,320p"
  fi
' sh {}

echo "== Exact-head GitHub check suites and workflow runs =="
gh api --paginate \
  "repos/${repo}/commits/${head}/check-suites" \
  --jq '.check_suites[] | {app: .app.slug, status, conclusion, head_sha, url}'
gh run list --repo "$repo" --commit "$head" --limit 100 \
  --json databaseId,name,status,conclusion,event,headSha,workflowName,url

Length of output: 1383

---

🏁 Script executed:

#!/bin/bash
set -u

head="f345af94f949374c5ee6ce3f48479736d85f4c41"

echo "== Organization .github PR `#2073` =="
gh pr view 2073 --repo ContextualWisdomLab/.github \
  --json number,state,isDraft,title,url,baseRefName,baseRefOid,headRefName,headRefOid,body,statusCheckRollup \
  || true

echo "== Organization .github issue `#2073`, if it is not a pull request =="
gh issue view 2073 --repo ContextualWisdomLab/.github \
  --json number,state,title,url,body \
  || true

echo "== Exact-head check suites =="
gh api "repos/ContextualWisdomLab/naruon/commits/${head}/check-suites" \
  --jq '.check_suites[] | {app: .app.slug, status, conclusion, head_sha, url}' \
  || true

echo "== Exact-head workflow runs =="
gh run list --repo ContextualWisdomLab/naruon --commit "$head" --limit 100 \
  --json databaseId,name,status,conclusion,event,headSha,workflowName,url \
  || true

Length of output: 8511

---

@seonghobae Review complete for exact range 17a7618eda2b212b691f08fa936e042b34258fc9..f345af94f949374c5ee6ce3f48479736d85f4c41.

Finding

• P1 — Exact-head PostgreSQL acceptance evidence is still unavailable. backend/tests/test_provider_writeback_retry_postgres.py skips when PostgreSQL is unavailable. The exact range has no PostgreSQL CI workflow. Exact head f345af94f949374c5ee6ce3f48479736d85f4c41 has zero GitHub Actions workflow runs. The referenced .github#2073 is an open issue, not an implemented CI path. Therefore, the two-session acceptance test and the nonpositive-limit regression have no terminal exact-head execution evidence. Keep this PR Draft until the canonical owner path produces a trustworthy exact-base/head run. Do not use predecessor receipts.

Verified

• backend/services/provider_writeback_retry_service.py::_due_retry_query() rejects 0 and negative batch_limit values before it constructs or executes SQL. The error is ValueError("batch_limit must be positive").
• The query keeps ascending next_retry_at ordering, applies the positive batch limit, and uses FOR UPDATE SKIP LOCKED.
• process_due_provider_writeback_retries() executes the claim query before dispatch. It commits after it processes the claimed rows. The transaction therefore retains the row lock during dispatch.
• backend/tests/test_provider_writeback_retry_service.py verifies that a positive limit is retained and that 0 and -1 fail.
• backend/tests/test_provider_writeback_retry_postgres.py uses two independent AsyncSession instances. The first session claims and retains the row lock. The second session runs the same production query under a two-second timeout and must not receive that row.
• The PostgreSQL test rolls back both worker sessions. Its cleanup deletes only the generated retry_item_uid.
• docs/doctoring/provider-writeback-retry-claim.md correctly states the boundary. The change prevents concurrent live-worker claims. It does not claim exactly-once recovery after a remote provider accepts a write and the process fails before the local commit.

I did not treat any predecessor review or check result as current-head approval evidence.

⚠️ Action not completed

Head commit changed.

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]

@coderabbitai review

현재 exact range 17a7618eda2b212b691f08fa936e042b34258fc9..a8fcba66f7235ac7d8f4a7a29818246f2703e5b3만 검토해 주세요. a8fcba66...은 PostgreSQL negative LIMIT semantics를 정확히 바로잡은 doctoring correction입니다: negative LIMIT은 DB에서 거절되고, zero는 no-work이므로 Naruon이 둘 다 invalid batch configuration으로 선제 거절합니다. Production RED/fix 58bf1c65... → da0073d5..., FOR UPDATE SKIP LOCKED, real PostgreSQL two-session regression, cleanup scope와 non-exactly-once boundary를 함께 확인해 주세요. Earlier-head review/check evidence는 current-head approval로 간주하지 않습니다.

[coderabbitai]

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="17a7618eda2b212b691f08fa936e042b34258fc9"
head="a8fcba66f7235ac7d8f4a7a29818246f2703e5b3"
repo="ContextualWisdomLab/naruon"

echo "== Exact range topology =="
git cat-file -e "${base}^{commit}"
git cat-file -e "${head}^{commit}"
printf 'merge-base: '
git merge-base "$base" "$head"
printf 'range count: '
git rev-list --count "${base}..${head}"

echo "== Exact range changed files =="
git diff --name-status "$base" "$head"

echo "== Exact range diff =="
git diff --find-renames --find-copies --no-ext-diff "$base" "$head"

echo "== Changed Python outlines =="
git diff --name-only "$base" "$head" | while IFS= read -r file; do
  case "$file" in
    *.py)
      echo "--- $file"
      ast-grep outline "$file" --items all || true
      ;;
  esac
done

echo "== Retry service =="
nl -ba backend/services/provider_writeback_retry_service.py | sed -n '1,300p'

echo "== Changed tests and doctoring =="
for file in \
  backend/tests/test_provider_writeback_retry_service.py \
  backend/tests/test_provider_writeback_retry_postgres.py \
  docs/doctoring/provider-writeback-retry-claim.md
do
  if [ -f "$file" ]; then
    echo "--- $file"
    nl -ba "$file" | sed -n '1,420p'
  fi
done

echo "== Retry model and production call sites =="
rg -n -C 10 --glob '*.py' \
  'class\s+ProviderWritebackRetryItem\b|\b(process_due_provider_writeback_retries|_due_retry_query)\s*\(' \
  backend

echo "== Exact-head PR state, check suites, and workflow runs =="
gh pr view 1654 --repo "$repo" \
  --json number,state,isDraft,baseRefOid,headRefOid,statusCheckRollup
gh api "repos/${repo}/commits/${head}/check-suites" \
  --jq '.check_suites[] | {app: .app.slug, status, conclusion, head_sha, url}' || true
gh run list --repo "$repo" --commit "$head" --limit 100 \
  --json databaseId,name,status,conclusion,event,headSha,workflowName,url

Length of output: 72839

---

@seonghobae Review complete for exact range 17a7618eda2b212b691f08fa936e042b34258fc9..a8fcba66f7235ac7d8f4a7a29818246f2703e5b3.

Finding

• P1 — Exact-head PostgreSQL acceptance evidence is unavailable. backend/tests/test_provider_writeback_retry_postgres.py skips when PostgreSQL is unavailable. The exact head has no GitHub Actions workflow run, and its visible check suites are queued. Therefore, this review cannot treat the two-session concurrency regression or the nonpositive-limit regression as executed acceptance evidence. Keep the PR Draft until the canonical stacked-verification path produces a terminal successful run for this exact base and head. Do not transfer earlier-head evidence.

Verified

• _due_retry_query() rejects batch_limit=0 and negative values with ValueError("batch_limit must be positive") before SQL execution.
• The query retains ascending next_retry_at order, applies the positive limit, and uses FOR UPDATE SKIP LOCKED.
• process_due_provider_writeback_retries() holds the claim transaction through dispatch_command() and commits only after item processing.
• The PostgreSQL regression uses two independent AsyncSession instances. The first session claims the generated due row. The second session executes the production query under a two-second bound and must not receive that row.
• The test rolls back both worker sessions. Cleanup deletes only the generated retry_item_uid.
• The unit regression verifies the retained positive limit and rejection of 0 and -1.
• The doctoring correction accurately states PostgreSQL behavior: negative LIMIT is rejected and LIMIT 0 returns no rows.
• The doctoring document correctly limits the guarantee to concurrent live-worker claims. It does not claim exactly-once recovery after a remote side effect succeeds but the local transaction fails before commit.

No additional source-level finding was identified in the requested scope.

You are interacting with an AI system.

[seonghobae]

@coderabbitai review

Please review the current exact stacked range 17a7618eda2b212b691f08fa936e042b34258fc9..a8fcba66f7235ac7d8f4a7a29818246f2703e5b3. Focus on the PostgreSQL FOR UPDATE SKIP LOCKED claim semantics, the real two-session concurrency acceptance, nonpositive batch_limit fail-closed behavior, and whether the doctoring accurately limits the claim to concurrent-live-worker duplicate dispatch rather than exactly-once remote side effects. Treat predecessor workflow receipts as stale.

[coderabbitai]

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="17a7618eda2b212b691f08fa936e042b34258fc9"
head="a8fcba66f7235ac7d8f4a7a29818246f2703e5b3"
repo="ContextualWisdomLab/naruon"

echo "== Exact topology =="
git cat-file -e "${base}^{commit}"
git cat-file -e "${head}^{commit}"
printf 'merge-base: '; git merge-base "$base" "$head"
printf 'base..head commits: '; git rev-list --count "${base}..${head}"

echo "== Changed files =="
git diff --name-status "$base" "$head"

echo "== Exact diff =="
git diff --find-renames --find-copies --no-ext-diff "$base" "$head"

echo "== Changed Python outlines =="
git diff --name-only "$base" "$head" | while IFS= read -r file; do
  case "$file" in
    *.py) echo "--- $file"; ast-grep outline "$file" --items all ;;
  esac
done

echo "== Production retry service =="
nl -ba backend/services/provider_writeback_retry_service.py | sed -n '1,300p'

echo "== Affected tests and doctoring =="
for file in \
  backend/tests/test_provider_writeback_retry_service.py \
  backend/tests/test_provider_writeback_retry_postgres.py \
  docs/doctoring/provider-writeback-retry-claim.md
do
  echo "--- $file"
  nl -ba "$file" | sed -n '1,430p'
done

echo "== Retry model and all production processing call sites =="
rg -n -C 10 --glob '*.py' \
  'class\s+ProviderWritebackRetryItem\b|\b(process_due_provider_writeback_retries|_due_retry_query)\s*\(' \
  backend

echo "== Exact-head PR checks and Actions executions =="
gh pr view 1654 --repo "$repo" \
  --json state,isDraft,baseRefOid,headRefOid,statusCheckRollup
gh api "repos/${repo}/commits/${head}/check-suites" \
  --jq '.check_suites[] | {app: .app.slug,status,conclusion,head_sha,url}' || true
gh run list --repo "$repo" --commit "$head" --limit 100 \
  --json databaseId,name,status,conclusion,event,headSha,workflowName,url

Length of output: 72758

---

@seonghobae Review complete for exact range 17a7618eda2b212b691f08fa936e042b34258fc9..a8fcba66f7235ac7d8f4a7a29818246f2703e5b3.

Finding

• P1 — Exact-head PostgreSQL acceptance evidence is unavailable. The real PostgreSQL test in backend/tests/test_provider_writeback_retry_postgres.py can skip when PostgreSQL is unavailable. Exact head a8fcba66f7235ac7d8f4a7a29818246f2703e5b3 has no GitHub Actions execution, and the visible check suites remain queued. Therefore, the two-session concurrency assertion and the nonpositive-limit regression do not yet have terminal exact-head execution evidence. Keep the PR Draft until the canonical stacked-verification path completes successfully for this exact base and head. Predecessor receipts are stale and do not satisfy this condition.

Verified

• backend/services/provider_writeback_retry_service.py::_due_retry_query() filters due pending rows, preserves ascending next_retry_at order, applies the positive batch limit, and uses FOR UPDATE SKIP LOCKED.
• _due_retry_query() raises ValueError("batch_limit must be positive") for batch_limit=0 and negative values before database execution.
• process_due_provider_writeback_retries() executes the claim query before dispatch_command(). It commits only after processing the claimed rows. The transaction therefore retains each claim lock during dispatch.
• backend/tests/test_provider_writeback_retry_postgres.py creates two independent AsyncSession instances. The first session claims the generated due retry item. The second session executes the same production query under a two-second bound and must not receive the locked item.
• The PostgreSQL test rolls back both worker sessions. Cleanup deletes only the generated retry_item_uid.
• backend/tests/test_provider_writeback_retry_service.py verifies skip_locked=True, retention of a positive limit, and rejection of 0 and -1.
• docs/doctoring/provider-writeback-retry-claim.md correctly states that PostgreSQL rejects a negative LIMIT and that LIMIT 0 returns no rows.
• The doctoring correctly limits the contract to duplicate claims by concurrent live workers. It does not claim exactly-once recovery when a remote provider accepts a write before the local transaction commits.

No additional source-level finding was identified in the requested scope.

You are interacting with an AI system.

[seonghobae]

@coderabbitai review

Fresh current-head review request after a real source/test repair. Please bind review to exact stacked range 17a7618eda2b212b691f08fa936e042b34258fc9..3ec872cd38cfcace4d885422af4668ed690ee374.

New focus since the prior request: predecessor a8fcba66... used one FOR UPDATE SKIP LOCKED LIMIT batch_limit query and held every selected row lock across serial provider I/O until one final commit. RED 7b8071fa... proves with two PostgreSQL sessions that a second worker must be able to claim the later due row while worker A is blocked dispatching the first; unit RED c9fc20b... requires a commit between dispatches. Fix 5ddb10cf... claims one row per transaction/dispatch and commits before claiming the next. Please check concurrency safety, fairness/ordering, transaction/crash semantics, the fake-session test validity, and whether doctoring 3ec872cd... accurately avoids an exactly-once claim. Current exact head has zero PR-triggered workflows because the stacked-base verification owner path remains open; do not treat predecessor receipts as current evidence.

[coderabbitai]

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="17a7618eda2b212b691f08fa936e042b34258fc9"
head="3ec872cd38cfcace4d885422af4668ed690ee374"
repo="ContextualWisdomLab/naruon"

echo "== Exact topology =="
git cat-file -e "${base}^{commit}"
git cat-file -e "${head}^{commit}"
printf 'merge-base: '; git merge-base "$base" "$head"
printf 'left-right: '; git rev-list --left-right --count "${base}...${head}"
echo

echo "== Effective changed files =="
git diff --name-status "$base" "$head"
echo

echo "== Exact diff =="
git diff --find-renames --find-copies --no-ext-diff "$base" "$head"
echo

echo "== Changed Python structure =="
git diff --name-only "$base" "$head" | while IFS= read -r file; do
  case "$file" in
    *.py)
      echo "--- $file"
      ast-grep outline "$file" --items all
      ;;
  esac
done
echo

echo "== Production retry service =="
nl -ba backend/services/provider_writeback_retry_service.py | sed -n '1,340p'
echo

echo "== Affected tests and doctoring =="
for file in \
  backend/tests/test_provider_writeback_retry_service.py \
  backend/tests/test_provider_writeback_retry_postgres.py \
  docs/doctoring/provider-writeback-retry-claim.md
do
  echo "--- $file"
  nl -ba "$file" | sed -n '1,500p'
done
echo

echo "== Retry model, transaction management, and worker call sites =="
rg -n -C 10 --glob '*.py' \
  'class\s+ProviderWritebackRetryItem\b|\b(process_due_provider_writeback_retries|_due_retry_query|schedule_provider_writeback_retry|dispatch_command|commit|rollback)\s*\(' \
  backend
echo

echo "== Exact-head state and workflow evidence =="
gh pr view 1654 --repo "$repo" \
  --json state,isDraft,baseRefOid,headRefOid,statusCheckRollup
gh run list --repo "$repo" --commit "$head" --limit 100 \
  --json databaseId,name,status,conclusion,event,headSha,workflowName,url

Length of output: 184005

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="17a7618eda2b212b691f08fa936e042b34258fc9"
head="3ec872cd38cfcace4d885422af4668ed690ee374"

echo "== Effective files and compact patch statistics =="
git diff --name-status "$base" "$head"
git diff --stat "$base" "$head"
echo

echo "== Retry worker complete processing path =="
nl -ba backend/services/provider_writeback_retry_service.py | sed -n '185,330p'
echo

echo "== Fake session and per-item transaction regression =="
nl -ba backend/tests/test_provider_writeback_retry_service.py | sed -n '1,80p'
nl -ba backend/tests/test_provider_writeback_retry_service.py | sed -n '220,285p'
echo

echo "== PostgreSQL worker-contention acceptance, cleanup, and cancellation =="
nl -ba backend/tests/test_provider_writeback_retry_postgres.py | sed -n '1,245p'
echo

echo "== Doctoring contract and correction history =="
nl -ba docs/doctoring/provider-writeback-retry-claim.md | sed -n '1,380p'
echo

echo "== Exact patch for the four requested files =="
git diff --no-ext-diff "$base" "$head" -- \
  backend/services/provider_writeback_retry_service.py \
  backend/tests/test_provider_writeback_retry_service.py \
  backend/tests/test_provider_writeback_retry_postgres.py \
  docs/doctoring/provider-writeback-retry-claim.md

Length of output: 59301

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Complete transient-reschedule regression =="
nl -ba backend/tests/test_provider_writeback_retry_service.py | sed -n '280,350p'
echo

echo "== Fake-session retryable-failure call sites and surrounding configuration =="
rg -n -C 12 --glob 'test_provider_writeback_retry_service.py' \
  'FakeRetrySession\(|runner_response_timeout|retry_delay_seconds=0|batch_limit=' \
  backend/tests/test_provider_writeback_retry_service.py
echo

echo "== Current worker configuration validation =="
nl -ba backend/services/provider_writeback_retry_service.py | sed -n '45,120p'
rg -n -C 6 --glob '*.py' \
  'ProviderWritebackRetryWorker\(' backend

Length of output: 14276

---

@seonghobae Review complete for exact range 17a7618eda2b212b691f08fa936e042b34258fc9..3ec872cd38cfcace4d885422af4668ed690ee374.

Finding

• P1 — Make FakeRetrySession enforce the due-time predicate. backend/tests/test_provider_writeback_retry_service.py, Line 25, filters fake results only by retry_state and LIMIT. Production _due_retry_query() also requires next_retry_at <= current_time. In test_process_due_provider_writeback_retries_reschedules_transient_failure(), the first failed dispatch sets next_retry_at to now + 60 seconds. The fake session returns that row again on the next claim cycle. With the default batch_limit=25, the worker dispatches the row again and marks it exhausted. The test assertions for one reschedule, attempt_count == 2, and one commit will fail. Make the fake session apply the same due-time rule, or use a fake query result sequence that excludes the rescheduled row.

Verified

• _due_retry_query() rejects nonpositive batch_limit values before SQL execution.
• The query filters due pending rows, orders by ascending next_retry_at, uses FOR UPDATE SKIP LOCKED, and receives a positive limit.
• process_due_provider_writeback_retries() claims one row per iteration with _due_retry_query(current_time, 1).
• The worker commits each item after its provider dispatch and state update. It does not pre-lock later rows.
• The PostgreSQL worker-contention test uses two independent AsyncSession instances. It blocks worker A during the first dispatch. It requires worker B to process the later due row within a bounded wait.
• The PostgreSQL test cleanup deletes only the generated retry item IDs.
• The doctoring correctly limits the guarantee to concurrent live-worker duplicate claims. It does not claim exactly-once recovery after a remote provider accepts a write before the local commit.
• Current head has no GitHub Actions workflow runs. I did not use predecessor receipts as current-head execution evidence.

You are interacting with an AI system.

[seonghobae]

@coderabbitai review

Fresh review requested for exact stacked range 17a7618eda2b212b691f08fa936e042b34258fc9..cf1e6220bcc5ad7f2927d7f0fb86b155dd91a0b3 after repairing the valid predecessor finding.

On 3ec872cd..., the fake retry session ignored production's next_retry_at <= current_time predicate, so a transiently rescheduled future row could be returned again in the same pass and spuriously exhausted. 050e5f0... makes the fake preserve due-time filtering and ordering, and strengthens the transient-reschedule regression; cf1e622... updates doctoring only. Production retry source is unchanged by this review repair.

Please verify the harness now matches the production due predicate without masking the per-item FOR UPDATE SKIP LOCKED concurrency contract, and re-check the real PostgreSQL two-session acceptance, transaction/crash boundary, and non-exactly-once claim. Current exact head still has zero PR-triggered workflows on the feature base, so predecessor receipts are not current evidence.

[coderabbitai]

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="17a7618eda2b212b691f08fa936e042b34258fc9"
head="cf1e6220bcc5ad7f2927d7f0fb86b155dd91a0b3"
repo="ContextualWisdomLab/naruon"

echo "== Exact range topology =="
git cat-file -e "${base}^{commit}"
git cat-file -e "${head}^{commit}"
printf 'merge-base: '; git merge-base "$base" "$head"
printf 'left-right: '; git rev-list --left-right --count "${base}...${head}"

echo "== Effective changed files and stat =="
git diff --name-status "$base" "$head"
git diff --stat "$base" "$head"

echo "== Exact patch for requested scope =="
git diff --no-ext-diff "$base" "$head" -- \
  backend/services/provider_writeback_retry_service.py \
  backend/tests/test_provider_writeback_retry_service.py \
  backend/tests/test_provider_writeback_retry_postgres.py \
  docs/doctoring/provider-writeback-retry-claim.md

echo "== Changed Python structure =="
git diff --name-only "$base" "$head" | while IFS= read -r file; do
  case "$file" in
    *.py)
      echo "--- $file"
      ast-grep outline "$file" --items all
      ;;
  esac
done

echo "== Production retry query and per-item processing path =="
nl -ba backend/services/provider_writeback_retry_service.py | sed -n '1,360p'

echo "== Fake session and unit regressions =="
nl -ba backend/tests/test_provider_writeback_retry_service.py | sed -n '1,420p'

echo "== Real PostgreSQL concurrency acceptance and cleanup =="
nl -ba backend/tests/test_provider_writeback_retry_postgres.py | sed -n '1,360p'

echo "== Doctoring contract boundary =="
nl -ba docs/doctoring/provider-writeback-retry-claim.md | sed -n '1,440p'

echo "== Relevant production call sites and model fields =="
rg -n -C 8 --glob '*.py' \
  'class\s+ProviderWritebackRetryItem\b|\b(process_due_provider_writeback_retries|_due_retry_query|schedule_provider_writeback_retry|dispatch_command|commit|rollback)\s*\(' \
  backend

echo "== Exact-head PR and workflow evidence =="
gh pr view 1654 --repo "$repo" \
  --json number,state,isDraft,baseRefOid,headRefOid,statusCheckRollup
gh run list --repo "$repo" --commit "$head" --limit 100 \
  --json databaseId,name,status,conclusion,event,headSha,workflowName,url

Length of output: 239864

---

@seonghobae Review complete for exact range 17a7618eda2b212b691f08fa936e042b34258fc9..cf1e6220bcc5ad7f2927d7f0fb86b155dd91a0b3.

Finding

• P1 — Exact-head PostgreSQL acceptance evidence is still unavailable. The PostgreSQL tests can skip when PostgreSQL is unavailable. Exact head cf1e6220bcc5ad7f2927d7f0fb86b155dd91a0b3 has zero GitHub Actions workflow runs. The visible successful CodeRabbit and Devin Review status contexts do not execute the real PostgreSQL two-session contracts. Keep PR #1654 Draft until the canonical stacked-verification path produces terminal successful execution for this exact base and head. Do not use predecessor receipts as current-head evidence.

Verified

• FakeRetrySession.execute() now filters rows by retry_state == "pending" and next_retry_at <= current_time. It obtains current_time from the query parameters.
• The fake now orders candidate rows by next_retry_at before it applies the limit. This matches _due_retry_query().
• The transient-reschedule regression requires one processed row, one reschedule, zero exhaustion, and one commit. A row rescheduled 60 seconds into the future cannot be claimed again in the same pass.
• The fake does not simulate row locks. It does not mask the concurrency contract because backend/tests/test_provider_writeback_retry_postgres.py tests that contract with independent PostgreSQL AsyncSession instances.
• _due_retry_query() uses due-time ordering, FOR UPDATE SKIP LOCKED, and a positive limit. It rejects batch_limit <= 0 before SQL execution.
• process_due_provider_writeback_retries() claims one row per iteration with _due_retry_query(current_time, 1). It holds that row lock during provider I/O and commits before it claims the next row.
• The PostgreSQL worker-contention test blocks worker A on the first due row. Worker B processes the later due row within a bounded wait. The test therefore detects pre-locking of later rows.
• The PostgreSQL tests roll back active worker sessions. Their cleanup deletes only generated retry item IDs.
• docs/doctoring/provider-writeback-retry-claim.md correctly limits the guarantee to concurrent live-worker claims. It does not claim exactly-once delivery when a remote provider accepts a write before the local transaction commits.

No additional source-level finding was identified in the requested scope.

You are interacting with an AI system.

[seonghobae]

@coderabbitai review

Fresh independent review requested for exact stacked range 17a7618eda2b212b691f08fa936e042b34258fc9..ffc5df0a40f888cdbfb6003419ffa1efffe80e88.

New source-order RED efd526ad... covers invalid retry policy at public boundaries: negative retry delay must fail before persistence/query/dispatch; nonpositive worker interval and attempt counts must fail closed. Causal source fix 18c825d... adds only semantic validation and deliberately leaves zero retry delay valid; ffc5df0... updates code-current doctoring with Python 3.14.7 asyncio sleeping semantics. Please re-check that these guards do not invent arbitrary operational caps or disturb the existing per-item FOR UPDATE SKIP LOCKED concurrency/crash boundary.

Current exact head still has zero PR-triggered workflow runs on the feature base, so no predecessor receipt should be treated as current hosted evidence.

[coderabbitai]

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="17a7618eda2b212b691f08fa936e042b34258fc9"
head="ffc5df0a40f888cdbfb6003419ffa1efffe80e88"
repo="ContextualWisdomLab/naruon"

echo "== Exact range topology and files =="
git cat-file -e "${base}^{commit}"
git cat-file -e "${head}^{commit}"
printf 'merge-base: '; git merge-base "$base" "$head"
printf 'left-right: '; git rev-list --left-right --count "${base}...${head}"
git diff --name-status "$base" "$head"
git diff --stat "$base" "$head"

echo "== Exact patch for review scope =="
git diff --no-ext-diff "$base" "$head" -- \
  backend/services/provider_writeback_retry_service.py \
  backend/tests/test_provider_writeback_retry_service.py \
  backend/tests/test_provider_writeback_retry_postgres.py \
  docs/doctoring/provider-writeback-retry-claim.md

echo "== Retry service outline and implementation =="
ast-grep outline backend/services/provider_writeback_retry_service.py --items all
nl -ba backend/services/provider_writeback_retry_service.py | sed -n '1,380p'

echo "== Affected tests and doctoring =="
for file in \
  backend/tests/test_provider_writeback_retry_service.py \
  backend/tests/test_provider_writeback_retry_postgres.py \
  docs/doctoring/provider-writeback-retry-claim.md
do
  echo "--- $file"
  nl -ba "$file" | sed -n '1,500p'
done

echo "== Public call sites and scheduling/persistence paths =="
rg -n -C 10 --glob '*.py' \
  '\b(schedule_provider_writeback_retry|process_due_provider_writeback_retries|ProviderWritebackRetryWorker|_due_retry_query)\s*\(' \
  backend

echo "== Exact-head PR and workflow evidence =="
gh pr view 1654 --repo "$repo" \
  --json number,state,isDraft,baseRefOid,headRefOid,statusCheckRollup
gh run list --repo "$repo" --commit "$head" --limit 100 \
  --json databaseId,name,status,conclusion,event,headSha,workflowName,url

Length of output: 139486

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="17a7618eda2b212b691f08fa936e042b34258fc9"
head="ffc5df0a40f888cdbfb6003419ffa1efffe80e88"

echo "== Configuration RED, fix, and doctoring commits =="
for commit in \
  efd526ad707e7098b4f453a92e414c585e1b0905 \
  18c825dbb17905d0f7e2ed54fdbf36f03f1b76a1 \
  ffc5df0a40f888cdbfb6003419ffa1efffe80e88
do
  echo "--- $commit"
  git show --format=fuller --no-ext-diff --stat "$commit"
  git show --format= --no-ext-diff "$commit" -- \
    backend/services/provider_writeback_retry_service.py \
    backend/tests/test_provider_writeback_retry_service.py \
    docs/doctoring/provider-writeback-retry-claim.md
done

echo "== Zero-delay coverage in the exact range =="
git grep -n -E 'retry_delay_seconds[[:space:]]*=[[:space:]]*0|retry_delay_seconds.*0' "$base..$head" -- \
  'backend/**/*.py' || true

echo "== Pytest selection and PostgreSQL workflow coverage =="
fd -t f -a 'pytest.ini' -a 'pyproject.toml' -a 'tox.ini' -a 'setup.cfg' . -x sh -c \
  'echo "--- $1"; rg -n -C 5 "(markers|addopts|postgres|testpaths)" "$1" || true' sh {}
fd -t f -e yml -e yaml .github/workflows -x sh -c \
  'echo "--- $1"; rg -n -C 6 "(postgres|DATABASE_URL|alembic|provider_writeback_retry_postgres|pytest)" "$1" || true' sh {}

echo "== Direct scheduling call shapes =="
ast-grep run --lang python \
  --pattern 'await schedule_provider_writeback_retry($$$)' backend

Length of output: 20347

---

@seonghobae Review complete for exact range 17a7618eda2b212b691f08fa936e042b34258fc9..ffc5df0a40f888cdbfb6003419ffa1efffe80e88.

Finding

• P1 — Exact-head acceptance evidence is unavailable. Exact head ffc5df0a40f888cdbfb6003419ffa1efffe80e88 has zero GitHub Actions workflow runs. The PostgreSQL tests can skip when PostgreSQL is unavailable. The visible CodeRabbit and Devin Review contexts do not execute the real two-session PostgreSQL contracts or the invalid-policy regressions. Keep #1654 Draft until the canonical stacked-verification path produces terminal successful results for this exact base and head. Do not use predecessor receipts as current-head evidence.

Verified

• ProviderWritebackRetryWorker.__init__() rejects interval_seconds <= 0, batch_limit <= 0, and max_attempts <= 0.
• schedule_provider_writeback_retry() rejects negative retry_delay_seconds before it creates or commits a retry item.
• process_due_provider_writeback_retries() rejects negative retry delay and nonpositive attempt counts before its first database query or provider dispatch.
• Zero retry_delay_seconds remains valid. The new guards add no fixed maximum delay, attempt count, or batch size.
• _due_retry_query() retains due-time filtering, ascending next_retry_at ordering, a strictly positive limit, and FOR UPDATE SKIP LOCKED.
• The worker still claims one row per iteration, holds that row lock during provider I/O, commits the item result, and then claims the next row.
• The two-session PostgreSQL tests retain independent AsyncSession instances, bounded waits, transaction rollback, and cleanup limited to generated retry item IDs.
• The fake session preserves the production due-time predicate and ordering.
• The doctoring correctly limits the contract to duplicate claims by concurrent live workers. It does not claim exactly-once remote side effects after a provider write succeeds before the local commit.

No additional source-level finding was identified in the requested scope.

You are interacting with an AI system.

[seonghobae]

@coderabbitai review

Please review the exact current range 17a7618eda2b212b691f08fa936e042b34258fc9..23b2cae3073df5edba9323abe01c7fd00cb03c0f after the latest source/test/doc changes. Focus on the new worker-policy separation: interval_seconds must remain polling cadence only, retry_delay_seconds must remain the provider backoff base, zero retry delay must stay explicitly valid, and no predecessor approval/check should be treated as current-head evidence. Also re-check the retained PostgreSQL SKIP LOCKED per-item claim/commit semantics, retry-attempt counting, crash/idempotency boundary, and the new focused test double for _sync().

[coderabbitai]

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="17a7618eda2b212b691f08fa936e042b34258fc9"
head="23b2cae3073df5edba9323abe01c7fd00cb03c0f"
repo="ContextualWisdomLab/naruon"

echo "== Exact topology and effective change set =="
git cat-file -e "${base}^{commit}"
git cat-file -e "${head}^{commit}"
printf 'merge-base: '; git merge-base "$base" "$head"
printf 'left-right: '; git rev-list --left-right --count "${base}...${head}"
git diff --name-status "$base" "$head"
git diff --stat "$base" "$head"

echo "== Exact diff =="
git diff --find-renames --find-copies --no-ext-diff "$base" "$head"

echo "== Changed Python structure =="
git diff --name-only "$base" "$head" | while IFS= read -r file; do
  case "$file" in
    *.py) echo "--- $file"; ast-grep outline "$file" --items all ;;
  esac
done

echo "== Retry service and worker =="
nl -ba backend/services/provider_writeback_retry_service.py | sed -n '1,420p'

echo "== Unit tests, PostgreSQL tests, and doctoring =="
for file in \
  backend/tests/test_provider_writeback_retry_service.py \
  backend/tests/test_provider_writeback_retry_postgres.py \
  docs/doctoring/provider-writeback-retry-claim.md \
  docs/doctoring/provider-writeback-retry-policy-cadence.md
do
  if [ -f "$file" ]; then
    echo "--- $file"
    nl -ba "$file" | sed -n '1,560p'
  fi
done

echo "== Public call sites and retry attempt/state updates =="
rg -n -C 10 --glob '*.py' \
  '\b(schedule_provider_writeback_retry|process_due_provider_writeback_retries|ProviderWritebackRetryWorker|_due_retry_query|dispatch_command)\s*\(|\b(attempt_count|next_retry_at|retry_delay_seconds|interval_seconds|max_attempts)\b' \
  backend

echo "== Exact-head PR status and workflow evidence =="
gh pr view 1654 --repo "$repo" \
  --json number,state,isDraft,baseRefOid,headRefOid,reviews,statusCheckRollup
gh run list --repo "$repo" --commit "$head" --limit 100 \
  --json databaseId,name,status,conclusion,event,headSha,workflowName,url
gh api "repos/${repo}/commits/${head}/check-suites" \
  --jq '.check_suites[] | {app: .app.slug,status,conclusion,head_sha,url}' || true

Length of output: 245757

---

@seonghobae Review complete for exact range 17a7618eda2b212b691f08fa936e042b34258fc9..23b2cae3073df5edba9323abe01c7fd00cb03c0f.

Finding

• P1 — Exact-head acceptance evidence is unavailable. Exact head 23b2cae3073df5edba9323abe01c7fd00cb03c0f has zero GitHub Actions workflow runs. The visible check suites are queued. The successful CodeRabbit and Devin Review status contexts do not execute the focused worker-policy regression or the real PostgreSQL two-session contracts. Keep #1654 Draft until the canonical stacked-verification path produces terminal successful execution for this exact base and head. Do not use predecessor receipts as current-head evidence.

Verified

• ProviderWritebackRetryWorker._run_loop() uses interval_seconds only for asyncio.sleep(self.interval_seconds).
• ProviderWritebackRetryWorker._sync() passes self.retry_delay_seconds to process_due_provider_writeback_retries(). It no longer derives retry backoff from polling cadence.
• retry_delay_seconds=0 remains valid. Negative retry_delay_seconds fails at worker construction and processing boundaries. No maximum delay, batch size, or attempt count is added.
• backend/tests/test_provider_writeback_retry_worker_policy.py uses the real _sync() boundary. Its controlled session context and process double verify that interval_seconds=7 and retry_delay_seconds=300 remain independent.
• _due_retry_query() preserves due-time filtering and ascending next_retry_at ordering. It uses FOR UPDATE SKIP LOCKED and requires a positive limit.
• process_due_provider_writeback_retries() claims one row per iteration, holds only that row lock during provider I/O, commits the result, and then claims the next row.
• Retry rows begin with attempt_count=1. The worker increments the count before a dispatch. It exhausts a row before dispatch when the stored count already reaches max_attempts. It exhausts a retryable failed dispatch when the incremented count reaches max_attempts.
• The PostgreSQL acceptance tests retain independent AsyncSession instances, bounded waits, rollback or session-scope cleanup, and deletion limited to generated retry-item IDs. The blocked-first-dispatch test verifies that a second worker can process the later due row.
• The fake retry session preserves the production pending-state, due-time, ordering, and limit behavior. It does not replace the PostgreSQL lock test.
• Both doctoring documents state the intended boundary. The row lock prevents concurrent live-worker claims. It does not provide exactly-once recovery if the remote provider accepts a write before the local transaction commits.

No additional source-level finding was identified in the requested scope.

You are interacting with an AI system.

[seonghobae]

@coderabbitai review

Fresh exact-head review request for 17a7618eda2b212b691f08fa936e042b34258fc9..23b2cae3073df5edba9323abe01c7fd00cb03c0f. Please verify the PostgreSQL FOR UPDATE SKIP LOCKED per-item claim/commit contract, the due-time predicate fidelity, retry-policy validation, and the polling-cadence vs retry-backoff separation. Current effective scope is six files and there are still zero PR-triggered workflow runs on this stacked base, so do not treat predecessor execution as current evidence.

[coderabbitai]

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="17a7618eda2b212b691f08fa936e042b34258fc9"
head="23b2cae3073df5edba9323abe01c7fd00cb03c0f"
repo="ContextualWisdomLab/naruon"

echo "== Exact range topology and effective files =="
git cat-file -e "${base}^{commit}"
git cat-file -e "${head}^{commit}"
printf 'merge-base: '; git merge-base "$base" "$head"
printf 'left-right commits: '; git rev-list --left-right --count "${base}...${head}"
git diff --name-status "$base" "$head"
git diff --stat "$base" "$head"

echo "== Exact patch =="
git diff --find-renames --find-copies --no-ext-diff "$base" "$head"

echo "== Changed Python outlines =="
git diff --name-only "$base" "$head" | while IFS= read -r file; do
  case "$file" in
    *.py) echo "--- $file"; ast-grep outline "$file" --items all ;;
  esac
done

echo "== Production retry service =="
nl -ba backend/services/provider_writeback_retry_service.py | sed -n '1,430p'

echo "== Focused unit, PostgreSQL, and worker-policy tests =="
for file in \
  backend/tests/test_provider_writeback_retry_service.py \
  backend/tests/test_provider_writeback_retry_postgres.py \
  backend/tests/test_provider_writeback_retry_worker_policy.py
do
  echo "--- $file"
  nl -ba "$file" | sed -n '1,560p'
done

echo "== Doctoring boundaries =="
for file in \
  docs/doctoring/provider-writeback-retry-claim.md \
  docs/doctoring/provider-writeback-retry-policy-cadence.md
do
  echo "--- $file"
  nl -ba "$file" | sed -n '1,520p'
done

echo "== Definitions and production call sites =="
rg -n -C 10 --glob '*.py' \
  '\b(schedule_provider_writeback_retry|process_due_provider_writeback_retries|ProviderWritebackRetryWorker|_due_retry_query|dispatch_command)\s*\(|\b(attempt_count|next_retry_at|retry_delay_seconds|interval_seconds|max_attempts|batch_limit)\b' \
  backend

echo "== Exact-head hosted evidence only =="
gh pr view 1654 --repo "$repo" \
  --json number,state,isDraft,baseRefOid,headRefOid,reviews,statusCheckRollup
gh run list --repo "$repo" --commit "$head" --limit 100 \
  --json databaseId,name,status,conclusion,event,headSha,workflowName,url
gh api "repos/${repo}/commits/${head}/check-suites" \
  --jq '.check_suites[] | {app: .app.slug,status,conclusion,head_sha,url}' || true

Length of output: 191596

---

@seonghobae Review complete for exact range 17a7618eda2b212b691f08fa936e042b34258fc9..23b2cae3073df5edba9323abe01c7fd00cb03c0f.

Finding

• P1 — Exact-head acceptance evidence is unavailable. Commit 23b2cae3073df5edba9323abe01c7fd00cb03c0f has zero GitHub Actions workflow runs. Its visible check suites are queued. The successful CodeRabbit and Devin Review status contexts do not execute the real PostgreSQL concurrency contracts or the focused worker-policy test. Keep #1654 as Draft until the canonical stacked-verification path produces terminal successful results for this exact base and head. Do not use predecessor receipts as current evidence.

Verified

• _due_retry_query() selects only pending rows where next_retry_at <= current_time. It orders rows by ascending next_retry_at. It rejects batch_limit <= 0 before SQL execution. It uses FOR UPDATE SKIP LOCKED.
• process_due_provider_writeback_retries() claims one row with _due_retry_query(current_time, 1) on each iteration. It holds only that row lock during provider I/O. It persists the item outcome and commits before its next claim.
• The unit-test FakeRetrySession applies the production pending-state predicate, due-time predicate, due-time ordering, and limit. A row rescheduled into the future cannot be claimed again in the same pass.
• The PostgreSQL acceptance tests use independent AsyncSession instances. One test proves that a second worker skips a locked row. The blocked-dispatch test proves that a second worker can claim and process the later due row.
• PostgreSQL test cleanup deletes only the retry-item identifiers that the test created. The test also releases the blocked dispatch during cleanup.
• Retry-policy validation rejects nonpositive interval_seconds, batch_limit, and max_attempts. It rejects negative retry_delay_seconds at worker construction, scheduling, and direct-processing boundaries. retry_delay_seconds=0 remains valid.
• ProviderWritebackRetryWorker._run_loop() uses interval_seconds only as the polling sleep duration. _sync() passes the independent retry_delay_seconds value to process_due_provider_writeback_retries().
• backend/tests/test_provider_writeback_retry_worker_policy.py exercises the real _sync() boundary and verifies that a 7-second poll interval does not replace a 300-second retry delay.
• The retry attempt behavior remains consistent. New rows start at attempt_count=1; processing increments the count before dispatch; exhausted rows do not dispatch.
• The doctoring documents limit the guarantee to concurrent live-worker claim safety. They do not claim exactly-once recovery after a remote provider accepts a write before the local transaction commits.

No additional source-level finding was identified in the requested scope.

You are interacting with an AI system.