Skip to content

Fix Codex exec approval handling#535

Open
fancyboi999 wants to merge 2 commits intoslopus:mainfrom
fancyboi999:fancy/fix-codex-permisiion
Open

Fix Codex exec approval handling#535
fancyboi999 wants to merge 2 commits intoslopus:mainfrom
fancyboi999:fancy/fix-codex-permisiion

Conversation

@fancyboi999
Copy link

Background

Users reported two related issues where Codex exec approvals either failed validation or left the session stuck after selecting “Stop and explain.” This produced invalid MCP elicitation responses and prevented follow-up messages from being processed. (Refs: #503, #522)

Solution

  • Preserve codex_* fields from the elicitation payload so we can reliably detect Codex exec approvals.
  • Return an MCP-compliant result (action) while also including legacy fields (decision, optional content) required by older Codex runtimes.
  • Track exec approval requests and propagate the “abort” decision to the run loop, so a “Stop and explain” cleanly ends the current turn and new user messages are handled normally.

Compatibility

This keeps compatibility with older Codex clients by continuing to send decision/content while remaining valid for the MCP schema. Non-Codex elicitation behavior is unchanged.

Testing

  • yarn --cwd packages/happy-cli typecheck
  • Manual: verified all three permission paths (accept, decline, stop/abort) in CLI + mobile, and follow-up messages are processed after stop.

Copy link

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

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: 26de9b1729

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

Comment on lines 208 to 215
} else {
const pending = this.pendingExecApprovals.shift();
if (pending) {
toolCallId = pending.callId;
toolInput = {
command: pending.command,
cwd: pending.cwd,
reason: pending.reason ?? undefined

Choose a reason for hiding this comment

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

P2 Badge Avoid consuming exec approvals for unrelated elicitations

The new fallback path shifts pendingExecApprovals for any elicitation without Codex fields. That means an unrelated MCP elicitation (e.g., another tool asking for user input) will pop and reuse the next exec-approval entry, so the permission handler sees the wrong command/callId and the real exec approval later falls back to a random id with missing command details. This can lead to approvals/denials being applied to the wrong exec command whenever a non-Codex elicitation arrives while an exec approval is pending.

Useful? React with 👍 / 👎.

Copy link
Author

Choose a reason for hiding this comment

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

Addressed review feedback: we now only consume pending exec approvals when the elicitation looks like an exec-approval prompt (Codex message patterns / empty schema). Otherwise we leave the queue intact to avoid mismatched approvals. Commit: ea57ac3.

@fancyboi999 fancyboi999 force-pushed the fancy/fix-codex-permisiion branch from c3eceaf to ea57ac3 Compare February 4, 2026 10:52
@HQ-Zhang-233
Copy link

very important pull, and hope merged ASAP(Codex user need this)

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.

3 participants