Expand ~ in MDM workspace write roots

[evawong-oai]

Summary

• Reuse the existing config path resolver for the macOS MDM managed preferences layer so writable_roots = ["~/code"] expands the same way as file backed config
• keep the change scoped to the MDM branch in config_loader; the current net diff is only config_loader/mod.rs plus focused regression tests in config_loader/tests.rs and config/service_tests.rs
• research note: resolve_relative_paths_in_config_toml(...) is already used in several existing configuration paths, including CLI overrides, file-backed managed config, normal config-file loading, project .codex/config.toml loading, and role config loading

Validation

• cargo fmt --all --check
• cargo test -p codex-core managed_preferences_expand_home_directory_in_workspace_write_roots -- --nocapture
• cargo test -p codex-core write_value_succeeds_when_managed_preferences_expand_home_directory_paths -- --nocapture

[evawong-oai]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

codex/codex-rs/core/src/config/service.rs

Line 618 in 42d1050

let _: ConfigToml = value.clone().try_into()?;

Validate merged config with home-expansion guard

read() now accepts MDM ~/... paths via deserialize_merged_config_toml, but apply_edits() still runs validate_config(), which deserializes with plain try_into() and no AbsolutePathBufGuard. That makes behavior inconsistent: the same managed config can be read successfully, then any write fails with ConfigValidationError (e.g. AbsolutePathBuf deserialized without a base path).

ℹ️ 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".

[evawong-oai]

@codex address that feedback

[chatgpt-codex-connector]

Summary

• Updated ConfigService::apply_edits to validate user config only through the guarded deserialize_config_toml_with_base(...) path before feature checks, removing the redundant unguarded deserialization pass that could reject ~/... path fields during writes. codex-rs/core/src/config/service.rsL336-L364
• Added a regression test that verifies writes still succeed when config.toml already contains a home-relative path (model_instructions_file = "~/instructions.md"), ensuring this flow remains supported on non-Windows platforms. codex-rs/core/src/config/service_tests.rsL338-L366

Testing

• ✅ just fmt
• ⚠️ cargo test -p codex-core (failed in this environment due crates.io access errors while resolving dependency v8).
• ⚠️ just argument-comment-lint (failed because prebuilt linter wrapper requires dotslash in this environment).
• ⚠️ ./tools/argument-comment-lint/run.sh (failed because cargo-dylint/dylint-link are not installed).
• ⚠️ cargo install --locked cargo-dylint dylint-link (install failed due crates.io/network access restrictions).

Committed on the current branch and created a follow-up PR message with make_pr.

View task →

[evawong-oai]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 75947d8963

ℹ️ 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".

[evawong-oai]

Addressing the earlier Codex review note about apply_edits() still failing on managed ~/... paths: this is fixed in efe354ffb.

apply_edits() still uses validate_config(&user_config) for raw user-file schema checks, but the path-sensitive deserialization during writes now goes through the guarded helpers:

• deserialize_config_toml_with_base(user_config.clone(), &self.codex_home) before feature validation in codex-rs/core/src/config/service.rs
• validate_merged_config(&effective, &self.codex_home) on the merged config, which uses deserialize_merged_config_toml(...)

So managed ~/... values are now validated with home expansion on write, not just on read. There is also a regression test in write_value_succeeds_when_managed_preferences_expand_home_directory_paths() in codex-rs/core/src/config/service_tests.rs that sets managed writable_roots = ["~/code"] and verifies write_value(...) succeeds.

[evawong-oai]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Can't wait for the next one!

ℹ️ 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".

[rreichel3-oai]

Few open questions! Looks like the actual logic is functional

[rreichel3-oai]

What are these changes actually testing with this PR? These seem unnecessary?

[evawong-oai]

These are changes that are needed for CI related to Mac changes to go green. The x86 macOS Bazel failure is stalling because shell_snapshot.rs submits the turn with approval_policy: AskForApproval::Never, but the test process is running under managed config that only allows OnRequest. The turn gets rejected up front in the session settings path in codex.rs line 2319, emits an Error event, and never reaches the shell tool or intercepted apply_patch path. The test then keeps waiting for TurnComplete or patch events until its timeout, which makes it look like a slow shell snapshot problem :(

[rreichel3-oai]

Why are we downgrading Windows versions here?

[evawong-oai]

Removed from this PR. That lockfile churn came from an unrelated tar bump and re resolution, not from the MDM path fix itself, so I dropped it while narrowing the branch back to the bug fix only.

[bolinfest]

Hmm, I'm debating whether we should do this or bring back your alternative path guard that honors ~ but not ./.

This is certainly simpler.

I don't know if there's a valid use case to specify a relative path in an MDM config. In the worst case, it could be an attack vector, but because MDM should be set by a trusted person, I guess I'm not that worried about that.

Maybe please just add a comment that we use codex_home as the base so that ~ and ./ are honored using our AbsolutePathBufGuard logic, though in practice, we expect ~ to be used but not ./.

[evawong-oai]

Sounds good. Following up with a comment.