diff --git a/bunfig.toml b/bunfig.toml index 90483aef2..f28d70ba7 100644 --- a/bunfig.toml +++ b/bunfig.toml @@ -8,6 +8,7 @@ saveTextLockfile = true [loader] ".md" = "text" ".py" = "text" +".lark" = "text" [run] bun = true diff --git a/docs/apply_patch_spec.md b/docs/apply_patch_spec.md new file mode 100644 index 000000000..ba9afb785 --- /dev/null +++ b/docs/apply_patch_spec.md @@ -0,0 +1,846 @@ +# `apply_patch`: Codex Patch Format Specification + +This document is a full, reimplementation-quality specification of the `apply_patch` +tool used by the Codex coding harness. It covers: + +1. How the tool is exposed to the model (schema + freeform grammar variants). +2. The exact prompt / instruction text shown to the model. +3. The patch format grammar. +4. The parser (lexical rules, lenient mode, streaming mode, errors). +5. The application algorithm (Add / Delete / Update / Move), including the + `seek_sequence` fuzzy matcher. +6. Invocation forms the harness accepts (freeform args, JSON args, shell + heredoc wrappers, stdin). +7. Result presentation and error reporting. +8. Test-derived edge cases. + +All normative behavior below is drawn from the `codex-rs/apply-patch` crate +(the parser in `src/parser.rs`, the applier in `src/lib.rs`, the matcher in +`src/seek_sequence.rs`) and the tool registration in `codex-rs/tools`. + +--- + +## 1. Tool registration + +`apply_patch` is registered in the tool registry when the current model's +`apply_patch_tool_type` metadata is set (see +`codex-rs/models-manager/models.json` — GPT-5.x variants use `"freeform"`; +older / non-reasoning models use `"function"`). It is registered as +`supports_parallel_tool_calls = false`, meaning the harness will not issue +two concurrent `apply_patch` calls. (Registration in +`codex-rs/tools/src/tool_registry_plan.rs`.) + +There are two wire formats for the same underlying command. + +### 1.1 Freeform (GPT-5 and later) + +The freeform variant uses OpenAI's custom-tool mechanism: the model emits a +single opaque string whose shape is constrained by a Lark grammar. + +```jsonc +{ + "type": "custom", + "name": "apply_patch", + "description": "Use the `apply_patch` tool to edit files. This is a FREEFORM tool, so do not wrap the patch in JSON.", + "format": { + "type": "grammar", + "syntax": "lark", + "definition": "" + } +} +``` + +(See `codex-rs/tools/src/apply_patch_tool.rs::create_apply_patch_freeform_tool`.) + +The freeform call payload is the patch text itself — no JSON envelope, no +wrapping quotes. Example (the model types this verbatim as the tool's +"input"): + +``` +*** Begin Patch +*** Add File: hello.txt ++Hello, world! +*** End Patch +``` + +### 1.2 JSON function (legacy / gpt-oss) + +For providers that only support function-style tool calls, a JSON variant is +registered: + +```jsonc +{ + "type": "function", + "name": "apply_patch", + "description": "", + "strict": false, + "parameters": { + "type": "object", + "additionalProperties": false, + "required": ["input"], + "properties": { + "input": { + "type": "string", + "description": "The entire contents of the apply_patch command" + } + } + } +} +``` + +(See `codex-rs/tools/src/apply_patch_tool.rs::create_apply_patch_json_tool`.) + +### 1.3 Handler dispatch + +Both variants land in the same handler, which normalizes the arguments into +`ApplyPatchToolArgs { input: String }` and passes `input` to +`apply_patch::apply_patch(...)` (see +`codex-rs/core/src/tools/handlers/apply_patch.rs`). The `input` is the full +patch text, including the `*** Begin Patch` / `*** End Patch` envelope. + +--- + +## 2. Agent prompt (verbatim) + +The model is taught the format via two equivalent pieces of text: the +Markdown file `codex-rs/apply-patch/apply_patch_tool_instructions.md` +(embedded in system prompts), and the string constant +`APPLY_PATCH_JSON_TOOL_DESCRIPTION` in +`codex-rs/tools/src/apply_patch_tool.rs` which is used as the JSON tool's +`description`. They are identical in content. + +A reimplementation SHOULD ship the following text verbatim (note: the +original uses Unicode curly quotes in a few places — they are reproduced +here; the final "smart quote" versus "typewriter quote" choice is not +semantic): + +````markdown +## `apply_patch` + +Use the `apply_patch` shell command to edit files. +Your patch language is a stripped‑down, file‑oriented diff format designed to be easy to parse and safe to apply. You can think of it as a high‑level envelope: + +*** Begin Patch +[ one or more file sections ] +*** End Patch + +Within that envelope, you get a sequence of file operations. +You MUST include a header to specify the action you are taking. +Each operation starts with one of three headers: + +*** Add File: - create a new file. Every following line is a + line (the initial contents). +*** Delete File: - remove an existing file. Nothing follows. +*** Update File: - patch an existing file in place (optionally with a rename). + +May be immediately followed by *** Move to: if you want to rename the file. +Then one or more "hunks", each introduced by @@ (optionally followed by a hunk header). +Within a hunk each line starts with: + +For instructions on [context_before] and [context_after]: +- By default, show 3 lines of code immediately above and 3 lines immediately below each change. If a change is within 3 lines of a previous change, do NOT duplicate the first change's [context_after] lines in the second change's [context_before] lines. +- If 3 lines of context is insufficient to uniquely identify the snippet of code within the file, use the @@ operator to indicate the class or function to which the snippet belongs. For instance, we might have: +@@ class BaseClass +[3 lines of pre-context] +- [old_code] ++ [new_code] +[3 lines of post-context] + +- If a code block is repeated so many times in a class or function such that even a single `@@` statement and 3 lines of context cannot uniquely identify the snippet of code, you can use multiple `@@` statements to jump to the right context. For instance: + +@@ class BaseClass +@@ def method(): +[3 lines of pre-context] +- [old_code] ++ [new_code] +[3 lines of post-context] + +The full grammar definition is below: +Patch := Begin { FileOp } End +Begin := "*** Begin Patch" NEWLINE +End := "*** End Patch" NEWLINE +FileOp := AddFile | DeleteFile | UpdateFile +AddFile := "*** Add File: " path NEWLINE { "+" line NEWLINE } +DeleteFile := "*** Delete File: " path NEWLINE +UpdateFile := "*** Update File: " path NEWLINE [ MoveTo ] { Hunk } +MoveTo := "*** Move to: " newPath NEWLINE +Hunk := "@@" [ header ] NEWLINE { HunkLine } [ "*** End of File" NEWLINE ] +HunkLine := (" " | "-" | "+") text NEWLINE + +A full patch can combine several operations: + +*** Begin Patch +*** Add File: hello.txt ++Hello world +*** Update File: src/app.py +*** Move to: src/main.py +@@ def greet(): +-print("Hi") ++print("Hello, world!") +*** Delete File: obsolete.txt +*** End Patch + +It is important to remember: + +- You must include a header with your intended action (Add/Delete/Update) +- You must prefix new lines with `+` even when creating a new file +- File references can only be relative, NEVER ABSOLUTE. + +You can invoke apply_patch like: + +``` +shell {"command":["apply_patch","*** Begin Patch\n*** Add File: hello.txt\n+Hello, world!\n*** End Patch\n"]} +``` +```` + +Notes for implementers: + +- The "You can invoke apply_patch like ..." footer is only shown when the + patch command is exposed as a `shell` command (the `codex` fallback path). + When the freeform tool is registered, the model invokes the tool directly + and the footer is redundant but harmless. +- The prompt asserts some things the parser is lenient about in practice: + - *"File references can only be relative"* — the parser accepts absolute + paths, but the model is instructed to produce relative ones. + - *"+ line even when creating a new file"* — `+` MUST be the first + character of every content line in an Add File section. + +--- + +## 3. Patch grammar + +### 3.1 EBNF (canonical) + +``` +Patch := Begin { FileOp }+ End +Begin := "*** Begin Patch" NEWLINE +End := "*** End Patch" [NEWLINE] + +FileOp := AddFile | DeleteFile | UpdateFile +AddFile := "*** Add File: " path NEWLINE { "+" text NEWLINE }+ +DeleteFile := "*** Delete File: " path NEWLINE +UpdateFile := "*** Update File: " path NEWLINE + [ "*** Move to: " path NEWLINE ] + Change? + +Change := (ChangeContext | ChangeLine)+ EofLine? +ChangeContext := ("@@" | "@@ " text) NEWLINE +ChangeLine := (" " | "-" | "+") text NEWLINE +EofLine := "*** End of File" NEWLINE +``` + +The Lark grammar that the OpenAI freeform tool uses to constrain model +output is in `codex-rs/tools/src/tool_apply_patch.lark`: + +```lark +start: begin_patch hunk+ end_patch +begin_patch: "*** Begin Patch" LF +end_patch: "*** End Patch" LF? + +hunk: add_hunk | delete_hunk | update_hunk +add_hunk: "*** Add File: " filename LF add_line+ +delete_hunk: "*** Delete File: " filename LF +update_hunk: "*** Update File: " filename LF change_move? change? + +filename: /(.+)/ +add_line: "+" /(.*)/ LF -> line + +change_move: "*** Move to: " filename LF +change: (change_context | change_line)+ eof_line? +change_context: ("@@" | "@@ " /(.+)/) LF +change_line: ("+" | "-" | " ") /(.*)/ LF +eof_line: "*** End of File" LF + +%import common.LF +``` + +### 3.2 Reserved tokens + +| Token | Meaning | +| ---------------------------- | --------------------------------------------------------------- | +| `*** Begin Patch` | Required first significant line of the envelope. | +| `*** End Patch` | Required last significant line (trailing LF optional). | +| `*** Add File: ` | Start of an Add File section. | +| `*** Delete File: ` | Standalone Delete File directive. | +| `*** Update File: ` | Start of an Update File section. | +| `*** Move to: ` | Optional rename target, immediately after `*** Update File:`. | +| `@@` or `@@
` | Starts a chunk inside an Update File. | +| `*** End of File` | Terminates a chunk; asserts the chunk ended at EOF. | +| `+` / `-` / ` ` | Added / deleted / context line inside a chunk. | + +Action headers match **with a trailing space**: the parser uses literal +`strip_prefix("*** Add File: ")` etc. Everything after the space is the +path; no escaping, no quoting. + +### 3.3 Lines and newlines + +- Input is split on `\n` (LF only). CRLF is not supported by the parser — + producers MUST use LF. (Tool output goes through Rust string handling which + preserves CRs as literal bytes on the content lines, which then fail to + match.) +- Each content line in a hunk starts with exactly one byte (`+`, `-`, or + space) followed by the line's text and then a newline. An empty ` `-prefixed + line is representable as a single space followed by LF; a bare `+\n` is a + one-character added empty line. +- A completely blank line (no prefix byte at all) inside an Update File + section is **skipped**, and is used only for visual separation between + chunks. This is a deliberate leniency; do not rely on blank lines to carry + data. + +--- + +## 4. Parser + +Implementation: `codex-rs/apply-patch/src/parser.rs`. + +### 4.1 Public entry points + +- `parse_patch(text: &str) -> Result` — + production parse. Uses `ParseMode::Lenient` (see §4.3). +- `parse_patch_streaming(text: &str) -> Result` — + same format, but tolerates a missing `*** End Patch` (the harness calls + this from a streaming response handler to show progress). Its output + MUST NOT be used to actually apply the patch. + +The result is: + +```rust +pub struct ApplyPatchArgs { + pub patch: String, // canonicalized patch text (heredoc stripped) + pub hunks: Vec, + pub workdir: Option, // populated only when parsing a shell + // invocation that begins with `cd &&` +} +``` + +### 4.2 Lexical canonicalization + +Before parsing, the input is `trim()`ed, then split into lines by `\n`. +Marker-line matching is done against `line.trim()`, so arbitrary +leading/trailing whitespace around the sentinel lines (`*** Begin Patch`, +etc.) is accepted. Content lines (those starting with `+`, `-`, ` `) are +NOT trimmed — their whitespace is significant. + +### 4.3 Parse modes + +The parser operates in one of three modes: + +1. **Strict** — requires line 0 = `*** Begin Patch` and the last line to be + `*** End Patch`. Not used by the harness today (`PARSE_IN_STRICT_MODE = + false`); retained as a fallback. + +2. **Lenient** (default). Tries strict first. If that fails, attempts to + strip a heredoc wrapper: the first line must be exactly one of + `< parse AddFile + "*** Delete File: " -> parse DeleteFile + "*** Update File: " -> parse UpdateFile + otherwise -> InvalidHunkError at line i +``` + +**AddFile** consumes subsequent lines while they start with `+`. Each line +becomes one element of `contents`, with the leading `+` stripped. The lines +are joined with `\n`; an additional `\n` is appended after the join so the +resulting file ends with exactly one newline. + +**DeleteFile** consumes only the header line; no content follows. + +**UpdateFile** consumes the header, optionally a `*** Move to: ` line, +then zero or more chunks. The UpdateFile section ends when the next line +starts with `***` (the next hunk header, or `*** End Patch`) or input is +exhausted. An UpdateFile section with zero chunks is an error +(`"Update file hunk for path '

' is empty"`). + +**Chunk** parsing within an UpdateFile: + +``` +chunk: + optional context line: + "@@" -> change_context = None (empty marker) + "@@

" -> change_context = Some("
") + otherwise -> if this is the FIRST chunk of the hunk, fall through + (context-less first chunk); otherwise error. + then: + loop over lines: + if line starts with "*" -> stop (end of chunk; next hunk or end) + if line == "" -> append empty string to BOTH old_lines + and new_lines (empty context line) + if line starts with ' ' -> append line[1..] to BOTH old_lines and + new_lines + if line starts with '-' -> append line[1..] to old_lines + if line starts with '+' -> append line[1..] to new_lines + if line == "*** End of File" -> set is_end_of_file = true, stop + otherwise -> error: unexpected line in update hunk +``` + +Invariants enforced by the parser: + +- A chunk must contain at least one non-context line (pure context chunks + are rejected). +- `*** End of File` cannot be the first line of a chunk. +- Chunks are stored in order and the applier relies on each chunk's match + position being ≥ the previous chunk's match position. + +### 4.5 Error taxonomy + +```rust +pub enum ParseError { + InvalidPatchError(String), // envelope errors + InvalidHunkError { message: String, line_number: usize }, // content errors +} +``` + +Specific messages (implementers SHOULD match these literally so tests and +downstream tools keep working): + +| Error | Condition | +| ------------------------------------ | ---------------------------------------------------------------------- | +| `The first line of the patch must be '*** Begin Patch'` | Missing/wrong `Begin` marker. | +| `The last line of the patch must be '*** End Patch'` | Missing/wrong `End` marker (strict/lenient only). | +| `'' is not a valid hunk header. Valid hunk headers: '*** Add File: {path}', '*** Delete File: {path}', '*** Update File: {path}'` | Unknown file-level directive. | +| `Update file hunk for path '

' is empty` | UpdateFile section has zero chunks. | +| `Expected update hunk to start with a @@ context marker, got: ''` | Missing `@@` when required (non-first chunk). | +| `Update hunk does not contain any lines` | Chunk with a context but no +/-/space line before EOF/next chunk. | +| `Unexpected line found in update hunk: ''. Every line should start with ' ' (context line), '+' (added line), or '-' (removed line)` | Invalid diff prefix. | + +--- + +## 5. Intermediate representation + +```rust +pub enum Hunk { + AddFile { path: PathBuf, contents: String }, + DeleteFile { path: PathBuf }, + UpdateFile { + path: PathBuf, + move_path: Option, + chunks: Vec, // MUST be non-empty + }, +} + +pub struct UpdateFileChunk { + pub change_context: Option, // text after "@@ "; None for bare "@@" + pub old_lines: Vec, // lines to match in the file + pub new_lines: Vec, // replacement lines + pub is_end_of_file: bool, // "*** End of File" present +} +``` + +Paths are stored exactly as the patch wrote them — no canonicalization. +Resolution to an absolute path happens at apply time via +`AbsolutePathBuf::resolve_path_against_base(path, cwd)`. + +--- + +## 6. Application algorithm + +Implementation: `codex-rs/apply-patch/src/lib.rs`. + +### 6.1 Top-level flow + +``` +apply_patch(input, cwd, fs, sandbox): + args := parse_patch(input)? + if args.hunks is empty: error "No files were modified." + affected := { added: [], modified: [], deleted: [] } + for each hunk in args.hunks: + apply_hunk(hunk, cwd, fs, sandbox, affected)? + return affected +``` + +Hunks are applied **in the order they appear in the patch** and **not +atomically**: if hunk N fails, hunks `0..N-1` have already written to disk +and are not rolled back. Hunks `N+1..` are skipped. (Test scenario +`015_failure_after_partial_success_leaves_changes` pins this behavior.) + +A reimplementation MAY add transactional semantics, but MUST document the +deviation — callers today rely on partial application being observable. + +### 6.2 Add File + +``` +path_abs := resolve_path_against_base(hunk.path, cwd) +try: fs.write_file(path_abs, contents) + on NotFound: fs.create_directory(parent(path_abs), { recursive: true }) + fs.write_file(path_abs, contents) +append hunk.path to affected.added +``` + +Characteristics: + +- Parent directories are created on demand (recursive). This is done lazily + — only after a first write fails with `NotFound`. +- An existing file at `path_abs` is **silently overwritten** (scenario + `011_add_overwrites_existing_file`). No confirmation, no diff. +- `contents` is the literal joined string from the parser — the parser + already terminates it with `\n`. + +### 6.3 Delete File + +``` +path_abs := resolve_path_against_base(hunk.path, cwd) +meta := fs.get_metadata(path_abs) +if meta.is_directory: error "path is a directory" +fs.remove(path_abs, { recursive: false, force: false }) +append hunk.path to affected.deleted +``` + +- If the file does not exist, the metadata call returns `NotFound` and the + whole `apply_patch` fails with `Failed to delete file

: ...`. +- Deleting directories is explicitly rejected. + +### 6.4 Update File + +``` +applied := derive_new_contents_from_chunks(path_abs, hunk.chunks, fs)? +if hunk.move_path.is_some(): + dest_abs := resolve_path_against_base(move_path, cwd) + write_with_missing_parent_retry(dest_abs, applied.new_contents) + ensure source isn't a directory; fs.remove(path_abs) +else: + fs.write_file(path_abs, applied.new_contents) +append hunk.path to affected.modified +``` + +Note: a renamed file is reported as **modified** (`M`) with the original +path, not as `D` + `A`. This is intentional — it mirrors git's rename +detection. + +`derive_new_contents_from_chunks`: + +``` +text := fs.read_file_text(path_abs) // utf-8, error if missing +lines := text.split('\n') // retains trailing '' if text ends '\n' +if lines.last() == "": lines.pop() // normalize off trailing-\n artifact + +replacements := compute_replacements(lines, chunks)? +new_lines := apply_replacements(lines, replacements) +if new_lines.last() != "": new_lines.push("") // re-add trailing newline +return new_lines.join('\n') +``` + +Postcondition: **every update produces a file ending in exactly one `\n`**, +regardless of whether the input had one. Reimplementations MAY choose to +preserve "no trailing newline" when present — doing so is a deliberate +deviation. + +### 6.5 Computing replacements + +``` +line_index := 0 +replacements := [] +for each chunk: + # 1. If the chunk has a "@@ ctx" marker, locate that line first. + if chunk.change_context.is_some(): + idx := seek_sequence([ctx], lines, start=line_index, eof=false)? + line_index := idx + 1 # search for old_lines AFTER ctx + + # 2. Pure-addition chunks (old_lines empty): + if chunk.old_lines.is_empty(): + insertion_idx := (if lines.last() == "" then lines.len() - 1 + else lines.len()) + replacements.push((insertion_idx, 0, chunk.new_lines)) + continue + + # 3. Normal replacement: match old_lines in the file. + pattern := chunk.old_lines + new_slc := chunk.new_lines + found := seek_sequence(lines, pattern, line_index, chunk.is_end_of_file) + if found.is_none() and pattern.last() == "": + pattern := pattern[..pattern.len()-1] # drop trailing empty sentinel + if new_slc.last() == "": + new_slc := new_slc[..new_slc.len()-1] + found := seek_sequence(lines, pattern, line_index, chunk.is_end_of_file) + match found: + Some(start): replacements.push((start, pattern.len(), new_slc)) + line_index := start + pattern.len() + None: error "Failed to find expected lines in :\n" + +replacements.sort_by_start_index() +``` + +Key invariants: + +- Context (`@@ ctx`) is matched by *one line*; it is a locator only and is + never modified. After a successful context match, the old_lines search + begins **immediately after** the context line. +- A chunk's `old_lines` search starts at `line_index` (the cursor after the + previous chunk), so chunks must appear in file order. +- Pure-addition chunks (no `-` or ` ` lines, only `+`) append at the end of + the file. They do NOT honor `line_index`; they always go to the end. +- The "trailing empty sentinel" retry exists because unified-diff-style + tools often emit a blank line at the end of `old_lines` representing the + file's terminal newline. Our line-splitting strips that element from the + file, so a literal match fails; we retry with the sentinel removed. + +Replacements are then applied **in reverse order of `start_index`** so +earlier edits do not shift later edits' indices: + +``` +apply_replacements(lines, replacements): + for (start, old_len, new_seg) in replacements.reversed(): + delete lines[start .. start + old_len] + insert new_seg at position start +``` + +### 6.6 `seek_sequence` — the fuzzy matcher + +Implementation: `codex-rs/apply-patch/src/seek_sequence.rs`. + +Signature: + +```rust +fn seek_sequence( + lines: &[String], pattern: &[String], + start: usize, eof: bool, +) -> Option +``` + +Contract: + +- Returns the smallest `i ≥ search_start` such that + `lines[i..i + pattern.len()]` matches `pattern` under one of four match + predicates (tried in order). `search_start = lines.len() - pattern.len()` + if `eof` and the pattern fits, else `start`. +- Empty `pattern` → `Some(start)`. +- `pattern.len() > lines.len()` → `None` (MUST NOT panic). + +The four match predicates, tried in order (first success wins): + +1. **Exact**. `lines[i + k] == pattern[k]` for all k. +2. **Rstrip**. `lines[i + k].trim_end() == pattern[k].trim_end()`. +3. **Full trim**. `lines[i + k].trim() == pattern[k].trim()`. +4. **Unicode-normalized trim**. Trim, then fold common typographic + punctuation to ASCII, then compare. + +Normalization table (MUST be implemented identically): + +| Folded to | Source code points | +| --------- | ------------------ | +| `-` | U+2010 HYPHEN, U+2011 NON-BREAKING HYPHEN, U+2012 FIGURE DASH, U+2013 EN DASH, U+2014 EM DASH, U+2015 HORIZONTAL BAR, U+2212 MINUS SIGN | +| `'` | U+2018, U+2019, U+201A, U+201B | +| `"` | U+201C, U+201D, U+201E, U+201F | +| ` ` (space) | U+00A0 NBSP, U+2002, U+2003, U+2004, U+2005, U+2006, U+2007, U+2008, U+2009, U+200A, U+202F, U+205F, U+3000 | + +All other code points are passed through unchanged. This lets the model +emit ASCII hyphens/quotes/spaces even when the source file contains +typographic variants (e.g. an em-dash pasted from a doc). + +**Per-chunk EOF hint.** When `eof == true` (set from `is_end_of_file`), the +matcher first tries to match at the tail of the file +(`i = lines.len() - pattern.len()`) before falling through to the normal +forward search from `start`. + +**What's deliberately not supported.** + +- No "floating" / best-effort match. If all four passes fail, the chunk + fails; there is no nearest-match heuristic. +- No matching across non-adjacent lines — the pattern must appear as a + contiguous block. +- No multi-match disambiguation: the **first** match wins. Chunks must + carry enough context (or a `@@ header`) that the first hit at or after + `line_index` is the intended one. + +--- + +## 7. Path resolution + +- `cwd` is the `AbsolutePathBuf` passed to `apply_patch(...)`. In the CLI / + harness it defaults to the process working directory, possibly further + qualified by a workdir extracted from a `cd X && apply_patch < && apply_patch <<'EOF' ... EOF` +shell invocation (parsed via Tree-sitter in +`codex-rs/apply-patch/src/invocation.rs`). The `` is extracted into +`ApplyPatchArgs.workdir` and used to qualify `cwd` before applying hunks. +Any other pre- or post-commands cause the parse to fail over to "treat as +a regular shell command" rather than `apply_patch`. + +### 8.4 stdin (standalone executable) + +`codex-rs/apply-patch/src/standalone_executable.rs` lets the binary be +invoked as `apply_patch` with the patch on argv[1], OR with no args and the +patch piped on stdin. + +--- + +## 9. Result presentation + +### 9.1 Success + +After a successful apply, the caller renders a git-style summary +(`codex-rs/apply-patch/src/lib.rs::print_summary`): + +``` +Success. Updated the following files: +A +A +M +D +``` + +- Sections appear in the order Added / Modified / Deleted. +- Paths are the ones spelled in the patch (not canonicalized). +- Renamed files appear under `M` with the **original** path, not the + destination. +- Exit status 0. + +### 9.2 Failure + +- Parse errors: written to stderr as + `Invalid patch: ` or + `Invalid patch hunk on line : `. +- Apply errors (context miss / old_lines miss / IO): written to stderr + with the Rust `anyhow` chain, e.g.: + - `Failed to find context '' in ` + - `Failed to find expected lines in :\n` + - `Failed to read file to update : ` + - `Failed to write file : ` + - `Failed to delete file : ` + - `Failed to remove original : ` + - `Failed to create parent directories for : ` +- Exit status 1 (apply/parse failure) or 2 (argv usage error). + +### 9.3 Harness-side tool call result + +When invoked through the harness, the handler wraps the above in +`ExecToolCallOutput { exit_code, stdout, stderr, aggregated_output, duration, +timed_out }`. The model sees `aggregated_output`. + +### 9.4 Progress events + +The `PatchApplyUpdatedEvent` is emitted to the TUI as each hunk is applied +(only when the progress feature is on). This is a UX detail and is not part +of the observable patch semantics. + +--- + +## 10. Edge cases (test-derived) + +| Case | Behavior | +| --------------------------------------------------- | -------- | +| Patch with zero hunks | Error: `No files were modified.` | +| Add File overwriting an existing file | Silent overwrite. | +| Delete File on a directory | Error: `path is a directory`. | +| Delete File on a nonexistent file | Error propagated from `fs.get_metadata` / `fs.remove`. | +| Move to an existing destination | Destination overwritten; source removed. | +| Update File with 0 chunks | Parse error: `Update file hunk for path '

' is empty`. | +| Chunk with only `+` lines (pure addition) | Inserts at end of file (before final empty line if any). | +| Chunk whose `old_lines` end in an empty string | Retry without the trailing empty; lets EOF edits match. | +| Patch with `*** End of File` marker | `is_end_of_file = true`; matcher tries tail-of-file first. | +| Unicode dash/quote/NBSP mismatch between patch and file | Normalized-trim match (4th seek pass) matches. | +| Leading/trailing whitespace on a sentinel line | Ignored (`line.trim()` before marker compare). | +| Blank line inside an Update File between chunks | Ignored (used as visual separator). | +| Heredoc wrapper around the whole patch | Stripped in Lenient mode. | +| Streaming: `*** End Patch` absent yet | OK in `parse_patch_streaming`; last incomplete hunk is dropped. | +| First chunk of an Update File lacks `@@` | Allowed (context-less first chunk). | +| Non-first chunk missing `@@` | Parse error. | +| Absolute path in patch | Accepted by parser; model is told not to emit these. | +| Multiple chunks touching the same file | Applied in reverse start-order; must be in file-order in the patch. | +| One hunk of N fails | Prior hunks remain applied; later hunks skipped. | +| File with no trailing newline as input | Output gains one (post-condition). | + +--- + +## 11. Reimplementation checklist + +To reimplement this format end-to-end, a conforming implementation MUST: + +- [ ] Accept the exact sentinel tokens in §3.2 with the trailing space + where required; match marker lines after `trim()` only. +- [ ] Parse the grammar in §3.1 including the context-less first chunk + allowance, the `*** End of File` terminator, blank-line separation + between chunks, and the `*** Move to:` renames. +- [ ] Implement Lenient mode (heredoc strip) and Streaming mode as + described in §4.3. +- [ ] Emit the error messages in §4.5 verbatim (test compatibility). +- [ ] For Update File, read the target with UTF-8, split on `\n`, drop the + trailing empty element, apply chunks via the replacement machinery in + §6.5, and re-add a trailing newline before writing. +- [ ] Implement `seek_sequence` with the four-pass strictness hierarchy and + the exact Unicode normalization table in §6.6, including the + pattern-longer-than-input → `None` guard and the `eof` tail-first + search. +- [ ] Apply hunks sequentially and non-atomically; do not rollback on + partial failure. +- [ ] Silently overwrite existing destinations for Add File and Move. +- [ ] Emit the `Success. Updated the following files:` / `A`/`M`/`D` + summary in §9.1 on success, and stderr messages in §9.2 on failure. +- [ ] Register both freeform-grammar and JSON-function tool variants with + `supports_parallel_tool_calls = false`. +- [ ] Ship the agent prompt in §2 verbatim. + +Optional / harness features (not required for correctness): + +- Heredoc `cd

&& apply_patch <<'EOF' ... EOF` shell-form detection + with workdir extraction. +- Streaming progress events to the UI. +- Unified-diff rendering (`unified_diff_from_chunks`) for displaying a + user-visible diff after apply. diff --git a/packages/agent/src/agent-loop.ts b/packages/agent/src/agent-loop.ts index c1d88e74a..b3d396ed3 100644 --- a/packages/agent/src/agent-loop.ts +++ b/packages/agent/src/agent-loop.ts @@ -454,7 +454,13 @@ async function executeToolCalls( const records = toolCalls.map(toolCall => ({ toolCall, - tool: tools?.find(t => t.name === toolCall.name), + // Tools emitted via OpenAI's custom-tool path (e.g. `apply_patch` on GPT-5) + // come back under their wire-level name, which may differ from the + // harness-internal `name`. Match on either, preferring `name` for + // determinism if both somehow collide. + tool: + tools?.find(t => t.name === toolCall.name) ?? + tools?.find(t => t.customWireName !== undefined && t.customWireName === toolCall.name), args: toolCall.arguments as Record, started: false, result: undefined as AgentToolResult | undefined, diff --git a/packages/ai/src/models.json b/packages/ai/src/models.json index d18247c24..a0c2097cb 100644 --- a/packages/ai/src/models.json +++ b/packages/ai/src/models.json @@ -32460,7 +32460,8 @@ "mode": "effort", "minLevel": "minimal", "maxLevel": "high" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5-chat-latest": { "id": "gpt-5-chat-latest", @@ -32480,7 +32481,8 @@ "cacheWrite": 0 }, "contextWindow": 128000, - "maxTokens": 16384 + "maxTokens": 16384, + "applyPatchToolType": "freeform" }, "gpt-5-codex": { "id": "gpt-5-codex", @@ -32505,7 +32507,8 @@ "mode": "effort", "minLevel": "minimal", "maxLevel": "high" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5-mini": { "id": "gpt-5-mini", @@ -32530,7 +32533,8 @@ "mode": "effort", "minLevel": "minimal", "maxLevel": "high" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5-nano": { "id": "gpt-5-nano", @@ -32555,7 +32559,8 @@ "mode": "effort", "minLevel": "minimal", "maxLevel": "high" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5-pro": { "id": "gpt-5-pro", @@ -32580,7 +32585,8 @@ "mode": "effort", "minLevel": "minimal", "maxLevel": "high" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.1": { "id": "gpt-5.1", @@ -32605,7 +32611,8 @@ "mode": "effort", "minLevel": "minimal", "maxLevel": "high" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.1-chat-latest": { "id": "gpt-5.1-chat-latest", @@ -32630,7 +32637,8 @@ "mode": "effort", "minLevel": "minimal", "maxLevel": "high" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.1-codex": { "id": "gpt-5.1-codex", @@ -32655,7 +32663,8 @@ "mode": "effort", "minLevel": "minimal", "maxLevel": "high" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.1-codex-max": { "id": "gpt-5.1-codex-max", @@ -32680,7 +32689,8 @@ "mode": "effort", "minLevel": "minimal", "maxLevel": "high" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.1-codex-mini": { "id": "gpt-5.1-codex-mini", @@ -32705,7 +32715,8 @@ "mode": "effort", "minLevel": "medium", "maxLevel": "high" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.2": { "id": "gpt-5.2", @@ -32730,7 +32741,8 @@ "mode": "effort", "minLevel": "low", "maxLevel": "xhigh" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.2-chat-latest": { "id": "gpt-5.2-chat-latest", @@ -32755,7 +32767,8 @@ "mode": "effort", "minLevel": "low", "maxLevel": "xhigh" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.2-codex": { "id": "gpt-5.2-codex", @@ -32780,7 +32793,8 @@ "mode": "effort", "minLevel": "low", "maxLevel": "xhigh" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.2-pro": { "id": "gpt-5.2-pro", @@ -32805,7 +32819,8 @@ "mode": "effort", "minLevel": "low", "maxLevel": "xhigh" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.3-chat-latest": { "id": "gpt-5.3-chat-latest", @@ -32825,7 +32840,8 @@ "cacheWrite": 0 }, "contextWindow": 128000, - "maxTokens": 16384 + "maxTokens": 16384, + "applyPatchToolType": "freeform" }, "gpt-5.3-codex": { "id": "gpt-5.3-codex", @@ -32850,7 +32866,8 @@ "mode": "effort", "minLevel": "low", "maxLevel": "xhigh" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.3-codex-spark": { "id": "gpt-5.3-codex-spark", @@ -32876,7 +32893,8 @@ "minLevel": "low", "maxLevel": "xhigh" }, - "contextPromotionTarget": "openai/gpt-5.3-codex" + "contextPromotionTarget": "openai/gpt-5.3-codex", + "applyPatchToolType": "freeform" }, "gpt-5.4": { "id": "gpt-5.4", @@ -32901,7 +32919,8 @@ "mode": "effort", "minLevel": "low", "maxLevel": "xhigh" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.4-mini": { "id": "gpt-5.4-mini", @@ -32926,7 +32945,8 @@ "mode": "effort", "minLevel": "low", "maxLevel": "xhigh" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.4-nano": { "id": "gpt-5.4-nano", @@ -32951,7 +32971,8 @@ "mode": "effort", "minLevel": "low", "maxLevel": "xhigh" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.4-pro": { "id": "gpt-5.4-pro", @@ -32976,7 +32997,8 @@ "mode": "effort", "minLevel": "low", "maxLevel": "xhigh" - } + }, + "applyPatchToolType": "freeform" }, "o1": { "id": "o1", @@ -33231,7 +33253,8 @@ "mode": "effort", "minLevel": "minimal", "maxLevel": "high" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5-codex": { "id": "gpt-5-codex", @@ -33258,7 +33281,8 @@ "mode": "effort", "minLevel": "minimal", "maxLevel": "high" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5-codex-mini": { "id": "gpt-5-codex-mini", @@ -33285,7 +33309,8 @@ "mode": "effort", "minLevel": "minimal", "maxLevel": "high" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.1": { "id": "gpt-5.1", @@ -33312,7 +33337,8 @@ "mode": "effort", "minLevel": "minimal", "maxLevel": "high" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.1-codex": { "id": "gpt-5.1-codex", @@ -33339,7 +33365,8 @@ "mode": "effort", "minLevel": "minimal", "maxLevel": "high" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.1-codex-max": { "id": "gpt-5.1-codex-max", @@ -33366,7 +33393,8 @@ "mode": "effort", "minLevel": "minimal", "maxLevel": "high" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.1-codex-mini": { "id": "gpt-5.1-codex-mini", @@ -33393,7 +33421,8 @@ "mode": "effort", "minLevel": "medium", "maxLevel": "high" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.2": { "id": "gpt-5.2", @@ -33420,7 +33449,8 @@ "mode": "effort", "minLevel": "low", "maxLevel": "xhigh" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.2-codex": { "id": "gpt-5.2-codex", @@ -33447,7 +33477,8 @@ "mode": "effort", "minLevel": "low", "maxLevel": "xhigh" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.3-codex": { "id": "gpt-5.3-codex", @@ -33474,7 +33505,8 @@ "mode": "effort", "minLevel": "low", "maxLevel": "xhigh" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.3-codex-spark": { "id": "gpt-5.3-codex-spark", @@ -33500,7 +33532,8 @@ "mode": "effort", "minLevel": "low", "maxLevel": "xhigh" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.4": { "id": "gpt-5.4", @@ -33527,7 +33560,8 @@ "mode": "effort", "minLevel": "low", "maxLevel": "xhigh" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.4-mini": { "id": "gpt-5.4-mini", @@ -33554,7 +33588,8 @@ "mode": "effort", "minLevel": "low", "maxLevel": "xhigh" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.4-nano": { "id": "gpt-5.4-nano", @@ -33581,7 +33616,8 @@ "mode": "effort", "minLevel": "low", "maxLevel": "xhigh" - } + }, + "applyPatchToolType": "freeform" }, "gpt-5.5": { "id": "gpt-5.5", @@ -33608,7 +33644,8 @@ "mode": "effort", "minLevel": "low", "maxLevel": "xhigh" - } + }, + "applyPatchToolType": "freeform" } }, "opencode": { @@ -51549,4 +51586,4 @@ } } } -} \ No newline at end of file +} diff --git a/packages/ai/src/providers/openai-codex-responses.ts b/packages/ai/src/providers/openai-codex-responses.ts index c9e58f3d4..cf2b9a9d9 100644 --- a/packages/ai/src/providers/openai-codex-responses.ts +++ b/packages/ai/src/providers/openai-codex-responses.ts @@ -2,6 +2,7 @@ import * as os from "node:os"; import { $env, $flag, abortableSleep, asRecord, logger, readSseJson, structuredCloneJSON } from "@oh-my-pi/pi-utils"; import type OpenAI from "openai"; import type { + ResponseCustomToolCall, ResponseFunctionToolCall, ResponseInput, ResponseInputContent, @@ -112,7 +113,7 @@ const CODEX_WEBSOCKET_FATAL_PATTERNS = ["websocket error:", "websocket closed be const CODEX_RATE_LIMIT_BUDGET_MS = 5 * 60 * 1000; type CodexTransport = "sse" | "websocket"; -type CodexEventItem = ResponseReasoningItem | ResponseOutputMessage | ResponseFunctionToolCall; +type CodexEventItem = ResponseReasoningItem | ResponseOutputMessage | ResponseFunctionToolCall | ResponseCustomToolCall; type CodexOutputBlock = ThinkingContent | TextContent | (ToolCall & { partialJson: string }); type CodexWebSocketSessionState = { @@ -498,13 +499,23 @@ async function buildTransformedCodexRequestBody( params.service_tier = options.serviceTier; } if (context.tools && context.tools.length > 0) { - params.tools = convertTools(context.tools); + params.tools = convertTools(context.tools, model); if (options?.toolChoice) { const toolChoice = normalizeCodexToolChoice(options.toolChoice); if (toolChoice) { params.tool_choice = toolChoice; } } + // When a custom-tool is active, force serial tool-calling. OpenAI's + // `parallel_tool_calls` is request-scoped — disabling it here affects + // every tool in the turn, not just the custom one. That's coarser + // than spec §1's "supports_parallel_tool_calls = false" (which + // strictly targets `apply_patch`), but the platform API offers no + // per-tool flag. + const emittedTools = params.tools as CodexToolPayload[]; + if (emittedTools.some(t => t.type === "custom")) { + params.parallel_tool_calls = false; + } } const systemPrompt = buildCodexSystemPrompt({ userSystemPrompt: context.systemPrompt }); @@ -802,6 +813,16 @@ function handleCodexStreamEvent(args: { return firstTokenTime; } + if (eventType === "response.custom_tool_call_input.delta") { + handleCustomToolCallInputDelta(runtime.currentItem, runtime.currentBlock, rawEvent, stream, output, blockIndex); + return firstTokenTime; + } + + if (eventType === "response.custom_tool_call_input.done") { + handleCustomToolCallInputDone(runtime.currentItem, runtime.currentBlock, rawEvent); + return firstTokenTime; + } + if (eventType === "response.output_item.done") { handleOutputItemDone(model, output, stream, runtime, rawEvent, blockIndex); return firstTokenTime; @@ -839,6 +860,19 @@ function createOutputBlockForItem(item: CodexEventItem): CodexOutputBlock | null partialJson: item.arguments || "", }; } + if (item.type === "custom_tool_call") { + // Wire name flows through unchanged; the agent-loop dispatcher also + // matches `Tool.customWireName`. Reuse `partialJson` as the + // accumulation buffer for the raw input string. + return { + type: "toolCall", + id: `${item.call_id}|${item.id ?? ""}`, + name: item.name, + arguments: { input: item.input ?? "" }, + customWireName: item.name, + partialJson: item.input ?? "", + }; + } return null; } @@ -948,6 +982,34 @@ function handleToolCallArgumentsDone( } } +function handleCustomToolCallInputDelta( + currentItem: CodexEventItem | null, + currentBlock: CodexOutputBlock | null, + rawEvent: Record, + stream: AssistantMessageEventStream, + output: AssistantMessage, + blockIndex: () => number, +): void { + if (currentItem?.type !== "custom_tool_call" || currentBlock?.type !== "toolCall") return; + const delta = (rawEvent as { delta?: string }).delta || ""; + currentBlock.partialJson += delta; + currentBlock.arguments = { input: currentBlock.partialJson }; + stream.push({ type: "toolcall_delta", contentIndex: blockIndex(), delta, partial: output }); +} + +function handleCustomToolCallInputDone( + currentItem: CodexEventItem | null, + currentBlock: CodexOutputBlock | null, + rawEvent: Record, +): void { + if (currentItem?.type !== "custom_tool_call" || currentBlock?.type !== "toolCall") return; + const input = (rawEvent as { input?: string }).input; + if (typeof input === "string") { + currentBlock.partialJson = input; + currentBlock.arguments = { input }; + } +} + function handleOutputItemDone( model: Model<"openai-codex-responses">, output: AssistantMessage, @@ -1000,6 +1062,23 @@ function handleOutputItemDone( return; } + if (item.type === "custom_tool_call") { + const rawInput = + runtime.currentBlock?.type === "toolCall" && runtime.currentBlock.partialJson + ? runtime.currentBlock.partialJson + : (item.input ?? ""); + const toolCall: ToolCall = { + type: "toolCall", + id: `${item.call_id}|${item.id ?? ""}`, + name: item.name, + arguments: { input: rawInput }, + customWireName: item.name, + }; + runtime.canSafelyReplayWebsocketOverSse = false; + stream.push({ type: "toolcall_end", contentIndex: blockIndex(), toolCall, partial: output }); + return; + } + void model; } @@ -2081,6 +2160,10 @@ function convertMessages(model: Model<"openai-codex-responses">, context: Contex const transformedMessages = transformMessages(context.messages, model, normalizeToolCallId); let msgIndex = 0; + // Track call_ids that originated as custom tool calls so paired tool-result + // messages can be replayed as `custom_tool_call_output` rather than + // `function_call_output` (OpenAI rejects mismatched pairs). + const customCallIds = new Set(); for (const msg of transformedMessages) { if (msg.role === "user" || msg.role === "developer") { @@ -2089,6 +2172,12 @@ function convertMessages(model: Model<"openai-codex-responses">, context: Contex | Array | undefined; if (historyItems) { + for (const item of historyItems) { + const maybe = item as { type?: string; call_id?: string }; + if (maybe.type === "custom_tool_call" && typeof maybe.call_id === "string") { + customCallIds.add(maybe.call_id); + } + } messages.push(...historyItems); msgIndex += 1; continue; @@ -2110,10 +2199,17 @@ function convertMessages(model: Model<"openai-codex-responses">, context: Contex ); const historyItems = providerPayload?.items as Array | undefined; if (historyItems) { + for (const item of historyItems) { + const maybe = item as { type?: string; call_id?: string }; + if (maybe.type === "custom_tool_call" && typeof maybe.call_id === "string") { + customCallIds.add(maybe.call_id); + } + } if (providerPayload?.dt) { messages.push(...historyItems); } else { messages.splice(0, messages.length, ...historyItems); + // Keep customCallIds from the pre-splice state since historyItems may re-introduce them. } msgIndex += 1; continue; @@ -2149,6 +2245,18 @@ function convertMessages(model: Model<"openai-codex-responses">, context: Contex if (block.type === "toolCall") { const toolCall = block as ToolCall; const normalized = normalizeResponsesToolCallId(toolCall.id); + if (toolCall.customWireName) { + const rawInput = typeof toolCall.arguments?.input === "string" ? toolCall.arguments.input : ""; + customCallIds.add(normalized.callId); + outputItems.push({ + type: "custom_tool_call", + id: normalized.itemId, + call_id: normalized.callId, + name: toolCall.customWireName, + input: rawInput, + } as ResponseInput[number]); + continue; + } outputItems.push({ type: "function_call", id: normalized.itemId, @@ -2172,11 +2280,20 @@ function convertMessages(model: Model<"openai-codex-responses">, context: Contex .join("\n"); const hasImages = msg.content.some(content => content.type === "image"); const normalized = normalizeResponsesToolCallId(msg.toolCallId); - messages.push({ - type: "function_call_output", - call_id: normalized.callId, - output: (textResult.length > 0 ? textResult : "(see attached image)").toWellFormed(), - }); + const output = (textResult.length > 0 ? textResult : "(see attached image)").toWellFormed(); + if (customCallIds.has(normalized.callId)) { + messages.push({ + type: "custom_tool_call_output", + call_id: normalized.callId, + output, + } as ResponseInput[number]); + } else { + messages.push({ + type: "function_call_output", + call_id: normalized.callId, + output, + }); + } if (hasImages && model.input.includes("image")) { const contentParts: ResponseInputContent[] = [ { type: "input_text", text: "Attached image(s) from tool result:" } satisfies ResponseInputText, @@ -2226,14 +2343,48 @@ function normalizeInputMessageContent( return maybeWithoutImages.filter(item => item.type !== "input_text" || item.text.trim().length > 0); } -function convertTools(tools: Tool[]): Array<{ - type: "function"; - name: string; - description: string; - parameters: Record; - strict?: boolean; -}> { - return tools.map(tool => { +/** + * Whether this Codex-backend model should get the custom-tool grammar + * variant for `apply_patch`. codex-rs uses a single serializer for both + * the public Responses endpoint and `chatgpt.com/backend-api`, so the + * backend already accepts `{type: "custom"}` tools in production — the + * gate is the per-model `applyPatchToolType` flag, nothing endpoint-specific. + */ +function supportsFreeformApplyPatchCodex(model: Model<"openai-codex-responses">): boolean { + return model.applyPatchToolType === "freeform"; +} + +type CodexToolPayload = + | { + type: "function"; + name: string; + description: string; + parameters: Record; + strict?: boolean; + } + | { + type: "custom"; + name: string; + description: string; + format: { type: "grammar"; syntax: "lark" | "regex"; definition: string }; + }; + +/** @internal Exported for tests. */ +export function convertTools(tools: Tool[], model: Model<"openai-codex-responses">): CodexToolPayload[] { + const allowFreeform = supportsFreeformApplyPatchCodex(model); + return tools.map((tool): CodexToolPayload => { + if (allowFreeform && tool.customFormat) { + return { + type: "custom", + name: tool.customWireName ?? tool.name, + description: tool.description || "", + format: { + type: "grammar", + syntax: tool.customFormat.syntax, + definition: tool.customFormat.definition, + }, + }; + } const strict = !!(!NO_STRICT && tool.strict); const baseParameters = tool.parameters as unknown as Record; const { schema: parameters, strict: effectiveStrict } = adaptSchemaForStrict(baseParameters, strict); diff --git a/packages/ai/src/providers/openai-responses-shared.ts b/packages/ai/src/providers/openai-responses-shared.ts index d02956792..f704409c5 100644 --- a/packages/ai/src/providers/openai-responses-shared.ts +++ b/packages/ai/src/providers/openai-responses-shared.ts @@ -1,6 +1,7 @@ import { structuredCloneJSON } from "@oh-my-pi/pi-utils"; import type OpenAI from "openai"; import type { + ResponseCustomToolCall, ResponseFunctionToolCall, ResponseInput, ResponseInputContent, @@ -82,11 +83,30 @@ export function collectKnownCallIds(messages: ResponseInput): Set { for (const item of messages) { if (item.type === "function_call" && typeof item.call_id === "string") { knownCallIds.add(item.call_id); + } else if ( + (item as { type?: string }).type === "custom_tool_call" && + typeof (item as { call_id?: string }).call_id === "string" + ) { + knownCallIds.add((item as { call_id: string }).call_id); } } return knownCallIds; } +/** Scan replay items for call_ids that were originally custom tool calls. */ +export function collectCustomCallIds(messages: ResponseInput): Set { + const customCallIds = new Set(); + for (const item of messages) { + if ( + (item as { type?: string }).type === "custom_tool_call" && + typeof (item as { call_id?: string }).call_id === "string" + ) { + customCallIds.add((item as { call_id: string }).call_id); + } + } + return customCallIds; +} + export function convertResponsesInputContent( content: string | Array, supportsImages: boolean, @@ -122,6 +142,7 @@ export function convertResponsesAssistantMessage( msgIndex: number, knownCallIds: Set, includeThinkingSignatures = true, + customCallIds?: Set, ): ResponseInput { const outputItems: ResponseInput = []; const isDifferentModel = @@ -167,6 +188,18 @@ export function convertResponsesAssistantMessage( itemId = undefined; } knownCallIds.add(normalized.callId); + if (block.customWireName) { + const rawInput = typeof block.arguments?.input === "string" ? block.arguments.input : ""; + customCallIds?.add(normalized.callId); + outputItems.push({ + type: "custom_tool_call", + id: itemId, + call_id: normalized.callId, + name: block.customWireName, + input: rawInput, + } as ResponseInput[number]); + continue; + } outputItems.push({ type: "function_call", id: itemId, @@ -185,6 +218,7 @@ export function appendResponsesToolResultMessages( model: Model, strictResponsesPairing: boolean, knownCallIds: ReadonlySet, + customCallIds?: ReadonlySet, ): void { const textResult = toolResult.content .filter((block): block is TextContent => block.type === "text") @@ -196,11 +230,20 @@ export function appendResponsesToolResultMessages( return; } - messages.push({ - type: "function_call_output", - call_id: normalized.callId, - output: (textResult.length > 0 ? textResult : "(see attached image)").toWellFormed(), - }); + const output = (textResult.length > 0 ? textResult : "(see attached image)").toWellFormed(); + if (customCallIds?.has(normalized.callId)) { + messages.push({ + type: "custom_tool_call_output", + call_id: normalized.callId, + output, + } as ResponseInput[number]); + } else { + messages.push({ + type: "function_call_output", + call_id: normalized.callId, + output, + }); + } if (!hasImages || !model.input.includes("image")) { return; @@ -233,7 +276,12 @@ export async function processResponsesStream( model: Model, options?: ProcessResponsesStreamOptions, ): Promise { - let currentItem: ResponseReasoningItem | ResponseOutputMessage | ResponseFunctionToolCall | null = null; + let currentItem: + | ResponseReasoningItem + | ResponseOutputMessage + | ResponseFunctionToolCall + | ResponseCustomToolCall + | null = null; let currentBlock: ThinkingContent | TextContent | (ToolCall & { partialJson: string }) | null = null; const blocks = output.content; const blockIndex = () => blocks.length - 1; @@ -269,6 +317,24 @@ export async function processResponsesStream( }; output.content.push(currentBlock); stream.push({ type: "toolcall_start", contentIndex: blockIndex(), partial: output }); + } else if (item.type === "custom_tool_call") { + currentItem = item; + currentBlock = { + type: "toolCall", + id: `${item.call_id}|${item.id ?? ""}`, + // Preserve the raw wire name (e.g. `apply_patch`). The agent-loop + // dispatcher matches it against both `Tool.name` and + // `Tool.customWireName`, so this stays wire-accurate through + // history replay while still routing to the right handler. + name: item.name, + arguments: { input: item.input ?? "" }, + customWireName: item.name, + // Custom tools stream a raw string, but we reuse `partialJson` as the + // accumulation buffer so later code that inspects the field still works. + partialJson: item.input ?? "", + }; + output.content.push(currentBlock); + stream.push({ type: "toolcall_start", contentIndex: blockIndex(), partial: output }); } } else if (event.type === "response.reasoning_summary_part.added") { if (currentItem?.type === "reasoning") { @@ -368,6 +434,22 @@ export async function processResponsesStream( currentBlock.partialJson = event.arguments; currentBlock.arguments = parseStreamingJson(currentBlock.partialJson); } + } else if (event.type === "response.custom_tool_call_input.delta") { + if (currentItem?.type === "custom_tool_call" && currentBlock?.type === "toolCall") { + currentBlock.partialJson += event.delta; + currentBlock.arguments = { input: currentBlock.partialJson }; + stream.push({ + type: "toolcall_delta", + contentIndex: blockIndex(), + delta: event.delta, + partial: output, + }); + } + } else if (event.type === "response.custom_tool_call_input.done") { + if (currentItem?.type === "custom_tool_call" && currentBlock?.type === "toolCall") { + currentBlock.partialJson = event.input; + currentBlock.arguments = { input: event.input }; + } } else if (event.type === "response.output_item.done") { const item = structuredCloneJSON(event.item); options?.onOutputItemDone?.(item); @@ -418,6 +500,20 @@ export async function processResponsesStream( }; currentBlock = null; stream.push({ type: "toolcall_end", contentIndex: blockIndex(), toolCall, partial: output }); + } else if (item.type === "custom_tool_call") { + const rawInput = + currentBlock?.type === "toolCall" && currentBlock.partialJson + ? currentBlock.partialJson + : (item.input ?? ""); + const toolCall: ToolCall = { + type: "toolCall", + id: `${item.call_id}|${item.id ?? ""}`, + name: item.name, + arguments: { input: rawInput }, + customWireName: item.name, + }; + currentBlock = null; + stream.push({ type: "toolcall_end", contentIndex: blockIndex(), toolCall, partial: output }); } } else if (event.type === "response.completed") { const response = event.response; diff --git a/packages/ai/src/providers/openai-responses.ts b/packages/ai/src/providers/openai-responses.ts index 8db1dab6e..e7c456eba 100644 --- a/packages/ai/src/providers/openai-responses.ts +++ b/packages/ai/src/providers/openai-responses.ts @@ -48,6 +48,7 @@ import { } from "./github-copilot-headers"; import { appendResponsesToolResultMessages, + collectCustomCallIds, collectKnownCallIds, convertResponsesAssistantMessage, convertResponsesInputContent, @@ -388,10 +389,19 @@ function buildParams( } if (context.tools) { - params.tools = convertTools(context.tools, supportsStrictMode(model)); + params.tools = convertTools(context.tools, supportsStrictMode(model), model); if (options?.toolChoice) { params.tool_choice = mapToOpenAIResponsesToolChoice(options.toolChoice); } + // The apply_patch spec §1 marks only `apply_patch` itself as + // `supports_parallel_tool_calls = false`. OpenAI's Responses API + // exposes `parallel_tool_calls` as a request-scoped flag, not a + // per-tool one, so when a custom grammar tool is in the list we + // disable parallelism for the whole turn. Slightly coarser than + // the spec requires — but the platform API offers no finer knob. + if (params.tools.some(t => (t as { type?: string }).type === "custom")) { + params.parallel_tool_calls = false; + } } if (model.reasoning) { @@ -459,6 +469,7 @@ function convertConversationMessages( ): ResponseInput { const messages: ResponseInput = []; let knownCallIds = new Set(); + const customCallIds = new Set(); const shouldReplayNativeHistory = canReplayOpenAIResponsesNativeHistory(providerSessionState); const transformedMessages = transformMessages(context.messages, model, normalizeResponsesToolCallIdForTransform); @@ -478,6 +489,7 @@ function convertConversationMessages( if (historyItems && shouldReplayPayloadItems) { messages.push(...sanitizeOpenAIResponsesHistoryItemsForReplay(historyItems)); knownCallIds = collectKnownCallIds(messages); + for (const id of collectCustomCallIds(messages)) customCallIds.add(id); msgIndex++; continue; } @@ -498,6 +510,7 @@ function convertConversationMessages( messages.splice(0, messages.length, ...sanitizedHistoryItems); } knownCallIds = collectKnownCallIds(messages); + for (const id of collectCustomCallIds(messages)) customCallIds.add(id); msgIndex++; continue; } @@ -508,11 +521,12 @@ function convertConversationMessages( msgIndex, knownCallIds, shouldReplayNativeHistory, + customCallIds, ); if (outputItems.length === 0) continue; messages.push(...outputItems); } else if (msg.role === "toolResult") { - appendResponsesToolResultMessages(messages, msg, model, strictResponsesPairing, knownCallIds); + appendResponsesToolResultMessages(messages, msg, model, strictResponsesPairing, knownCallIds, customCallIds); } msgIndex++; } @@ -520,8 +534,35 @@ function convertConversationMessages( return messages; } -function convertTools(tools: Tool[], strictMode: boolean): OpenAITool[] { +/** + * Whether this model should get the OpenAI custom-tool grammar variant + * for `apply_patch`. Explicit opt-in via `model.applyPatchToolType` in + * `models.json` — no auto-detection. See Phase 2a.1 in the plan. + * @internal Exported for tests. + */ +export function supportsFreeformApplyPatch(model: Model<"openai-responses">): boolean { + return model.applyPatchToolType === "freeform"; +} + +/** @internal Exported for tests. */ +export function convertTools(tools: Tool[], strictMode: boolean, model: Model<"openai-responses">): OpenAITool[] { + const allowFreeform = supportsFreeformApplyPatch(model); return tools.map(tool => { + if (allowFreeform && tool.customFormat) { + return { + type: "custom", + // Tool advertises its wire-level name (e.g. `apply_patch`) — the + // agent-loop dispatcher will match incoming calls by either the + // internal `name` or `customWireName`. + name: tool.customWireName ?? tool.name, + description: tool.description || "", + format: { + type: "grammar", + syntax: tool.customFormat.syntax, + definition: tool.customFormat.definition, + }, + } as unknown as OpenAITool; + } const strict = !NO_STRICT && strictMode && tool.strict !== false; const baseParameters = tool.parameters as unknown as Record; const { schema: parameters, strict: effectiveStrict } = adaptSchemaForStrict(baseParameters, strict); diff --git a/packages/ai/src/types.ts b/packages/ai/src/types.ts index b8fb672f3..26259b0cc 100644 --- a/packages/ai/src/types.ts +++ b/packages/ai/src/types.ts @@ -283,6 +283,14 @@ export interface ToolCall { arguments: Record; thoughtSignature?: string; // Google-specific: opaque signature for reusing thought context intent?: string; // Harness-level intent metadata extracted from traced tool arguments + /** + * Original wire-level name when the tool was invoked via OpenAI's custom-tool + * mechanism (e.g., `apply_patch`). Set by `openai-responses` on receive so + * the history-replay path can re-emit the call as `custom_tool_call` with + * its paired tool-result as `custom_tool_call_output`. Absent for regular + * JSON function tools. + */ + customWireName?: string; } export interface Usage { @@ -409,6 +417,23 @@ export interface Tool { parameters: TParameters; /** If true, tool is strictly typed and validated against the parameters schema before execution */ strict?: boolean; + /** + * Optional grammar constraint for OpenAI custom-tool emission. + * When set, providers that support grammar-constrained tools (currently only + * `openai-responses` against models with the right capability flag) may emit + * this tool as `{type: "custom", format: {type: "grammar", …}}` instead of a + * JSON function tool. Other providers ignore the field. + */ + customFormat?: { syntax: "lark" | "regex"; definition: string }; + /** + * Optional wire-level name used when this tool is emitted as a custom tool + * (e.g. OpenAI's `{type: "custom"}` shape). Models trained on specific tool + * names — like GPT-5 on `apply_patch` — need to see that exact name on the + * wire, but it may differ from the harness-internal `name`. The agent-loop + * dispatcher matches both `name` and `customWireName` so returned tool + * calls route correctly. Absent for regular JSON function tools. + */ + customWireName?: string; } export interface Context { @@ -542,4 +567,11 @@ export interface Model { thinking?: ThinkingConfig; /** Compatibility overrides for openai-completions API. If not set, auto-detected from baseUrl. */ compat?: TApi extends "openai-completions" ? OpenAICompat : never; + /** + * Which shape to use when exposing the Codex `apply_patch` tool to this model. + * - `"freeform"`: OpenAI custom-tool with a Lark grammar (spec §1.1). Raw patch + * string, no JSON envelope. Only honored by the `openai-responses` provider. + * - `"function"` or undefined: JSON function-tool with `{input: string}` (spec §1.2). + */ + applyPatchToolType?: "freeform" | "function"; } diff --git a/packages/ai/test/apply-patch-freeform.test.ts b/packages/ai/test/apply-patch-freeform.test.ts new file mode 100644 index 000000000..99a1b78f1 --- /dev/null +++ b/packages/ai/test/apply-patch-freeform.test.ts @@ -0,0 +1,382 @@ +import { describe, expect, test } from "bun:test"; +import { convertTools, supportsFreeformApplyPatch } from "@oh-my-pi/pi-ai/providers/openai-responses"; +import { + appendResponsesToolResultMessages, + convertResponsesAssistantMessage, + processResponsesStream, +} from "@oh-my-pi/pi-ai/providers/openai-responses-shared"; +import type { AssistantMessage, Model, Tool, ToolResultMessage } from "@oh-my-pi/pi-ai/types"; +import { Type } from "@sinclair/typebox"; + +const GRAMMAR = 'start: "*** Begin Patch" LF'; + +function makeModel(overrides: Partial> = {}): Model<"openai-responses"> { + return { + id: "gpt-5", + name: "GPT-5", + api: "openai-responses", + provider: "openai", + baseUrl: "https://api.openai.com/v1", + reasoning: true, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 400000, + maxTokens: 128000, + ...overrides, + }; +} + +const editTool: Tool = { + name: "edit", + customWireName: "apply_patch", + description: "edit files", + parameters: Type.Object({ input: Type.String() }), + customFormat: { syntax: "lark", definition: GRAMMAR }, +}; + +const plainTool: Tool = { + name: "read_file", + description: "read a file", + parameters: Type.Object({ path: Type.String() }), +}; + +describe("supportsFreeformApplyPatch", () => { + test("absent flag returns false", () => { + // No auto-detection — requires explicit opt-in in models.json. + expect(supportsFreeformApplyPatch(makeModel())).toBe(false); + }); + + test("applyPatchToolType: freeform enables", () => { + expect(supportsFreeformApplyPatch(makeModel({ applyPatchToolType: "freeform" }))).toBe(true); + }); + + test("applyPatchToolType: function disables", () => { + expect(supportsFreeformApplyPatch(makeModel({ applyPatchToolType: "function" }))).toBe(false); + }); + + test("flag is the sole signal — id/baseUrl are irrelevant", () => { + expect( + supportsFreeformApplyPatch( + makeModel({ id: "gpt-4", baseUrl: "https://proxy.example/", applyPatchToolType: "freeform" }), + ), + ).toBe(true); + expect(supportsFreeformApplyPatch(makeModel({ id: "gpt-5", baseUrl: "https://api.openai.com/v1" }))).toBe(false); + }); +}); + +describe("convertTools: freeform emission", () => { + const freeformModel = makeModel({ applyPatchToolType: "freeform" }); + + test("edit tool with customFormat becomes a custom grammar tool", () => { + const [out] = convertTools([editTool], false, freeformModel) as unknown as Array>; + expect(out.type).toBe("custom"); + expect(out.name).toBe("apply_patch"); // wire name from tool.customWireName + expect(out.format).toEqual({ type: "grammar", syntax: "lark", definition: GRAMMAR }); + }); + + test("regular tools remain function-type alongside a custom one", () => { + const out = convertTools([editTool, plainTool], false, freeformModel) as unknown as Array< + Record + >; + expect(out[0].type).toBe("custom"); + expect(out[1].type).toBe("function"); + expect(out[1].name).toBe("read_file"); + }); + + test("falls back to function tool when flag is absent", () => { + const [out] = convertTools([editTool], false, makeModel()) as unknown as Array>; + expect(out.type).toBe("function"); + expect(out.name).toBe("edit"); + }); + + test("applyPatchToolType=function explicitly disables", () => { + const [out] = convertTools([editTool], false, makeModel({ applyPatchToolType: "function" })) as unknown as Array< + Record + >; + expect(out.type).toBe("function"); + }); +}); + +describe("custom_tool_call stream receive", () => { + async function* makeStream(events: unknown[]): AsyncIterable { + for (const e of events) yield e; + } + + test("aggregates delta events into a ToolCall with input arg", async () => { + const output: AssistantMessage = { + role: "assistant", + content: [], + timestamp: Date.now(), + provider: "openai", + model: "gpt-5", + api: "openai-responses", + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "stop", + }; + const emitted: unknown[] = []; + const stream = { + push: (e: unknown) => emitted.push(e), + end: () => {}, + } as never; + + const events = [ + { + type: "response.output_item.added", + item: { + type: "custom_tool_call", + id: "ctc_1", + call_id: "call_1", + name: "apply_patch", + input: "", + }, + }, + { + type: "response.custom_tool_call_input.delta", + delta: "*** Begin Patch\n", + }, + { + type: "response.custom_tool_call_input.delta", + delta: "*** End Patch\n", + }, + { + type: "response.custom_tool_call_input.done", + input: "*** Begin Patch\n*** End Patch\n", + }, + { + type: "response.output_item.done", + item: { + type: "custom_tool_call", + id: "ctc_1", + call_id: "call_1", + name: "apply_patch", + input: "*** Begin Patch\n*** End Patch\n", + }, + }, + ]; + + await processResponsesStream(makeStream(events), output, stream, makeModel()); + + const block = output.content[0]; + expect(block?.type).toBe("toolCall"); + const tool = block as { + type: "toolCall"; + name: string; + arguments: Record; + customWireName?: string; + }; + // Wire name passes through unchanged — the agent-loop dispatcher + // matches against both `Tool.name` and `Tool.customWireName`. + expect(tool.name).toBe("apply_patch"); + expect(tool.customWireName).toBe("apply_patch"); + expect(tool.arguments.input).toBe("*** Begin Patch\n*** End Patch\n"); + + // toolcall_end event carries the final ToolCall + const endEvent = emitted.find( + ( + e, + ): e is { + type: string; + toolCall: { name: string; arguments: Record; customWireName?: string }; + } => !!e && typeof e === "object" && (e as { type?: string }).type === "toolcall_end", + ); + expect(endEvent?.toolCall.name).toBe("apply_patch"); + expect(endEvent?.toolCall.customWireName).toBe("apply_patch"); + }); +}); + +describe("codex-backend convertTools (chatgpt.com/backend-api)", () => { + // Dynamic import: loading the codex provider pulls in heavy SDK code we + // don't want mixed into module-resolve for unrelated tests. + async function getCodexConvertTools() { + const mod = (await import("@oh-my-pi/pi-ai/providers/openai-codex-responses")) as { + convertTools: (tools: Tool[], model: Model<"openai-codex-responses">) => Array>; + }; + return mod.convertTools; + } + + function makeCodexModel(overrides: Partial> = {}): Model<"openai-codex-responses"> { + return { + id: "gpt-5", + name: "GPT-5", + api: "openai-codex-responses", + provider: "openai-codex", + baseUrl: "https://chatgpt.com/backend-api", + reasoning: true, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 272000, + maxTokens: 128000, + ...overrides, + }; + } + + test("edit tool with customFormat becomes a custom grammar tool when flag is set", async () => { + const codexConvertTools = await getCodexConvertTools(); + const [out] = codexConvertTools([editTool], makeCodexModel({ applyPatchToolType: "freeform" })); + expect(out.type).toBe("custom"); + expect(out.name).toBe("apply_patch"); + expect(out.format).toEqual({ type: "grammar", syntax: "lark", definition: GRAMMAR }); + }); + + test("wire shape matches direct-OpenAI convertTools (single serializer contract)", async () => { + const codexConvertTools = await getCodexConvertTools(); + const [codexOut] = codexConvertTools([editTool], makeCodexModel({ applyPatchToolType: "freeform" })); + const [openaiOut] = convertTools( + [editTool], + false, + makeModel({ applyPatchToolType: "freeform" }), + ) as unknown as Array>; + expect(codexOut).toEqual(openaiOut); + }); + + test("falls back to function tool when flag is absent", async () => { + const codexConvertTools = await getCodexConvertTools(); + const [out] = codexConvertTools([editTool], makeCodexModel()); + expect(out.type).toBe("function"); + expect(out.name).toBe("edit"); + }); +}); + +describe("dispatcher wire-name matching", () => { + test("ToolCall.name matches a Tool via its customWireName", () => { + // Simulate what agent-loop.ts:455-465 does. + const editLikeTool: Tool & { customWireName?: string } = { + name: "edit", + customWireName: "apply_patch", + description: "edit files", + parameters: Type.Object({ input: Type.String() }), + customFormat: { syntax: "lark", definition: GRAMMAR }, + }; + const readTool: Tool = { + name: "read_file", + description: "read", + parameters: Type.Object({ path: Type.String() }), + }; + const tools = [editLikeTool, readTool]; + const toolCall = { name: "apply_patch" }; + + const matched = + tools.find(t => t.name === toolCall.name) ?? + tools.find( + (t): t is typeof t & { customWireName: string } => + (t as { customWireName?: string }).customWireName !== undefined && + (t as { customWireName?: string }).customWireName === toolCall.name, + ); + expect(matched).toBe(editLikeTool); + }); + + test("prefers name over customWireName when both would match", () => { + // A pathological tool set: one tool named `foo`, another with + // customWireName `foo`. Internal name wins. + const nameMatch: Tool = { + name: "foo", + description: "", + parameters: Type.Object({}), + }; + const wireMatch: Tool & { customWireName: string } = { + name: "bar", + customWireName: "foo", + description: "", + parameters: Type.Object({}), + }; + const tools = [wireMatch, nameMatch]; // wireMatch listed first + const toolCall = { name: "foo" }; + + const matched = + tools.find(t => t.name === toolCall.name) ?? + tools.find( + (t): t is typeof t & { customWireName: string } => + (t as { customWireName?: string }).customWireName !== undefined && + (t as { customWireName?: string }).customWireName === toolCall.name, + ); + expect(matched).toBe(nameMatch); + }); +}); + +describe("history replay: custom_tool_call round-trip", () => { + test("assistant tool-call block with customWireName replays as custom_tool_call", () => { + const assistantMsg: AssistantMessage = { + role: "assistant", + content: [ + { + type: "toolCall", + id: "call_1|ctc_1", + name: "edit", + arguments: { input: "*** Begin Patch\n*** End Patch\n" }, + customWireName: "apply_patch", + }, + ], + timestamp: Date.now(), + provider: "openai", + model: "gpt-5", + api: "openai-responses", + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "stop", + }; + const knownCallIds = new Set(); + const customCallIds = new Set(); + const items = convertResponsesAssistantMessage(assistantMsg, makeModel(), 0, knownCallIds, true, customCallIds); + + expect(items).toHaveLength(1); + const item = items[0] as { type: string; name?: string; input?: string }; + expect(item.type).toBe("custom_tool_call"); + expect(item.name).toBe("apply_patch"); + expect(item.input).toBe("*** Begin Patch\n*** End Patch\n"); + expect(customCallIds.has("call_1")).toBe(true); + }); + + test("paired tool result emits custom_tool_call_output when custom id is tracked", () => { + const messages: unknown[] = []; + const toolResult: ToolResultMessage = { + role: "toolResult", + toolCallId: "call_1", + toolName: "edit", + isError: false, + content: [{ type: "text", text: "Success. Updated the following files:\nM foo.txt" }], + timestamp: Date.now(), + }; + const knownCallIds = new Set(["call_1"]); + const customCallIds = new Set(["call_1"]); + + appendResponsesToolResultMessages(messages as never, toolResult, makeModel(), true, knownCallIds, customCallIds); + + expect(messages).toHaveLength(1); + const item = messages[0] as { type: string; call_id: string; output: string }; + expect(item.type).toBe("custom_tool_call_output"); + expect(item.call_id).toBe("call_1"); + expect(item.output).toContain("Success"); + }); + + test("tool result for a non-custom call still emits function_call_output", () => { + const messages: unknown[] = []; + const toolResult: ToolResultMessage = { + role: "toolResult", + toolCallId: "call_2", + toolName: "read_file", + isError: false, + content: [{ type: "text", text: "ok" }], + timestamp: Date.now(), + }; + const knownCallIds = new Set(["call_2"]); + const customCallIds = new Set(); // call_2 not custom + + appendResponsesToolResultMessages(messages as never, toolResult, makeModel(), true, knownCallIds, customCallIds); + + const item = messages[0] as { type: string }; + expect(item.type).toBe("function_call_output"); + }); +}); diff --git a/packages/coding-agent/src/bun-imports.d.ts b/packages/coding-agent/src/bun-imports.d.ts index e7dc00dff..f783792aa 100644 --- a/packages/coding-agent/src/bun-imports.d.ts +++ b/packages/coding-agent/src/bun-imports.d.ts @@ -20,3 +20,9 @@ declare module "*.py" { const content: string; export default content; } + +// Lark grammar files imported as text +declare module "*.lark" { + const content: string; + export default content; +} diff --git a/packages/coding-agent/src/config/settings-schema.ts b/packages/coding-agent/src/config/settings-schema.ts index 6aaa2b2ca..eef337827 100644 --- a/packages/coding-agent/src/config/settings-schema.ts +++ b/packages/coding-agent/src/config/settings-schema.ts @@ -953,12 +953,12 @@ export const SETTINGS_SCHEMA = { // Edit tool "edit.mode": { type: "enum", - values: ["replace", "patch", "hashline", "chunk", "vim"] as const, + values: ["replace", "patch", "hashline", "chunk", "vim", "apply_patch"] as const, default: "hashline", ui: { tab: "editing", label: "Edit Mode", - description: "Select the edit tool variant (replace, patch, hashline, chunk, or vim)", + description: "Select the edit tool variant (replace, patch, hashline, chunk, vim, or apply_patch)", }, }, diff --git a/packages/coding-agent/src/edit/apply-patch/index.ts b/packages/coding-agent/src/edit/apply-patch/index.ts new file mode 100644 index 000000000..b60f7106f --- /dev/null +++ b/packages/coding-agent/src/edit/apply-patch/index.ts @@ -0,0 +1,87 @@ +/** + * Multi-file orchestrator for the Codex `apply_patch` envelope. + * + * Decoupled from tool-registration: takes raw patch text + options, parses + * it, and applies each hunk via the existing single-file `applyPatch` in + * `../modes/patch.ts`. A future OpenAI freeform/grammar tool variant can + * call this directly with the raw grammar output. + * + * Per spec §6.1, hunks are applied in order and NOT atomically — if hunk + * N fails, hunks `0..N-1` are already on disk. We surface that by + * returning the per-file results alongside the error when it happens. + */ + +import { ApplyPatchError } from "../diff"; +import { type ApplyPatchOptions, type ApplyPatchResult, applyPatch, type PatchInput } from "../modes/patch"; +import { parseApplyPatch } from "./parser"; + +export { parseApplyPatch } from "./parser"; + +export interface ApplyCodexPatchResult { + /** Single-file apply results in the order they were attempted. */ + results: ApplyPatchResult[]; + /** Affected file paths grouped by operation, for the §9.1 summary. */ + affected: { + added: string[]; + modified: string[]; + deleted: string[]; + }; +} + +/** + * Apply a full Codex `*** Begin Patch` envelope. + * + * Note: renames are reported under `modified` with the original path (spec + * §9.1), not as a delete + add. + */ +export async function applyCodexPatch(patchText: string, options: ApplyPatchOptions): Promise { + const hunks = parseApplyPatch(patchText); + + if (hunks.length === 0) { + throw new ApplyPatchError("No files were modified."); + } + + const results: ApplyPatchResult[] = []; + const affected = { + added: [] as string[], + modified: [] as string[], + deleted: [] as string[], + }; + + for (const hunk of hunks) { + const result = await applyPatch(hunk, options); + results.push(result); + recordAffected(affected, hunk, result); + } + + return { results, affected }; +} + +function recordAffected( + affected: ApplyCodexPatchResult["affected"], + hunk: PatchInput, + _result: ApplyPatchResult, +): void { + switch (hunk.op) { + case "create": + affected.added.push(hunk.path); + break; + case "delete": + affected.deleted.push(hunk.path); + break; + case "update": + affected.modified.push(hunk.path); + break; + } +} + +/** + * Format the A/M/D summary described in spec §9.1. + */ +export function formatApplyCodexPatchSummary(affected: ApplyCodexPatchResult["affected"]): string { + const lines = ["Success. Updated the following files:"]; + for (const p of affected.added) lines.push(`A ${p}`); + for (const p of affected.modified) lines.push(`M ${p}`); + for (const p of affected.deleted) lines.push(`D ${p}`); + return lines.join("\n"); +} diff --git a/packages/coding-agent/src/edit/apply-patch/parser.ts b/packages/coding-agent/src/edit/apply-patch/parser.ts new file mode 100644 index 000000000..713b2300b --- /dev/null +++ b/packages/coding-agent/src/edit/apply-patch/parser.ts @@ -0,0 +1,148 @@ +/** + * Parser for the Codex `apply_patch` envelope format. + * + * *** Begin Patch + * *** Add File: + * + + * *** Delete File: + * *** Update File: + * *** Move to: + * @@ + * -old + * +new + * context + * *** End of File + * *** End Patch + * + * Input is the full envelope text (optionally heredoc-wrapped). Output is a + * list of `PatchInput` records, each ready to hand to the existing + * single-file `applyPatch()` in `../modes/patch.ts`. + * + * Per spec §4.3 Lenient mode: a `<= 2) { + const first = lines[0]; + const last = lines[lines.length - 1].trim(); + const validOpeners = new Set(["< 0) { + // Blank separator lines between hunks are ignored (spec §3.3). + if (remaining[0].trim() === "") { + remaining = remaining.slice(1); + lineNumber++; + continue; + } + + const firstLine = remaining[0].trim(); + + if (firstLine.startsWith(ADD_FILE_MARKER)) { + const path = firstLine.slice(ADD_FILE_MARKER.length); + let contents = ""; + let consumed = 1; + + for (let i = 1; i < remaining.length; i++) { + const line = remaining[i]; + if (line.startsWith("+")) { + contents += `${line.slice(1)}\n`; + consumed++; + } else { + break; + } + } + + hunks.push({ path, op: "create", diff: contents }); + remaining = remaining.slice(consumed); + lineNumber += consumed; + continue; + } + + if (firstLine.startsWith(DELETE_FILE_MARKER)) { + const path = firstLine.slice(DELETE_FILE_MARKER.length); + hunks.push({ path, op: "delete" }); + remaining = remaining.slice(1); + lineNumber++; + continue; + } + + if (firstLine.startsWith(UPDATE_FILE_MARKER)) { + const path = firstLine.slice(UPDATE_FILE_MARKER.length); + remaining = remaining.slice(1); + lineNumber++; + + let movePath: string | undefined; + if (remaining.length > 0 && remaining[0].startsWith(MOVE_TO_MARKER)) { + movePath = remaining[0].slice(MOVE_TO_MARKER.length); + remaining = remaining.slice(1); + lineNumber++; + } + + // The body runs until the next file-op marker or end of input. + // `*** End of File` is a chunk-terminator and stays inside the body — + // the downstream unified-diff parser handles it. + const diffLines: string[] = []; + while (remaining.length > 0) { + const line = remaining[0]; + if ( + line.startsWith("*** Add File:") || + line.startsWith("*** Delete File:") || + line.startsWith("*** Update File:") + ) { + break; + } + diffLines.push(line); + remaining = remaining.slice(1); + lineNumber++; + } + + if (diffLines.length === 0) { + throw new ParseError(`Update file hunk for path '${path}' is empty`, lineNumber); + } + + hunks.push({ path, op: "update", rename: movePath, diff: diffLines.join("\n") }); + continue; + } + + throw new ParseError( + `'${firstLine}' is not a valid hunk header. Valid hunk headers: '*** Add File: {path}', '*** Delete File: {path}', '*** Update File: {path}'`, + lineNumber, + ); + } + + return hunks; +} diff --git a/packages/coding-agent/src/edit/index.ts b/packages/coding-agent/src/edit/index.ts index c658be514..0728d541e 100644 --- a/packages/coding-agent/src/edit/index.ts +++ b/packages/coding-agent/src/edit/index.ts @@ -8,6 +8,7 @@ import { type WritethroughDeferredHandle, writethroughNoop, } from "../lsp"; +import applyPatchDescription from "../prompts/tools/apply-patch.md" with { type: "text" }; import chunkEditDescription from "../prompts/tools/chunk-edit.md" with { type: "text" }; import hashlineDescription from "../prompts/tools/hashline.md" with { type: "text" }; import patchDescription from "../prompts/tools/patch.md" with { type: "text" }; @@ -16,6 +17,13 @@ import type { ToolSession } from "../tools"; import { VimTool, vimSchema } from "../tools/vim"; import { type EditMode, normalizeEditMode, resolveEditMode } from "../utils/edit-mode"; import type { VimToolDetails } from "../vim/types"; +import { + type ApplyPatchParams, + applyPatchSchema, + expandApplyPatchToEntries, + isApplyPatchParams, +} from "./modes/apply-patch"; +import applyPatchGrammar from "./modes/apply-patch.lark" with { type: "text" }; import { type ChunkParams, type ChunkToolEdit, @@ -50,8 +58,10 @@ import { import { type EditToolDetails, type EditToolPerFileResult, getLspBatchRequest, type LspBatchRequest } from "./renderer"; export { DEFAULT_EDIT_MODE, type EditMode, normalizeEditMode } from "../utils/edit-mode"; +export * from "./apply-patch"; export * from "./diff"; export * from "./line-hash"; +export * from "./modes/apply-patch"; export * from "./modes/chunk"; export * from "./modes/hashline"; export * from "./modes/patch"; @@ -64,10 +74,11 @@ type TInput = | typeof patchEditSchema | typeof hashlineEditParamsSchema | typeof chunkEditParamsSchema - | typeof vimSchema; + | typeof vimSchema + | typeof applyPatchSchema; type VimParams = Static; -type EditParams = ReplaceParams | PatchParams | HashlineParams | ChunkParams | VimParams; +type EditParams = ReplaceParams | PatchParams | HashlineParams | ChunkParams | VimParams | ApplyPatchParams; type EditToolResultDetails = EditToolDetails | VimToolDetails; type EditModeDefinition = { @@ -265,6 +276,28 @@ export class EditTool implements AgentTool { return this.#getModeDefinition().parameters; } + /** + * When in `apply_patch` mode, expose the Codex Lark grammar so providers + * that support OpenAI-style custom tools can emit a grammar-constrained + * variant. Providers that don't support custom tools ignore this field + * and fall back to emitting a JSON function tool from `parameters`. + */ + get customFormat(): { syntax: "lark"; definition: string } | undefined { + if (this.mode !== "apply_patch") return undefined; + return { syntax: "lark", definition: applyPatchGrammar }; + } + + /** + * Wire-level tool name used when the custom-tool variant is active. GPT-5+ + * is trained on the literal name `apply_patch`; internally this is just a + * mode of the `edit` tool. The agent-loop dispatcher matches both the + * internal `name` and `customWireName`, so returned calls route correctly. + */ + get customWireName(): string | undefined { + if (this.mode !== "apply_patch") return undefined; + return "apply_patch"; + } + async execute( _toolCallId: string, params: EditParams, @@ -346,6 +379,36 @@ export class EditTool implements AgentTool { return executePerFile(entries, batchRequest, onUpdate); }, }, + apply_patch: { + description: () => prompt.render(applyPatchDescription), + parameters: applyPatchSchema, + invalidParamsMessage: "Invalid edit parameters for apply_patch mode.", + validate: isApplyPatchParams, + execute: ( + tool: EditTool, + params: EditParams, + signal: AbortSignal | undefined, + batchRequest: LspBatchRequest | undefined, + onUpdate?: (partialResult: AgentToolResult) => void, + ) => { + const entries = expandApplyPatchToEntries(params as ApplyPatchParams); + const perFile = entries.map((entry: PatchEditEntry) => ({ + path: entry.path, + run: (br: LspBatchRequest | undefined) => + executePatchSingle({ + session: tool.session, + params: entry, + signal, + batchRequest: br, + allowFuzzy: tool.#allowFuzzy, + fuzzyThreshold: tool.#fuzzyThreshold, + writethrough: tool.#writethrough, + beginDeferredDiagnosticsForPath: p => tool.#beginDeferredDiagnosticsForPath(p), + }), + })); + return executePerFile(perFile, batchRequest, onUpdate); + }, + }, hashline: { description: () => prompt.render(hashlineDescription), parameters: hashlineEditParamsSchema, diff --git a/packages/coding-agent/src/edit/modes/apply-patch.lark b/packages/coding-agent/src/edit/modes/apply-patch.lark new file mode 100644 index 000000000..5aa41b0af --- /dev/null +++ b/packages/coding-agent/src/edit/modes/apply-patch.lark @@ -0,0 +1,19 @@ +start: begin_patch hunk+ end_patch +begin_patch: "*** Begin Patch" LF +end_patch: "*** End Patch" LF? + +hunk: add_hunk | delete_hunk | update_hunk +add_hunk: "*** Add File: " filename LF add_line+ +delete_hunk: "*** Delete File: " filename LF +update_hunk: "*** Update File: " filename LF change_move? change? + +filename: /(.+)/ +add_line: "+" /(.*)/ LF -> line + +change_move: "*** Move to: " filename LF +change: (change_context | change_line)+ eof_line? +change_context: ("@@" | "@@ " /(.+)/) LF +change_line: ("+" | "-" | " ") /(.*)/ LF +eof_line: "*** End of File" LF + +%import common.LF diff --git a/packages/coding-agent/src/edit/modes/apply-patch.ts b/packages/coding-agent/src/edit/modes/apply-patch.ts new file mode 100644 index 000000000..16dba69cb --- /dev/null +++ b/packages/coding-agent/src/edit/modes/apply-patch.ts @@ -0,0 +1,51 @@ +/** + * Edit mode wrapper for the Codex `apply_patch` envelope format. + * + * The mode accepts a single `input` string containing a full + * `*** Begin Patch ... *** End Patch` block, parses it, and fans out to + * the existing `executePatchSingle` — so all the machinery (plan mode, + * LSP writethrough, fs-cache invalidation, diagnostics) is shared with + * the `patch` mode. + */ + +import { type Static, Type } from "@sinclair/typebox"; +import { parseApplyPatch } from "../apply-patch/parser"; +import { ApplyPatchError } from "../diff"; +import type { PatchEditEntry } from "./patch"; + +export const applyPatchSchema = Type.Object({ + input: Type.String({ + description: + "Full Codex apply_patch envelope, including '*** Begin Patch' and '*** End Patch'. Contains any mix of Add/Delete/Update (with optional Move to) file operations.", + }), +}); + +export type ApplyPatchParams = Static; + +export function isApplyPatchParams(params: unknown): params is ApplyPatchParams { + return ( + typeof params === "object" && + params !== null && + "input" in params && + typeof (params as { input: unknown }).input === "string" + ); +} + +/** + * Parse the envelope and lower each hunk to a `PatchEditEntry` so it can + * be routed through `executePatchSingle`. + */ +export function expandApplyPatchToEntries(params: ApplyPatchParams): PatchEditEntry[] { + const hunks = parseApplyPatch(params.input); + if (hunks.length === 0) { + throw new ApplyPatchError("No files were modified."); + } + return hunks.map( + (h): PatchEditEntry => ({ + path: h.path, + op: h.op, + rename: h.rename, + diff: h.diff, + }), + ); +} diff --git a/packages/coding-agent/src/edit/renderer.ts b/packages/coding-agent/src/edit/renderer.ts index f002603a7..b1d5fd750 100644 --- a/packages/coding-agent/src/edit/renderer.ts +++ b/packages/coding-agent/src/edit/renderer.ts @@ -26,9 +26,10 @@ import { type VimRenderArgs, vimToolRenderer } from "../tools/vim"; import { Hasher, type RenderCache, renderStatusLine, truncateToWidth } from "../tui"; import type { VimToolDetails } from "../vim/types"; import type { DiffError, DiffResult } from "./diff"; +import { expandApplyPatchToEntries } from "./modes/apply-patch"; import { type ChunkToolEdit, parseChunkEditPath } from "./modes/chunk"; import type { HashlineToolEdit } from "./modes/hashline"; -import type { Operation } from "./modes/patch"; +import type { Operation, PatchEditEntry } from "./modes/patch"; // ═══════════════════════════════════════════════════════════════════════════ // LSP Batching @@ -79,6 +80,7 @@ interface EditRenderArgs { oldText?: string; newText?: string; patch?: string; + input?: string; all?: boolean; // Patch mode fields op?: Operation; @@ -89,7 +91,19 @@ interface EditRenderArgs { */ previewDiff?: string; // Hashline / chunk mode fields - edits?: Partial[]; + edits?: EditRenderEntry[]; +} + +type EditRenderEntry = { + path?: string; + rename?: string; + move?: string; + op?: Operation; +}; + +interface ApplyPatchRenderSummary { + entries: PatchEditEntry[]; + error?: string; } function isVimRenderArgs(args: EditRenderArgs | VimRenderArgs): args is VimRenderArgs { @@ -145,8 +159,8 @@ function filePathFromEditEntry(p: string | undefined): string | undefined { } /** Count distinct file paths in an edits array. */ -function countEditFiles(edits: any[]): number { - return new Set(edits.map((e: any) => filePathFromEditEntry(e?.path)).filter(Boolean)).size; +function countEditFiles(edits: EditRenderEntry[]): number { + return new Set(edits.map(edit => filePathFromEditEntry(edit.path)).filter(Boolean)).size; } function countLines(text: string): number { @@ -352,6 +366,18 @@ function getCallPreview(args: EditRenderArgs, rawPath: string, uiTheme: Theme): return ""; } +function getApplyPatchRenderSummary(args: EditRenderArgs): ApplyPatchRenderSummary | undefined { + if (typeof args.input !== "string") { + return undefined; + } + + try { + return { entries: expandApplyPatchToEntries({ input: args.input }) }; + } catch (err) { + return { entries: [], error: err instanceof Error ? err.message : String(err) }; + } +} + function renderDiffSection( diff: string, rawPath: string, @@ -419,21 +445,28 @@ export const editToolRenderer = { return vimToolRenderer.renderCall(args, options, uiTheme); } + const applyPatchSummary = getApplyPatchRenderSummary(args); + const firstApplyPatchEntry = applyPatchSummary?.entries[0]; // Extract path from first edit entry when top-level path is absent (new schema) const firstEdit = Array.isArray(args.edits) && args.edits.length > 0 ? args.edits[0] : undefined; - const rawPath = args.file_path || args.path || filePathFromEditEntry((firstEdit as any)?.path) || ""; - const rename = args.rename || (firstEdit as any)?.rename; - const op = args.op || (firstEdit as any)?.op; + const rawPath = args.file_path || args.path || filePathFromEditEntry(firstEdit?.path) || firstApplyPatchEntry?.path || ""; + const rename = args.rename || firstEdit?.rename || firstEdit?.move || firstApplyPatchEntry?.rename; + const op = args.op || firstEdit?.op || firstApplyPatchEntry?.op; const { description } = formatEditDescription(rawPath, uiTheme, { rename }); const spinner = options?.spinnerFrame !== undefined ? formatStatusIcon("running", uiTheme, options.spinnerFrame) : ""; let text = `${formatTitle(getOperationTitle(op), uiTheme)} ${spinner ? `${spinner} ` : ""}${description}`; // Show file count hint for multi-file edits - const fileCount = Array.isArray(args.edits) ? countEditFiles(args.edits as any[]) : 0; + const fileCount = Array.isArray(args.edits) + ? countEditFiles(args.edits) + : (applyPatchSummary?.entries.length ?? 0); if (fileCount > 1) { text += uiTheme.fg("dim", ` (+${fileCount - 1} more)`); } text += getCallPreview(args, rawPath, uiTheme); + if (applyPatchSummary?.error) { + text += `\n\n${uiTheme.fg("error", truncateToWidth(replaceTabs(applyPatchSummary.error), CALL_TEXT_PREVIEW_WIDTH))}`; + } return new Text(text, 0, 0); }, diff --git a/packages/coding-agent/src/modes/components/tool-execution.ts b/packages/coding-agent/src/modes/components/tool-execution.ts index b9bcac4a0..6d2ca4525 100644 --- a/packages/coding-agent/src/modes/components/tool-execution.ts +++ b/packages/coding-agent/src/modes/components/tool-execution.ts @@ -14,7 +14,14 @@ import { type TUI, } from "@oh-my-pi/pi-tui"; import { getProjectDir, logger } from "@oh-my-pi/pi-utils"; -import { computeEditDiff, computeHashlineDiff, computePatchDiff, type DiffError, type DiffResult } from "../../edit"; +import { + computeEditDiff, + computeHashlineDiff, + computePatchDiff, + type DiffError, + type DiffResult, + expandApplyPatchToEntries, +} from "../../edit"; import type { Theme } from "../../modes/theme/theme"; import { theme } from "../../modes/theme/theme"; import { BASH_DEFAULT_PREVIEW_LINES } from "../../tools/bash"; @@ -54,6 +61,10 @@ function cloneToolArgs(args: T): T { } } +function isEditLikeToolName(toolName: string): boolean { + return toolName === "edit" || toolName === "apply_patch"; +} + export interface ToolExecutionOptions { showImages?: boolean; // default: true (only used if terminal supports images) editFuzzyThreshold?: number; @@ -166,7 +177,7 @@ export class ToolExecutionComponent extends Container { /** * Signal that args are complete (tool is about to execute). - * This triggers diff computation for edit tool. + * This triggers diff computation for edit-like tools. */ setArgsComplete(_toolCallId?: string): void { this.#argsComplete = true; @@ -179,10 +190,39 @@ export class ToolExecutionComponent extends Container { * This runs async and updates display when done. */ #maybeComputeEditDiff(): void { - if (this.#toolName !== "edit") return; + if (!isEditLikeToolName(this.#toolName)) return; const edits = this.#args?.edits; - if (!Array.isArray(edits) || edits.length === 0) return; + if (!Array.isArray(edits) || edits.length === 0) { + if (this.#toolName !== "apply_patch" || typeof this.#args?.input !== "string") { + return; + } + + const input = this.#args.input; + const argsKey = JSON.stringify({ input }); + if (this.#editDiffArgsKey === argsKey) return; + this.#editDiffArgsKey = argsKey; + + try { + const first = expandApplyPatchToEntries({ input })[0]; + if (!first?.path) return; + computePatchDiff(first, this.#cwd, { + fuzzyThreshold: this.#editFuzzyThreshold, + allowFuzzy: this.#editAllowFuzzy, + }).then(result => { + if (this.#editDiffArgsKey === argsKey) { + this.#editDiffPreview = result; + this.#updateDisplay(); + this.#ui.requestRender(); + } + }); + } catch (err) { + this.#editDiffPreview = { error: err instanceof Error ? err.message : String(err) }; + this.#updateDisplay(); + this.#ui.requestRender(); + } + return; + } const first = edits[0]; if (!first || typeof first !== "object") return; @@ -309,7 +349,7 @@ export class ToolExecutionComponent extends Container { */ #updateSpinnerAnimation(): void { // Spinner for: task tool with partial result, or edit/write while args streaming - const isStreamingArgs = !this.#argsComplete && (this.#toolName === "edit" || this.#toolName === "write"); + const isStreamingArgs = !this.#argsComplete && (isEditLikeToolName(this.#toolName) || this.#toolName === "write"); const isBackgroundAsyncTask = this.#toolName === "task" && (this.#result?.details as { async?: { state?: string } } | undefined)?.async?.state === "running"; @@ -603,7 +643,7 @@ export class ToolExecutionComponent extends Container { } #getCallArgsForRender(): any { - if (this.#toolName !== "edit") { + if (!isEditLikeToolName(this.#toolName)) { return this.#args; } if (!this.#editDiffPreview || !("diff" in this.#editDiffPreview) || !this.#editDiffPreview.diff) { @@ -639,7 +679,7 @@ export class ToolExecutionComponent extends Container { context.expanded = this.#expanded; context.previewLines = PYTHON_DEFAULT_PREVIEW_LINES; context.timeout = normalizeTimeoutSeconds(this.#args?.timeout, 600); - } else if (this.#toolName === "edit") { + } else if (isEditLikeToolName(this.#toolName)) { // Edit needs diff preview and renderDiff function context.editDiffPreview = this.#editDiffPreview; context.renderDiff = renderDiff; diff --git a/packages/coding-agent/src/prompts/tools/apply-patch.md b/packages/coding-agent/src/prompts/tools/apply-patch.md new file mode 100644 index 000000000..317df3361 --- /dev/null +++ b/packages/coding-agent/src/prompts/tools/apply-patch.md @@ -0,0 +1,69 @@ +## `apply_patch` + +Use the `apply_patch` shell command to edit files. +Your patch language is a stripped‑down, file‑oriented diff format designed to be easy to parse and safe to apply. You can think of it as a high‑level envelope: + +*** Begin Patch +[ one or more file sections ] +*** End Patch + +Within that envelope, you get a sequence of file operations. +You MUST include a header to specify the action you are taking. +Each operation starts with one of three headers: + +*** Add File: - create a new file. Every following line is a + line (the initial contents). +*** Delete File: - remove an existing file. Nothing follows. +*** Update File: - patch an existing file in place (optionally with a rename). + +May be immediately followed by *** Move to: if you want to rename the file. +Then one or more "hunks", each introduced by @@ (optionally followed by a hunk header). +Within a hunk each line starts with: + +For instructions on [context_before] and [context_after]: +- By default, show 3 lines of code immediately above and 3 lines immediately below each change. If a change is within 3 lines of a previous change, do NOT duplicate the first change's [context_after] lines in the second change's [context_before] lines. +- If 3 lines of context is insufficient to uniquely identify the snippet of code within the file, use the @@ operator to indicate the class or function to which the snippet belongs. For instance, we might have: +@@ class BaseClass +[3 lines of pre-context] +- [old_code] ++ [new_code] +[3 lines of post-context] + +- If a code block is repeated so many times in a class or function such that even a single `@@` statement and 3 lines of context cannot uniquely identify the snippet of code, you can use multiple `@@` statements to jump to the right context. For instance: + +@@ class BaseClass +@@ def method(): +[3 lines of pre-context] +- [old_code] ++ [new_code] +[3 lines of post-context] + +The full grammar definition is below: +Patch := Begin { FileOp } End +Begin := "*** Begin Patch" NEWLINE +End := "*** End Patch" NEWLINE +FileOp := AddFile | DeleteFile | UpdateFile +AddFile := "*** Add File: " path NEWLINE { "+" line NEWLINE } +DeleteFile := "*** Delete File: " path NEWLINE +UpdateFile := "*** Update File: " path NEWLINE [ MoveTo ] { Hunk } +MoveTo := "*** Move to: " newPath NEWLINE +Hunk := "@@" [ header ] NEWLINE { HunkLine } [ "*** End of File" NEWLINE ] +HunkLine := (" " | "-" | "+") text NEWLINE + +A full patch can combine several operations: + +*** Begin Patch +*** Add File: hello.txt ++Hello world +*** Update File: src/app.py +*** Move to: src/main.py +@@ def greet(): +-print("Hi") ++print("Hello, world!") +*** Delete File: obsolete.txt +*** End Patch + +It is important to remember: + +- You must include a header with your intended action (Add/Delete/Update) +- You must prefix new lines with `+` even when creating a new file +- File references can only be relative, NEVER ABSOLUTE. diff --git a/packages/coding-agent/src/tools/renderers.ts b/packages/coding-agent/src/tools/renderers.ts index e14e89569..cda88dc08 100644 --- a/packages/coding-agent/src/tools/renderers.ts +++ b/packages/coding-agent/src/tools/renderers.ts @@ -51,6 +51,7 @@ export const toolRenderers: Record = { python: pythonToolRenderer as ToolRenderer, calc: calculatorToolRenderer as ToolRenderer, edit: editToolRenderer as ToolRenderer, + apply_patch: editToolRenderer as ToolRenderer, find: findToolRenderer as ToolRenderer, grep: grepToolRenderer as ToolRenderer, lsp: lspToolRenderer as ToolRenderer, diff --git a/packages/coding-agent/src/utils/edit-mode.ts b/packages/coding-agent/src/utils/edit-mode.ts index 6f187ec7e..1f795338c 100644 --- a/packages/coding-agent/src/utils/edit-mode.ts +++ b/packages/coding-agent/src/utils/edit-mode.ts @@ -1,10 +1,11 @@ import { $env, $flag } from "@oh-my-pi/pi-utils"; -export type EditMode = "replace" | "patch" | "hashline" | "chunk" | "vim"; +export type EditMode = "replace" | "patch" | "hashline" | "chunk" | "vim" | "apply_patch"; export const DEFAULT_EDIT_MODE: EditMode = "hashline"; const EDIT_MODE_IDS = { + apply_patch: "apply_patch", chunk: "chunk", hashline: "hashline", patch: "patch", diff --git a/packages/coding-agent/test/core/apply-patch.test.ts b/packages/coding-agent/test/core/apply-patch.test.ts index 1e66d69b6..6b000874b 100644 --- a/packages/coding-agent/test/core/apply-patch.test.ts +++ b/packages/coding-agent/test/core/apply-patch.test.ts @@ -4,24 +4,20 @@ import * as os from "node:os"; import * as path from "node:path"; import { ApplyPatchError, + applyCodexPatch, applyPatch, ParseError, - type PatchInput, + parseApplyPatch, parseDiffHunks, seekSequence, } from "@oh-my-pi/pi-coding-agent/edit"; // ═══════════════════════════════════════════════════════════════════════════ -// Legacy parser for test fixtures (*** Begin Patch format) +// Test-local adapters over the production Codex envelope API. +// (Kept as thin shims so the pre-existing tests keep their original shape; +// the production API returns PatchInput[] directly.) // ═══════════════════════════════════════════════════════════════════════════ -const BEGIN_PATCH_MARKER = "*** Begin Patch"; -const END_PATCH_MARKER = "*** End Patch"; -const ADD_FILE_MARKER = "*** Add File: "; -const DELETE_FILE_MARKER = "*** Delete File: "; -const UPDATE_FILE_MARKER = "*** Update File: "; -const MOVE_TO_MARKER = "*** Move to: "; - type LegacyHunk = | { type: "add"; path: string; contents: string } | { type: "delete"; path: string } @@ -32,139 +28,16 @@ interface LegacyParseResult { } function parseLegacyPatch(patch: string): LegacyParseResult { - let lines = patch.trim().split("\n"); - - // Try lenient heredoc mode first - if ( - lines.length >= 4 && - (lines[0] === "< 0) { - if (remainingLines[0].trim() === "") { - remainingLines = remainingLines.slice(1); - lineNumber++; - continue; - } - - const firstLine = remainingLines[0].trim(); - - // Add File - if (firstLine.startsWith(ADD_FILE_MARKER)) { - const path = firstLine.slice(ADD_FILE_MARKER.length); - let contents = ""; - let linesConsumed = 1; - - for (let i = 1; i < remainingLines.length; i++) { - const line = remainingLines[i]; - if (line.startsWith("+")) { - contents += `${line.slice(1)}\n`; - linesConsumed++; - } else { - break; - } - } - - hunks.push({ type: "add", path, contents }); - remainingLines = remainingLines.slice(linesConsumed); - lineNumber += linesConsumed; - continue; - } - - // Delete File - if (firstLine.startsWith(DELETE_FILE_MARKER)) { - const path = firstLine.slice(DELETE_FILE_MARKER.length); - hunks.push({ type: "delete", path }); - remainingLines = remainingLines.slice(1); - lineNumber++; - continue; - } - - // Update File - if (firstLine.startsWith(UPDATE_FILE_MARKER)) { - const path = firstLine.slice(UPDATE_FILE_MARKER.length); - remainingLines = remainingLines.slice(1); - lineNumber++; - - let movePath: string | undefined; - if (remainingLines.length > 0 && remainingLines[0].startsWith(MOVE_TO_MARKER)) { - movePath = remainingLines[0].slice(MOVE_TO_MARKER.length); - remainingLines = remainingLines.slice(1); - lineNumber++; - } - - // Collect diff body until next file marker or end - const diffLines: string[] = []; - while (remainingLines.length > 0) { - const line = remainingLines[0]; - // Stop at file operation markers (but not *** End of File) - if ( - line.startsWith("*** Add File:") || - line.startsWith("*** Delete File:") || - line.startsWith("*** Update File:") - ) { - break; - } - diffLines.push(remainingLines[0]); - remainingLines = remainingLines.slice(1); - lineNumber++; - } - - if (diffLines.length === 0) { - throw new ParseError(`Update file hunk for path '${path}' is empty`, lineNumber); - } - - hunks.push({ type: "update", path, movePath, diffBody: diffLines.join("\n") }); - continue; - } - - throw new ParseError( - `'${firstLine}' is not a valid hunk header. Valid: '*** Add File:', '*** Delete File:', '*** Update File:'`, - lineNumber, - ); - } - + const hunks = parseApplyPatch(patch).map((h): LegacyHunk => { + if (h.op === "create") return { type: "add", path: h.path, contents: h.diff ?? "" }; + if (h.op === "delete") return { type: "delete", path: h.path }; + return { type: "update", path: h.path, movePath: h.rename, diffBody: h.diff ?? "" }; + }); return { hunks }; } -/** Convert legacy hunk to new PatchInput format */ -function legacyHunkToInput(hunk: LegacyHunk): PatchInput { - if (hunk.type === "add") { - return { path: hunk.path, op: "create", diff: hunk.contents }; - } - if (hunk.type === "delete") { - return { path: hunk.path, op: "delete" }; - } - return { path: hunk.path, op: "update", rename: hunk.movePath, diff: hunk.diffBody }; -} - -/** Apply a legacy format patch (for test fixtures) */ async function applyLegacyPatch(patch: string, options: { cwd: string }) { - const { hunks } = parseLegacyPatch(patch); - - if (hunks.length === 0) { - throw new ApplyPatchError("No files were modified."); - } - - for (const hunk of hunks) { - const input = legacyHunkToInput(hunk); - await applyPatch(input, options); - } + await applyCodexPatch(patch, options); } // ═══════════════════════════════════════════════════════════════════════════ @@ -696,3 +569,151 @@ describe("simple replace mode", () => { ).rejects.toThrow(/2 occurrences/); }); }); + +// ═══════════════════════════════════════════════════════════════════════════ +// Production Codex envelope API — spec §10 edge-case coverage +// ═══════════════════════════════════════════════════════════════════════════ + +describe("parseApplyPatch (production)", () => { + const wrap = (body: string) => `*** Begin Patch\n${body}\n*** End Patch`; + + test("returns PatchInput[] shape directly", () => { + const result = parseApplyPatch(wrap("*** Add File: foo.txt\n+hi")); + expect(result).toEqual([{ path: "foo.txt", op: "create", diff: "hi\n" }]); + }); + + test("maps update with rename to op=update + rename field", () => { + const result = parseApplyPatch(wrap("*** Update File: a.py\n*** Move to: b.py\n@@\n-old\n+new")); + expect(result[0]).toMatchObject({ path: "a.py", op: "update", rename: "b.py" }); + expect(result[0].diff).toContain("-old"); + }); + + test("zero-hunk patch returns empty array", () => { + expect(parseApplyPatch(wrap(""))).toEqual([]); + }); + + test("heredoc wrapper with double quotes is stripped", () => { + const inner = wrap("*** Add File: x.txt\n+content"); + const wrapped = `<<"EOF"\n${inner}\nEOF`; + const result = parseApplyPatch(wrapped); + expect(result).toHaveLength(1); + expect(result[0].op).toBe("create"); + }); + + test("heredoc wrapper with bare EOF is stripped", () => { + const inner = wrap("*** Add File: x.txt\n+content"); + const wrapped = `< { + const inner = wrap("*** Add File: x.txt\n+content"); + // `<<"EOF'` — opener has mismatched quotes; parser should not strip it, + // so the begin-patch check fails. + const bad = `<<"EOF'\n${inner}\nEOF`; + expect(() => parseApplyPatch(bad)).toThrow(ParseError); + }); + + test("unknown file directive is rejected with spec message", () => { + expect(() => parseApplyPatch(wrap("*** Rename File: a"))).toThrow(/is not a valid hunk header/); + }); + + test("preserves *** End of File marker inside update body", () => { + const result = parseApplyPatch(wrap("*** Update File: a.py\n@@\n-x\n+y\n*** End of File")); + expect(result[0].diff).toContain("*** End of File"); + }); +}); + +describe("applyCodexPatch (production)", () => { + let tempDir: string; + + beforeEach(() => { + tempDir = path.join(os.tmpdir(), `codex-patch-${Date.now()}-${Math.random().toString(36).slice(2)}`); + fs.mkdirSync(tempDir, { recursive: true }); + }); + + afterEach(() => { + try { + fs.rmSync(tempDir, { recursive: true, force: true }); + } catch { + // ignore + } + }); + + test("zero-hunk patch throws 'No files were modified.'", async () => { + await expect(applyCodexPatch("*** Begin Patch\n*** End Patch", { cwd: tempDir })).rejects.toThrow( + "No files were modified.", + ); + }); + + test("multi-op patch (add + update + delete) applies in order", async () => { + await Bun.write(path.join(tempDir, "old.txt"), "to delete\n"); + await Bun.write(path.join(tempDir, "keep.txt"), "hello\n"); + + const patch = [ + "*** Begin Patch", + "*** Add File: new.txt", + "+brand new", + "*** Update File: keep.txt", + "@@", + "-hello", + "+HELLO", + "*** Delete File: old.txt", + "*** End Patch", + ].join("\n"); + + const result = await applyCodexPatch(patch, { cwd: tempDir }); + + expect(result.affected.added).toEqual(["new.txt"]); + expect(result.affected.modified).toEqual(["keep.txt"]); + expect(result.affected.deleted).toEqual(["old.txt"]); + + expect(await Bun.file(path.join(tempDir, "new.txt")).text()).toBe("brand new\n"); + expect(await Bun.file(path.join(tempDir, "keep.txt")).text()).toBe("HELLO\n"); + expect(fs.existsSync(path.join(tempDir, "old.txt"))).toBe(false); + }); + + test("rename reports modified under original path (spec §9.1)", async () => { + await Bun.write(path.join(tempDir, "src.txt"), "body\n"); + + const patch = [ + "*** Begin Patch", + "*** Update File: src.txt", + "*** Move to: dst.txt", + "@@", + "-body", + "+body2", + "*** End Patch", + ].join("\n"); + + const result = await applyCodexPatch(patch, { cwd: tempDir }); + + expect(result.affected.modified).toEqual(["src.txt"]); + expect(result.affected.added).toEqual([]); + expect(result.affected.deleted).toEqual([]); + expect(fs.existsSync(path.join(tempDir, "src.txt"))).toBe(false); + expect(await Bun.file(path.join(tempDir, "dst.txt")).text()).toBe("body2\n"); + }); + + test("partial success: earlier ops stay applied when a later op fails", async () => { + await Bun.write(path.join(tempDir, "first.txt"), "a\n"); + + const patch = [ + "*** Begin Patch", + "*** Update File: first.txt", + "@@", + "-a", + "+A", + "*** Update File: missing.txt", + "@@", + "-x", + "+y", + "*** End Patch", + ].join("\n"); + + await expect(applyCodexPatch(patch, { cwd: tempDir })).rejects.toThrow(); + + // First op should have landed before the failure. + expect(await Bun.file(path.join(tempDir, "first.txt")).text()).toBe("A\n"); + }); +}); diff --git a/packages/coding-agent/test/tools/apply-patch-renderer.test.ts b/packages/coding-agent/test/tools/apply-patch-renderer.test.ts new file mode 100644 index 000000000..6358014b3 --- /dev/null +++ b/packages/coding-agent/test/tools/apply-patch-renderer.test.ts @@ -0,0 +1,116 @@ +import { describe, expect, it } from "bun:test"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { ToolExecutionComponent } from "@oh-my-pi/pi-coding-agent/modes/components/tool-execution"; +import * as themeModule from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; +import { editToolRenderer } from "@oh-my-pi/pi-coding-agent/edit/renderer"; +import { toolRenderers } from "@oh-my-pi/pi-coding-agent/tools/renderers"; +import type { TUI } from "@oh-my-pi/pi-tui"; + +async function getUiTheme() { + await themeModule.initTheme(false, undefined, undefined, "dark", "light"); + const theme = await themeModule.getThemeByName("dark"); + expect(theme).toBeDefined(); + return theme!; +} + +describe("apply_patch rendering", () => { + it("registers apply_patch to use the edit renderer", () => { + expect(toolRenderers.apply_patch).toBe(editToolRenderer); + }); + + it("renders apply_patch results through edit UI instead of generic fallback", async () => { + await getUiTheme(); + const uiStub = { requestRender() {} } as unknown as TUI; + + const component = new ToolExecutionComponent( + "apply_patch", + { + input: "*** Begin Patch\n*** Update File: src/demo.ts\n@@\n-old\n+new\n*** End Patch", + }, + {}, + undefined, + uiStub, + ); + + component.updateResult( + { + content: [{ type: "text", text: "" }], + details: { + path: "src/demo.ts", + op: "update", + diff: "@@\n-old\n+new", + }, + }, + false, + ); + + const rendered = Bun.stripANSI(component.render(140).join("\n")); + expect(rendered).toContain("src/demo.ts"); + expect(rendered).toContain("+new"); + expect(rendered).not.toContain("(no output)"); + }); + + it("derives call path, operation, and file-count hints from apply_patch input", async () => { + const uiTheme = await getUiTheme(); + const input = [ + "*** Begin Patch", + "*** Update File: src/first.ts", + "@@", + "-before", + "+after", + "*** Add File: src/new.ts", + "+hello", + "*** End Patch", + ].join("\n"); + + const component = editToolRenderer.renderCall({ input }, { expanded: false, isPartial: true }, uiTheme); + const rendered = Bun.stripANSI(component.render(160).join("\n")); + + expect(rendered).toContain("src/first.ts"); + expect(rendered).toContain("Edit"); + expect(rendered).toContain("(+1 more)"); + }); + + it("shows an apply_patch parse error preview for malformed input", async () => { + const uiTheme = await getUiTheme(); + const malformedInput = ["*** Begin Patch", "*** Update File: src/bad.ts", "*** End Patch"].join("\n"); + + const component = editToolRenderer.renderCall({ input: malformedInput }, { expanded: false, isPartial: true }, uiTheme); + const rendered = Bun.stripANSI(component.render(160).join("\n")); + + expect(rendered).toContain("src/bad.ts"); + expect(rendered).toContain("is empty"); + }); + + it("shows apply_patch preview diffs after args complete", async () => { + await getUiTheme(); + const uiStub = { requestRender() {} } as unknown as TUI; + const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "apply-patch-preview-")); + try { + await Bun.write(path.join(tmpDir, "preview.ts"), "const value = 1;\n"); + const input = [ + "*** Begin Patch", + "*** Update File: preview.ts", + "@@", + "-const value = 1;", + "+const value = 2;", + "*** End Patch", + ].join("\n"); + + const component = new ToolExecutionComponent("apply_patch", { input }, {}, undefined, uiStub, tmpDir); + const before = Bun.stripANSI(component.render(160).join("\n")); + expect(before).not.toContain("(preview)"); + + component.setArgsComplete(); + await Bun.sleep(50); + + const after = Bun.stripANSI(component.render(160).join("\n")); + expect(after).toContain("(preview)"); + expect(after).toContain("const value = 2;"); + } finally { + await fs.rm(tmpDir, { recursive: true, force: true }); + } + }); +});