Fire-and-Forget Is a Scheduler: Reviewing Agent PRs That Drop Awaits

When an agent-generated pull request removes an await, wraps a call in void, or starts a background task so the handler can return faster, the change is not a performance patch. It is a scheduler. The request path no longer owns completion, failure, or ordering.

Reviewers should treat that as a new runtime. Then decide whether to trust, revert, or prove it. The rest of this article is a protocol for that pattern: an illustrative patch, a decision table, and a test plan that does not depend on the agent's own comments.

This is adjacent to a current debate about calling vibe output "engineering." The interesting failure is not sloppy prose. It is a merge that looks like latency work and ships an undocumented job system.

The pattern

Agents produce this shape often. The prompt said "make the endpoint faster" or "don't block on the model." The model then detaches work.

The TypeScript below is illustrative. It is not a production dump.

// before
export async function completeJob(jobId: string): Promise<JobResult> {
  const job = await loadJob(jobId);
  const result = await runInference(job.payload);
  await persist(jobId, result);
  return result;
}

// after (agent PR)
export async function completeJob(jobId: string): Promise<JobResult> {
  const job = await loadJob(jobId);
  void runInference(job.payload)
    .then((result) => persist(jobId, result))
    .catch((err) => console.error("inference failed", err));
  return { status: "accepted" } as JobResult;
}

Python agents reach the same place with asyncio.create_task. Same review. Same missing owner.

# after (illustrative)
async def complete_job(job_id: str) -> JobResult:
    job = await load_job(job_id)
    asyncio.create_task(_run_and_persist(job))
    return JobResult(status="accepted")

Three contract changes hide in both diffs. The return type lies. Failures leave the request. Persistence becomes unordered.

What to read in the diff, not the summary

Agent PR titles under-describe this class of change. "Speed up completion handler" is a product claim. The code is a job queue with no queue.

Scan the patch for these tokens first:

• void somePromise or a floating promise on an exported path
• .then( without a matching await on the public function
• asyncio.create_task, unawaited, gevent.spawn, go func( inside a handler
• setImmediate, setTimeout(..., 0), queueMicrotask, process.nextTick
• as JobResult / as any covering a newly invented body
• comments that say fire and forget, don't block, or best effort

Each token is a control-flow edit. None of them is evidence that a worker, a retry policy, or a dead-letter path exists.

A cheap first pass on the branch:

git diff origin/main...HEAD -- '*.ts' '*.js' '*.py' '*.go'

git grep -nE 'void .+(Promise|runInference)|create_task\(|nextTick\(|setImmediate\(|fire-and-forget' -- '*.ts' '*.py' '*.go'

# if the repo already lints floating promises, keep that signal
git grep -nE 'no-floating-promises|unawaited-task|RUF006' -- '*.json' '*.toml' '*.yml'

If the linter already forbids the pattern and the PR disables the rule, that disable is part of the review. It is not a style nit.

Trust, revert, or prove

Use this table on the PR. Do not use the model's explanation as evidence.

Signal in the PR	Trust?	Revert?	Prove with
Await dropped, caller still types JobResult with output fields	No	Yes, unless the public API was already "accepted"	Contract test on status code and body shape
New background task, existing enqueue helper in the repo unused	No	Yes	Search for the canonical queue API and require it
Detach plus console.error / logger.warn only	No	Default to revert	Kill the process mid-task; assert durable state
Detach plus an idempotency key, durable queue, and retry policy already in tree	Maybe	No, if it actually calls those APIs	Duplicate-delivery test
Comment says "safe to background" with no test	No	Yes	Cancellation and restart tests below

Default is revert. Trust is the exception. Proof is a failing test you would keep if the agent disappeared.

Review checklist

Work top to bottom. Stop at the first unmet item and request a revision rather than stacking nits.

1. Public contract. Did the HTTP status, return type, or event payload change from "done" to "accepted"? If yes, this is an API PR. It needs a changelog, caller updates, and a version note.
2. Ownership of failure. After detach, who retries? Who pages? A .catch(console.error) is not an owner.
3. Ownership of success. persist in a then can run after the client has already read a success body. Or never run.
4. Idempotency. A timeout client will retry completeJob. Two background tasks can persist twice. Require a job key.
5. Cancellation. Request abort, client disconnect, and server shutdown must have a defined behavior. "Undefined" is a reject.
6. Observability. You need a correlation id that survives the detach. A log line without the job id is not a trace.
7. Backpressure. Unbounded create_task is a memory leak under load. Ask for a queue depth or a reject-when-full path.
8. The unused queue. If the repo already has a domain enqueue(), Bull, Sidekiq, SQS, or Cloud Tasks, the agent invented a second scheduler. Revert the invention.

Reproducible test plan (unexecuted here)

Do not accept the agent's tests as coverage for this pattern. They often assert that the handler returned 200. That is the bug.

Proposed tests. Mark them as new requirements on the PR:

// proposed: contract tests, not executed in this article
describe("completeJob", () => {
  it("does not report completed while persist is still pending", async () => {
    const persistGate = deferred();
    fakePersist.mockImplementation(() => persistGate.promise);

    const pending = completeJob("job-1");
    const snapshot = await Promise.race([
      pending,
      timeout(50).then(() => "still-pending"),
    ]);

    // A detached PR returns an accepted body while persistGate is unresolved.
    // That must be an explicit API change, not an accident.
    expect(snapshot).not.toMatchObject({ output: expect.anything() });
  });

  it("does not start a second inference for the same job id", async () => {
    await Promise.all([completeJob("job-1"), completeJob("job-1")]);
    await flushBackground();
    expect(fakeInference).toHaveBeenCalledTimes(1);
  });
});

Python equivalent, also proposed:

# proposed; not executed here
@pytest.mark.asyncio
async def test_duplicate_post_does_not_double_infer(fake_infer):
    await asyncio.gather(complete_job("job-1"), complete_job("job-1"))
    await asyncio.sleep(0)
    assert fake_infer.call_count == 1

Add process-lifetime checks outside the unit runner. Unit tests will not catch a lost write on SIGKILL.

# proposed commands; replace the start script with the service's real one
node --test ./complete-job.test.ts

node ./server.js &
pid=$!
curl -sS -D - -X POST localhost:3000/jobs/job-1/complete
kill -9 "$pid"

node ./server.js &
curl -sS localhost:3000/jobs/job-1

The question is binary. After SIGKILL, is the job completed, pending, or missing? An agent PR that cannot answer that has not added a scheduler. It has added a race.

What "faster" actually measured

Agents justify detach with latency. Measure the right clock.

• T_handler: time until the HTTP response is flushed.
• T_durable: time until persist has committed.
• T_visible: time until another node can read the result.

Dropping await can shrink T_handler and leave T_durable unchanged or worse. Callers that poll on T_handler now race T_durable. That shows up as flaky clients, not as a faster product.

A one-line check on the PR:

Which of T_handler, T_durable, T_visible did you measure, and on how many runs?
If the answer is none, the performance claim is unreviewed.

No fabricated speedup numbers belong in the merge message. If the author did not record a method, the claim is out of scope.

A second model, on a throwaway server

Local review still needs a place to run the kill-and-inspect loop. A laptop works. A disposable server works better when the patch also touches paths, permissions, or process managers.

MonkeyCode is relevant here only as a reproduction bench: it currently offers free model access and a free server option, which is enough to re-run the original prompt and to host the kill test without using a production box.

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

The useful move is not "ask the same model to confirm its PR." The useful move is to give a second model the before tree and a constrained instruction:

Do not background work. Keep completeJob awaited end-to-end.
If latency is the issue, return 202 from a real enqueue helper
already in this repo. If none exists, say so and stop.

Compare the two diffs. If the second model refuses to drop the await, the first PR is a prompt failure, not an architecture finding. If both models detach, the prompt is asking for a job system. Write the job system on purpose, or reject the latency request.

If you run that reproduction on a free server, attach the crash transcript and the second diff to the PR so the next reviewer can see the disagreement.

Limitations

This protocol is for request-path code that used to be synchronous with its side effects. It is the wrong tool for systems that already document 202 + worker + idempotency key. It is also the wrong tool for fire-and-forget metrics where loss is an accepted SLO.

It does not prove semantic correctness of the inference itself. A perfectly awaited handler can still persist garbage. Pair this review with a contract test on the payload, which is a different PR class.

Throwaway models and throwaway servers are not production capacity. They have unspecified limits, no implied uptime, and no implied model identity. Do not load customer data onto a free server. Do not treat a second-model disagreement as a formal proof; it is only a prompt to write a test.

Who should not use this approach

• Reviewers who cannot run the service at all. Read the checklist, then assign someone who can execute the SIGKILL step.
• Teams whose runtime already forbids detached tasks via lint (@typescript-eslint/no-floating-promises, Ruff unawaited-task rules). Keep the lint. Do not replace it with a review essay.
• Anyone merging under a prompt that said "just make it work." Detach is how that prompt ships.

If the PR still needs a background path after this review, the follow-up is a queue PR: enqueue API, worker, idempotency, retry, dead letter, and a 202 contract. That is engineering. Dropping await is not.