Review Agent PRs That Leave Promises Unawaited

A labeled incident from a review queue starts here.
A payments worker shipped a faster webhook handler on Tuesday.
The request path returned 202 in twenty milliseconds.
Downstream billing events never reached the ledger service.
The agent patch had removed three await keywords for speed.
This playbook treats that class of diff as hostile.

Why agents drop await

Coding agents optimize the visible request timer first.
Logs, mailers, webhooks, and cache writes look optional.
The handler reads as clean sequential business logic.
The side effects become unhandled rejections after the response.
Unit tests stay green because mocks swallow the I/O.
The merge then ships silent work that never runs.

Scope of this review

The article walks one agent-generated JavaScript pull request.
It shows which hunks to trust, revert, and test.
A scan script and a regression test form the artifact.

A concrete agent diff

The TypeScript below is a labeled example, not production code.

// example only: src/webhooks/invoicePaid.ts
export async function handleInvoicePaid(req: Request, res: Response) {
  const event = InvoicePaid.parse(req.body);

  auditLog.write({ kind: "invoice.paid", id: event.id });
  cache.delete("invoice:" + event.id);
  billing.enqueueLedger(event);

  res.status(202).json({ ok: true });
}

The three calls look parallel, cheap, and complete.
None of the three calls is awaited or joined.
A thrown TypeError dies as an unhandled rejection.
The HTTP client already received a 202 Accepted body.
The ledger service never sees the invoice identifier.

Status codes that lie

Agents often keep 200 after making the handler async.
A 200 claims the side effect already completed.
A 202 claims the work is accepted and durable.
Unawaited in-process promises support neither status code.
Reviewers should match the status to a durable ack.

What to trust

Trust schema parsing that matches an existing contract file.
Trust a 202 only when a durable outbox row exists.
Trust comments that cite a verified queue acknowledgment.
Trust metrics calls that cannot change money movement.
Trust refactors that add await without changing order.

What to revert

Revert dropped await on money, mail, auth, or deletion.
Revert fire-and-forget cache deletes without a sweeper.
Revert empty catch handlers added to silence the linter.
Revert void wrappers around product-critical promise calls.
Revert setImmediate used to hide work after res.json.
Revert Promise.all on mixed critical and optional I/O.

What to test

Test handler rejection when ledger enqueue fails closed.
Test a crash after enqueue still leaves a durable record.
Test the process for zero unhandled promise rejections.
Test that 202 is absent when the outbox write fails.
Test restored await order against the original sequence.

Common agent rationales

The agent commit message often cites p95 latency.
The attached benchmark is usually a unit test clock.
That clock ignores broker round trips and disk fsync.
A reviewer should demand an outbox write latency instead.
Another common rationale is at-least-once delivery elsewhere.
That claim is true only with a durable producer ack.
In-memory enqueue inside the web process is not durable.

Ordering bugs after dropped await

Dropped await also reorders side effects under load.
Cache deletes can finish before the ledger write starts.
Readers then observe a missing invoice that later appears.
Joined awaits restore a single happens-before edge.

Numbered review workflow

1. Isolate the async hunks

Fetch the agent branch before opening the web UI.

git fetch origin
git checkout -B review-agent origin/agent/faster-webhooks
git diff origin/main...HEAD -- '*.ts' '*.js' > /tmp/agent.diff

Save the diff to a file for grep and later notes.
Reviewers should not rely on GitHub hunk scrolling alone.

2. Flag dropped await

The scan below is a heuristic, not a type checker.

#!/usr/bin/env node
// example only: scan-unawaited.mjs
import fs from "node:fs";

const text = fs.readFileSync(process.argv[2], "utf8");
const lines = text.split("\n");
const call = /^\s+([a-zA-Z0-9_$.]+)\((.*)\)\s*;\s*$/;
const ignored = /^(return|throw|if|for|while|switch|catch)/;

