-
Notifications
You must be signed in to change notification settings - Fork 58
feat: update support for codex #41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
cc: @matifali @hugodutka |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the message formatting support for the Codex agent type by introducing a specialized formatter that handles Codex-specific message box patterns. The changes enable proper parsing and extraction of relevant content from Codex agent responses.
- Adds a new
formatCodexMessage
function to handle Codex-specific message formatting - Implements
removeCodexMessageBox
function to parse Codex's unique message box patterns - Updates the agent message formatter to use the new Codex-specific logic
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
lib/msgfmt/msgfmt.go | Adds formatCodexMessage function and updates FormatAgentMessage to use Codex-specific formatting |
lib/msgfmt/message_box.go | Implements removeCodexMessageBox function to handle Codex message box parsing |
lib/msgfmt/testdata/format/codex/* | Updates test data files to reflect new Codex message formatting behavior |
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for contributing. The PR largely look good. I have 2 asks:
- Could you add some tests for multi-line user input?
- Could you note in the README that if you're using Codex you should make sure to specify agent type when starting
agentapi server
? Since we used the same logic for every agent before it didn't matter, but now it does.
@hugodutka made the required changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. Two minor nits and we can merge it.
Co-authored-by: Hugo Dutka <[email protected]>
Co-authored-by: Hugo Dutka <[email protected]>
https://www.loom.com/share/af76ebf12da14b83b84f53298d947343?sid=ff871cfa-4cd7-4fe7-a576-f0b92e65ad95