mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat(app-server): add optional thread_id to experimentalFeature/list (#23335)
## Why `experimentalFeature/list` reports effective feature enablement, but currently does not resolve it against a working directory where project-local config.toml files can exist and toggle on/off features when merged into the effective config after resolving the various config layers. That means we effectively (and incorrectly) ignore features set in project-local config. To address that, this PR exposes an optional `thread_id` param which allows us to load the thread's `cwd. ## Testing - `cargo test -p codex-app-server-protocol` - `cargo test -p codex-app-server experimental_feature_list`
This commit is contained in:
committed by
GitHub
Unverified
parent
8e52578e66
commit
139365a4bb
@@ -611,6 +611,13 @@
|
||||
"integer",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"threadId": {
|
||||
"description": "Optional loaded thread id. Pass this when showing feature state for an existing thread so enablement is computed from that thread's refreshed config, including project-local config for the thread's cwd.",
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
}
|
||||
},
|
||||
"type": "object"
|
||||
|
||||
+7
@@ -8194,6 +8194,13 @@
|
||||
"integer",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"threadId": {
|
||||
"description": "Optional loaded thread id. Pass this when showing feature state for an existing thread so enablement is computed from that thread's refreshed config, including project-local config for the thread's cwd.",
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
}
|
||||
},
|
||||
"title": "ExperimentalFeatureListParams",
|
||||
|
||||
+7
@@ -4583,6 +4583,13 @@
|
||||
"integer",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"threadId": {
|
||||
"description": "Optional loaded thread id. Pass this when showing feature state for an existing thread so enablement is computed from that thread's refreshed config, including project-local config for the thread's cwd.",
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
}
|
||||
},
|
||||
"title": "ExperimentalFeatureListParams",
|
||||
|
||||
+7
@@ -16,6 +16,13 @@
|
||||
"integer",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"threadId": {
|
||||
"description": "Optional loaded thread id. Pass this when showing feature state for an existing thread so enablement is computed from that thread's refreshed config, including project-local config for the thread's cwd.",
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
}
|
||||
},
|
||||
"title": "ExperimentalFeatureListParams",
|
||||
|
||||
+7
-1
@@ -10,4 +10,10 @@ cursor?: string | null,
|
||||
/**
|
||||
* Optional page size; defaults to a reasonable server-side value.
|
||||
*/
|
||||
limit?: number | null, };
|
||||
limit?: number | null,
|
||||
/**
|
||||
* Optional loaded thread id. Pass this when showing feature state for an
|
||||
* existing thread so enablement is computed from that thread's refreshed
|
||||
* config, including project-local config for the thread's cwd.
|
||||
*/
|
||||
threadId?: string | null, };
|
||||
|
||||
@@ -2721,7 +2721,33 @@ mod tests {
|
||||
"id": 8,
|
||||
"params": {
|
||||
"cursor": null,
|
||||
"limit": null
|
||||
"limit": null,
|
||||
"threadId": null
|
||||
}
|
||||
}),
|
||||
serde_json::to_value(&request)?,
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn serialize_list_experimental_features_with_thread_id() -> Result<()> {
|
||||
let request = ClientRequest::ExperimentalFeatureList {
|
||||
request_id: RequestId::Integer(8),
|
||||
params: v2::ExperimentalFeatureListParams {
|
||||
cursor: Some("3".to_string()),
|
||||
limit: Some(2),
|
||||
thread_id: Some("00000000-0000-4000-8000-000000000001".to_string()),
|
||||
},
|
||||
};
|
||||
assert_eq!(
|
||||
json!({
|
||||
"method": "experimentalFeature/list",
|
||||
"id": 8,
|
||||
"params": {
|
||||
"cursor": "3",
|
||||
"limit": 2,
|
||||
"threadId": "00000000-0000-4000-8000-000000000001"
|
||||
}
|
||||
}),
|
||||
serde_json::to_value(&request)?,
|
||||
|
||||
@@ -14,6 +14,11 @@ pub struct ExperimentalFeatureListParams {
|
||||
/// Optional page size; defaults to a reasonable server-side value.
|
||||
#[ts(optional = nullable)]
|
||||
pub limit: Option<u32>,
|
||||
/// Optional loaded thread id. Pass this when showing feature state for an
|
||||
/// existing thread so enablement is computed from that thread's refreshed
|
||||
/// config, including project-local config for the thread's cwd.
|
||||
#[ts(optional = nullable)]
|
||||
pub thread_id: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)]
|
||||
|
||||
@@ -188,7 +188,7 @@ Example with notification opt-out:
|
||||
- `fs/changed` — notification emitted when watched paths change, including the `watchId` and `changedPaths`.
|
||||
- `model/list` — list available models (set `includeHidden: true` to include entries with `hidden: true`), with reasoning effort options, `additionalSpeedTiers`, optional legacy `upgrade` model ids, optional `upgradeInfo` metadata (`model`, `upgradeCopy`, `modelLink`, `migrationMarkdown`), and optional `availabilityNux` metadata.
|
||||
- `modelProvider/capabilities/read` — read provider-level capabilities for the currently configured model provider.
|
||||
- `experimentalFeature/list` — list feature flags with stage metadata (`beta`, `underDevelopment`, `stable`, etc.), enabled/default-enabled state, and cursor pagination. For non-beta flags, `displayName`/`description`/`announcement` are `null`.
|
||||
- `experimentalFeature/list` — list feature flags with stage metadata (`beta`, `underDevelopment`, `stable`, etc.), enabled/default-enabled state, and cursor pagination. Pass `threadId` when showing feature state for an existing loaded thread so `enabled` is computed from that thread's refreshed config, including project-local config for the thread's cwd; if omitted, the server uses its default config resolution context. For non-beta flags, `displayName`/`description`/`announcement` are `null`.
|
||||
- `experimentalFeature/enablement/set` — patch the in-memory process-wide runtime feature enablement for the currently supported feature keys (`apps`, `memories`, `plugins`, `tool_search`, `tool_suggest`, `tool_call_mcp_elicitation`). For each feature, precedence is: cloud requirements > --enable <feature_name> > config.toml > experimentalFeature/enablement/set (new) > code default.
|
||||
- `environment/add` — experimental; add or replace a named remote environment by `environmentId` and `execServerUrl` for later selection by `thread/start` or `turn/start`; returns `{}` and does not change the default environment.
|
||||
- `collaborationMode/list` — list available collaboration mode presets (experimental, no pagination). Built-in presets do not select a model; the Plan preset selects medium reasoning effort. This response omits built-in developer instructions; clients should either pass `settings.developer_instructions: null` when setting a mode to use Codex's built-in instructions, or provide their own instructions explicitly.
|
||||
|
||||
@@ -285,8 +285,28 @@ impl CatalogRequestProcessor {
|
||||
&self,
|
||||
params: ExperimentalFeatureListParams,
|
||||
) -> Result<ExperimentalFeatureListResponse, JSONRPCErrorError> {
|
||||
let ExperimentalFeatureListParams { cursor, limit } = params;
|
||||
let config = self.load_latest_config(/*fallback_cwd*/ None).await?;
|
||||
let ExperimentalFeatureListParams {
|
||||
cursor,
|
||||
limit,
|
||||
thread_id,
|
||||
} = params;
|
||||
let config = match thread_id.as_deref() {
|
||||
Some(thread_id) => {
|
||||
let thread_id = ThreadId::from_string(thread_id)
|
||||
.map_err(|err| invalid_request(format!("invalid thread id: {err}")))?;
|
||||
let thread = self
|
||||
.thread_manager
|
||||
.get_thread(thread_id)
|
||||
.await
|
||||
.map_err(|_| invalid_request(format!("thread not found: {thread_id}")))?;
|
||||
let thread_config = thread.config().await;
|
||||
self.config_manager
|
||||
.load_latest_config_for_thread(thread_config.as_ref())
|
||||
.await
|
||||
.map_err(|err| internal_error(format!("failed to reload config: {err}")))?
|
||||
}
|
||||
None => self.load_latest_config(/*fallback_cwd*/ None).await?,
|
||||
};
|
||||
let auth = self.auth_manager.auth().await;
|
||||
let workspace_codex_plugins_enabled = self
|
||||
.workspace_codex_plugins_enabled(&config, auth.as_ref())
|
||||
|
||||
@@ -3,6 +3,7 @@ use std::time::Duration;
|
||||
use anyhow::Result;
|
||||
use app_test_support::ChatGptAuthFixture;
|
||||
use app_test_support::McpProcess;
|
||||
use app_test_support::create_mock_responses_server_repeating_assistant;
|
||||
use app_test_support::to_response;
|
||||
use app_test_support::write_chatgpt_auth;
|
||||
use codex_app_server_protocol::ConfigReadParams;
|
||||
@@ -16,6 +17,8 @@ use codex_app_server_protocol::ExperimentalFeatureStage;
|
||||
use codex_app_server_protocol::JSONRPCError;
|
||||
use codex_app_server_protocol::JSONRPCResponse;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_app_server_protocol::ThreadStartParams;
|
||||
use codex_app_server_protocol::ThreadStartResponse;
|
||||
use codex_config::LoaderOverrides;
|
||||
use codex_config::types::AuthCredentialsStoreMode;
|
||||
use codex_core::config::ConfigBuilder;
|
||||
@@ -156,6 +159,104 @@ async fn experimental_feature_list_marks_apps_and_plugins_disabled_by_workspace_
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn experimental_feature_list_resolves_thread_project_config() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
let workspace = TempDir::new()?;
|
||||
let server_uri = server.uri();
|
||||
let workspace_key = workspace.path().to_string_lossy().replace('\\', "\\\\");
|
||||
std::fs::write(
|
||||
codex_home.path().join("config.toml"),
|
||||
format!(
|
||||
r#"model = "mock-model"
|
||||
approval_policy = "never"
|
||||
sandbox_mode = "read-only"
|
||||
model_provider = "mock_provider"
|
||||
|
||||
[projects."{workspace_key}"]
|
||||
trust_level = "trusted"
|
||||
|
||||
[model_providers.mock_provider]
|
||||
name = "Mock provider for test"
|
||||
base_url = "{server_uri}/v1"
|
||||
wire_api = "responses"
|
||||
request_max_retries = 0
|
||||
stream_max_retries = 0
|
||||
"#
|
||||
),
|
||||
)?;
|
||||
let project_config_dir = workspace.path().join(".codex");
|
||||
std::fs::create_dir_all(&project_config_dir)?;
|
||||
std::fs::write(
|
||||
project_config_dir.join("config.toml"),
|
||||
r#"[features]
|
||||
memories = true
|
||||
"#,
|
||||
)?;
|
||||
|
||||
let mut mcp = McpProcess::new_without_managed_config(codex_home.path()).await?;
|
||||
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let thread_start_id = mcp
|
||||
.send_thread_start_request(ThreadStartParams {
|
||||
cwd: Some(workspace.path().display().to_string()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let ThreadStartResponse { thread, .. } =
|
||||
read_response::<ThreadStartResponse>(&mut mcp, thread_start_id).await?;
|
||||
|
||||
let request_id = mcp
|
||||
.send_experimental_feature_list_request(ExperimentalFeatureListParams {
|
||||
cursor: None,
|
||||
limit: None,
|
||||
thread_id: Some(thread.id),
|
||||
})
|
||||
.await?;
|
||||
|
||||
let actual = read_response::<ExperimentalFeatureListResponse>(&mut mcp, request_id).await?;
|
||||
let memories = actual
|
||||
.data
|
||||
.iter()
|
||||
.find(|feature| feature.name == "memories")
|
||||
.expect("memories feature should be present");
|
||||
assert!(memories.enabled);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn experimental_feature_list_rejects_unknown_thread_id() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let request_id = mcp
|
||||
.send_experimental_feature_list_request(ExperimentalFeatureListParams {
|
||||
cursor: None,
|
||||
limit: None,
|
||||
thread_id: Some("00000000-0000-4000-8000-000000000001".to_string()),
|
||||
})
|
||||
.await?;
|
||||
let JSONRPCError { error, .. } = timeout(
|
||||
DEFAULT_TIMEOUT,
|
||||
mcp.read_stream_until_error_message(RequestId::Integer(request_id)),
|
||||
)
|
||||
.await??;
|
||||
|
||||
assert_eq!(error.code, -32600);
|
||||
assert!(
|
||||
error
|
||||
.message
|
||||
.contains("thread not found: 00000000-0000-4000-8000-000000000001"),
|
||||
"{}",
|
||||
error.message
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn experimental_feature_enablement_set_applies_to_global_and_thread_config_reads()
|
||||
-> Result<()> {
|
||||
|
||||
Reference in New Issue
Block a user