for (let i = 0; i < lines.length; i++) {
  const line = lines[i];
  if (!call.test(line) || ignored.test(line.trim())) continue;
  if (/\bawait\b/.test(line)) continue;
  if (/void\s+/.test(line)) {
    console.log(`${i + 1}: void call ${line.trim()}`);
    continue;
  }
  if (/\.(then|catch|finally)\(/.test(line)) {
    console.log(`${i + 1}: detached chain ${line.trim()}`);
    continue;
  }
  if (/\b(send|write|enqueue|publish|delete|emit|notify|mail|charge)\b/.test(line)) {
    console.log(`${i + 1}: possible unawaited side effect ${line.trim()}`);
  }
}

Run it against every file in the pull request.

git diff --name-only origin/main...HEAD -- '*.ts' '*.js' |
while read -r f; do
  echo "## $f"
  node scan-unawaited.mjs "$f"
done

False positives are expected and useful as spotlights.
The script never issues a merge decision by itself.

3. Classify each hit

Use this decision table during the human review pass.

Signal in the hunk	Default action	Extra test
Dropped await on parse	Usually safe	Keep schema test
Dropped await on DB write	Revert	Fail-closed integration test
Dropped await on queue publish	Revert unless outbox	Crash-after-publish test
Dropped await on metrics	Trust if documented	Counter still increments
void plus empty catch	Revert	Unhandled rejection test
Promise.all without error split	Review	Partial failure test
setImmediate or queueMicrotask	Revert by default	Timing test under load

The table is a default, not a policy engine.
Written team rules still override any cell above.

4. Prove the request boundary

Add one test that does not mock the queue away.
The Node test below is a labeled example only.

import test from "node:test";
import assert from "node:assert/strict";

test("ledger enqueue failure fails the handler", async () => {
  const calls = [];
  const billing = {
    async enqueueLedger(event) {
      calls.push(event.id);
      throw new Error("broker down");
    },
  };
  const handler = makeHandler({
    billing,
    auditLog: { write: async () => {} },
    cache: { delete: async () => {} },
  });
  const req = { body: { id: "in_1" } };
  const res = {
    status(code) { this.code = code; return this; },
    json(body) { this.body = body; },
  };

  await assert.rejects(() => handler(req, res), /broker down/);
  assert.equal(res.code, undefined);
  assert.deepEqual(calls, ["in_1"]);
});

A green 202 with an empty broker fails the review.
The handler must not report success after a failed enqueue.

5. Watch unhandled rejections

Agents hide failures by detaching work after res.json.

test("handler leaves no unhandled rejection", async () => {
  const leaks = [];
  const onLeak = (err) => leaks.push(err);
  process.on("unhandledRejection", onLeak);
  try {
    const handler = makeHandler({
      billing: { enqueueLedger: async () => {} },
      auditLog: { write: async () => { throw new Error("audit"); } },
      cache: { delete: async () => {} },
    });
    const req = { body: { id: "in_2" } };
    const res = { status() { return this; }, json() {} };
    await handler(req, res);
    await new Promise((r) => setImmediate(r));
    assert.equal(leaks.length, 0);
  } finally {
    process.off("unhandledRejection", onLeak);
  }
});

A failing test here means the PR shipped a landmine.
Restore await or route the work through a durable outbox.

Parallelism without dropping work

Restoring await does not forbid real concurrent I/O.
The next example keeps failures on the request boundary.

// example only: join critical work, then respond
const event = InvoicePaid.parse(req.body);
const [outbox] = await Promise.all([
  billing.enqueueLedger(event),
  auditLog.write({ kind: "invoice.paid", id: event.id }),
]);
if (!outbox.acked) {
  throw new Error("outbox not durable");
}
res.status(202).json({ ok: true, id: event.id });

Optional cache deletes can follow a durable write.
They still need logging when they fail after the ack.
Do not hide those failures with an empty catch block.

Optional hosted run

Review scripts need a clean Node runtime, not a GPU.
Disclosure: This article was prepared as part of MonkeyCode's product outreach.
MonkeyCode is an open-source project with free model access.
A free server option can run this small review job.
Reviewers without a local Node install can run the same scan there.
The human still classifies each hit with the table above.
Model commentary is never a merge approval on its own.

Limitations

The regex scan misses aliased methods and dynamic calls.
It cannot see work behind Promise.resolve then chains.
It cannot prove durability of an outbox table.
TypeScript no-floating-promises is stricter and should stay enabled.
This workflow does not replace a queue with retry semantics.
It does not measure latency after restoring the await keywords.
It does not inspect Python asyncio or Go goroutine leaks.
Generated frontend-only PRs will produce noisy false positives.

Who should not use this approach

Do not use this scan as a merge gate on CSS-only changes.
Do not apply it to workers that already use explicit job runners.
Do not paste secret-bearing diffs into any hosted review box.
Do not accept fire-and-forget on payment, auth, or deletion paths.
Teams without an outbox should not drop those awaits for speed.
Do not treat a green unit suite with mocks as durability proof.

Closing notes

Agent PRs often sell speed by dropping await keywords.
The cost shows up as silent ledger drift later.
Reviewers should restore the request boundary before merge.
Then they can add real parallelism with Promise.all.
The merge is ready when failed side effects fail the handler.
Green CI is not enough if rejections happen after 202.