›_ Codex Issues
Codex Issues / #35018

“Approve for Me” mode makes apply_patch 3,128× slower on a large repository (Windows)

Open 💬 3 comments Opened Jul 23, 2026 by david-rzepa
💡 Likely answer: A maintainer (github-actions[bot], contributor) responded on this thread — see the highlighted reply below.

What version of the Codex App are you using (From “About Codex” dialog)?

Codex App 26.715.10079.0; bundled engine 0.145.0-alpha.30 (confirmed in two of the controlled runs).

What subscription do you have?

Not included; this is a local filesystem/sandbox A/B comparison.

What platform is your computer?

Microsoft Windows 11 Home 10.0.26200 x64.

What issue are you seeing?

On Windows, the same native apply_patch create/update/delete cycle is dramatically slower in Approve for Me mode than in Full Access. On a large Unity workspace the measured end-to-end time was 325.359 seconds vs 104 milliseconds: 3,128.45× slower.

The controlled tests also show that the slowdown grows with workspace file count, while Full Access remains essentially flat:

| Workspace | Approve for Me | Full Access | Slowdown | FS-helper starts | Setup-refresh duration |
|---|---:|---:|---:|---:|---:|
| 10 tracked files | 5.850 s | 67 ms | 87.31× | 12 | 40.557–140.373 ms |
| 100 tracked files | 6.430 s | 61 ms | 105.41× | 12 | 51.594–176.562 ms |
| 1,000 tracked files | 6.768 s | 55 ms | 123.05× | 12 | 118.932–188.105 ms |
| 10,000 tracked files | 14.852 s | 68 ms | 218.41× | 12 | 667–1,065 ms |
| Large Unity workspace | 325.359 s | 104 ms | 3,128.45× | 12 | approximately 22–33 s |

The large workspace contained:

  • 50,466 tracked files
  • 180,681 filesystem files excluding .git
  • 7,195 directories excluding .git and junction traversal
  • approximately 58.4 GiB excluding .git

Full Access produced no filesystem-helper or setup-refresh activity during any measured window. Approve for Me produced exactly 12 filesystem-helper starts, 12 write-ACE grants, and 12 setup refreshes for the three patch operations.

The helper count is deterministic by operation:

  • create: 3 helpers
  • update: 4 helpers
  • delete: 5 helpers

The sandbox log repeatedly contains events of this form for every helper:

START: ... codex.exe --codex-run-as-fs-helper
setup refresh: spawning ... codex-windows-sandbox-setup.exe ...
granting write ACE to <workspace-root>
setup refresh: processed 3 write roots (read roots delegated); errors=[]

The workspace root already inherits a Codex sandbox-group ACE equivalent to:

CodexSandboxUsers:(I)(OI)(CI)(M,DC)

Nevertheless, every helper refresh logs another granting write ACE operation. On the large workspace, twelve approximately 22–33-second refreshes account for nearly the entire 325-second patch cycle. The create/update/delete durations also grow with the 3/4/5 helper counts.

This appears to be repeated, non-converging ACL/setup work for each filesystem RPC in one logical patch operation, rather than patch parsing or normal disk-write cost.

What steps can reproduce the bug?

Controlled fixture setup
  1. On Windows, create four clean local Git repositories outside any production repository.
  2. In each repository, create a single flat data directory containing respectively 10, 100, 1,000, and 10,000 tracked files. Each file in this test was exactly 1,024 bytes.
  3. Commit all files, run git gc, and verify git status --short is empty.
  4. Place the fixtures under a parent directory that inherits the normal Codex sandbox-group modify/delete-child ACE. No custom ACLs are required.
  5. Open each repository as its own Codex App workspace/task. Run the workspaces sequentially, not concurrently.

The important properties are a clean baseline commit, exactly the expected tracked-file count, a flat directory layout to isolate file count, and no pre-existing probe file.

Measured patch cycle

For each repository:

  1. Set the Codex App permission mode to Approve for Me.
  2. Record the local and UTC start timestamp.
  3. Time each native apply_patch call end-to-end, from dispatch until the tool returns. Do not use PowerShell, Python, or shell redirection to edit the file.
  4. Create the probe using exactly one patch:
*** Begin Patch
*** Add File: .codex-apply-patch-benchmark.txt
+iteration=1
*** End Patch
  1. Update it using exactly one patch:
*** Begin Patch
*** Update File: .codex-apply-patch-benchmark.txt
@@
-iteration=1
+iteration=2
*** End Patch
  1. Delete it using exactly one patch:
*** Begin Patch
*** Delete File: .codex-apply-patch-benchmark.txt
*** End Patch
  1. Record the finish timestamp. Verify the probe is absent and git status --short is empty. Do not retry an operation while its completion state is unknown.
  2. Inspect %USERPROFILE%\.codex\.sandbox\sandbox.YYYY-MM-DD.log between the recorded UTC timestamps. Count filesystem-helper starts, granting write ACE messages, and completed setup refreshes; record minimum and maximum setup-refresh durations.
  3. Switch the same task/workspace to Full Access and repeat steps 2–8.
  4. Repeat for each fixture, sequentially.

The Codex App was updated and restarted before testing. The same create/update/delete content and timing method were used in both modes. Each run ended cleanly with no errors or benchmark residue.

Raw operation timings

| Files | Mode | Create | Update | Delete | Total |
|---:|---|---:|---:|---:|---:|
| 10 | Approve for Me | 996 ms | 1,788 ms | 3,066 ms | 5,850 ms |
| 10 | Full Access | 22 ms | 23 ms | 22 ms | 67 ms |
| 100 | Approve for Me | 1,918 ms | 2,071 ms | 2,441 ms | 6,430 ms |
| 100 | Full Access | 20 ms | 21 ms | 20 ms | 61 ms |
| 1,000 | Approve for Me | 2,287 ms | 1,894 ms | 2,587 ms | 6,768 ms |
| 1,000 | Full Access | 19 ms | 17 ms | 19 ms | 55 ms |
| 10,000 | Approve for Me | 3,212 ms | 5,093 ms | 6,547 ms | 14,852 ms |
| 10,000 | Full Access | 25 ms | 22 ms | 21 ms | 68 ms |
| Large workspace | Approve for Me | 95,437 ms | 101,861 ms | 128,061 ms | 325,359 ms |
| Large workspace | Full Access | 53 ms | 23 ms | 28 ms | 104 ms |

What is the expected behavior?

A three-line create/update/delete benchmark should have bounded sandbox overhead and should not become thousands of times slower in Approve for Me mode.

In particular:

  • ACL/setup refresh should converge or be cached when the required workspace ACE is already present.
  • One logical apply_patch operation should not repeat expensive workspace-wide sandbox setup separately for every internal filesystem RPC.
  • Latency should not scale from milliseconds to several minutes based on unrelated workspace contents.
  • If ACL refresh is genuinely required, Codex should expose that work and its timing rather than appearing to hang.

Additional information

This adds a permission-mode A/B and controlled scaling data to related reports such as #32477 and #34290.

PR #34392 (Ignore inherited ACEs when refreshing Windows write roots) merged on 2026-07-20. This benchmark was run on 2026-07-23 with App 26.715.10079.0 / engine 0.145.0-alpha.30, and every Approve for Me helper still logged a write-ACE grant and setup refresh. It would be useful to verify whether #34392 is included and active in this build, and whether it addresses only inherited-ACE classification rather than the repeated per-filesystem-RPC setup.

The controlled data suggests two compounding costs:

  1. a fixed helper/orchestration cost (12 helper starts per create/update/delete cycle), and
  2. workspace-size-dependent ACL propagation repeated for every helper.

The large workspace likely amplifies the second cost through its file/directory count and generated Unity cache, but Full Access completing in 104 ms shows that patch parsing and ordinary local filesystem mutation are not the bottleneck.

View original on GitHub ↗

3 Comments

github-actions[bot] contributor · 1 month ago

Potential duplicates detected. Please review them and close your issue if it is a duplicate.

  • #34889
  • #33737

Powered by Codex Action

bbingz · 1 month ago

Analysis (community)

Thank you for the excellent controlled fixtures and log counts — they make the bottleneck unusually clear.

  • Environment (from report): Windows 11 Home 10.0.26200 x64; Codex App 26.715.10079.0; engine 0.145.0-alpha.30.
  • Repro status: Not re-run here (no Windows sandbox host in this pass). Treating the issue’s multi-size A/B table, per-op timings, and sandbox-log counters as excellent primary evidence and correlating them with current main sources.
  • Observed: Same create/update/delete apply_patch cycle is ~87×–3,128× slower under Approve for Me than Full Access, scaling with workspace size. Large Unity tree: 325.359 s vs 104 ms. Logs show a fixed shape: 12 FS-helper starts, 12 “granting write ACE”, 12 setup refreshes (create≈3 / update≈4 / delete≈5 helpers), with refresh alone ~22–33 s on the large tree. Full Access shows zero helper/setup activity.
  • Expected: Sandbox overhead for a three-line probe should be bounded; ACL/setup should converge or cache once rights are present; one logical apply_patch should not pay full workspace ACL work per internal FS RPC; latency should not track unrelated workspace file count.

Root-cause hypothesis

Two multipliers appear to compound; neither is “patch parse / ordinary disk I/O” (Full Access ~100 ms rules that out).

1) Process-per-RPC FS helper
FileSystemSandboxRunner / sandbox_exec_request in codex-rs/exec-server/src/fs_sandbox.rs appears to launch a fresh sandboxed process per FsHelperRequest (--codex-run-as-fs-helper). codex-rs/core/src/tools/runtimes/apply_patch.rs wires FileSystemSandboxContext into codex_apply_patch::apply_patch, and codex-rs/apply-patch/src/lib.rs issues multiple FS ops (metadata/read/write/remove/create_directory) per hunk — matching the reported 3/4/5 helper counts.

2) Unconditional setup/ACL refresh on every sandboxed spawn
Windows spawn prep (codex-rs/windows-sandbox-rs/src/spawn_prep.rs → prepare_elevated_spawn_context_for_permissions → require_logon_sandbox_creds) leads to run_setup_refresh_* in identity.rs / setup.rs. Source comments/path around identity.rs (~230–247) indicate always refresh ACLs; run_setup_singleflight only coalesces concurrent identical payloads, so sequential helpers each pay a full refresh.

When path_write_aces_need_refresh is true (setup_main/win.rs ~851–904, 987–994), setup logs “granting write ACE” and ensure_allow_write_aces (acl.rs ~351–507) applies write allow via SetNamedSecurityInfoW with CONTAINER_INHERIT | OBJECT_INHERIT on the writable root — cost that can scale with tree size (matches 22–33 s refreshes and O(workspace)×12 total). Related: capability SIDs in cap.rs (workspace_write_cap_sid_for_root / writable_root_by_path).

Why #34392 may not fix this benchmark: PR #34392 (ignore inherited FILE_DELETE_CHILD when deciding refresh) improves classification of whether a grant is needed for inherited DC, but does not remove always-on per-spawn refresh or process-per-RPC helperization. The reporter still sees a grant on every helper on 0.145.0-alpha.30 (post-merge date of the PR vs app/engine version is an open question below). Related context: #32477, #34290, #34889, #33158.

Confidence: high on the multiplicative structure (N helpers × full setup/ACL path); medium-high on the exact fraction of wall time inside SetNamedSecurityInfo propagation vs setup process startup vs DACL enumeration without Windows ETW/sampling on this machine.

High-level fix outline (not a PR)

Intended as design discussion only — happy to revise if this fights an existing plan.

  1. Primary (highest leverage): Stop unconditional ACL setup refresh on every sandboxed spawn. Cache a successful refresh keyed by something like (codex_home, permissions, write/read roots, cap SID set) for the session/process; re-run only on root/policy change or ACL-shaped failure. Touch require_logon_sandbox_creds / setup orchestration — not apply_patch semantics.
  2. Secondary: Amortize apply_patch FS RPCs with a long-lived sandboxed FS helper (or batch RPC) so one tool call is not N process launches + N setup paths.
  3. Tertiary: Ensure write-root ACE repair converges in one pass for both sandbox-group and capability SIDs; avoid full-tree CI|OI propagation when only root-level token rights are required (if security model allows).

Alternatives considered: one preflight refresh when entering Approve for Me; short-TTL coalesce for sequential refreshes (weaker than a success-keyed cache).

Risks / non-goals:

  • Stale grants after mid-session root/policy changes → must invalidate cache on root/policy change and on ACL-shaped failures.
  • Under-granting breaks sandboxed writes → prefer fail-open to refresh on doubt.
  • Do not change: Full Access path, approval-policy UX, deny-write carveouts, or patch application correctness.

Test ideas

