mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
fix(app-server): suppress TUI rollback warning (#30124)
## Why The TUI uses `thread/rollback` internally for user-facing flows such as prompt cancellation/backtracking. After `thread/rollback` was marked deprecated, those internal calls started surfacing `deprecationNotice` messages in the TUI, even though the user did not explicitly call the deprecated app-server API. The endpoint should remain deprecated for external app-server clients, but the built-in `codex-tui` client should not show this implementation-detail warning during normal interaction. ## What changed - Pass the initialized app-server client name into the `thread/rollback` request processor. - Suppress the `thread/rollback` deprecation notice only for `codex-tui`. - Preserve the existing `deprecationNotice` behavior for non-TUI clients. - Add regression coverage for the `codex-tui` suppression path. ## How to Test 1. Start Codex TUI from this branch. 2. Type text into the composer and press `Esc` to cancel/backtrack. 3. Confirm the TUI restores/cancels the prompt without showing `thread/rollback is deprecated and will be removed soon`. 4. Also verify an external app-server client that calls `thread/rollback` still receives `deprecationNotice`. Targeted tests: - `just test -p codex-app-server thread_rollback` - `just argument-comment-lint`
This commit is contained in:
committed by
GitHub
Unverified
parent
c9e6d9783d
commit
b80fbb70cd
@@ -1205,7 +1205,7 @@ impl MessageProcessor {
|
||||
}
|
||||
ClientRequest::ThreadRollback { params, .. } => {
|
||||
self.thread_processor
|
||||
.thread_rollback(&request_id, params)
|
||||
.thread_rollback(&request_id, params, app_server_client_name.as_deref())
|
||||
.await
|
||||
}
|
||||
ClientRequest::ThreadList { params, .. } => {
|
||||
|
||||
@@ -8,6 +8,7 @@ use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE;
|
||||
|
||||
const THREAD_LIST_DEFAULT_LIMIT: usize = 25;
|
||||
const THREAD_LIST_MAX_LIMIT: usize = 100;
|
||||
const CODEX_TUI_CLIENT_NAME: &str = "codex-tui";
|
||||
const THREAD_ROLLBACK_DEPRECATION_SUMMARY: &str =
|
||||
"thread/rollback is deprecated and will be removed soon";
|
||||
|
||||
@@ -634,9 +635,12 @@ impl ThreadRequestProcessor {
|
||||
&self,
|
||||
request_id: &ConnectionRequestId,
|
||||
params: ThreadRollbackParams,
|
||||
app_server_client_name: Option<&str>,
|
||||
) -> Result<Option<ClientResponsePayload>, JSONRPCErrorError> {
|
||||
self.send_thread_rollback_deprecation_notice(request_id.connection_id)
|
||||
.await;
|
||||
if app_server_client_name != Some(CODEX_TUI_CLIENT_NAME) {
|
||||
self.send_thread_rollback_deprecation_notice(request_id.connection_id)
|
||||
.await;
|
||||
}
|
||||
self.thread_rollback_inner(request_id, params)
|
||||
.await
|
||||
.map(|()| None)
|
||||
|
||||
@@ -3,6 +3,7 @@ use app_test_support::TestAppServer;
|
||||
use app_test_support::create_final_assistant_message_sse_response;
|
||||
use app_test_support::create_mock_responses_server_sequence_unchecked;
|
||||
use app_test_support::to_response;
|
||||
use codex_app_server_protocol::ClientInfo;
|
||||
use codex_app_server_protocol::DeprecationNoticeNotification;
|
||||
use codex_app_server_protocol::JSONRPCMessage;
|
||||
use codex_app_server_protocol::JSONRPCResponse;
|
||||
@@ -24,6 +25,48 @@ use tokio::time::timeout;
|
||||
|
||||
const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_rollback_does_not_emit_deprecation_notice_to_codex_tui() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let mut mcp = TestAppServer::new_with_auto_env(codex_home.path()).await?;
|
||||
let initialized = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.initialize_with_client_info(ClientInfo {
|
||||
name: "codex-tui".to_string(),
|
||||
title: None,
|
||||
version: "0.1.0".to_string(),
|
||||
}),
|
||||
)
|
||||
.await??;
|
||||
let JSONRPCMessage::Response(_) = initialized else {
|
||||
panic!("expected initialize response, got {initialized:?}");
|
||||
};
|
||||
mcp.clear_message_buffer();
|
||||
|
||||
let rollback_id = mcp
|
||||
.send_thread_rollback_request(ThreadRollbackParams {
|
||||
thread_id: "00000000-0000-0000-0000-000000000001".to_string(),
|
||||
num_turns: 1,
|
||||
})
|
||||
.await?;
|
||||
loop {
|
||||
let message = timeout(DEFAULT_READ_TIMEOUT, mcp.read_next_message()).await??;
|
||||
match message {
|
||||
JSONRPCMessage::Notification(notification) => {
|
||||
assert_ne!(notification.method, "deprecationNotice");
|
||||
}
|
||||
JSONRPCMessage::Error(error) if error.id == RequestId::Integer(rollback_id) => {
|
||||
break;
|
||||
}
|
||||
message => {
|
||||
panic!("expected rollback error response, got {message:?}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_rollback_drops_last_turns_and_persists_to_rollout() -> Result<()> {
|
||||
// Three Codex turns hit the mock model (session start + two turn/start calls).
|
||||
|
||||
Reference in New Issue
Block a user