mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
app-server-protocol: mark permission profiles experimental (#19899)
## Why `PermissionProfile` is now the canonical internal permissions representation, but the app-server wire shape is still intentionally unstable while the migration continues. Stable app-server clients should not see or generate code for these fields until the wire format settles. ## What changed - Marks every app-server v2 field that sends `PermissionProfile` as experimental, including `command/exec`, `thread/start`, `thread/resume`, `thread/fork`, and `turn/start` request/response payloads. - Enables per-field experimental inspection for `command/exec`, so `permissionProfile` is gated without making the entire method experimental. - Fixes the generated TypeScript schema filter to be comment-aware. The previous scanner treated apostrophes inside doc comments as string delimiters, so some experimental fields leaked into stable TypeScript even though stable JSON was filtered correctly. ## Verification - `cargo test -p codex-app-server-protocol` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/19899). * #19900 * __->__ #19899
This commit is contained in:
@@ -736,11 +736,11 @@ fn find_top_level_brace_span(input: &str) -> Option<(usize, usize)> {
|
||||
let mut state = ScanState::default();
|
||||
let mut open_index = None;
|
||||
for (index, ch) in input.char_indices() {
|
||||
if !state.in_string() && ch == '{' && state.depth.is_top_level() {
|
||||
if !state.in_ignored_syntax() && ch == '{' && state.depth.is_top_level() {
|
||||
open_index = Some(index);
|
||||
}
|
||||
state.observe(ch);
|
||||
if !state.in_string()
|
||||
if !state.in_ignored_syntax()
|
||||
&& ch == '}'
|
||||
&& state.depth.is_top_level()
|
||||
&& let Some(open) = open_index
|
||||
@@ -760,7 +760,7 @@ fn split_top_level_multi(input: &str, delimiters: &[char]) -> Vec<String> {
|
||||
let mut start = 0usize;
|
||||
let mut parts = Vec::new();
|
||||
for (index, ch) in input.char_indices() {
|
||||
if !state.in_string() && state.depth.is_top_level() && delimiters.contains(&ch) {
|
||||
if !state.in_ignored_syntax() && state.depth.is_top_level() && delimiters.contains(&ch) {
|
||||
let part = input[start..index].trim();
|
||||
if !part.is_empty() {
|
||||
parts.push(part.to_string());
|
||||
@@ -882,22 +882,58 @@ struct ScanState {
|
||||
depth: Depth,
|
||||
string_delim: Option<char>,
|
||||
escape: bool,
|
||||
block_comment: bool,
|
||||
line_comment: bool,
|
||||
previous_char: Option<char>,
|
||||
}
|
||||
|
||||
impl ScanState {
|
||||
fn observe(&mut self, ch: char) {
|
||||
if self.line_comment {
|
||||
if ch == '\n' {
|
||||
self.line_comment = false;
|
||||
}
|
||||
self.previous_char = Some(ch);
|
||||
return;
|
||||
}
|
||||
|
||||
if self.block_comment {
|
||||
if self.previous_char == Some('*') && ch == '/' {
|
||||
self.block_comment = false;
|
||||
self.previous_char = None;
|
||||
} else {
|
||||
self.previous_char = Some(ch);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
if let Some(delim) = self.string_delim {
|
||||
if self.escape {
|
||||
self.escape = false;
|
||||
self.previous_char = Some(ch);
|
||||
return;
|
||||
}
|
||||
if ch == '\\' {
|
||||
self.escape = true;
|
||||
self.previous_char = Some(ch);
|
||||
return;
|
||||
}
|
||||
if ch == delim {
|
||||
self.string_delim = None;
|
||||
}
|
||||
self.previous_char = Some(ch);
|
||||
return;
|
||||
}
|
||||
|
||||
if self.previous_char == Some('/') && ch == '/' {
|
||||
self.line_comment = true;
|
||||
self.previous_char = Some(ch);
|
||||
return;
|
||||
}
|
||||
|
||||
if self.previous_char == Some('/') && ch == '*' {
|
||||
self.block_comment = true;
|
||||
self.previous_char = Some(ch);
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -919,10 +955,11 @@ impl ScanState {
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
self.previous_char = Some(ch);
|
||||
}
|
||||
|
||||
fn in_string(&self) -> bool {
|
||||
self.string_delim.is_some()
|
||||
fn in_ignored_syntax(&self) -> bool {
|
||||
self.string_delim.is_some() || self.block_comment || self.line_comment
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2694,6 +2731,79 @@ export type Config = { stableField: Keep, unstableField: string | null } & ({ [k
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn experimental_type_fields_ts_filter_handles_generated_command_params_shape() -> Result<()> {
|
||||
let output_dir = std::env::temp_dir().join(format!("codex_ts_filter_{}", Uuid::now_v7()));
|
||||
fs::create_dir_all(&output_dir)?;
|
||||
|
||||
struct TempDirGuard(PathBuf);
|
||||
|
||||
impl Drop for TempDirGuard {
|
||||
fn drop(&mut self) {
|
||||
let _ = fs::remove_dir_all(&self.0);
|
||||
}
|
||||
}
|
||||
|
||||
let _guard = TempDirGuard(output_dir.clone());
|
||||
let path = output_dir.join("CommandExecParams.ts");
|
||||
let content = r#"import type { CommandExecTerminalSize } from "./CommandExecTerminalSize";
|
||||
import type { PermissionProfile } from "./PermissionProfile";
|
||||
import type { SandboxPolicy } from "./SandboxPolicy";
|
||||
|
||||
export type CommandExecParams = {/**
|
||||
* Command argv vector. Empty arrays are rejected.
|
||||
*/
|
||||
command: Array<string>, /**
|
||||
* Optional environment overrides merged into the server-computed
|
||||
* environment.
|
||||
*/
|
||||
env?: { [key in string]?: string | null } | null, /**
|
||||
* Optional initial PTY size in character cells. Only valid when `tty` is
|
||||
* true.
|
||||
*/
|
||||
size?: CommandExecTerminalSize | null, /**
|
||||
* Optional sandbox policy for this command.
|
||||
*
|
||||
* Uses the same shape as thread/turn execution sandbox configuration and
|
||||
* defaults to the user's configured policy when omitted. Cannot be
|
||||
* combined with `permissionProfile`.
|
||||
*/
|
||||
sandboxPolicy?: SandboxPolicy | null,
|
||||
/**
|
||||
* Optional full permissions profile for this command.
|
||||
*
|
||||
* Defaults to the user's configured permissions when omitted. Cannot be
|
||||
* combined with `sandboxPolicy`.
|
||||
*/
|
||||
permissionProfile?: PermissionProfile | null};
|
||||
"#;
|
||||
fs::write(&path, content)?;
|
||||
|
||||
static CUSTOM_FIELD: crate::experimental_api::ExperimentalField =
|
||||
crate::experimental_api::ExperimentalField {
|
||||
type_name: "CommandExecParams",
|
||||
field_name: "permissionProfile",
|
||||
reason: "command/exec.permissionProfile",
|
||||
};
|
||||
filter_experimental_type_fields_ts(&output_dir, &[&CUSTOM_FIELD])?;
|
||||
|
||||
let filtered = fs::read_to_string(&path)?;
|
||||
assert_eq!(
|
||||
filtered.contains("permissionProfile?: PermissionProfile"),
|
||||
false
|
||||
);
|
||||
assert_eq!(
|
||||
filtered.contains(r#"import type { PermissionProfile } from "./PermissionProfile";"#),
|
||||
false
|
||||
);
|
||||
assert_eq!(filtered.contains("sandboxPolicy?: SandboxPolicy"), true);
|
||||
assert_eq!(
|
||||
filtered.contains(r#"import type { SandboxPolicy } from "./SandboxPolicy";"#),
|
||||
true
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn stable_schema_filter_removes_mock_experimental_method() -> Result<()> {
|
||||
let output_dir = std::env::temp_dir().join(format!("codex_schema_{}", Uuid::now_v7()));
|
||||
|
||||
@@ -581,6 +581,7 @@ client_request_definitions! {
|
||||
/// Execute a standalone command (argv vector) under the server's sandbox.
|
||||
OneOffCommandExec => "command/exec" {
|
||||
params: v2::CommandExecParams,
|
||||
inspect_params: true,
|
||||
response: v2::CommandExecResponse,
|
||||
},
|
||||
/// Write stdin bytes to a running `command/exec` session or close stdin.
|
||||
@@ -2049,6 +2050,33 @@ mod tests {
|
||||
let reason = crate::experimental_api::ExperimentalApi::experimental_reason(&request);
|
||||
assert_eq!(reason, Some("mock/experimentalMethod"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn command_exec_permission_profile_is_marked_experimental() {
|
||||
let request = ClientRequest::OneOffCommandExec {
|
||||
request_id: RequestId::Integer(1),
|
||||
params: v2::CommandExecParams {
|
||||
command: vec!["pwd".to_string()],
|
||||
process_id: None,
|
||||
tty: false,
|
||||
stream_stdin: false,
|
||||
stream_stdout_stderr: false,
|
||||
output_bytes_cap: None,
|
||||
disable_output_cap: false,
|
||||
disable_timeout: false,
|
||||
timeout_ms: None,
|
||||
cwd: None,
|
||||
env: None,
|
||||
size: None,
|
||||
sandbox_policy: None,
|
||||
permission_profile: Some(v2::PermissionProfile::Disabled),
|
||||
},
|
||||
};
|
||||
|
||||
let reason = crate::experimental_api::ExperimentalApi::experimental_reason(&request);
|
||||
assert_eq!(reason, Some("command/exec.permissionProfile"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn thread_realtime_start_is_marked_experimental() {
|
||||
let request = ClientRequest::ThreadRealtimeStart {
|
||||
|
||||
@@ -3163,7 +3163,7 @@ pub struct CommandExecTerminalSize {
|
||||
/// The final `command/exec` response is deferred until the process exits and is
|
||||
/// sent only after all `command/exec/outputDelta` notifications for that
|
||||
/// connection have been emitted.
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS, ExperimentalApi)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct CommandExecParams {
|
||||
@@ -3242,6 +3242,7 @@ pub struct CommandExecParams {
|
||||
///
|
||||
/// Defaults to the user's configured permissions when omitted. Cannot be
|
||||
/// combined with `sandboxPolicy`.
|
||||
#[experimental("command/exec.permissionProfile")]
|
||||
#[ts(optional = nullable)]
|
||||
pub permission_profile: Option<PermissionProfile>,
|
||||
}
|
||||
@@ -3364,6 +3365,7 @@ pub struct ThreadStartParams {
|
||||
pub sandbox: Option<SandboxMode>,
|
||||
/// Full permissions override for this thread. Cannot be combined with
|
||||
/// `sandbox`.
|
||||
#[experimental("thread/start.permissionProfile")]
|
||||
#[ts(optional = nullable)]
|
||||
pub permission_profile: Option<PermissionProfile>,
|
||||
#[ts(optional = nullable)]
|
||||
@@ -3447,6 +3449,7 @@ pub struct ThreadStartResponse {
|
||||
/// view.
|
||||
pub sandbox: SandboxPolicy,
|
||||
/// Canonical active permissions view for this thread.
|
||||
#[experimental("thread/start.permissionProfile")]
|
||||
#[serde(default)]
|
||||
pub permission_profile: Option<PermissionProfile>,
|
||||
pub reasoning_effort: Option<ReasoningEffort>,
|
||||
@@ -3508,6 +3511,7 @@ pub struct ThreadResumeParams {
|
||||
pub sandbox: Option<SandboxMode>,
|
||||
/// Full permissions override for the resumed thread. Cannot be combined
|
||||
/// with `sandbox`.
|
||||
#[experimental("thread/resume.permissionProfile")]
|
||||
#[ts(optional = nullable)]
|
||||
pub permission_profile: Option<PermissionProfile>,
|
||||
#[ts(optional = nullable)]
|
||||
@@ -3551,6 +3555,7 @@ pub struct ThreadResumeResponse {
|
||||
/// view.
|
||||
pub sandbox: SandboxPolicy,
|
||||
/// Canonical active permissions view for this thread.
|
||||
#[experimental("thread/resume.permissionProfile")]
|
||||
#[serde(default)]
|
||||
pub permission_profile: Option<PermissionProfile>,
|
||||
pub reasoning_effort: Option<ReasoningEffort>,
|
||||
@@ -3603,6 +3608,7 @@ pub struct ThreadForkParams {
|
||||
pub sandbox: Option<SandboxMode>,
|
||||
/// Full permissions override for the forked thread. Cannot be combined
|
||||
/// with `sandbox`.
|
||||
#[experimental("thread/fork.permissionProfile")]
|
||||
#[ts(optional = nullable)]
|
||||
pub permission_profile: Option<PermissionProfile>,
|
||||
#[ts(optional = nullable)]
|
||||
@@ -3646,6 +3652,7 @@ pub struct ThreadForkResponse {
|
||||
/// view.
|
||||
pub sandbox: SandboxPolicy,
|
||||
/// Canonical active permissions view for this thread.
|
||||
#[experimental("thread/fork.permissionProfile")]
|
||||
#[serde(default)]
|
||||
pub permission_profile: Option<PermissionProfile>,
|
||||
pub reasoning_effort: Option<ReasoningEffort>,
|
||||
@@ -5184,6 +5191,7 @@ pub struct TurnStartParams {
|
||||
pub sandbox_policy: Option<SandboxPolicy>,
|
||||
/// Override the full permissions profile for this turn and subsequent
|
||||
/// turns. Cannot be combined with `sandboxPolicy`.
|
||||
#[experimental("turn/start.permissionProfile")]
|
||||
#[ts(optional = nullable)]
|
||||
pub permission_profile: Option<PermissionProfile>,
|
||||
/// Override the model for this turn and subsequent turns.
|
||||
|
||||
Reference in New Issue
Block a user