[codex] Support Codex home instructions directory#28838
[codex] Support Codex home instructions directory#28838charlesgong-openai wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3b191bc08
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
|
|
||
| let data = match tokio::fs::read(&path).await { |
There was a problem hiding this comment.
Cap home instruction snippets before injection
When a user places a large .md file under ~/.codex/instructions, this reads the entire file and later concatenates it into UserInstructions sent to the model; unlike project AGENTS.md loading, the new directory path has no per-file or aggregate hard cap, so a single file can exceed the model-visible context budget and startup memory expectations. Please enforce a bounded size/token budget before accepting these snippets.
AGENTS.md reference: AGENTS.md:L98-L100
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| if has_multiple_sources { | ||
| source = self.codex_home.join(INSTRUCTIONS_DIR_NAME); |
There was a problem hiding this comment.
Report actual instruction files, not the directory
When both an existing home AGENTS file and any instructions/*.md file are loaded, this collapses the reported source to ~/.codex/instructions, so app-server instructionSources clients receive a directory and lose the actual AGENTS.md/snippet file paths that supplied the thread instructions. That breaks the API’s existing “loaded instruction source files” semantics for users who add the new directory alongside their current AGENTS file; please preserve and return each contributing file path instead of replacing them with the directory.
AGENTS.md reference: AGENTS.md:L103-L108
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn instructions_directory_markdown_files_are_appended_in_sorted_order() { |
There was a problem hiding this comment.
Add an integration test for directory instructions
These new tests only exercise the provider in isolation, but the change alters the model-visible instructions that fresh threads send through ThreadManager/Session; the repo requires integration coverage for agent-logic changes, so a wiring regression could still pass codex-home tests while instructions/*.md never reaches the actual Responses request. Please add a core/suite test_codex integration test that starts a thread with a home instructions/*.md file and asserts the rendered AGENTS fragment includes it.
AGENTS.md reference: AGENTS.md:L114-L116
Useful? React with 👍 / 👎.
Summary
*.mdfiles directly under~/.codex/instructions/as global Codex home instructions.AGENTS.override.md/AGENTS.mdprecedence and append instruction-directory snippets in deterministic sorted order.Validation
just fmtjust test -p codex-homegit diff --check