Skip to content

Commit ccf8d76

Browse files
authored
[app-server] feat: v2 apply_patch approval flow (openai#6760)
This PR adds the API V2 version of the apply_patch approval flow, which centers around `ThreadItem::FileChange`. This PR wires the new RPC (`item/fileChange/requestApproval`, V2 only) and related events (`item/started`, `item/completed` for `ThreadItem::FileChange`, which are emitted in both V1 and V2) through the app-server protocol. The new approval RPC is only sent when the user initiates a turn with the new `turn/start` API so we don't break backwards compatibility with VSCE. Similar to openai#6758, the approach I took was to make as few changes to the Codex core as possible, leveraging existing `EventMsg` core events, and translating those in app-server. I did have to add a few additional fields to `EventMsg::PatchApplyBegin` and `EventMsg::PatchApplyEnd`, but those were fairly lightweight. However, the `EventMsg`s emitted by core are the following: ``` 1) Auto-approved (no request for approval)
 - EventMsg::PatchApplyBegin - EventMsg::PatchApplyEnd 2) Approved by user - EventMsg::ApplyPatchApprovalRequest - EventMsg::PatchApplyBegin - EventMsg::PatchApplyEnd 3) Declined by user - EventMsg::ApplyPatchApprovalRequest - EventMsg::PatchApplyBegin - EventMsg::PatchApplyEnd ``` For a request triggering an approval, this would result in: ``` item/fileChange/requestApproval item/started item/completed ``` which is different from the `ThreadItem::CommandExecution` flow introduced in openai#6758, which does the below and is preferable: ``` item/started item/commandExecution/requestApproval item/completed ``` To fix this, we leverage `TurnSummaryStore` on codex_message_processor to store a little bit of state, allowing us to fire `item/started` and `item/fileChange/requestApproval` whenever we receive the underlying `EventMsg::ApplyPatchApprovalRequest`, and no-oping when we receive the `EventMsg::PatchApplyBegin` later. This is much less invasive than modifying the order of EventMsg within core (I tried). The resulting payloads: ``` { "method": "item/started", "params": { "item": { "changes": [ { "diff": "Hello from Codex!\n", "kind": "add", "path": "/Users/owen/repos/codex/codex-rs/APPROVAL_DEMO.txt" } ], "id": "call_Nxnwj7B3YXigfV6Mwh03d686", "status": "inProgress", "type": "fileChange" } } } ``` ``` { "id": 0, "method": "item/fileChange/requestApproval", "params": { "grantRoot": null, "itemId": "call_Nxnwj7B3YXigfV6Mwh03d686", "reason": null, "threadId": "019a9e11-8295-7883-a283-779e06502c6f", "turnId": "1" } } ``` ``` { "id": 0, "result": { "decision": "accept" } } ``` ``` { "method": "item/completed", "params": { "item": { "changes": [ { "diff": "Hello from Codex!\n", "kind": "add", "path": "/Users/owen/repos/codex/codex-rs/APPROVAL_DEMO.txt" } ], "id": "call_Nxnwj7B3YXigfV6Mwh03d686", "status": "completed", "type": "fileChange" } } } ```
1 parent 558e3b0 commit ccf8d76

File tree

16 files changed

+693
-22
lines changed

16 files changed

+693
-22
lines changed

codex-rs/app-server-protocol/src/protocol/common.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,13 @@ server_request_definitions! {
438438
response: v2::CommandExecutionRequestApprovalResponse,
439439
},
440440

441+
/// Sent when approval is requested for a specific file change.
442+
/// This request is used for Turns started via turn/start.
443+
FileChangeRequestApproval => "item/fileChange/requestApproval" {
444+
params: v2::FileChangeRequestApprovalParams,
445+
response: v2::FileChangeRequestApprovalResponse,
446+
},
447+
441448
/// DEPRECATED APIs below
442449
/// Request to approve a patch.
443450
/// This request is used for Turns started via the legacy APIs (i.e. SendUserTurn, SendUserMessage).

codex-rs/app-server-protocol/src/protocol/v2.rs

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -794,20 +794,23 @@ pub struct FileUpdateChange {
794794
}
795795

796796
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
797-
#[serde(rename_all = "camelCase")]
797+
#[serde(tag = "type", rename_all = "camelCase")]
798+
#[ts(tag = "type")]
798799
#[ts(export_to = "v2/")]
799800
pub enum PatchChangeKind {
800801
Add,
801802
Delete,
802-
Update,
803+
Update { move_path: Option<PathBuf> },
803804
}
804805

805806
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
806807
#[serde(rename_all = "camelCase")]
807808
#[ts(export_to = "v2/")]
808809
pub enum PatchApplyStatus {
810+
InProgress,
809811
Completed,
810812
Failed,
813+
Declined,
811814
}
812815

813816
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
@@ -982,6 +985,26 @@ pub struct CommandExecutionRequestApprovalResponse {
982985
pub accept_settings: Option<CommandExecutionRequestAcceptSettings>,
983986
}
984987

988+
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
989+
#[serde(rename_all = "camelCase")]
990+
#[ts(export_to = "v2/")]
991+
pub struct FileChangeRequestApprovalParams {
992+
pub thread_id: String,
993+
pub turn_id: String,
994+
pub item_id: String,
995+
/// Optional explanatory reason (e.g. request for extra write access).
996+
pub reason: Option<String>,
997+
/// [UNSTABLE] When set, the agent is asking the user to allow writes under this root
998+
/// for the remainder of the session (unclear if this is honored today).
999+
pub grant_root: Option<PathBuf>,
1000+
}
1001+
1002+
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
1003+
#[ts(export_to = "v2/")]
1004+
pub struct FileChangeRequestApprovalResponse {
1005+
pub decision: ApprovalDecision,
1006+
}
1007+
9851008
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
9861009
#[serde(rename_all = "camelCase")]
9871010
#[ts(export_to = "v2/")]

codex-rs/app-server-test-client/src/main.rs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ use codex_app_server_protocol::ClientRequest;
2424
use codex_app_server_protocol::CommandExecutionRequestAcceptSettings;
2525
use codex_app_server_protocol::CommandExecutionRequestApprovalParams;
2626
use codex_app_server_protocol::CommandExecutionRequestApprovalResponse;
27+
use codex_app_server_protocol::FileChangeRequestApprovalParams;
28+
use codex_app_server_protocol::FileChangeRequestApprovalResponse;
2729
use codex_app_server_protocol::GetAccountRateLimitsResponse;
2830
use codex_app_server_protocol::InitializeParams;
2931
use codex_app_server_protocol::InitializeResponse;
@@ -677,6 +679,9 @@ impl CodexClient {
677679
ServerRequest::CommandExecutionRequestApproval { request_id, params } => {
678680
self.handle_command_execution_request_approval(request_id, params)?;
679681
}
682+
ServerRequest::FileChangeRequestApproval { request_id, params } => {
683+
self.approve_file_change_request(request_id, params)?;
684+
}
680685
other => {
681686
bail!("received unsupported server request: {other:?}");
682687
}
@@ -717,6 +722,37 @@ impl CodexClient {
717722
Ok(())
718723
}
719724

725+
fn approve_file_change_request(
726+
&mut self,
727+
request_id: RequestId,
728+
params: FileChangeRequestApprovalParams,
729+
) -> Result<()> {
730+
let FileChangeRequestApprovalParams {
731+
thread_id,
732+
turn_id,
733+
item_id,
734+
reason,
735+
grant_root,
736+
} = params;
737+
738+
println!(
739+
"\n< fileChange approval requested for thread {thread_id}, turn {turn_id}, item {item_id}"
740+
);
741+
if let Some(reason) = reason.as_deref() {
742+
println!("< reason: {reason}");
743+
}
744+
if let Some(grant_root) = grant_root.as_deref() {
745+
println!("< grant root: {}", grant_root.display());
746+
}
747+
748+
let response = FileChangeRequestApprovalResponse {
749+
decision: ApprovalDecision::Accept,
750+
};
751+
self.send_server_request_response(request_id, &response)?;
752+
println!("< approved fileChange request for item {item_id}");
753+
Ok(())
754+
}
755+
720756
fn send_server_request_response<T>(&mut self, request_id: RequestId, response: &T) -> Result<()>
721757
where
722758
T: Serialize,

0 commit comments

Comments
 (0)