DEV Community

Jordan Li
Jordan Li

Posted on

Review Agent PRs That Add Sleep, Retry, or Timeout

The agent labeled the pull request as a flake fix.
The webhook handler gained a hard-coded 500 millisecond sleep.
The HTTP client gained four retries with no jitter.

This walkthrough uses a synthetic agent diff as the artifact.
The article claims no production incident and no measured win.
The goal is a repeatable review order for time hunks.

Why time hunks deserve their own pass

Agent patches often hide race conditions behind delays.
A sleep can make one local test pass by accident.
A retry can turn a fast failure into a slow outage.

Timeouts also change user-visible latency budgets under load.
Catch blocks around retries can swallow cancellation errors.
Those edits look small and read like hygiene.

They are not hygiene until proven with a clock.
They are control-flow changes with production cost.

A webhook that sleeps holds a worker thread idle.
Idle workers shrink throughput during a retry storm.
The failure mode is latency, not an obvious stack trace.

Retry without idempotency keys duplicates side effects.
A double charge is worse than a flaky test.
The reviewer treats new retry loops as payment-adjacent risk.

Trust, revert, or test

The reviewer sorts every time-related hunk into three buckets.
Each hunk gets Trust, Revert, or Test before merge.

Trust means the hunk matches an existing documented policy.
Revert means the hunk invents delay without a stated budget.
Test means the hunk might be valid after a clock-based check.

Pattern Default bucket Why
sleep / setTimeout on a request path Revert Hides races and blocks the event loop
retry without cap, jitter, or deadline Revert Amplifies load during an incident
timeout copied from an existing client Trust Policy already reviewed elsewhere
new timeout constant in one module Test Needs load and cancellation checks
Date.now used for auth or billing Test Clock skew and freeze tests required
test-only fake timers Trust Isolated if production code is untouched
catch that ignores timeout errors Revert Turns deadline misses into success

This table is a proposal, not a legal policy.
Teams should replace the defaults with their own SLOs.

Review order

Follow these steps on every agent pull request.
The order stays fixed even when the patch is large.

1. Isolate the time surface

The reviewer fetches the branch and produces a focused diff.
A three-dot range keeps merge commits out of the noise.

git fetch origin
git diff origin/main...HEAD -- '*.js' '*.ts' > /tmp/agent.patch
Enter fullscreen mode Exit fullscreen mode

The reviewer greps the patch for time verbs first.
The pattern list is a starting point, not complete.

grep -nE 'sleep|setTimeout|setInterval|retry|backoff|Date\.now|performance\.now|AbortSignal|timeout|delay\(' /tmp/agent.patch
Enter fullscreen mode Exit fullscreen mode

Empty grep output does not end the review.
Generated helpers may hide timers behind new names.

2. Classify each hunk

The reviewer walks hunks in file order, not commit order.
Agents squash unrelated timer edits into one commit.
File order keeps the HTTP client next to its tests.

For each match, the reviewer fills three fields.
The fields are bucket, budget, and blast radius.
Budget is the new maximum wait in milliseconds.

Blast radius is the caller set that inherits the wait.
Missing budget means the hunk starts in Revert.
Missing caller set means the hunk starts in Test.

3. Split production files from test files

The classifier does not know test directories by default.
The reviewer prefixes test paths by hand.
A sleep in src/ starts as Revert.

A sleep in test/ starts as Test.
Fake timers in tests can stay in Trust.
The production import graph must stay free of those timers.

4. Revert invented delays on the request path

Production handlers should not sleep to win a race.
The correct fix is ordering, locking, or an explicit queue.
A 500ms sleep is a load-bearing guess, not a design.

The reviewer restores the handler to the prior control flow.
The reviewer leaves a comment naming the missing invariant.
The agent can retry the task after that invariant is stated.

5. Test remaining timeouts with a fake clock

Trusted timeout copies still need one cancellation test.
The test must advance fake time past the deadline.
The test must prove the abort path runs once.

A passing real-clock test is not enough evidence.
Network jitter will not reproduce in CI consistently.

// Example only. Unexecuted illustration of a fake-clock test.
const { describe, it, mock } = require('node:test');
const assert = require('node:assert/strict');
const { withDeadline } = require('../src/http/deadline');

describe('withDeadline', () => {
  it('rejects after the budget', async (t) => {
    t.mock.timers.enable({ apis: ['setTimeout'] });
    const pending = withDeadline((signal) => waitFor(signal), 30_000);
    t.mock.timers.tick(30_000);
    await assert.rejects(pending, { name: 'TimeoutError' });
  });
});
Enter fullscreen mode Exit fullscreen mode

6. Record the decision on the patch

The reviewer pastes a short table into the review.
Each row names the file, bucket, and follow-up test.
Unbucketed time hunks must block the merge.

TIME-HUNK REVIEW
file:
bucket: trust | revert | test
budget_ms:
blast_radius:
follow_up_test:
Enter fullscreen mode Exit fullscreen mode

Artifact: a local time-hunk classifier

The script below is labeled example code only.
It reads a unified diff from stdin.
It prints JSON lines for review, not a merge verdict.

#!/usr/bin/env node
// Example only: classify time-related hunks in a unified diff.
// It does not execute the patch and does not call a model.

const fs = require('fs');

