Fail-Open Defaults in Agent PRs: A Provenance Review

The following scene is a composite review scenario.
It is not a named customer story.
A billing service received an agent pull request on Friday.

The patch added retries, caching, and a thin HTTP client.
Generated tests were green on the CI run.
The reviewer still blocked the merge that afternoon.
Three invented defaults would have hidden production failures.

Agent patches often look complete at first glance.
They compile, they format, and they add comments.
The real risk is quiet invention of runtime contracts.

This article is a provenance review for those inventions.
Silent defaults are treated as merge blockers.
Style nits wait until the default list is empty.

What fail-open invention looks like

Fail-open code turns an outage into reported success.
Agents insert it because tests stay green that way.
Reviewers should treat that pattern as a contract bug.

Invented defaults are values nobody in the repo proved.
They include guessed hosts, guessed timeouts, and guessed env keys.
They also include empty catch blocks that swallow transport errors.

A typical invented fragment looks harmless in the diff.

const baseUrl = process.env.BILLING_URL || "https://billing.internal";
const timeoutMs = Number(process.env.BILLING_TIMEOUT_MS) || 8000;

async function charge(payload) {
  try {
    return await client.post("/v1/charges", payload, {
      baseUrl,
      timeout: timeoutMs,
    });
  } catch (err) {
    console.warn("billing retry later", err.message);
    return { ok: true, deferred: true };
  }
}

The host string is invented.
The timeout number is invented.
The catch path reports success after a hard failure.
None of that should merge without a cited source.

Review sequence

Follow this sequence on contract-changing agent PRs.
Do not start with naming or import order.
The first pass is provenance, not taste.

1. Extract the new contract surface

List every new input, output, and environment key.
Put the list in the review thread, not in private chat.
Missing documentation means the contract is still unproven.

NEW CONTRACT
- env BILLING_URL
- env BILLING_TIMEOUT_MS
- POST /v1/charges
- return shape { ok, deferred? }

Commands that fill the list from a diff are enough.
They do not prove correctness.
They only stop reviewers from missing keys.

git fetch origin pull/1842/head:pr-1842
git checkout pr-1842
git diff main...HEAD -- '*.js' '*.ts' > /tmp/agent.patch
rg -n "process\.env\.\w+" /tmp/agent.patch
rg -n "\|\|\s*[\"'`]https?:" /tmp/agent.patch
rg -n "catch\s*\(" /tmp/agent.patch

If the repo has no owner for a new host, strip that hunk.
Do not leave an internal URL as a convenience fallback.
Convenience fallbacks become production dependencies overnight.

2. Classify fail-open versus fail-closed

Search the diff for swallowed errors and success flags.
Fail-open code is common in agent retries and caches.
It keeps demos alive and hides broken dependencies.

Require fail-closed behavior unless a written SLA exists.
A ticket, a runbook, or an OpenAPI file can be that SLA.
A generated comment is not an SLA.

// strip this unless a written SLA allows deferral
catch (err) {
  return { ok: true, deferred: true };
}

// keep this until product confirms a durable queue
catch (err) {
  throw new BillingUnavailableError(err);
}

Log-and-continue is still fail-open when callers see success.
console.warn does not repair a missing billing host.
The return value is the contract the rest of the system trusts.

3. Demand a source for every default

A default is allowed only with a cited source.
Acceptable sources include runbooks, OpenAPI files, and tickets.
"It seemed reasonable" is not a source.

Build a provenance table inside the review comment.
One row per default is enough.
Empty source columns mean the hunk is stripped.

| Default | Cited source | Review action |
| timeout 8000 | none | strip |
| https://billing.internal | none | strip |
| retry count 3 | runbook §4 | add fail-closed test |
| cache TTL 60s | guessed | strip |

Agent comments often claim a default "matches existing config".
Those comments are claims, not evidence.
Grep the main branch for the claimed value before keeping it.

rg -n "BILLING_TIMEOUT_MS|billing.internal|timeoutMs" $(git rev-parse main)

No hit on main means the comment is false.
False comments are a second reason to strip the default.
Leave a review note on the lying comment itself.

4. Scan the diff with a labeled heuristic

Visual review still misses fallbacks in large patches.
A short scanner can mark common invention smells.
This script is a heuristic, not a verifier.

#!/usr/bin/env node
// assumption-scan.mjs — labeled heuristic, not a proof of safety
import { readFileSync } from "node:fs";

