Review Agent PRs That Ignore Client Abort Signals

A billing enrichment pull request landed just after midnight. The coding agent wired a partner HTTP call into checkout. The recorded happy-path unit test stayed completely green.

The handler kept the outbound fetch alive anyway. Client disconnects did not cancel remaining database work. A small host then burned sockets on abandoned requests.

The failure mode in agent diffs

Coding agents optimize for a successful JSON body. They copy fetch calls from common public tutorials. They rarely thread the incoming request signal downward.

The diff still looks finished to a tired reviewer. Client cancellation, refresh, and proxy timeouts never appear. The merge then leaks work onto every dropped tab.

This cancellation bug is not exotic in production traffic. Modern browsers abort in-flight calls on navigation. Mobile clients abort calls during aggressive backgrounding.

Each abort should stop remaining outbound I/O. Agent-generated pull requests often keep going instead. That waste shows first on a free or small server.

What the agent usually commits

The following snippet is a labeled, unexecuted example. It shows a review pattern, not a recipe.

// unexecuted example — agent-style handler
export async function enrichOrder(req, res) {
  const orderId = req.params.id;
  const order = await db.orders.findById(orderId);
  const extra = await fetch(order.partnerUrl).then((r) => r.json());
  await db.orders.update(orderId, { extra });
  res.json({ ok: true, extra });
}

The partner call has no timeout budget. The client abort never reaches fetch. Updates still run after the requester leaves.

A second pattern shows up in helper modules. The agent starts work with void and moves on.

// unexecuted example — fire-and-forget after response
res.json({ ok: true });
void fetch(order.partnerUrl);

That form is worse under modest load. Errors vanish into unhandled rejections. Cancellation becomes impossible to test later.

Scan the PR with commands

Run these commands on the agent branch. They do not prove correctness. They only list suspects for the human review.

git fetch origin
git checkout pr-agent-enrich
git grep -nE "fetch\(|axios\(|got\(|undici" -- '*.js' '*.ts'
git grep -nE "void fetch|void axios" -- '*.js' '*.ts'
git grep -nE "AbortSignal|req\.signal|signal:" -- '*.js' '*.ts'
git grep -nE "res\.json\(|res\.send\(" -- '*.js' '*.ts'

A fetch hit without a nearby signal is a revert candidate. A res.json hit above a later await is another. Record both paths in the review notes.

How to read the diff

Follow this numbered pass before leaving comments.

1. Find every outbound HTTP helper in the changed files.
2. Check whether req.signal is an argument into that helper.
3. Check for a timeout signal beside the client signal.
4. Check that no await remains after the response is sent.
5. Check that retries reuse the original abort signal object.

Stop the pass when any step fails. One missing signal is enough to block merge. Style nits can wait until cancellation is real.

What to trust

Reviewers can trust naming and the intended checkout flow. The agent often picks the correct route file. It may reuse an existing order lookup helper.

Trust stubbed 200 tests for the partner shape. Those tests prove parsing on the happy path. They do not prove abort behavior at all.

Trust comments that mention timeouts only after a diff check. Agents leave timeout comments after removing the timeout. The comment is not evidence.

What to revert before merge

Use this numbered revert list during review.

1. Revert fetch calls that omit signal and a timeout.
2. Revert void or unawaited promises in request handlers.
3. Revert partner writes that continue after res.json.
4. Revert retries that recreate fetch without the same signal.
5. Revert debug logs that dump the entire partner payload.

Do not keep a temporary unbounded call. Temporary fetch helpers become permanent fixtures. Small servers feel that leak first and loudest.

Also revert partner URLs taken from the client body. Handlers should allowlist stored hosts instead. Cancellation cannot fix an unbounded destination list.

What to test instead

The original artifact is this abort test plan. Run it on the PR branch. Do not accept green CI that never aborts.

Step 1. Prove the handler accepts a signal

Pass req.signal or an equivalent AbortController. The partner fetch must receive that same signal. A unit stub should reject with AbortError when aborted.

Step 2. Abort before the partner responds

Start the request, then abort at ten milliseconds. The test must see no orders.update call. The process must not log an unhandled rejection.

Step 3. Abort during a slow write

If the write cannot cancel, document that driver limit. The handler should still stop follow-up fetches. It should not retry after the abort fires.

Step 4. Bound every outbound call

Set an explicit timeout besides the client abort. Client abort and server timeout differ in source. Both must close the outbound socket.

Step 5. Reject unknown partner hosts

Feed a URL whose host is not allowlisted. The handler must not open a socket. This check belongs beside cancellation, not after it.

Reproducible harness

Label this harness as a proposal. Operators should adapt names to the real app. The test uses Node.js node:test and built-in fetch.

