Skip to content

fix(selfhost): guard pg queue coalesce UPDATEs with status=pending - #3617

Closed
RealDiligent wants to merge 1 commit into
JSONbored:mainfrom
RealDiligent:fix/pg-queue-coalesce-pending-guard
Closed

fix(selfhost): guard pg queue coalesce UPDATEs with status=pending#3617
RealDiligent wants to merge 1 commit into
JSONbored:mainfrom
RealDiligent:fix/pg-queue-coalesce-pending-guard

Conversation

@RealDiligent

Copy link
Copy Markdown
Contributor

Summary

  • Add status='pending' guards to Postgres queue supersede and simple-coalesce UPDATE paths
  • On a lost SELECT-then-UPDATE race (rowCount === 0), fall through to INSERT instead of silently overwriting a row another instance claimed
  • Mirrors the existing merge-path gate finding in the same enqueue() function

Replaces closed #3595 (same change, rebased on latest main).

Test plan

  • npm run typecheck
  • npx vitest run test/unit/selfhost-pg-queue.test.ts

Supersede and simple-coalesce paths now mirror the merge-path gate finding:
lose the SELECT-then-UPDATE race cleanly (rowCount 0) and fall through to
INSERT instead of overwriting a row another instance already claimed.

Co-authored-by: Cursor <cursoragent@cursor.com>
@RealDiligent
RealDiligent requested a review from JSONbored as a code owner July 5, 2026 19:21
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 5, 2026
@loopover-orb

loopover-orb Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - reject/close recommended

Review updated: 2026-07-05 19:26:08 UTC

2 files · 1 AI reviewer · 1 blocker · readiness 80/100 · CI pending · blocked

🛑 Suggested Action - Reject/Close

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.

Review summary
This PR adds `status='pending'` guards to the two remaining unguarded coalesce UPDATE paths (supersede and simple-coalesce) in `enqueue()`, checking `rowCount` and falling through to INSERT when a concurrent claim already moved the row out of `pending`, mirroring the existing merge-path gate fix in the same function. The change is narrow, logically sound (the fallthrough correctly skips the DELETE/metric/kickOne/return only on a lost race and proceeds to the next coalesce check and eventual INSERT), and is backed by two new regression tests plus updated mock plumbing (`setCoalesceUpdateRowCount`) that exercise both the supersede and simple-coalesce loss paths. This directly closes the race the PR describes with no scope creep.

Nits — 5 non-blocking
  • The PR description doesn't cite an open issue number, only a closed/superseded predecessor PR (fix(selfhost): guard pg queue coalesce UPDATEs with status=pending #3595) — worth confirming this satisfies the repo's issue-linkage policy for external contributions.
  • test/unit/selfhost-pg-queue.test.ts:625 loosened the prior assertion from a specific `expect.arrayContaining([...])` payload check to `expect.anything()`, slightly reducing precision of that pre-existing test alongside the intended `WHERE id=$6 AND status='pending'` addition.
  • The shared mock matcher `q.includes("SET payload=$1, run_after=GREATEST") && q.includes("WHERE id=")` in the test file matches both the supersede and simple-coalesce UPDATEs identically, so `setCoalesceUpdateRowCount` can't distinguish between them within a single test — currently worked around via `mockResolvedValueOnce` ordering, but worth a comment noting the coupling.
  • Consider a debug-level log or counter when a coalesce/supersede race is lost and falls through to INSERT, to help observe race frequency in production self-host deployments.
  • Since both guarded UPDATEs now share near-identical rowCount-check-and-fallthrough shape, a small local helper could reduce duplication, though at only two call sites this is optional.

Why this is blocked

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
Signal Result Evidence
Code review ❌ 1 blocker 1 reviewer
Linked issue ✅ Linked #3595
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ❌ 5/25 Preflight is holding this PR: the review lane is unavailable, so it is not ready for automated review.
Contributor workload ✅ 10/10 Author activity: 208 registered-repo PR(s), 51 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor RealDiligent; Gittensor profile; 208 PR(s), 0 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: RealDiligent
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: JavaScript, Ruby, Svelte, TypeScript, Cuda, Markdown
  • Official Gittensor activity: 208 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Await review-lane availability.
  • Refresh registry data or choose a registered active repo.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb

loopover-orb Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Gittensory is closing this pull request on the maintainer's behalf (No linked issue detected). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

@loopover-orb loopover-orb Bot closed this Jul 5, 2026
@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.19%. Comparing base (bfa9c2d) to head (aa75729).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3617   +/-   ##
=======================================
  Coverage   93.19%   93.19%           
=======================================
  Files         313      313           
  Lines       31874    31874           
  Branches    11666    11666           
=======================================
  Hits        29705    29705           
  Misses       1517     1517           
  Partials      652      652           
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant