A payment team received an agent-generated pull request on Tuesday. The ticket asked only for webhook retries after timeouts. The agent also invented three extra operational defaults.
It set a 250ms timeout on the gateway client. It retried failed posts three times without jitter. It swallowed JSON parse errors and returned 200.
Unit tests remained green after the generated change. Staging stayed quiet for two full days. Duplicate captures then appeared in the merchant ledger.
The reviewer had approved the shape of the diff. Nobody had approved the policy inside those defaults. This walkthrough uses a fictional payment service as the specimen. It is a review protocol, not an incident report.
The defect is unspecified policy
Agent-generated code often compiles on the first pass. It often mirrors nearby naming and file layout. It still injects policy the ticket never named.
Timeouts and retry counts are product policy, not decoration. Fallback identities remain policy, not a coding convenience. Null coalescing is also policy, not cleanup.
A reviewer who only reads the hunk list misses that injection. The generated diff still looks local and tidy. Injected policy remains global, durable, and unreviewed.
Cheap agent output makes this failure more common. The model fills every unspecified gap with a confident literal. Reviewers then confuse fluency with authorization.
This protocol does not re-score hunks for trust. It inventories silent defaults against the original ticket. Each finding maps to one of three actions.
- Strip the default. The ticket never authorized it.
- Pin the default. Promote it to named configuration.
- Lock the default. Cover it with a contract test.
Specimen: the agent patch
The ticket text is short and incomplete. That incompleteness is the usual agent input.
Title: Retry payment webhooks on gateway timeout
Acceptance: POST /webhooks/payment again if the gateway times out.
Out of scope: changing success status codes, auth, or capture semantics.
The agent produced a handler that looks careful. The extra policy hides inside ordinary literals.
// proposed: src/webhooks/paymentRetry.js
const GATEWAY_TIMEOUT_MS = 250;
const MAX_RETRIES = 3;
async function deliverPaymentWebhook(event, gateway = defaultGateway) {
const payload = event.body || {};
const merchantId = payload.merchantId || process.env.DEFAULT_MERCHANT;
for (let attempt = 1; attempt <= MAX_RETRIES; attempt++) {
try {
await gateway.post("/capture", payload, { timeout: GATEWAY_TIMEOUT_MS });
return { ok: true, attempt };
} catch (err) {
if (attempt === MAX_RETRIES) {
return { ok: true, skipped: true };
}
}
}
}
module.exports = { deliverPaymentWebhook };
Four silent defaults sit in that short file. The 250ms timeout is not in the ticket. The retry count is not in the ticket.
The fallback merchant id is not in the ticket. The success response on total failure is not in the ticket. The last default is the ledger bug. The caller treats { ok: true } as a completed capture.
Step 1. Extract the ticket contract first
Reviewers should not open the generated diff yet. The reviewer writes the ticket contract as executable statements. Each contract statement must stay strictly falsifiable.
The contract file lives beside the pull request. Name it review/contract.md so later comments can link it.
# Contract: payment webhook retry
1. Retry only after a gateway timeout error.
2. Do not invent a timeout budget.
3. Do not invent a retry ceiling.
4. Do not invent a merchant identity.
5. Do not report success when delivery failed.
6. Do not change HTTP success semantics.
Those six lines become the review oracle. The generated code is not the oracle. The diff is evidence against the contract, nothing more.
Step 2. Inventory literals that encode policy
The reviewer scans the pull request for policy-shaped literals. A small Node scanner is enough for a first pass. Hits are review suspects, not proof of a defect.
// tools/assumption-scan.js
const fs = require("fs");
const path = require("path");
const RULES = [
{ id: "timeout-literal", re: /timeout\s*[:=]\s*\d+/i, why: "timeout budget" },
{ id: "retry-literal", re: /(retry|retries|maxRetries|MAX_RETRIES)\s*[:=]\s*\d+/i, why: "retry policy" },
{ id: "or-empty-object", re: /\|\|\s*\{\s*\}/, why: "shape fallback" },
{ id: "env-fallback", re: /process\.env\.\w+\s*\|\|/, why: "identity fallback" },
{ id: "empty-catch", re: /catch\s*\([^)]*\)\s*\{\s*\}/m, why: "swallowed error" },
{ id: "success-in-catch", re: /catch\s*\([^)]*\)\s*\{[\s\S]{0,200}\bok\s*:\s*true/m, why: "failure reported as success" },
];
function walk(dir, acc = []) {
for (const ent of fs.readdirSync(dir, { withFileTypes: true })) {
if (ent.name === "node_modules" || ent.name === ".git") continue;
const p = path.join(dir, ent.name);
if (ent.isDirectory()) walk(p, acc);
else if (p.endsWith(".js")) acc.push(p);
}
return acc;
}
const root = process.argv[2] || "src";
for (const file of walk(root)) {
const text = fs.readFileSync(file, "utf8");
const lines = text.split("\n");
for (const rule of RULES) {
lines.forEach((line, i) => {
if (rule.re.test(line)) {
console.log(`${rule.id}\t${file}:${i + 1}\t${rule.why}\t${line.trim()}`);
}
});
}
}
The scan runs against patch files only. Full-tree noise hides the review signal.
git fetch origin main
git diff --name-only origin/main...HEAD -- "*.js" > /tmp/pr-files.txt
node tools/assumption-scan.js src | grep -F -f /tmp/pr-files.txt
Expected output for the specimen looks like this.
timeout-literal src/webhooks/paymentRetry.js:2 timeout budget const GATEWAY_TIMEOUT_MS = 250;
retry-literal src/webhooks/paymentRetry.js:3 retry policy const MAX_RETRIES = 3;
env-fallback src/webhooks/paymentRetry.js:7 identity fallback const merchantId = payload.merchantId || process.env.DEFAULT_MERCHANT;
success-in-catch src/webhooks/paymentRetry.js:16 failure reported as success return { ok: true, skipped: true };
The scanner is a labeled heuristic. It misses semantic assumptions on purpose. It also flags honest constants. A human still decides.
Step 3. Map each hit to strip, pin, or lock
The reviewer uses a table instead of taste. The only debate is ticket authorization.
| Hit | Ticket authority | Action | Review comment |
|---|---|---|---|
GATEWAY_TIMEOUT_MS = 250 |
none | strip or pin | Timeout budget is unspecified. Move to config or remove. |
MAX_RETRIES = 3 |
none | pin | Retry ceiling needs an explicit product value. |
| `payload.merchantId \ | \ | process.env.DEFAULT_MERCHANT` | forbidden |
{ ok: true, skipped: true } in catch |
forbidden | strip | Failure must remain failure. |
| `event.body \ | \ | {}` | none |
Review comments stay imperative and local. They cite the contract file, not reviewer mood.
Contract violation: merchant identity fallback.
Ticket out of scope includes auth and capture semantics.
This line invents a merchant when the payload omits one.
Please strip the `|| process.env.DEFAULT_MERCHANT` branch.
Please reject the event when merchantId is missing.
A second comment covers the success-on-failure path.
Contract violation: failure reported as success.
`{ ok: true, skipped: true }` after exhausted retries hides a capture miss.
Return or throw a delivery failure. Do not keep ok:true.
Step 4. Turn each remaining default into a failing test
A pinned default still needs a lock. The next agent pass will move the number again. Contract tests belong in the pull request, not in a later cleanup ticket.
// test/paymentRetry.contract.test.js
const test = require("node:test");
const assert = require("node:assert/strict");
const { deliverPaymentWebhook } = require("../src/webhooks/paymentRetry");
test("does not invent a merchant identity", async () => {
const gateway = {
post: async () => {
throw new Error("should not run");
},
};
await assert.rejects(
() => deliverPaymentWebhook({ body: { amount: 10 } }, gateway),
/merchantId/
);
});
test("does not report success after exhausted retries", async () => {
const gateway = {
post: async () => {
const err = new Error("timeout");
err.code = "ETIMEDOUT";
throw err;
},
};
await assert.rejects(
() =>
deliverPaymentWebhook(
{ body: { merchantId: "m_1", amount: 10 } },
gateway,
{ timeoutMs: 1000, maxRetries: 2 }
),
/DELIVERY_FAILED|webhook delivery failed/
);
});
test("does not retry non-timeout errors", async () => {
let calls = 0;
const gateway = {
post: async () => {
calls += 1;
const err = new Error("bad request");
err.status = 400;
throw err;
},
};
await assert.rejects(() =>
deliverPaymentWebhook(
{ body: { merchantId: "m_1", amount: 10 } },
gateway,
{ timeoutMs: 1000, maxRetries: 3 }
)
);
assert.equal(calls, 1);
});
Those tests run on the pull request head only.
node --test test/paymentRetry.contract.test.js
A red test here is a successful review. The agent filled a gap. The test reopened that gap before merge.
Step 5. Ask a model for counterexamples, not more code
After the scanner and tests, one optional pass remains. The reviewer sends the ticket contract and flagged hunks to a hosted coding model. The prompt asks only for inputs that should fail. It does not ask for a rewrite.
Disclosure: This article was prepared as part of MonkeyCode's product outreach.
MonkeyCode offers free model access and a free server option. Those two facts are the only product claims used here. The reviewer pastes the contract and flagged hunks there. The prompt stays narrow.
You are not allowed to edit production code.
Ticket contract:
- Retry only after a gateway timeout error.
- Do not invent timeout, retry, or identity defaults.
- Do not report success when delivery failed.
Flagged hunks:
<paste scanner output>
List 8 concrete event payloads and gateway behaviors that should be rejected.
Return JSON only: [{id, event, gatewayBehavior, expectedFailure}].
Do not propose a patch.
Model output is untrusted fixture text. Each case is copied into the contract test file. Cases the ticket does not require are discarded. Tests can run on the free server when local runners are busy.
This is the only product-shaped step in the protocol. The rest runs with git, Node, and a review form.
Step 6. Require a policy changelog in the PR body
Agents rarely write the policy they invented. The pull request body must do that writing. No changelog means no merge.
## Policy changelog
- Timeout budget: not in ticket → stripped / pinned to PAYMENT_GATEWAY_TIMEOUT_MS
- Retry ceiling: not in ticket → pinned to PAYMENT_WEBHOOK_MAX_RETRIES
- Merchant fallback: stripped
- Success-on-failure: stripped
- Contract tests added: merchant identity, timeout-only retry, failure remains failure
The next reviewer should see policy, not only files. Future agent patches should see the same list. Hidden literals then have a paper trail.
A repaired handler for the specimen
The repair is boring on purpose. Boring is the review outcome.
// src/webhooks/paymentRetry.js
function deliverPaymentWebhook(event, gateway, policy) {
if (!event?.body?.merchantId) {
const err = new Error("merchantId required");
err.code = "CONTRACT";
return Promise.reject(err);
}
if (!policy?.timeoutMs || !policy?.maxRetries) {
const err = new Error("retry policy required");
err.code = "CONTRACT";
return Promise.reject(err);
}
const payload = event.body;
let attempt = 0;
const run = () => {
attempt += 1;
return gateway
.post("/capture", payload, { timeout: policy.timeoutMs })
.then(() => ({ ok: true, attempt }))
.catch((err) => {
const timeout = err.code === "ETIMEDOUT";
if (!timeout) throw err;
if (attempt >= policy.maxRetries) {
const fail = new Error("webhook delivery failed");
fail.code = "DELIVERY_FAILED";
throw fail;
}
return run();
});
};
return run();
}
module.exports = { deliverPaymentWebhook };
The function no longer owns product numbers. The caller must pass policy. Missing policy fails closed. Missing merchant fails closed.
Non-timeout errors fail closed. Exhausted retries fail closed. The agent still wrote most of the control flow. The reviewer removed the invented law.
What this protocol will miss
Regex will not see a wrong status code mapping. Regex will not see a swapped idempotency key. Regex will not see a cache key that ignores tenant.
Semantic assumptions still need a human. The table does not replace that reading. Vague tickets also break the protocol.
If product never named the contract, strip-or-pin becomes guesswork. Work stops until a written value exists. The agent does not become the product manager.
This protocol is a review aid for humans. It is not a merge bot. Security-sensitive paths still need a dedicated review.
Teams without Node in the repo can keep the table and comments. The scanner is optional. The contract file is not.
Who should skip this
Skip this protocol on a one-line typo fix. Skip it when a human already specified every literal in the ticket. Skip it when the organization requires a formal threat model instead of a heuristic scan.
Skip the model pass when the ticket contains secrets. Production payloads do not belong in any hosted workspace. Local contract tests still apply in that case.
Closing
Agent pull requests fail in the gaps. Those gaps are policy, not style. The ticket contract is written first. Literals are scanned second. Each hit is stripped, pinned, or locked.
Optional counterexamples can come from free model access on a free server. The merge decision still belongs to the human who owns the ledger. If MonkeyCode is already in the workspace, use it only for that counterexample list and leave the rest in git comments plus contract tests.
Top comments (0)