Bad apply_patch error message "Invalid patch: The first line of the patch must be '*** Begin Patch'" results in agent looping

Potential duplicates detected. Please review them and close your issue if it is a duplicate.

• #34735

Powered by Codex Action

Analysis (community)

Residual analysis for #35361 (bad apply_patch “first line must be *** Begin Patch” error → agent retry loops) under docs/contributing.md — invitation-only. No unsolicited PR.

Root-cause hypothesis

The apply_patch parser enforces a strict first-line marker with only lightweight trim(), the lenient path only peels a narrow heredoc shape, and parse failures are returned via RespondToModel as plain strings — so common model mistakes (markdown fences, leading prose, missing spaces) produce a generic error that invites another nearly identical attempt.

Verified at HEAD 4c43465133:

1. Exact error site.
codex-rs/apply-patch/src/parser.rs:

• BEGIN_PATCH_MARKER = "*** Begin Patch"
• check_start_and_end_lines_strict: if first line ≠ marker →

InvalidPatchError("The first line of the patch must be '*** Begin Patch'")

• Display becomes invalid patch: … via thiserror.

Only trim on first/last lines — does not strip:

• markdown fences (` ` / `diff `)
• leading prose / explanations
• BOM / zero-width beyond trim
• ***Begin Patch (missing space) — fails equality

2. Lenient path is narrow.
check_patch_boundaries_lenient only peels <<EOF / <<'EOF' / <<"EOF" when the last line ends with EOF. Not markdown, not nested prose. So the failure mode reported in the issue (agents wrapping patches in fenced blocks or chatter) is outside the lenient envelope.

3. Retry fuel — handler surface.
codex-rs/core/src/tools/handlers/apply_patch.rs maps parse failure to
FunctionCallError::RespondToModel(format!("apply_patch verification failed: {parse_error}"))
(and a streaming finish path failed to parse apply_patch: …). That is a normal tool-error → model follow-up surface: the turn layer continues with needs_follow_up style re-entry, so the model is invited to retry without a structured “fatal envelope / abort this format” signal.

Contrast: format_exec_output_for_model always prefixes Exit code + wall time for exec failures; apply_patch parse failures are string-only and do not include a first-line preview.

4. Honest scope on “looping”.
OSS enables retry fuel; it does not implement an infinite server-side apply_patch loop. The agent-looping symptom is model policy + identical error + no progressive diagnostics. Still a high-value residual: better errors (and optional light lenience) would cut wasted turns.

Ranked fix outline (all OSS):

1. Enrich error: include escaped/truncated first line; detect ` ` fences; suggest “strip fences / ensure exact *** Begin Patch`”.
2. Optional lenient: strip a single outer markdown fence pair before the strict check.
3. Structured code e.g. apply_patch.invalid_envelope so prompts/models can switch strategy once.
4. Cap consecutive identical envelope failures per turn (session counter) to break pure retry storms.

Fail-first tests: (1) fenced patch → error mentions fence or first line; (2) ***Begin Patch (no space) → actionable hint; (3) identical envelope fail N times → stop or escalate once if (4) lands.

Happy to draft the error-string tests if useful. No unsolicited PR.

@bbingz I don't understand how stripping backticks will help, there are none in the first line reported above:

Ran apply_patch '{"command":["apply_patch","*** Begin Patch\n

I presume there is something else wrong, but I couldn't find any examples of what a valid apply_patch call should look like.

@DanTup Thanks for the careful pushback — you were right to question the backtick/fence framing for your log, and I appreciate you calling that out with the exact Ran apply_patch '…' line.

You're correct: stripping markdown backticks would not help the call you showed. There are no backticks in that payload. What matters is the leading {.

What the parser actually saw

Codex's apply_patch tool is a freeform tool: it passes the entire custom-tool input string straight into the patch parser and does not unwrap a JSON object like {"command":[...]}. The tool description also says not to wrap the patch in JSON.

In the shape from your report:

apply_patch '{"command":["apply_patch","*** Begin Patch\n*** Update File: ..."]}'

the string that reaches the first-line check begins with {, not *** Begin Patch. The marker you can see is one layer deeper — inside a nested JSON string (command[1]). So the error:

The first line of the patch must be *** Begin Patch

is technically true for the bytes received, but it is misleading for a human (and for a model): it does not say “you sent a JSON wrapper whose first character is {,” so the agent reasonably thinks it already started with the right marker and retries the same shape.

I should not have led with fence/backtick stripping for this issue; that is only a generic example of another invalid envelope, not the diagnosis of your log. Sorry for the noise there.

What a valid freeform apply_patch looks like

Send raw patch grammar only as the tool input (no JSON object, no command array, no nested apply_patch, no markdown fence around the body):

*** Begin Patch
*** Update File: .github/workflows/build-and-test.yml
@@
-old line
+new line
*** End Patch

Conceptually:

CustomToolCall {
  name: "apply_patch",
  input: "*** Begin Patch\n*** Update File: ...\n@@\n...\n*** End Patch"
}

Not:

{"command":["apply_patch","*** Begin Patch\n...\n*** End Patch"]}

(If something goes through the shell tool instead, the supported pattern is a heredoc apply_patch <<'PATCH' ... PATCH, not a quoted JSON blob as the single argument.)

Why the loop happens

There is no dedicated server-side “infinite apply_patch loop.” A failed tool result is returned to the model, which is invited to try again. With an error that does not surface the leading { / double-wrapping, a smaller model can keep resending the same nested JSON. Better diagnostics (preview first bytes; detect JSON/command nesting explicitly) would help more than fence stripping for this case.

Happy to refine further if you have a raw tool-call capture from the provider. Thanks again for the precise challenge — it fixed the attribution on this thread.

Codex's apply_patch tool is a freeform tool: it passes the entire custom-tool input string straight into the patch parser and does not unwrap a JSON object like {"command":[...]}. The tool description also says not to wrap the patch in JSON.

Ah, that sounds more likely. I wasn't sure what the format was supposed to be.

In that case, I think it would've helped the agent if:

• the error message included the first few characters of the string it got
• if the string starts with {, the message noted that it looks like JSON but should not be

I suspect if the error had been more specific like this, the model would've attempted to correct the format instead of just sending the same thing again.

Hi Codex team — I’d like to contribute a focused fix for this issue.
The proposed change keeps parsing behavior unchanged. When the trimmed first line begins with {, both the full and streaming apply_patch parsers return an actionable diagnostic explaining that the input looks JSON-wrapped and that raw patch text must be passed directly. Non-JSON errors remain unchanged.
I have regression coverage for both parser paths. The focused crate suite passes 86/86 tests, along with Clippy and formatting checks.
Would a Codex maintainer like me to submit this as a PR? If so, please explicitly invite me to open it in accordance with docs/contributing.md.