const RULES = [
  { re: /\bsleep\s*\(|\bsetTimeout\s*\(|\bsetInterval\s*\(/, tag: 'delay', bucket: 'revert' },
  { re: /\bretry\b|\bbackoff\b|\battempts?\s*[:=]/i, tag: 'retry', bucket: 'revert' },
  { re: /\btimeout\b|\bdeadline\b|\bAbortSignal\b/, tag: 'timeout', bucket: 'test' },
  { re: /\bDate\.now\s*\(|\bperformance\.now\s*\(|\bnew Date\s*\(/, tag: 'clock', bucket: 'test' },
];

function classify(line) {
  const hits = [];
  for (const rule of RULES) {
    if (rule.re.test(line)) hits.push(rule);
  }
  return hits;
}

const input = fs.readFileSync(0, 'utf8');
let file = null;

for (const raw of input.split('\n')) {
  if (raw.startsWith('+++ b/')) {
    file = raw.slice(6).trim();
    continue;
  }
  const added = raw.startsWith('+') && !raw.startsWith('+++');
  if (!added) continue;
  const text = raw.slice(1);
  for (const hit of classify(text)) {
    const row = {
      file,
      tag: hit.tag,
      bucket: hit.bucket,
      excerpt: text.trim().slice(0, 160),
    };
    process.stdout.write(JSON.stringify(row) + '\n');
  }
}
Enter fullscreen mode Exit fullscreen mode

The reviewer runs it against the saved patch.

node flag-time-hunks.js < /tmp/agent.patch
Enter fullscreen mode Exit fullscreen mode

Sample output follows for the synthetic webhook patch.

{"file":"src/webhooks/payment.js","tag":"delay","bucket":"revert","excerpt":"await sleep(500)"}
{"file":"src/http/client.js","tag":"retry","bucket":"revert","excerpt":"for (let attempt = 0; attempt < 4; attempt++) {"}
{"file":"src/http/client.js","tag":"timeout","bucket":"test","excerpt":"signal: AbortSignal.timeout(30_000)"}
Enter fullscreen mode Exit fullscreen mode

The first two JSON rows start as Revert.
The third row starts as Test until a cancel test exists.

A synthetic before-and-after

The agent proposed this handler change for review.

// agent-proposed, do not merge as-is
async function onPayment(event) {
  await sleep(500); // wait for ledger replica
  const row = await ledger.get(event.paymentId);
  if (!row) return { ok: true };
  return settle(row);
}
Enter fullscreen mode Exit fullscreen mode

The sleep invents a replica lag budget of 500ms.
The early return treats a miss as success.
The reviewer reverts both of those lines together.

A reviewable replacement keeps the failure path visible.

async function onPayment(event) {
  const row = await ledger.get(event.paymentId);
  if (!row) {
    const err = new Error('ledger miss');
    err.code = 'LEDGER_MISS';
    throw err;
  }
  return settle(row);
}
Enter fullscreen mode Exit fullscreen mode

The retry hunk needs that same review treatment.
Four attempts without jitter can stampede a sick dependency.
The reviewer asks for a shared retry helper with a deadline.

// Example only. Budget must come from an existing SLO document.
async function withDeadline(fn, ms) {
  const signal = AbortSignal.timeout(ms);
  return fn(signal);
}
Enter fullscreen mode Exit fullscreen mode

That helper remains a proposal in this article.
The team must pick ms from an existing SLO document.

Running the classifier on a free server

Large agent patches overflow a local terminal scrollback.
A small VPS can store patches and run the classifier on a schedule.

Disclosure: This article was prepared as part of MonkeyCode's product outreach.
MonkeyCode provides free model access and a free server option.
The classifier above does not require either capability.

A hosted model can turn JSON lines into a review comment.
The model must not change the bucket field.
The reviewer still applies Trust, Revert, and Test by hand.

A possible prompt for that summary step follows.
The prompt is a draft, not production advice.

Summarize each JSON line in one sentence.
Keep the bucket value unchanged.
Do not recommend merge.
List files that still lack a cancellation test.
Enter fullscreen mode Exit fullscreen mode

The summary is a reading aid, not evidence.
Evidence remains the diff, the SLO, and the fake-clock test.

Limitations

Regex rules miss renamed helpers and minified bundles.
They also flag comments and string literals by design.
That bias is intentional for a first review pass.

The script does not run tests and does not prove safety.
It does not know the service SLO or the retry policy.
It cannot detect sleeps hidden in native addons.

Fake timers lie when the code uses wall clocks.
Date.now calls still bypass most fake-timer suites.
Those hunks stay in Test until a frozen-clock test exists.

This workflow does not replace a full security review.
Timeouts can still leak timing data on auth paths.
Retry can still replay non-idempotent POST requests.

Who should not use this approach

Teams without a written latency budget should not auto-trust timeouts.
Solo merges without a second reader should not skip the Revert default.

Generated diffs that rewrite lock files need a different classifier.
Mobile clients with OS-level backoff need platform review, not this script.

The model summary must not act as a merge gate.
Empty grep output must not count as a green review.

Close

Time hunks are control flow dressed as cleanup.
The reviewer buckets them before reading the rest.
Sleep on a request path starts as Revert.

Retry without a hard deadline starts as Revert.
Copied timeout values still need one cancellation test.

Readers with a free hosted coding server can run the classifier there.
They can compare bucket notes on the next agent patch.

Top comments (0)