fix: ensure PatchApplyBeginEvent and PatchApplyEndEvent are dispatched reliably

[bolinfest]

This is a follow-up to #1705, as that PR inadvertently lost the logic where PatchApplyBeginEvent and PatchApplyEndEvent events were sent when patches were auto-approved.

Though as part of this fix, I believe this also makes an important safety fix to assess_patch_safety(), as there was a case that returned SandboxType::None, which arguably is the thing we were trying to avoid in #1705.

On a high level, we want there to be only one codepath where apply_patch happens, which should be unified with the patch to run exec, in general, so that sandboxing is applied consistently for both cases.

Prior to this change, apply_patch() in core would either:

• exit early, delegating to exec() to shell out to apply_patch using the appropriate sandbox
• proceed to run the logic for apply_patch in memory

codex/codex-rs/core/src/apply_patch.rs

Lines 61 to 63 in 549846b

	SafetyCheck::AutoApprove { .. } => {
	return InternalApplyPatchInvocation::DelegateToExec(action);
	}

In this implementation, only the latter would dispatch PatchApplyBeginEvent and PatchApplyEndEvent, though the former would dispatch ExecCommandBeginEvent and ExecCommandEndEvent for the apply_patch call (or, more specifically, the codex --codex-run-as-apply-patch PATCH call).

To unify things in this PR, we:

• Eliminate the back half of the apply_patch() function, and instead have it also return with DelegateToExec, though we add an extra field to the return value, user_explicitly_approved_this_action.
• In codex.rs where we process DelegateToExec, we use SandboxType::None when user_explicitly_approved_this_action is true. This means we no longer run the apply_patch logic in memory, as we always exec(). (Note this is what allowed us to delete so much code in apply_patch.rs.)
• In codex.rs, we further update notify_exec_command_begin() and notify_exec_command_end() to take additional fields to determine what type of notification to send: ExecCommand or PatchApply.

Admittedly, this PR also drops some of the functionality about giving the user the opportunity to expand the set of writable roots as part of approving the apply_patch command. I'm not sure how much that was used, and we should probably rethink how that works as we are currently tidying up the protocol to the TUI, in general.

[github-actions]

Summary
Unifies all apply_patch execution through exec, guarantees PatchApplyBegin/End events are emitted in every case, and tightens assess_patch_safety logic. Large chunk of in-memory patch-application code is deleted and replaced with a leaner delegation path.

Notes

• Adds ApplyPatchExec + richer ExecCommandContext to carry patch-specific metadata.
• notify_exec_command_{begin,end} now decide between ExecCommand* and PatchApply* events.
• assess_patch_safety no longer returns SandboxType::None when paths look writable; still sandboxes unless explicitly user-approved.
• Drops the “expand writable roots” prompt.

---

Review

Nice cleanup—single code-path for patches reduces complexity and the event stream is now consistent. 👏

A few minor thoughts:

• Losing the writable-root-escalation flow is worth a quick note in the changelog and maybe a follow-up issue.
• assess_patch_safety comment says we “should still run in a sandbox”, but SandboxType::None is used when user-approved; double-check that this cannot re-introduce the hard-link escape you called out.
• Consider a unit test that asserts PatchApplyBegin/End are produced for both auto- and user-approved patches.
• Public/helper fn order in codex.rs is drifting—might be time to split some of the exec logic into its own module as the file is getting large.

Otherwise looks solid.

---

View workflow run