mirror of
https://github.com/openai/codex.git
synced 2026-04-29 00:55:38 +00:00
chore: remove skill metadata from command approval payloads (#15906)
## Why This is effectively a follow-up to [#15812](https://github.com/openai/codex/pull/15812). That change removed the special skill-script exec path, but `skill_metadata` was still being threaded through command-approval payloads even though the approval flow no longer uses it to render prompts or resolve decisions. Keeping it around added extra protocol, schema, and client surface area without changing behavior. Removing it keeps the command-approval contract smaller and avoids carrying a dead field through app-server, TUI, and MCP boundaries. ## What changed - removed `ExecApprovalRequestSkillMetadata` and the corresponding `skillMetadata` field from core approval events and the v2 app-server protocol - removed the generated JSON and TypeScript schema output for that field - updated app-server, MCP server, TUI, and TUI app-server approval plumbing to stop forwarding the field - cleaned up tests that previously constructed or asserted `skillMetadata` ## Testing - `cargo test -p codex-app-server-protocol` - `cargo test -p codex-protocol` - `cargo test -p codex-app-server-test-client` - `cargo test -p codex-mcp-server` - `just argument-comment-lint`
This commit is contained in:
@@ -1705,7 +1705,6 @@ mod tests {
|
||||
}),
|
||||
macos: None,
|
||||
}),
|
||||
skill_metadata: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
available_decisions: None,
|
||||
@@ -1716,31 +1715,4 @@ mod tests {
|
||||
Some("item/commandExecution/requestApproval.additionalPermissions")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn command_execution_request_approval_skill_metadata_is_marked_experimental() {
|
||||
let params = v2::CommandExecutionRequestApprovalParams {
|
||||
thread_id: "thr_123".to_string(),
|
||||
turn_id: "turn_123".to_string(),
|
||||
item_id: "call_123".to_string(),
|
||||
approval_id: None,
|
||||
reason: None,
|
||||
network_approval_context: None,
|
||||
command: Some("cat file".to_string()),
|
||||
cwd: None,
|
||||
command_actions: None,
|
||||
additional_permissions: None,
|
||||
skill_metadata: Some(v2::CommandExecutionRequestApprovalSkillMetadata {
|
||||
path_to_skills_md: PathBuf::from("/tmp/SKILLS.md"),
|
||||
}),
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
available_decisions: None,
|
||||
};
|
||||
let reason = crate::experimental_api::ExperimentalApi::experimental_reason(¶ms);
|
||||
assert_eq!(
|
||||
reason,
|
||||
Some("item/commandExecution/requestApproval.skillMetadata")
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7,7 +7,6 @@ use crate::protocol::common::AuthMode;
|
||||
use codex_experimental_api_macros::ExperimentalApi;
|
||||
use codex_protocol::account::PlanType;
|
||||
use codex_protocol::approvals::ElicitationRequest as CoreElicitationRequest;
|
||||
use codex_protocol::approvals::ExecApprovalRequestSkillMetadata as CoreExecApprovalRequestSkillMetadata;
|
||||
use codex_protocol::approvals::ExecPolicyAmendment as CoreExecPolicyAmendment;
|
||||
use codex_protocol::approvals::NetworkApprovalContext as CoreNetworkApprovalContext;
|
||||
use codex_protocol::approvals::NetworkApprovalProtocol as CoreNetworkApprovalProtocol;
|
||||
@@ -3519,14 +3518,6 @@ impl From<CoreSkillMetadata> for SkillMetadata {
|
||||
}
|
||||
}
|
||||
|
||||
impl From<CoreExecApprovalRequestSkillMetadata> for CommandExecutionRequestApprovalSkillMetadata {
|
||||
fn from(value: CoreExecApprovalRequestSkillMetadata) -> Self {
|
||||
Self {
|
||||
path_to_skills_md: value.path_to_skills_md,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl From<CoreSkillInterface> for SkillInterface {
|
||||
fn from(value: CoreSkillInterface) -> Self {
|
||||
Self {
|
||||
@@ -5240,11 +5231,6 @@ pub struct CommandExecutionRequestApprovalParams {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional = nullable)]
|
||||
pub additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
/// Optional skill metadata when the approval was triggered by a skill script.
|
||||
#[experimental("item/commandExecution/requestApproval.skillMetadata")]
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional = nullable)]
|
||||
pub skill_metadata: Option<CommandExecutionRequestApprovalSkillMetadata>,
|
||||
/// Optional proposed execpolicy amendment to allow similar commands without prompting.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional = nullable)]
|
||||
@@ -5266,17 +5252,9 @@ impl CommandExecutionRequestApprovalParams {
|
||||
// We need a generic outbound compatibility design for stripping or
|
||||
// otherwise handling experimental server->client payloads.
|
||||
self.additional_permissions = None;
|
||||
self.skill_metadata = None;
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct CommandExecutionRequestApprovalSkillMetadata {
|
||||
pub path_to_skills_md: PathBuf,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
@@ -6100,7 +6078,6 @@ mod tests {
|
||||
},
|
||||
"macos": null
|
||||
},
|
||||
"skillMetadata": null,
|
||||
"proposedExecpolicyAmendment": null,
|
||||
"proposedNetworkPolicyAmendments": null,
|
||||
"availableDecisions": null
|
||||
@@ -6139,7 +6116,6 @@ mod tests {
|
||||
"contacts": "read_only"
|
||||
}
|
||||
},
|
||||
"skillMetadata": null,
|
||||
"proposedExecpolicyAmendment": null,
|
||||
"proposedNetworkPolicyAmendments": null,
|
||||
"availableDecisions": null
|
||||
@@ -6183,7 +6159,6 @@ mod tests {
|
||||
"contacts": "none"
|
||||
}
|
||||
},
|
||||
"skillMetadata": null,
|
||||
"proposedExecpolicyAmendment": null,
|
||||
"proposedNetworkPolicyAmendments": null,
|
||||
"availableDecisions": null
|
||||
@@ -6199,35 +6174,6 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn command_execution_request_approval_accepts_skill_metadata() {
|
||||
let params = serde_json::from_value::<CommandExecutionRequestApprovalParams>(json!({
|
||||
"threadId": "thr_123",
|
||||
"turnId": "turn_123",
|
||||
"itemId": "call_123",
|
||||
"command": "cat file",
|
||||
"cwd": "/tmp",
|
||||
"commandActions": null,
|
||||
"reason": null,
|
||||
"networkApprovalContext": null,
|
||||
"additionalPermissions": null,
|
||||
"skillMetadata": {
|
||||
"pathToSkillsMd": "/tmp/SKILLS.md"
|
||||
},
|
||||
"proposedExecpolicyAmendment": null,
|
||||
"proposedNetworkPolicyAmendments": null,
|
||||
"availableDecisions": null
|
||||
}))
|
||||
.expect("skill metadata should deserialize");
|
||||
|
||||
assert_eq!(
|
||||
params.skill_metadata,
|
||||
Some(CommandExecutionRequestApprovalSkillMetadata {
|
||||
path_to_skills_md: PathBuf::from("/tmp/SKILLS.md"),
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn permissions_request_approval_uses_request_permission_profile() {
|
||||
let read_only_path = if cfg!(windows) {
|
||||
|
||||
Reference in New Issue
Block a user