// abort-review.mjs — proposal for PR CI
import test from 'node:test';
import assert from 'node:assert/strict';

let updateCalls = 0;

const db = {
  orders: {
    async findById(id) {
      return { id, partnerUrl: 'http://127.0.0.1:9/slow' };
    },
    async update() {
      updateCalls += 1;
    },
  },
};

async function enrichOrder(req) {
  const order = await db.orders.findById(req.params.id);
  const extra = await fetch(order.partnerUrl, {
    signal: AbortSignal.any([
      req.signal,
      AbortSignal.timeout(2000),
    ]),
  }).then((r) => r.json());
  await db.orders.update(order.id, { extra });
  return extra;
}

test('enrichment stops when the client aborts', async () => {
  updateCalls = 0;
  const controller = new AbortController();
  const req = { params: { id: 'ord_1' }, signal: controller.signal };

  const pending = enrichOrder(req);
  controller.abort();

  await assert.rejects(pending, { name: 'AbortError' });
  assert.equal(updateCalls, 0);
});

Run it with a single command.

node --test abort-review.mjs

A failing agent PR throws later, during connect. It also increments updateCalls. Reviewers should reject that branch on sight.

The corrected production shape looks like the next snippet. It remains a proposal for the real service. Wire it to the real logger and host allowlist.

// proposal — forward abort, timeout, and host allowlist
const PARTNER_HOSTS = new Set(['partner.example.internal']);

export async function enrichOrder(req, res) {
  const order = await db.orders.findById(req.params.id);
  const url = new URL(order.partnerUrl);
  if (!PARTNER_HOSTS.has(url.hostname)) {
    res.status(400).json({ error: 'partner_host_rejected' });
    return;
  }

  const signal = AbortSignal.any([
    req.signal,
    AbortSignal.timeout(2000),
  ]);

  try {
    const response = await fetch(url, { signal, redirect: 'error' });
    if (!response.ok) throw new Error('partner_http');
    const extra = await response.json();
    if (req.signal.aborted) return;
    await db.orders.update(order.id, { extra });
    res.json({ ok: true });
  } catch (err) {
    if (err.name === 'AbortError') {
      if (!res.headersSent) res.status(499).end();
      return;
    }
    if (!res.headersSent) {
      res.status(502).json({ error: 'partner_unavailable' });
    }
  }
}

The allowlist is not optional on a small host. Unbounded destinations turn into outbound floods. Cancellation without an allowlist still wastes DNS and sockets.

Decision table for reviewers

Diff signal	Trust	Action	Required test
fetch(url) with no options	Intent only	Add signal and timeout	Abort at 10ms
void fetch(...)	Nothing	Revert	Unhandled rejection check
Retry wrapper around fetch	Backoff math only	Pass the original signal	Abort mid-retry
res.json then await	Nothing	Revert the order	No write after abort
Partner host from request body	Nothing	Allowlist stored hosts	Unknown host opens zero sockets

Keep this table in the PR template. Agents will still omit signal. The table makes the revert boring and repeatable.

Reviewing on a constrained box

Abandoned fetches hurt most on small shared hosts. File descriptors run out first. Outbound NAT slots run out second.

Disclosure: This article was prepared as part of MonkeyCode's product outreach.

MonkeyCode is an open-source project with free model access. It also includes a 10 million free token grant and a free server option. Those resources fit a cancellation review harness. The model can draft abort tests from a diff. The server can run node --test on each agent branch.

Do not treat generated notes as the review. The failing test is the review. Tokens only help sketch stubs. The free server is one always-on worker for that command.

Reviewers who need a small box for the harness can try that free server option.

Limitations

AbortSignal does not cancel every database driver. Some pools finish the in-flight query. Some ORMs ignore signal today.

AbortSignal.any needs a current Node.js runtime. Older runtimes need a manual abort listener. The two-second timeout is an example, not a default.

This workflow does not replace load testing. It only catches leaked work after disconnect. It will not catch slow queries that finish inside the timeout.

Status 499 is a conventional local choice. Proxies may map it differently. Teams can log and close without a body instead.

Who should not use this approach

Do not use this harness as the only production gate. Teams without Node 20 plus must adapt APIs. Codebases without fetch need their own client abort hooks.

Do not abort paths that already committed payments. Those paths need idempotent completion. Cancellation belongs to HTTP side effects, not ledger writes.

Skip this pattern for batch jobs without a client. Jobs need a separate deadline signal. Pretending req.signal exists there hides bugs.

Frontend-only PRs do not need this server harness. Browser AbortController still matters there. The tests above target Node request handlers.

Close

Agent PRs often look done at status 200. Cancellation is the missing branch. Review the signal path, then merge.