const smells = [
  { id: "fallback-url", re: /\|\|\s*["'`]https?:\/\// },
  { id: "empty-catch", re: /catch\s*\([^)]*\)\s*\{\s*\}/ },
  { id: "ok-true", re: /\bok:\s*true\b/ },
  { id: "bare-number-or", re: /\|\|\s*\d+/ },
  { id: "env-or", re: /process\.env\.\w+\s*\|\|/ },
  { id: "any-cast", re: /as any\b|: any\b/ },
  { id: "todo-fixme", re: /\b(TODO|FIXME|HACK)\b/ },
];

const diff = readFileSync(0, "utf8");
const lines = diff.split("\n");
const hits = [];

for (let i = 0; i < lines.length; i++) {
  const line = lines[i];
  if (!line.startsWith("+") || line.startsWith("+++")) continue;
  for (const smell of smells) {
    if (smell.re.test(line)) {
      hits.push({ line: i + 1, id: smell.id, text: line.slice(0, 120) });
    }
  }
}

console.log(JSON.stringify({ hitCount: hits.length, hits }, null, 2));
process.exit(hits.length ? 2 : 0);

Run the scanner against the pull request diff only.
Do not execute the new client on the review laptop yet.
Diff text is enough for this pass.

git diff main...HEAD | node assumption-scan.mjs

A non-zero exit only means humans must classify hits.
Legitimate || 0 counters will also fire.
The reviewer still decides strip versus keep.

5. Add tests that punish the invention

Green tests from the agent are not sufficient.
Those tests often encode the same invented defaults.
They assert the fallback path, not the real contract.

Write one test that withholds the environment key.
Write one test that forces the dependency to throw.
Both tests should fail closed if the patch is honest.

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

test("charge fails closed without BILLING_URL", async () => {
  delete process.env.BILLING_URL;
  const { charge } = await import("./billing.js");
  await assert.rejects(() => charge({ cents: 100 }), /BILLING_URL/);
});

test("charge does not report ok on transport error", async () => {
  process.env.BILLING_URL = "http://127.0.0.1:9";
  process.env.BILLING_TIMEOUT_MS = "50";
  const { charge } = await import("./billing.js");
  await assert.rejects(() => charge({ cents: 100 }));
});

If those tests cannot be written, the contract is still invented.
Strip that part of the pull request.
Do not accept a skip comment as a substitute test.

6. Use a second-pass model only after the table exists

A second reader helps after the scanner, not before.
The reviewer already has a contract list and smell JSON.
A hosted coding model can group remaining hunks by provenance risk.

Disclosure: This article was prepared as part of MonkeyCode's product outreach.
MonkeyCode is an open-source coding assistant for review work.
It offers free model access and a free server option.
This workflow does not name models, quotas, or hardware.

A prompt that stays inside review artifacts looks like this.
It is a proposal, not a recorded production run.

You are a second-pass reviewer, not an author.
Input 1: contract list from the human reviewer.
Input 2: assumption-scan.mjs JSON output.
Input 3: the unified diff.
Task: group hunks into strip, demand-test, or accept.
Rules:
- Do not invent new env keys or hosts.
- Do not praise style.
- Cite a diff line for every strip decision.
- If a default has no source, classify strip.
Return JSON only.

Run that pass on the free server when local machines are busy.
Keep the model output beside the human provenance table.
The human table still wins on conflict.

Sample review comment

Paste a short, structured comment after the table is filled.
Avoid vague requests for "more tests".
Name the defaults and the required fail-closed checks.

Provenance review (not a style pass)

Strip:
- billing.js fallback URL https://billing.internal (no source on main)
- catch path returning { ok: true } (fail-open, no SLA)

Demand tests before accept:
- missing BILLING_URL must reject
- refused TCP connection must reject, not return ok

Accept only after those tests land.
Generated suite is ignored until then.

This comment is reusable across languages.
The smells change.
The provenance rule does not.

Merge rubric

Use one row per hunk, not one mood for the whole PR.
Strip hunks that invent network identity.
Demand tests for control-flow changes that already have sources.

| Hunk | Smell | Source | Fail-closed test | Action |
| client timeout | number fallback | none | missing | strip |
| error mapper | none | ticket BILL-22 | added | demand-test |
| cache wrapper | env fallback URL | none | missing | strip |
| log redaction | none | security.md | added | accept |

Accept only hunks with a cited source and a fail-closed test.
Strip hunks that mint hosts, ports, or credentials.
Demand tests when behavior changes but defaults are documented.

Limitations

The scanner is a regex heuristic over unified diffs.
It misses multi-line catch blocks and helper wrappers.
It flags some legitimate numeric fallbacks.

The workflow assumes a reviewer can name the real contract.
It fails on greenfield repos with no runbooks.
It also fails when product wants fail-open deferral and never wrote that down.

Do not treat model grouping as evidence.
Models repeat the same invention if the prompt is weak.
The free server does not replace CI, staging, or secret scanning.

Who should not use this approach

Do not use this workflow as a security review substitute.
Secret scanning and dependency review stay separate gates.
Do not use it to auto-merge agent pull requests.

Skip it for pure documentation PRs with no runtime defaults.
Skip it for one-line typo fixes with no fallbacks.
The overhead is for contract-changing agent patches.

Teams without permission to run untrusted PR code should not execute the new client.
They can still run the scanner on diff text alone.
They can still require provenance tables in review.

What the review actually changes

The Friday patch lost its invented host and success-on-error catch.
Retry logic shipped later, after two fail-closed tests.
The merge took longer than a rubber stamp.

That delay is the useful part.
Agent throughput is cheap.
Unsourced defaults are not.