-
Notifications
You must be signed in to change notification settings - Fork 51
feat: codex qol updates #348
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
base: main
Are you sure you want to change the base?
Conversation
…ation script for new options
…ional MCP server settings
…configuration details
…r instruction prompts
…with default configuration details
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 refactors the Codex module for better usability and configuration management. It simplifies variable names, improves defaults, and consolidates configuration files outside the project directory.
- Replaced granular configuration variables with
base_config_toml
andadditional_mcp_servers
for more flexible configuration - Set secure defaults to enable minimal configuration while supporting advanced customization
- Moved all Codex-related files from the project directory to the
~/.codex
directory
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
main.tf | Consolidated configuration variables and updated variable descriptions with better defaults |
scripts/install.sh | Refactored configuration generation logic and moved AGENTS.md to ~/.codex directory |
scripts/start.sh | Improved logging output formatting and updated comments for clarity |
main.test.ts | Updated tests to reflect new configuration structure and file locations |
README.md | Updated documentation to reflect new configuration approach and examples |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
…y were just expressing the defaults
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.
A few suggestions and LGTM.
} | ||
``` | ||
|
||
> [!WARNING] | ||
> **Security Notice**: This module configures Codex with a `workspace-write` sandbox that allows AI tasks to read/write files in the specified folder. While the sandbox provides security boundaries, Codex can still modify files within the workspace. Use this module in trusted environments and be aware of the security implications. |
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.
Why this change? We do prefer GFM style alerts.
@@ -150,6 +140,6 @@ module "codex" { | |||
|
|||
## References | |||
|
|||
- [OpenAI API Documentation](https://platform.openai.com/docs) | |||
- [Codex CLI Documentation](https://github.com/openai/codex) |
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.
🎉
Co-authored-by: Atif Ali <[email protected]>
Co-authored-by: Atif Ali <[email protected]>
Co-authored-by: Atif Ali <[email protected]>
Co-authored-by: Atif Ali <[email protected]>
@@ -53,41 +53,21 @@ variable "codex_version" { | |||
default = "" # empty string means the latest available version | |||
} | |||
|
|||
variable "extra_codex_settings_toml" { | |||
variable "base_config_toml" { |
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.
@DevelopmentCats, are these breaking changes? We are changing the input names?
Description
Type of Change
Module Information
Path:
registry/coder-labs/modules/codex
New version:
v1.1.0
Breaking change: [X] Yes [ ] No
Testing & Validation
bun test
)bun run fmt
)Related Issues