[app-server] define new v2 approval request

[owenlin0]

Define the API v2 interface for the server making a request to the client. This only happens when codex is requesting the user's approval for executing a command or writing a file.

In API v2, a single item/requestApproval RPC replaces both applyPatchApproval and execCommandApproval.

This PR only defines the interface. Implementation to follow.

[owenlin0]

I made some updates to the server_request_definitions macro - I like the explicitness of referencing the params and response structs, and it now mirrors client_request_definitions which does the same thing.

[owenlin0]

@codex review this

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. 🎉

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[pakrym-oai]

can this purely reference the item?

[pakrym-oai]

If not I think there is no point in having 1 event than has to have internal dispatch for each item type.

[owenlin0]

So you're saying if we can do something like:

pub struct ItemRequestApprovalParams {
    pub thread_id: String,
    pub turn_id: String,
    pub item: ThreadItem,
}

then it's worth consolidating. If not, then we should have two methods?

[owenlin0]

The unfortunate thing about pub item: ThreadItem is that the type is too broad, in reality we only send approval requests for ThreadItem::CommandExecution and ThreadItem::FileChange.

This makes me lean towards having two separate methods actually

[pakrym-oai]

then it's worth consolidating. If not, then we should have two methods?

Yeah, my initial goal was to have something like:

pub struct ItemRequestApprovalParams {
    pub thread_id: String,
    pub turn_id: String,
    pub item_id: String ,
}

and have all relevant information be on the item.

But having item_id + item type specific payload makes this annoying.

two separate methods actually

Then we'll have approvals for web searches, maybe mcp calls and who knows what. I don't think we'll stop on just two.

[owenlin0]

Hmm yeah, neither seems ideal. Perhaps I'll go deeper on implementation to see if there's something clever we can do, I'm not super familiar with the underlying core event msgs yet and how they map to Items

[pakrym-oai]

can this be inferred from the item?

[pakrym-oai]

is this actually used in UI?