Skip to content

fix(codex): denormalize Edit/Write to apply_patch on export - #60

Open
Drix10 wants to merge 1 commit into
skillsynchq:mainfrom
Drix10:fix/codex-apply-patch-denormalization
Open

Drix10 wants to merge 1 commit into
skillsynchq:mainfrom
Drix10:fix/codex-apply-patch-denormalization

Conversation

@Drix10

@Drix10 Drix10 commented Sep 22, 2026

Copy link
Copy Markdown

Issue: #59

What changed and why?

to_common already normalizes Codex's native tools into canonical ones
(exec_command/shellBash, apply_patchEdit/Write), but
from_common had no reverse mapping: it emitted the canonical names back as
function_call lines, which the Codex runtime cannot validate, breaking
txcript continue <id> --with codex.

Now from_common replays native shapes:

  • Before: {"type":"function_call","name":"Bash","arguments":"{...}"}
    After: {"type":"function_call","name":"exec_command","arguments":"{\"cmd\":\"ls\"}"}
  • Before: {"type":"function_call","name":"Edit",...}
    After: {"type":"custom_tool_call","name":"apply_patch","input":"*** Begin Patch\n*** Update File: src/main.rs\n@@\n-old\n+new\n*** End Patch"}
  • Write*** Add File envelope; multi-file Raw ApplyPatch passes its
    envelope through unwrapped. Patch results emit custom_tool_call_output
    (with exit_code carrying the error bit) so they pair on replay.
  • Bash extras with no native slot (timeout, description, background) are
    dropped — same lossiness as the inbound normalizer, noted in a comment.
    replace_all degrades to a single application (no envelope equivalent).

Tests: extended the codex integration module per tests/README.md
exec_command emission asserts the parsed cmd field and that no "Bash"
name leaks; new test proves the Edit envelope round-trips through
to_common losslessly, including is_error. Reproduced first: the three
from_common_denormalizes_* tests fail on main, all 12 pass with the fix.

Checks: cargo test -p txcript --no-default-features --test integration codex
(12 passed), cargo fmt --check and cargo clippy -p txcript --all-targets
(zero warnings) — run under the GNU toolchain, as my machine has no MSVC
linker; CI's MSVC run will confirm.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant