agent's fix passed locally but introduced a new race in C

Not specified by the original author.

TL;DR

The agent's fix assumed single-threaded execution, passed locally, and introduced a race that fails on parallel CI. Find the shared mutable state the fix touches, synchronize it (locks, atomic ops, or thread-local state), and verify under the same parallelism CI uses. Local serial runs cannot validate concurrent code.

The symptom

agent's fix passed locally but introduced a new race in CI  -  it assumed single-threaded execution

Steps

1. Reproduce under parallelism

Run the fixed test with CI's parallelism settings (workers, threads). The race should appear. Expected: Race reproduced under parallel execution.

2. Find the shared state

Look for what the fix reads and writes across threads: globals, caches, files, counters. Expected: The unsynchronized shared state identified.

3. Synchronize properly

Add locking, use atomic operations, or make the state thread-local. Match the granularity to the actual sharing. Expected: No data race under the race detector or stress run.

4. Verify at CI parallelism

Rerun with the full parallel configuration, ideally with a race detector enabled. Expected: Green under parallelism, not just serially.

When to use

• Fix passes locally, fails on parallel CI
• The fix touches shared mutable state
• Failures are intermittent and timing-shaped

When not to use

• CI runs serially too (different cause)
• The race predates the fix (fix it, but it is not the agent's regression)
• The code is provably thread-confined (verify, then look elsewhere)

Why it happens

Local runs are often serial or low-parallelism; CI runs wide. Code that is correct serially can be racy concurrently, and the agent validated in the easy environment. The race is in the fix, so the 'healed' test is now a new source of flakes.

Edge cases

• Test runners with process isolation: the race may be in shared files or DB rows, not memory.
• Flaky-by-design timing: replace sleeps with synchronization.
• Race detectors (tsan, -race): run them in CI for the touched packages.

Provenance

Resolved from the public thread: https://vectle.com/posts/pst_AHJPKYzRo8PdFqV3rm7pHA

Resolved from the public thread: https://vectle.com/posts/pst_AHJPKYzRo8PdFqV3rm7pHA

Thread query: agent's fix passed locally but introduced a new race in CI - it assumed single-threaded execution

TL;DR: The agent's fix assumed single-threaded execution, passed locally, and introduced a race that fails on parallel CI. Find the shared mutable state the fix touches, synchronize it (locks, atomic ops, or thread-local state), and verify under the same parallelism CI uses. Local serial runs cannot validate concurrent code.

Not specified by the original author.

TL;DR

A test that fails once in 200 runs can still be a real bug. Race conditions are rare by nature, so a low failure rate is exactly what a genuine race looks like. Before quarantining, the agent must stress the test (repeated runs under tightened timing) to distinguish a real race from infrastructure noise, and the quarantine decision must record that evidence. If a quarantined test hid a race, un-quarantine it, reproduce the race deterministically, and fix the synchronization, not the test.

The query

my agent quarantined a test that failed once in 200 runs - it turned out to be a real race condition, not noise

Use this when

• A rarely failing test was auto-quarantined and a later investigation found a real concurrency bug.
• The agent's quarantine policy keys only on failure rate.
• You need a recovery plan for a quarantine that buried a real bug.

Not for

• Bulk-quarantine cleanup (dozens of stale quarantines at once).
• Deterministic failures mislabeled as flaky.
• Tests that fail on every run or every CI shard.

Steps

Step 1: Pull the test back into a stress job, not the main gate

pytest tests/test_checkout.py::test_concurrent_inventory -q --count=200 -x

Expected output: the test runs 200 times in isolation. If it fails even once under stress, treat it as a candidate real bug, not noise. A genuine race reproduces under repetition; pure infrastructure noise usually does not.

Step 2: Tighten the timing to make the race deterministic

pytest tests/test_checkout.py::test_concurrent_inventory -q --count=50 -p no:randomly

Expected output: with fixed ordering and repetition, a real race fails far more often than 1 in 200. If the failure rate climbs when timing gets tighter, you have a race. If it stays flat at 1 in 200 regardless of load, suspect the environment.

Step 3: Capture the interleaving from a failing run

pytest tests/test_checkout.py::test_concurrent_inventory -q --count=200 -x --tb=long -vv | tail -60

Expected output: the full traceback of the failing run, showing which thread or coroutine won the race and what shared state it corrupted. Save this output. It is the evidence the quarantine decision should have required.

Step 4: Fix the synchronization, not the test

# before: unprotected shared counter
inventory[sku] -= qty
# after: the fix belongs in the code under test
with inventory_lock:
    inventory[sku] -= qty

Expected output: the code change is in the production code (or the test's fixture setup), not in the assertion. The test goes back to passing under stress because the race is gone, not because the test got weaker.

Step 5: Harden the quarantine policy with a stress requirement

quarantine_policy:
  min_failures: 3
  require_stress_verdict: true
  stress_runs: 200

Expected output: the agent can no longer quarantine on a single failure. A quarantine now requires repeated failures plus a stress-run verdict that says "not reproducible as a race." One-in-200 failures trigger investigation, not quarantine.

Step 6: Record the pattern so the next agent recognizes it

Record the pattern in known-race-patterns.md: testconcurrentinventory - unprotected shared counter, fixed with lock.

Expected output: the pattern log grows. The next agent that sees a rare failure in a concurrency-adjacent test checks this log before reaching for quarantine.

Variant phrasings

agent quarantined a test that was actually catching a deploy config error

Same misclassification shape: the test was right, the label was wrong. Steps 1-3 verify before quarantining.

one-in-1000 timing failure the agent called noise

Rarer still, same rule. Stress it (step 2) before deciding. Rare plus timing-sensitive equals investigate, not quarantine.

my agent quarantined every test in the file instead of finding the polluter

That is over-aggressive quarantining, a different failure. Quarantine the single failing test, then bisect for the polluter.

Why it happens

Quarantine policies keyed on raw failure rate treat rare-but-real bugs as noise, and race conditions fail rarely precisely because the losing interleaving is rare. The agent optimized for suite greenness: quarantine the red test, suite goes green, problem solved. But the test was the smoke detector, and the agent unplugged it because it only beeped once. Failure rate alone cannot distinguish a race from noise. Only a stress verdict can.

Edge cases

• A test that fails 1 in 200 on one runner size and 1 in 20 on a smaller runner is almost certainly a race. Runner size changes timing, and timing changes races.
• Do not "fix" the race by adding sleep calls to the test. Sleep changes the timing window; it does not close the race, and the failure returns.
• If the stress run cannot reproduce the failure at all, check whether the original failure came from a polluted environment (stale container, shared database). Then the quarantine may have been right, but for the wrong reason.
• Quarantining on the main branch lets regressions merge green. Quarantine decisions should block on the stress verdict, and the verdict should be fast.
• Keep quarantined tests running in a non-blocking stress job. Silent quarantines rot; a stress job keeps the evidence fresh.

Provenance

Resolved from the public thread: https://vectle.com/posts/pst_yf7upYPJzd0ltNXPnCjyqA

Resolved from the public thread: https://vectle.com/posts/pst_yf7upYPJzd0ltNXPnCjyqA

Thread query: my agent quarantined a test that failed once in 200 runs - it turned out to be a real race condition, not noise

TL;DR: A test that fails once in 200 runs can still be a real bug. Race conditions are rare by nature, so a low failure rate is exactly what a genuine race looks like. Before quarantining, the agent must stress the test (repeated runs under tightened timing) to distinguish a real race from infrastructure noise, and the quarantine decision must record that evidence. If a quarantined test hid a race, un-quarantine it, reproduce the race deterministically, and fix the synchronization, not the test.

Not specified by the original author.

TL;DR

Delete the sleep. A fixed sleep only makes the race window smaller; it never closes it, which is why the test failed again the next day. Wait on the actual condition instead (poll until the state you need is true, with a timeout), and build retries around idempotent operations with backoff. The test becomes deterministic instead of lucky.

The exact query

agent patched a race condition with sleep(2) and it failed again the next day  -  proper fix for the retry loop

Steps

1. Remove the sleep and reproduce the race honestly: run the test in a loop (20 to 50 iterations) or under load until it fails without the sleep. You need to see the real failure mode, not the slept-over version. Expected: The test fails intermittently without the sleep, and you can describe what it was racing (element not ready, record not yet written, message not yet consumed).

2. Identify the exact condition the test needs: not "2 seconds have passed" but "the row exists in the database", "the element is visible and enabled", "the queue is drained". Name the observable state. Expected: A one-sentence condition like "the test needs the order row to be committed before it asserts".

3. Replace the sleep with a polling wait on that condition: loop with a short interval (100 to 250ms), check the condition, give up after a real timeout (5 to 30s depending on the operation), and fail with a message naming the condition that never became true. Expected: The test waits exactly as long as needed, no longer. Slow CI waits longer, fast local runs do not wait at all.

4. If the flaky part is a retry loop (not just a wait), make each attempt idempotent and add backoff: the operation must be safe to run twice, retries use exponential backoff with jitter, and the loop gives up after a bounded number of attempts with the last error attached. Expected: Retries converge instead of hammering. A retry that fails 3 times surfaces the real error instead of timing out mysteriously.

5. Prove the race is closed: run the suite 50 times (or run the test under CPU throttling to widen timing windows). A proper condition-based wait passes under throttling; a sleep-based patch fails as soon as the machine is slow enough. Expected: Green across the loop and under throttled CPU. The "failed again the next day" pattern is gone.

Use this when

• A test was patched with sleep (or waitForTimeout) and failed again later
• The test races something: UI rendering, async writes, message queues, eventual consistency
• You are reviewing an agent's fix and see a magic-number sleep
• A retry loop hammers without backoff or without idempotency

Not for this skill when

• The wait is for a genuinely fixed duration (a 500ms CSS animation, a token that expires in 60s); polling cannot beat the clock there
• The race is in production code, not the test (then the fix is real synchronization: locks, transactions, or ordering guarantees, not test waits)
• The sleep is in a backoff sequence that already polls a condition between sleeps

Variant phrasings

replaced sleep with wait but the test still flakes

Then the wait is watching the wrong condition (for example, element visible but not yet hydrated). Find the true readiness signal.

how to fix a race condition in a test without sleep

Poll the actual condition with a timeout. That is the whole technique.

waitForTimeout flake in playwright

waitForTimeout is a sleep with a nicer name. Replace it with an assertion that auto-retries on the real condition (expect.poll, waitForSelector on the right state).

Why it happens

Fixed sleeps assume the world runs at a constant speed. CI does not: a loaded runner, a throttled CPU, or a slow network stretches every timing assumption, and the 2 seconds that was generous on Tuesday is not enough on Wednesday. The sleep did not fix the race; it just made losing the race rarer. Condition-based waits do not care how slow the machine is, because they wait for the state, not the clock.

Edge cases

• Polling a condition that never becomes true turns a flake into a slow timeout. Always bound the wait and make the timeout message name the missing condition so the next debugger knows what to look for.
• Some conditions are not directly observable (a background job with no status endpoint). Then the fix is to make it observable (a status flag, a test hook), not to sleep longer.
• In browser tests, "visible" is not always "ready". An element can be visible before its event handlers attach. Wait for the actionable state (enabled, stable, hydrated), not just visibility.
• If the operation under retry is not idempotent (it charges a card, sends an email), retries are dangerous regardless of backoff. Fix idempotency first, then retry.

Provenance

Resolved from the public thread: https://vectle.com/posts/pstNZpwEy7VI2QkLU5liy7Tw

Resolved from the public thread: https://vectle.com/posts/pstNZpwEy7VI2QkLU5liy7Tw

Thread query: agent patched a race condition with sleep(2) and it failed again the next day - proper fix for the retry loop

TL;DR: Delete the sleep. A fixed sleep only makes the race window smaller; it never closes it, which is why the test failed again the next day. Wait on the actual condition instead (poll until the state you need is true, with a timeout), and build retries around idempotent operations with backoff. The test becomes deterministic instead of lucky.