Skip to content

[codex] Support Codex home instructions directory#28838

Draft
charlesgong-openai wants to merge 3 commits into
mainfrom
dev/charlesgong/home-instructions-dir
Draft

[codex] Support Codex home instructions directory#28838
charlesgong-openai wants to merge 3 commits into
mainfrom
dev/charlesgong/home-instructions-dir

Conversation

@charlesgong-openai

Copy link
Copy Markdown
Contributor

Summary

  • Add support for loading non-empty *.md files directly under ~/.codex/instructions/ as global Codex home instructions.
  • Preserve existing AGENTS.override.md / AGENTS.md precedence and append instruction-directory snippets in deterministic sorted order.
  • Add focused provider tests for sorted directory loading, AGENTS.md combination, and override precedence.

Validation

  • just fmt
  • just test -p codex-home
  • git diff --check

@charlesgong-openai charlesgong-openai marked this pull request as ready for review June 18, 2026 03:15

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0 Badge 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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

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