MotirBuilding in public
MOTIR · moooon
onMotir
You’re viewing a public project. Anyone can view it — no account needed. Sign in to submit, upvote, or comment on requests.View-only — you can’t edit work items
MOTIR-3739

(motir-core) `code-graph-refresh-engine.spec.ts` hand-rolls a TRUNCATE that bypasses `db-reset.ts`'s 40P01 retry — it deadlocks against the E2E job worker and reds unrelated PRs

Done
Description

⚠️ AMENDED ON 2026-08-28 BY THE RUN THAT FIXED THIS — two premises below are FALSIFIED. Read the amendment comment before the body.

  1. "The first two are protected against the job worker" is false for the second: truncateJobRuns() is a bare $executeRawUnsafe in tests/helpers/db.ts with no retry, truncating a strict SUPERSET of line 66's three tables. The Fix direction below would have moved the deadlock up one line.
  2. Line 66 was REDUNDANT — it cleared nothing line 65 had not already cleared. It is deleted, not wrapped.
  3. The population is EIGHT, not one (five raw statements + three bare helper calls), re-measured on origin/main @ bed2bbc52.

All four acceptance criteria stand and are met. Shipped as motir-core#2402; planning bug MOTIR-3740. The evidence and the commands are in the amendment comment.

tests/e2e/code-graph-refresh-engine.spec.ts's beforeEach runs three resets in a row. The first two are protected against the job worker; the third is not, and it is the one that fails.

test.beforeEach(async () => {
  await resetDatabase();      // 64 — HAS a 40P01 retry
  await truncateJobRuns();    // 65
  await adminDb.$executeRawUnsafe(   // 66 — NO retry. This is the one that deadlocks.
    'TRUNCATE TABLE "job_event", "job_queue", "job_step" RESTART IDENTITY CASCADE',
  );
  await adminDb.fleetInFlightSlot.deleteMany({});
});

Root cause — read off shipped code, not inferred

tests/e2e/_helpers/db-reset.ts already documents this exact contention and already retries on it (3 attempts):

// Retries on Postgres deadlock (40P01): the PREVIOUS test can leave [job]
// transactions in flight ... and TRUNCATE deadlocking against those
// [is a spurious] failure — the job finishes within moments and the retry succeeds.
const deadlock = err instanceof Error && /40P01|deadlock/i.test(err.message);
if (!deadlock || attempt >= 3) throw err;

Line 66 is a raw adminDb.$executeRawUnsafe that never goes through that helper, so it gets none of that protection — while truncating job_queue / job_event / job_step, which are precisely the tables the E2E job worker writes to continuously. The worker takes row locks via claimDueRuns (FOR UPDATE SKIP LOCKED) and the ledger; the TRUNCATE wants AccessExclusiveLock on the same three in a different order. Deadlock.

The lane runs a live worker for this spec file, so the contending party is not hypothetical — in the observed failure the log carries [e2e-job-worker] system.daily-health-check run … FAILED terminally in the same second as the deadlock.

Reproduction / evidence

Observed on PR #2398 (subtask/MOTIR-3731-job-concurrency-decision, head 4c187d7e9), job Playwright E2E (bulk-3), run 33154624497, 2026-08-28T08:29:16Z:

PrismaClientKnownRequestError:
Invalid `prisma.$executeRawUnsafe()` invocation:
Raw query failed. Code: `40P01`. Message: `deadlock detected`
  > 66 |   await adminDb.$executeRawUnsafe(
    at tests/e2e/code-graph-refresh-engine.spec.ts:66:3

It is environmental, and the run itself says so — two DIFFERENT tests in the file were hit and one passed on retry:

  • 1 failed:293 a worker KILLED mid-index resumes on the SAME container
  • 1 flaky:158 a same-repo BURST coalesces into ONE run carrying the LAST delivery (passed on retry #1)
  • 94 passed

Both failed in the beforeEach, so neither test body ran. That PR's diff was two Markdown files, a comment-only edit, and one new vitest spec — it touches nothing the E2E lane executes, and TypeScript, Vitest (1-3/3) and Structural guards were all green on the same commit.

Acceptance criteria

  • tests/e2e/code-graph-refresh-engine.spec.ts:66's TRUNCATE no longer runs unprotected — it goes through the same 40P01 retry resetDatabase() uses two lines above it.
  • The retry wrapper is reachable by any E2E truncate rather than living inside one helper's private closure, so the next hand-rolled truncate can use it instead of re-inventing the failure.
  • A guard makes an unprotected truncate in tests/e2e/** visible — a source scan in the shape of the existing tests/truncate-lock-order.test.ts, which already adjudicates truncate helpers this way. (If a scan is judged not to earn its keep, say so on the card and cite what replaces it — silence is not a disposition.)
  • No product code changes. This is a test-infrastructure defect; the engine's own locking is not implicated.

Fix direction

Route line 66 through the same retry as line 64 rather than hand-rolling it — either call the shared helper, or extract db-reset.ts's retry wrapper so any truncate in the E2E lane can use it. Prefer the extraction: a second hand-rolled truncate is how this recurs.

Note the retry is a MASK for a real lock-ordering fact, not a cure — the vitest-lane analysis of 40P01 (MOTIR-3066) traced its instance to an abandoned Promise.all arm holding a transaction, and was explicit that "make every truncate helper agree on an order" cannot be the fix for the class. That is an acceptable trade in the E2E lane, where a live worker running beside a truncating test is the intended shape; what is not acceptable is one truncate having the paper and its neighbour not.

Context refs

  • tests/e2e/code-graph-refresh-engine.spec.ts:64-69 — the three-reset beforeEach; line 66 is the unprotected one
  • tests/e2e/_helpers/db-reset.ts:26-39 — the retry that exists, its comment naming this exact cause, and the attempt >= 3 bound
  • tests/truncate-lock-order.test.ts — the precedent for asserting a truncate property by source scan
  • lib/repositories/jobQueueRepository.tsclaimDueRuns, the worker's side of the lock contention