Prefer counting setup spawns / ACE grants (sandbox log or test hooks) over multi-minute wall-clock-only tests.

  1. Unit / Windows: After ensure_allow_write_aces for group + cap SIDs, a second path_write_aces_need_refresh is false (extend setup_main write_root_refresh_* tests).
  2. Integration: N sequential identical sandboxed FS-helper spawns assert setup-refresh / helper-prep count ≪ N (ideally 1) after cache/amortize.
  3. apply_patch regression: Sandboxed create+update+delete of a one-line file on a large or instrumented write root — helper starts and “granting write ACE” / setup-refresh counts stay near-constant vs file count; total time does not scale like O(files)×RPCs.
  4. Safety: Missing cap ACE still grants; explicit stale FILE_DELETE_CHILD still repaired; inherited-only DC does not force refresh (#34392).

Questions for maintainers

  1. Is #34392 actually present/active in engine 0.145.0-alpha.30 used for this benchmark?
  2. After one successful grant on the large Unity root, does path_write_aces_need_refresh still return true for sandbox-group, capability SID, or both (worth logging which SID)?
  3. Is most of the 22–33 s in SetNamedSecurityInfo auto-propagation vs setup process startup vs DACL enumeration?
  4. Can capability-SID rights be granted without CI/OI tree rewrite, consistent with the security model?
  5. Is a long-lived FS helper acceptable security-wise versus process-per-RPC isolation, or is session-scoped setup caching the preferred first step?

Happy to refine this analysis (or dig into any of the open questions) if useful. If the team later wants an invited PR along an agreed approach, I’d be glad to help — no rush and no expectation.

bbingz · 1 month ago

Building on the root-cause analysis above with the origin archaeology for the always-refresh call, a sharpening of the #34392 framing, and a correction to my proposed test idea. Verified against main as of 5dd992a (2026-07-24).

Origin archaeology (hedged - not calling it a "defect/regression"). The always-refresh call and its // Always refresh ACLs comment originate in 13c0919bf / #7792 (2025-12-10, "Elevated Sandbox 2"); run_setup_refresh in the same file was later touched by b976e701a / #14568 (2026-04-09, "support split carveouts in windows elevated sandbox"). identity.rs comments the always-refresh as intentional design. A shallow-clone git blame boundary can pin the block at b7e39aa, but with full history the real origin is #7792 - so the always-refresh long predates the capability-SID grant work. (I am not claiming a regression from #7792; only pinning where the call came from.)

Sharpening the #34392 framing. Restating to be precise: #34392 (merge bd92b05, merged 2026-07-20) is classification-only - it touches dacl_allow_mask_needs_refresh (acl.rs:375) and adds the test write_root_refresh_ignores_inherited_delete_child_grant (win.rs:1156). It did not introduce the always-refresh call, and it still would not remove the per-spawn setup cost - which is consistent with the first comment's view that #34392 may not move this benchmark.

Correction to my proposed test idea #1 - it already exists. I suggested "after ensure_allow_write_aces, a second path_write_aces_need_refresh is false". That test already exists: write_root_refresh_replaces_stale_delete_child_grant (single capability SID) and write_root_refresh_checks_each_sid (two workspace_write_cap_sid_for_root capability SIDs for different roots, win.rs:~1133-1150) both call ensure_allow_write_aces then assert needs_refresh_after is false via path_write_aces_need_refresh. So ACL-level convergence is already asserted (I am not claiming these were run/passed on Windows here, only that the assertions exist in setup_main/win.rs). I withdraw the test idea.

Sharpened non-convergence mystery (the real open question). The unit tests assert path_write_aces_need_refresh converges to false after a grant at the ACL level, yet the issue logs 12 "granting write ACE" events (need_grant == true on every helper), including re-granting an inherited CodexSandboxUsers ACE. So the real-world non-convergence is not reproduced by any existing unit test - it is an integration-level gap (in-process ACL convergence vs the multi-process setup-binary spawn path), not an ACL-level one. One hypothesis (not the deduced cause): the capability SID ACE is not persisting across helper processes, so need_grant stays true every spawn; but the issue's own evidence of the inherited CodexSandboxUsers ACE still re-granting is at least as consistent with the inherited-ACE classification path. I'd leave the version/build-signal question open (as the first comment's Q1 already does) rather than infer a version timeline from the issue's abbreviated log template.

Blast radius. The always-refresh fires for every Windows sandboxed spawn through require_logon_sandbox_creds, not only apply_patch - any tool/operation using FileSystemSandboxRunner pays the same per-RPC setup-binary spawn cost.

Not fixed as of 5dd992a: only b115de97d / #35036 "Preserve Windows sandbox proxy settings in guardian sessions" touches any of identity.rs/setup.rs/fs_sandbox.rs/win.rs/acl.rs after the issue date (2026-07-23), and it is unrelated to ACL refresh performance.