Merge 7e295bf422 into sapling-pr-archive-bolinfest

This commit is contained in:
Michael Bolin
2026-04-17 14:24:05 -07:00
committed by GitHub
73 changed files with 2930 additions and 480 deletions
+2 -2
View File
@@ -30,7 +30,7 @@ Accept any of the following:
5. If the failure is likely caused by the current branch, patch code locally, commit, and push.
6. If `process_review_comment` is present, inspect surfaced review items and decide whether to address them.
7. If a review item is actionable and correct, patch code locally, commit, push, and then mark the associated review thread/comment as resolved once the fix is on GitHub.
8. If a review item from another author is non-actionable, already addressed, or not valid, post one reply on the comment/thread explaining that decision (for example answering the question or explaining why no change is needed). If the watcher later surfaces your own reply, treat that self-authored item as already handled and do not reply again.
8. If a review item from another author is non-actionable, already addressed, or not valid, post one reply on the comment/thread explaining that decision (for example answering the question or explaining why no change is needed). Prefix the GitHub reply body with `[codex]` so it is clear the response is automated. If the watcher later surfaces your own reply, treat that self-authored item as already handled and do not reply again.
9. If the failure is likely flaky/unrelated and `retry_failed_checks` is present, rerun failed jobs with `--retry-failed-now`.
10. If both actionable review feedback and `retry_failed_checks` are present, prioritize review feedback first; a new commit will retrigger CI, so avoid rerunning flaky checks on the old SHA unless you intentionally defer the review change.
11. On every loop, look for newly surfaced review feedback before acting on CI failures or mergeability state, then verify mergeability / merge-conflict status (for example via `gh pr view`) alongside CI.
@@ -99,7 +99,7 @@ When you agree with a comment and it is actionable:
5. Resume watching on the new SHA immediately (do not stop after reporting the push).
6. If monitoring was running in `--watch` mode, restart `--watch` immediately after the push in the same turn; do not wait for the user to ask again.
If you disagree or the comment is non-actionable/already addressed, reply once directly on the GitHub comment/thread so the reviewer gets an explicit answer, then continue the watcher loop. If the watcher later surfaces your own reply because the authenticated operator is treated as a trusted review author, treat that self-authored item as already handled and do not reply again.
If you disagree or the comment is non-actionable/already addressed, reply once directly on the GitHub comment/thread so the reviewer gets an explicit answer, then continue the watcher loop. Prefix any GitHub reply to a code review comment/thread with `[codex]` so it is clear the response is automated and not from the human user. If the watcher later surfaces your own reply because the authenticated operator is treated as a trusted review author, treat that self-authored item as already handled and do not reply again.
If a code review comment/thread is already marked as resolved in GitHub, treat it as non-actionable and safely ignore it unless new unresolved follow-up feedback appears.
## Git Safety Rules
+2
View File
@@ -1409,6 +1409,7 @@ version = "0.0.0"
dependencies = [
"anyhow",
"assert_matches",
"async-channel",
"async-trait",
"base64 0.22.1",
"bytes",
@@ -1842,6 +1843,7 @@ dependencies = [
name = "codex-code-mode"
version = "0.0.0"
dependencies = [
"async-channel",
"async-trait",
"codex-protocol",
"deno_core_icudata",
@@ -2046,6 +2046,16 @@
],
"type": "string"
},
"RateLimitReachedType": {
"enum": [
"rate_limit_reached",
"workspace_owner_credits_depleted",
"workspace_member_credits_depleted",
"workspace_owner_usage_limit_reached",
"workspace_member_usage_limit_reached"
],
"type": "string"
},
"RateLimitSnapshot": {
"properties": {
"credits": {
@@ -2090,6 +2100,16 @@
}
]
},
"rateLimitReachedType": {
"anyOf": [
{
"$ref": "#/definitions/RateLimitReachedType"
},
{
"type": "null"
}
]
},
"secondary": {
"anyOf": [
{
@@ -10888,6 +10888,16 @@
},
"type": "object"
},
"RateLimitReachedType": {
"enum": [
"rate_limit_reached",
"workspace_owner_credits_depleted",
"workspace_member_credits_depleted",
"workspace_owner_usage_limit_reached",
"workspace_member_usage_limit_reached"
],
"type": "string"
},
"RateLimitSnapshot": {
"properties": {
"credits": {
@@ -10932,6 +10942,16 @@
}
]
},
"rateLimitReachedType": {
"anyOf": [
{
"$ref": "#/definitions/v2/RateLimitReachedType"
},
{
"type": "null"
}
]
},
"secondary": {
"anyOf": [
{
@@ -7420,6 +7420,16 @@
},
"type": "object"
},
"RateLimitReachedType": {
"enum": [
"rate_limit_reached",
"workspace_owner_credits_depleted",
"workspace_member_credits_depleted",
"workspace_owner_usage_limit_reached",
"workspace_member_usage_limit_reached"
],
"type": "string"
},
"RateLimitSnapshot": {
"properties": {
"credits": {
@@ -7464,6 +7474,16 @@
}
]
},
"rateLimitReachedType": {
"anyOf": [
{
"$ref": "#/definitions/RateLimitReachedType"
},
{
"type": "null"
}
]
},
"secondary": {
"anyOf": [
{
@@ -39,6 +39,16 @@
],
"type": "string"
},
"RateLimitReachedType": {
"enum": [
"rate_limit_reached",
"workspace_owner_credits_depleted",
"workspace_member_credits_depleted",
"workspace_owner_usage_limit_reached",
"workspace_member_usage_limit_reached"
],
"type": "string"
},
"RateLimitSnapshot": {
"properties": {
"credits": {
@@ -83,6 +93,16 @@
}
]
},
"rateLimitReachedType": {
"anyOf": [
{
"$ref": "#/definitions/RateLimitReachedType"
},
{
"type": "null"
}
]
},
"secondary": {
"anyOf": [
{
@@ -39,6 +39,16 @@
],
"type": "string"
},
"RateLimitReachedType": {
"enum": [
"rate_limit_reached",
"workspace_owner_credits_depleted",
"workspace_member_credits_depleted",
"workspace_owner_usage_limit_reached",
"workspace_member_usage_limit_reached"
],
"type": "string"
},
"RateLimitSnapshot": {
"properties": {
"credits": {
@@ -83,6 +93,16 @@
}
]
},
"rateLimitReachedType": {
"anyOf": [
{
"$ref": "#/definitions/RateLimitReachedType"
},
{
"type": "null"
}
]
},
"secondary": {
"anyOf": [
{
@@ -0,0 +1,5 @@
// GENERATED CODE! DO NOT MODIFY BY HAND!
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
export type RateLimitReachedType = "rate_limit_reached" | "workspace_owner_credits_depleted" | "workspace_member_credits_depleted" | "workspace_owner_usage_limit_reached" | "workspace_member_usage_limit_reached";
@@ -3,6 +3,7 @@
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
import type { PlanType } from "../PlanType";
import type { CreditsSnapshot } from "./CreditsSnapshot";
import type { RateLimitReachedType } from "./RateLimitReachedType";
import type { RateLimitWindow } from "./RateLimitWindow";
export type RateLimitSnapshot = { limitId: string | null, limitName: string | null, primary: RateLimitWindow | null, secondary: RateLimitWindow | null, credits: CreditsSnapshot | null, planType: PlanType | null, };
export type RateLimitSnapshot = { limitId: string | null, limitName: string | null, primary: RateLimitWindow | null, secondary: RateLimitWindow | null, credits: CreditsSnapshot | null, planType: PlanType | null, rateLimitReachedType: RateLimitReachedType | null, };
@@ -245,6 +245,7 @@ export type { PluginUninstallParams } from "./PluginUninstallParams";
export type { PluginUninstallResponse } from "./PluginUninstallResponse";
export type { PluginsMigration } from "./PluginsMigration";
export type { ProfileV2 } from "./ProfileV2";
export type { RateLimitReachedType } from "./RateLimitReachedType";
export type { RateLimitSnapshot } from "./RateLimitSnapshot";
export type { RateLimitWindow } from "./RateLimitWindow";
export type { RawResponseItemCompletedNotification } from "./RawResponseItemCompletedNotification";
@@ -74,6 +74,7 @@ use codex_protocol::protocol::ModelRerouteReason as CoreModelRerouteReason;
use codex_protocol::protocol::NetworkAccess as CoreNetworkAccess;
use codex_protocol::protocol::NonSteerableTurnKind as CoreNonSteerableTurnKind;
use codex_protocol::protocol::PatchApplyStatus as CorePatchApplyStatus;
use codex_protocol::protocol::RateLimitReachedType as CoreRateLimitReachedType;
use codex_protocol::protocol::RateLimitSnapshot as CoreRateLimitSnapshot;
use codex_protocol::protocol::RateLimitWindow as CoreRateLimitWindow;
use codex_protocol::protocol::ReadOnlyAccess as CoreReadOnlyAccess;
@@ -6625,6 +6626,7 @@ pub struct RateLimitSnapshot {
pub secondary: Option<RateLimitWindow>,
pub credits: Option<CreditsSnapshot>,
pub plan_type: Option<PlanType>,
pub rate_limit_reached_type: Option<RateLimitReachedType>,
}
impl From<CoreRateLimitSnapshot> for RateLimitSnapshot {
@@ -6636,6 +6638,60 @@ impl From<CoreRateLimitSnapshot> for RateLimitSnapshot {
secondary: value.secondary.map(RateLimitWindow::from),
credits: value.credits.map(CreditsSnapshot::from),
plan_type: value.plan_type,
rate_limit_reached_type: value
.rate_limit_reached_type
.map(RateLimitReachedType::from),
}
}
}
#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema, TS)]
#[serde(rename_all = "snake_case")]
#[ts(export_to = "v2/", rename_all = "snake_case")]
pub enum RateLimitReachedType {
RateLimitReached,
WorkspaceOwnerCreditsDepleted,
WorkspaceMemberCreditsDepleted,
WorkspaceOwnerUsageLimitReached,
WorkspaceMemberUsageLimitReached,
}
impl From<CoreRateLimitReachedType> for RateLimitReachedType {
fn from(value: CoreRateLimitReachedType) -> Self {
match value {
CoreRateLimitReachedType::RateLimitReached => Self::RateLimitReached,
CoreRateLimitReachedType::WorkspaceOwnerCreditsDepleted => {
Self::WorkspaceOwnerCreditsDepleted
}
CoreRateLimitReachedType::WorkspaceMemberCreditsDepleted => {
Self::WorkspaceMemberCreditsDepleted
}
CoreRateLimitReachedType::WorkspaceOwnerUsageLimitReached => {
Self::WorkspaceOwnerUsageLimitReached
}
CoreRateLimitReachedType::WorkspaceMemberUsageLimitReached => {
Self::WorkspaceMemberUsageLimitReached
}
}
}
}
impl From<RateLimitReachedType> for CoreRateLimitReachedType {
fn from(value: RateLimitReachedType) -> Self {
match value {
RateLimitReachedType::RateLimitReached => Self::RateLimitReached,
RateLimitReachedType::WorkspaceOwnerCreditsDepleted => {
Self::WorkspaceOwnerCreditsDepleted
}
RateLimitReachedType::WorkspaceMemberCreditsDepleted => {
Self::WorkspaceMemberCreditsDepleted
}
RateLimitReachedType::WorkspaceOwnerUsageLimitReached => {
Self::WorkspaceOwnerUsageLimitReached
}
RateLimitReachedType::WorkspaceMemberUsageLimitReached => {
Self::WorkspaceMemberUsageLimitReached
}
}
}
}
@@ -2016,9 +2016,12 @@ async fn complete_file_change_item(
outgoing: &ThreadScopedOutgoingMessageSender,
thread_state: &Arc<Mutex<ThreadState>>,
) {
let mut state = thread_state.lock().await;
state.turn_summary.file_change_started.remove(&item_id);
drop(state);
thread_state
.lock()
.await
.turn_summary
.file_change_started
.remove(&item_id);
let notification = ItemCompletedNotification {
thread_id: conversation_id.to_string(),
@@ -2087,12 +2090,12 @@ async fn complete_command_execution_item(
outgoing: &ThreadScopedOutgoingMessageSender,
thread_state: &Arc<Mutex<ThreadState>>,
) {
let mut state = thread_state.lock().await;
let should_emit = state
let should_emit = thread_state
.lock()
.await
.turn_summary
.command_execution_started
.remove(&item_id);
drop(state);
if !should_emit {
return;
}
@@ -4138,6 +4141,7 @@ mod tests {
balance: Some("5".to_string()),
}),
plan_type: None,
rate_limit_reached_type: None,
};
handle_token_count_event(
@@ -3510,9 +3510,7 @@ impl CodexMessageProcessor {
// No ThreadRollback event will arrive if an error occurs.
// Clean up and reply immediately.
let thread_state = self.thread_state_manager.thread_state(thread_id).await;
let mut thread_state = thread_state.lock().await;
thread_state.pending_rollbacks = None;
drop(thread_state);
thread_state.lock().await.pending_rollbacks = None;
self.send_internal_error(request, format!("failed to start rollback: {err}"))
.await;
@@ -8051,8 +8049,10 @@ impl CodexMessageProcessor {
start_fuzzy_file_search_session(session_id.clone(), roots, self.outgoing.clone());
match session {
Ok(session) => {
let mut sessions = self.fuzzy_search_sessions.lock().await;
sessions.insert(session_id, session);
self.fuzzy_search_sessions
.lock()
.await
.insert(session_id, session);
self.outgoing
.send_response(request_id, FuzzyFileSearchSessionStartResponse {})
.await;
@@ -10446,13 +10446,9 @@ mod tests {
assert_eq!(cancel_rx.await, Ok(()));
let state = manager.thread_state(thread_id).await;
let subscribed_connection_ids = manager.subscribed_connection_ids(thread_id).await;
assert!(subscribed_connection_ids.is_empty());
let state = state.lock().await;
assert!(
manager
.subscribed_connection_ids(thread_id)
.await
.is_empty()
);
assert!(state.cancel_tx.is_none());
assert!(state.active_turn_snapshot().is_none());
Ok(())
+3 -1
View File
@@ -736,6 +736,7 @@ mod tests {
secondary: None,
credits: None,
plan_type: Some(PlanType::Plus),
rate_limit_reached_type: None,
},
});
@@ -754,7 +755,8 @@ mod tests {
},
"secondary": null,
"credits": null,
"planType": "plus"
"planType": "plus",
"rateLimitReachedType": null
}
},
}),
@@ -392,7 +392,14 @@ impl RemoteControlWebsocket {
ping_interval: std::time::Duration,
shutdown_token: CancellationToken,
) -> io::Result<()> {
for server_envelope in state.lock().await.outbound_buffer.server_envelopes() {
let server_envelopes = state
.lock()
.await
.outbound_buffer
.server_envelopes()
.cloned()
.collect::<Vec<_>>();
for server_envelope in server_envelopes {
let payload = match serde_json::to_string(&server_envelope) {
Ok(payload) => payload,
Err(err) => {
@@ -594,21 +601,22 @@ impl RemoteControlWebsocket {
}
};
let mut websocket_state = state.lock().await;
if let Some(cursor) = client_envelope.cursor.as_deref() {
websocket_state.subscribe_cursor = Some(cursor.to_string());
}
if let ClientEvent::Ack = &client_envelope.event
&& let Some(acked_seq_id) = client_envelope.seq_id
&& let Some(stream_id) = client_envelope.stream_id.as_ref()
{
websocket_state.outbound_buffer.ack(
&client_envelope.client_id,
stream_id,
acked_seq_id,
);
let mut websocket_state = state.lock().await;
if let Some(cursor) = client_envelope.cursor.as_deref() {
websocket_state.subscribe_cursor = Some(cursor.to_string());
}
if let ClientEvent::Ack = &client_envelope.event
&& let Some(acked_seq_id) = client_envelope.seq_id
&& let Some(stream_id) = client_envelope.stream_id.as_ref()
{
websocket_state.outbound_buffer.ack(
&client_envelope.client_id,
stream_id,
acked_seq_id,
);
}
}
drop(websocket_state);
if client_tracker
.handle_message(client_envelope)
@@ -7,6 +7,7 @@ use codex_app_server_protocol::GetAccountRateLimitsResponse;
use codex_app_server_protocol::JSONRPCError;
use codex_app_server_protocol::JSONRPCResponse;
use codex_app_server_protocol::LoginAccountResponse;
use codex_app_server_protocol::RateLimitReachedType;
use codex_app_server_protocol::RateLimitSnapshot;
use codex_app_server_protocol::RateLimitWindow;
use codex_app_server_protocol::RequestId;
@@ -118,6 +119,9 @@ async fn get_account_rate_limits_returns_snapshot() -> Result<()> {
"reset_at": secondary_reset_timestamp,
}
},
"rate_limit_reached_type": {
"type": "workspace_member_usage_limit_reached",
},
"additional_rate_limits": [
{
"limit_name": "codex_other",
@@ -173,6 +177,7 @@ async fn get_account_rate_limits_returns_snapshot() -> Result<()> {
}),
credits: None,
plan_type: Some(AccountPlanType::Pro),
rate_limit_reached_type: Some(RateLimitReachedType::WorkspaceMemberUsageLimitReached),
},
rate_limits_by_limit_id: Some(
[
@@ -193,6 +198,9 @@ async fn get_account_rate_limits_returns_snapshot() -> Result<()> {
}),
credits: None,
plan_type: Some(AccountPlanType::Pro),
rate_limit_reached_type: Some(
RateLimitReachedType::WorkspaceMemberUsageLimitReached,
),
},
),
(
@@ -208,6 +216,7 @@ async fn get_account_rate_limits_returns_snapshot() -> Result<()> {
secondary: None,
credits: None,
plan_type: Some(AccountPlanType::Pro),
rate_limit_reached_type: None,
},
),
]
+100
View File
@@ -1,6 +1,7 @@
use crate::types::CodeTaskDetailsResponse;
use crate::types::ConfigFileResponse;
use crate::types::PaginatedListTaskListItem;
use crate::types::RateLimitReachedKind as BackendRateLimitReachedKind;
use crate::types::RateLimitStatusPayload;
use crate::types::TurnAttemptsSiblingTurnsResponse;
use anyhow::Result;
@@ -9,6 +10,7 @@ use codex_login::CodexAuth;
use codex_login::default_client::get_codex_user_agent;
use codex_protocol::account::PlanType as AccountPlanType;
use codex_protocol::protocol::CreditsSnapshot;
use codex_protocol::protocol::RateLimitReachedType;
use codex_protocol::protocol::RateLimitSnapshot;
use codex_protocol::protocol::RateLimitWindow;
use reqwest::StatusCode;
@@ -412,12 +414,17 @@ impl Client {
payload: RateLimitStatusPayload,
) -> Vec<RateLimitSnapshot> {
let plan_type = Some(Self::map_plan_type(payload.plan_type));
let rate_limit_reached_type = payload
.rate_limit_reached_type
.flatten()
.and_then(|details| Self::map_rate_limit_reached_type(details.kind));
let mut snapshots = vec![Self::make_rate_limit_snapshot(
Some("codex".to_string()),
/*limit_name*/ None,
payload.rate_limit.flatten().map(|details| *details),
payload.credits.flatten().map(|details| *details),
plan_type,
rate_limit_reached_type,
)];
if let Some(additional) = payload.additional_rate_limits.flatten() {
snapshots.extend(additional.into_iter().map(|details| {
@@ -427,6 +434,7 @@ impl Client {
details.rate_limit.flatten().map(|rate_limit| *rate_limit),
/*credits*/ None,
plan_type,
/*rate_limit_reached_type*/ None,
)
}));
}
@@ -439,6 +447,7 @@ impl Client {
rate_limit: Option<crate::types::RateLimitStatusDetails>,
credits: Option<crate::types::CreditStatusDetails>,
plan_type: Option<AccountPlanType>,
rate_limit_reached_type: Option<RateLimitReachedType>,
) -> RateLimitSnapshot {
let (primary, secondary) = match rate_limit {
Some(details) => (
@@ -454,6 +463,30 @@ impl Client {
secondary,
credits: Self::map_credits(credits),
plan_type,
rate_limit_reached_type,
}
}
fn map_rate_limit_reached_type(
kind: BackendRateLimitReachedKind,
) -> Option<RateLimitReachedType> {
match kind {
BackendRateLimitReachedKind::RateLimitReached => {
Some(RateLimitReachedType::RateLimitReached)
}
BackendRateLimitReachedKind::WorkspaceOwnerCreditsDepleted => {
Some(RateLimitReachedType::WorkspaceOwnerCreditsDepleted)
}
BackendRateLimitReachedKind::WorkspaceMemberCreditsDepleted => {
Some(RateLimitReachedType::WorkspaceMemberCreditsDepleted)
}
BackendRateLimitReachedKind::WorkspaceOwnerUsageLimitReached => {
Some(RateLimitReachedType::WorkspaceOwnerUsageLimitReached)
}
BackendRateLimitReachedKind::WorkspaceMemberUsageLimitReached => {
Some(RateLimitReachedType::WorkspaceMemberUsageLimitReached)
}
BackendRateLimitReachedKind::Unknown => None,
}
}
@@ -521,6 +554,8 @@ impl Client {
mod tests {
use super::*;
use codex_backend_openapi_models::models::AdditionalRateLimitDetails;
use codex_backend_openapi_models::models::RateLimitReachedKind;
use codex_backend_openapi_models::models::RateLimitReachedType as BackendRateLimitReachedType;
use pretty_assertions::assert_eq;
#[test]
@@ -574,6 +609,9 @@ mod tests {
balance: Some(Some("9.99".to_string())),
..Default::default()
}))),
rate_limit_reached_type: Some(Some(BackendRateLimitReachedType {
kind: RateLimitReachedKind::WorkspaceMemberCreditsDepleted,
})),
};
let snapshots = Client::rate_limit_snapshots_from_payload(payload);
@@ -598,6 +636,10 @@ mod tests {
})
);
assert_eq!(snapshots[0].plan_type, Some(AccountPlanType::Pro));
assert_eq!(
snapshots[0].rate_limit_reached_type,
Some(RateLimitReachedType::WorkspaceMemberCreditsDepleted)
);
assert_eq!(snapshots[1].limit_id.as_deref(), Some("codex_other"));
assert_eq!(snapshots[1].limit_name.as_deref(), Some("codex_other"));
@@ -607,6 +649,7 @@ mod tests {
);
assert_eq!(snapshots[1].credits, None);
assert_eq!(snapshots[1].plan_type, Some(AccountPlanType::Pro));
assert_eq!(snapshots[1].rate_limit_reached_type, None);
}
#[test]
@@ -620,6 +663,7 @@ mod tests {
rate_limit: None,
}])),
credits: None,
rate_limit_reached_type: None,
};
let snapshots = Client::rate_limit_snapshots_from_payload(payload);
@@ -645,6 +689,7 @@ mod tests {
secondary: None,
credits: None,
plan_type: Some(AccountPlanType::Pro),
rate_limit_reached_type: None,
},
RateLimitSnapshot {
limit_id: Some("codex".to_string()),
@@ -657,6 +702,7 @@ mod tests {
secondary: None,
credits: None,
plan_type: Some(AccountPlanType::Pro),
rate_limit_reached_type: None,
},
];
@@ -667,4 +713,58 @@ mod tests {
.unwrap_or_else(|| snapshots[0].clone());
assert_eq!(preferred.limit_id.as_deref(), Some("codex"));
}
#[test]
fn usage_payload_maps_every_rate_limit_reached_type() {
let cases = [
(
RateLimitReachedKind::RateLimitReached,
Some(RateLimitReachedType::RateLimitReached),
),
(
RateLimitReachedKind::WorkspaceOwnerCreditsDepleted,
Some(RateLimitReachedType::WorkspaceOwnerCreditsDepleted),
),
(
RateLimitReachedKind::WorkspaceMemberCreditsDepleted,
Some(RateLimitReachedType::WorkspaceMemberCreditsDepleted),
),
(
RateLimitReachedKind::WorkspaceOwnerUsageLimitReached,
Some(RateLimitReachedType::WorkspaceOwnerUsageLimitReached),
),
(
RateLimitReachedKind::WorkspaceMemberUsageLimitReached,
Some(RateLimitReachedType::WorkspaceMemberUsageLimitReached),
),
(RateLimitReachedKind::Unknown, None),
];
for (kind, expected) in cases {
let payload = RateLimitStatusPayload {
plan_type: crate::types::PlanType::Plus,
rate_limit: None,
credits: None,
additional_rate_limits: None,
rate_limit_reached_type: Some(Some(BackendRateLimitReachedType { kind })),
};
let snapshots = Client::rate_limit_snapshots_from_payload(payload);
assert_eq!(snapshots[0].rate_limit_reached_type, expected);
}
}
#[test]
fn usage_payload_preserves_absent_rate_limit_reached_type() {
let payload = RateLimitStatusPayload {
plan_type: crate::types::PlanType::Plus,
rate_limit: None,
credits: None,
additional_rate_limits: None,
rate_limit_reached_type: None,
};
let snapshots = Client::rate_limit_snapshots_from_payload(payload);
assert_eq!(snapshots[0].rate_limit_reached_type, None);
}
}
+1
View File
@@ -2,6 +2,7 @@ pub use codex_backend_openapi_models::models::ConfigFileResponse;
pub use codex_backend_openapi_models::models::CreditStatusDetails;
pub use codex_backend_openapi_models::models::PaginatedListTaskListItem;
pub use codex_backend_openapi_models::models::PlanType;
pub use codex_backend_openapi_models::models::RateLimitReachedKind;
pub use codex_backend_openapi_models::models::RateLimitStatusDetails;
pub use codex_backend_openapi_models::models::RateLimitStatusPayload;
pub use codex_backend_openapi_models::models::RateLimitWindowSnapshot;
+1
View File
@@ -13,6 +13,7 @@ path = "src/lib.rs"
workspace = true
[dependencies]
async-channel = { workspace = true }
async-trait = { workspace = true }
codex-protocol = { workspace = true }
deno_core_icudata = { workspace = true }
+10 -13
View File
@@ -44,8 +44,8 @@ struct SessionHandle {
struct Inner {
stored_values: Mutex<HashMap<String, JsonValue>>,
sessions: Mutex<HashMap<String, SessionHandle>>,
turn_message_tx: mpsc::UnboundedSender<TurnMessage>,
turn_message_rx: Arc<Mutex<mpsc::UnboundedReceiver<TurnMessage>>>,
turn_message_tx: async_channel::Sender<TurnMessage>,
turn_message_rx: async_channel::Receiver<TurnMessage>,
next_cell_id: AtomicU64,
}
@@ -55,14 +55,14 @@ pub struct CodeModeService {
impl CodeModeService {
pub fn new() -> Self {
let (turn_message_tx, turn_message_rx) = mpsc::unbounded_channel();
let (turn_message_tx, turn_message_rx) = async_channel::unbounded();
Self {
inner: Arc::new(Inner {
stored_values: Mutex::new(HashMap::new()),
sessions: Mutex::new(HashMap::new()),
turn_message_tx,
turn_message_rx: Arc::new(Mutex::new(turn_message_rx)),
turn_message_rx,
next_cell_id: AtomicU64::new(1),
}),
}
@@ -146,16 +146,13 @@ impl CodeModeService {
pub fn start_turn_worker(&self, host: Arc<dyn CodeModeTurnHost>) -> CodeModeTurnWorker {
let (shutdown_tx, mut shutdown_rx) = oneshot::channel();
let inner = Arc::clone(&self.inner);
let turn_message_rx = Arc::clone(&self.inner.turn_message_rx);
let turn_message_rx = self.inner.turn_message_rx.clone();
tokio::spawn(async move {
loop {
let next_message = tokio::select! {
_ = &mut shutdown_rx => break,
message = async {
let mut turn_message_rx = turn_message_rx.lock().await;
turn_message_rx.recv().await
} => message,
message = turn_message_rx.recv() => message.ok(),
};
let Some(next_message) = next_message else {
break;
@@ -361,7 +358,7 @@ async fn run_session_control(
cell_id: cell_id.clone(),
call_id,
text,
});
}).await;
}
RuntimeEvent::ToolCall { id, name, input } => {
let _ = inner.turn_message_tx.send(TurnMessage::ToolCall {
@@ -369,7 +366,7 @@ async fn run_session_control(
id,
name,
input,
});
}).await;
}
RuntimeEvent::Result {
stored_values,
@@ -500,12 +497,12 @@ mod tests {
}
fn test_inner() -> Arc<Inner> {
let (turn_message_tx, turn_message_rx) = mpsc::unbounded_channel();
let (turn_message_tx, turn_message_rx) = async_channel::unbounded();
Arc::new(Inner {
stored_values: Mutex::new(HashMap::new()),
sessions: Mutex::new(HashMap::new()),
turn_message_tx,
turn_message_rx: Arc::new(Mutex::new(turn_message_rx)),
turn_message_rx,
next_cell_id: AtomicU64::new(1),
})
}
+1
View File
@@ -5,6 +5,7 @@ edition.workspace = true
license.workspace = true
[dependencies]
async-channel = { workspace = true }
async-trait = { workspace = true }
base64 = { workspace = true }
bytes = { workspace = true }
@@ -65,9 +65,9 @@ enum WsCommand {
impl WsStream {
fn new(
inner: WebSocketStream<MaybeTlsStream<TcpStream>>,
) -> (Self, mpsc::UnboundedReceiver<Result<Message, WsError>>) {
) -> (Self, async_channel::Receiver<Result<Message, WsError>>) {
let (tx_command, mut rx_command) = mpsc::channel::<WsCommand>(32);
let (tx_message, rx_message) = mpsc::unbounded_channel::<Result<Message, WsError>>();
let (tx_message, rx_message) = async_channel::unbounded::<Result<Message, WsError>>();
let pump_task = tokio::spawn(async move {
let mut inner = inner;
@@ -110,7 +110,7 @@ impl WsStream {
trace!(payload_len = payload.len(), "realtime websocket received ping");
if let Err(err) = inner.send(Message::Pong(payload)).await {
error!("realtime websocket failed to send pong: {err}");
let _ = tx_message.send(Err(err));
let _ = tx_message.send(Err(err)).await;
break;
}
}
@@ -138,7 +138,7 @@ impl WsStream {
}
Message::Ping(_) | Message::Pong(_) => {}
}
if tx_message.send(Ok(message)).is_err() {
if tx_message.send(Ok(message)).await.is_err() {
break;
}
if is_close {
@@ -147,7 +147,7 @@ impl WsStream {
}
Err(err) => {
error!("realtime websocket receive failed: {err}");
let _ = tx_message.send(Err(err));
let _ = tx_message.send(Err(err)).await;
break;
}
}
@@ -208,7 +208,7 @@ pub struct RealtimeWebsocketWriter {
#[derive(Clone)]
pub struct RealtimeWebsocketEvents {
rx_message: Arc<Mutex<mpsc::UnboundedReceiver<Result<Message, WsError>>>>,
rx_message: async_channel::Receiver<Result<Message, WsError>>,
active_transcript: Arc<Mutex<ActiveTranscriptState>>,
event_parser: RealtimeEventParser,
is_closed: Arc<AtomicBool>,
@@ -256,7 +256,7 @@ impl RealtimeWebsocketConnection {
fn new(
stream: WsStream,
rx_message: mpsc::UnboundedReceiver<Result<Message, WsError>>,
rx_message: async_channel::Receiver<Result<Message, WsError>>,
event_parser: RealtimeEventParser,
) -> Self {
let stream = Arc::new(stream);
@@ -268,7 +268,7 @@ impl RealtimeWebsocketConnection {
event_parser,
},
events: RealtimeWebsocketEvents {
rx_message: Arc::new(Mutex::new(rx_message)),
rx_message,
active_transcript: Arc::new(Mutex::new(ActiveTranscriptState::default())),
event_parser,
is_closed,
@@ -369,16 +369,16 @@ impl RealtimeWebsocketEvents {
}
loop {
let msg = match self.rx_message.lock().await.recv().await {
Some(Ok(msg)) => msg,
Some(Err(err)) => {
let msg = match self.rx_message.recv().await {
Ok(Ok(msg)) => msg,
Ok(Err(err)) => {
self.is_closed.store(true, Ordering::SeqCst);
error!("realtime websocket read failed: {err}");
return Err(ApiError::Stream(format!(
"failed to read websocket message: {err}"
)));
}
None => {
Err(_) => {
self.is_closed.store(true, Ordering::SeqCst);
info!("realtime websocket event stream ended");
return Ok(None);
+2
View File
@@ -93,6 +93,7 @@ pub fn parse_rate_limit_for_limit(
secondary,
credits,
plan_type: None,
rate_limit_reached_type: None,
})
}
@@ -156,6 +157,7 @@ pub fn parse_rate_limit_event(payload: &str) -> Option<RateLimitSnapshot> {
secondary,
credits,
plan_type: event.plan_type,
rate_limit_reached_type: None,
})
}
@@ -32,6 +32,8 @@ pub use self::additional_rate_limit_details::AdditionalRateLimitDetails;
pub(crate) mod rate_limit_status_payload;
pub use self::rate_limit_status_payload::PlanType;
pub use self::rate_limit_status_payload::RateLimitReachedKind;
pub use self::rate_limit_status_payload::RateLimitReachedType;
pub use self::rate_limit_status_payload::RateLimitStatusPayload;
pub(crate) mod rate_limit_status_details;
@@ -37,6 +37,13 @@ pub struct RateLimitStatusPayload {
skip_serializing_if = "Option::is_none"
)]
pub additional_rate_limits: Option<Option<Vec<models::AdditionalRateLimitDetails>>>,
#[serde(
rename = "rate_limit_reached_type",
default,
with = "::serde_with::rust::double_option",
skip_serializing_if = "Option::is_none"
)]
pub rate_limit_reached_type: Option<Option<RateLimitReachedType>>,
}
impl RateLimitStatusPayload {
@@ -46,10 +53,36 @@ impl RateLimitStatusPayload {
rate_limit: None,
credits: None,
additional_rate_limits: None,
rate_limit_reached_type: None,
}
}
}
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
pub struct RateLimitReachedType {
#[serde(rename = "type")]
pub kind: RateLimitReachedKind,
}
#[derive(
Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, Hash, Serialize, Deserialize, Default,
)]
pub enum RateLimitReachedKind {
#[serde(rename = "rate_limit_reached")]
RateLimitReached,
#[serde(rename = "workspace_owner_credits_depleted")]
WorkspaceOwnerCreditsDepleted,
#[serde(rename = "workspace_member_credits_depleted")]
WorkspaceMemberCreditsDepleted,
#[serde(rename = "workspace_owner_usage_limit_reached")]
WorkspaceOwnerUsageLimitReached,
#[serde(rename = "workspace_member_usage_limit_reached")]
WorkspaceMemberUsageLimitReached,
#[serde(rename = "unknown", other)]
#[default]
Unknown,
}
#[derive(
Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, Hash, Serialize, Deserialize, Default,
)]
@@ -50,8 +50,10 @@ use codex_protocol::protocol::McpStartupStatus;
use codex_protocol::protocol::McpStartupUpdateEvent;
use codex_protocol::protocol::SandboxPolicy;
use codex_rmcp_client::ElicitationResponse;
use codex_rmcp_client::LocalStdioServerLauncher;
use codex_rmcp_client::RmcpClient;
use codex_rmcp_client::SendElicitation;
use codex_rmcp_client::StdioServerLauncher;
use futures::future::BoxFuture;
use futures::future::FutureExt;
use futures::future::Shared;
@@ -1500,7 +1502,8 @@ async fn make_rmcp_client(
.map(|(key, value)| (key.into(), value.into()))
.collect::<HashMap<_, _>>()
});
RmcpClient::new_stdio_client(command_os, args_os, env_os, &env_vars, cwd)
let launcher = Arc::new(LocalStdioServerLauncher) as Arc<dyn StdioServerLauncher>;
RmcpClient::new_stdio_client(command_os, args_os, env_os, &env_vars, cwd, launcher)
.await
.map_err(|err| StartupOutcomeError::from(anyhow!(err)))
}
+51 -4
View File
@@ -158,11 +158,9 @@ where
let path = match next_token.as_deref() {
Some(token) => {
let encoded_token = urlencoding::encode(token);
format!(
"/connectors/directory/list?tier=categorized&token={encoded_token}&external_logos=true"
)
format!("/connectors/directory/list?token={encoded_token}&external_logos=true")
}
None => "/connectors/directory/list?tier=categorized&external_logos=true".to_string(),
None => "/connectors/directory/list?external_logos=true".to_string(),
};
let response = fetch_page(path).await?;
apps.extend(
@@ -416,6 +414,7 @@ mod tests {
use super::*;
use pretty_assertions::assert_eq;
use std::sync::Arc;
use std::sync::Mutex;
use std::sync::atomic::AtomicUsize;
use std::sync::atomic::Ordering;
@@ -558,4 +557,52 @@ mod tests {
assert_eq!(connectors[1].name, "Beta");
Ok(())
}
#[tokio::test]
async fn list_directory_connectors_omits_tier_for_all_pages() -> anyhow::Result<()> {
let requested_paths: Arc<Mutex<Vec<String>>> = Arc::new(Mutex::new(Vec::new()));
let paths = Arc::clone(&requested_paths);
let apps = list_directory_connectors(&mut move |path| {
let paths = Arc::clone(&paths);
async move {
paths
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.push(path.clone());
if path == "/connectors/directory/list?external_logos=true" {
Ok(DirectoryListResponse {
apps: vec![app("alpha", "Alpha")],
next_token: Some("page 2".to_string()),
})
} else {
assert_eq!(
path,
"/connectors/directory/list?token=page%202&external_logos=true"
);
Ok(DirectoryListResponse {
apps: vec![app("beta", "Beta")],
next_token: None,
})
}
}
})
.await?;
assert_eq!(
apps.iter().map(|app| app.id.as_str()).collect::<Vec<_>>(),
vec!["alpha", "beta"]
);
assert_eq!(
requested_paths
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.as_slice(),
&[
"/connectors/directory/list?external_logos=true".to_string(),
"/connectors/directory/list?token=page%202&external_logos=true".to_string(),
]
);
Ok(())
}
}
File diff suppressed because one or more lines are too long
+35 -28
View File
@@ -1385,37 +1385,44 @@ impl Session {
&self,
updates: SessionSettingsUpdate,
) -> ConstraintResult<()> {
let mut state = self.state.lock().await;
match state.session_configuration.apply(&updates) {
Ok(updated) => {
let previous_cwd = state.session_configuration.cwd.clone();
let sandbox_policy_changed =
state.session_configuration.sandbox_policy != updated.sandbox_policy;
let next_cwd = updated.cwd.clone();
let codex_home = updated.codex_home.clone();
let session_source = updated.session_source.clone();
state.session_configuration = updated;
drop(state);
self.maybe_refresh_shell_snapshot_for_cwd(
&previous_cwd,
&next_cwd,
&codex_home,
&session_source,
);
if sandbox_policy_changed {
self.refresh_managed_network_proxy_for_current_sandbox_policy()
.await;
let (previous_cwd, sandbox_policy_changed, next_cwd, codex_home, session_source) = {
let mut state = self.state.lock().await;
let updated = match state.session_configuration.apply(&updates) {
Ok(updated) => updated,
Err(err) => {
warn!("rejected session settings update: {err}");
return Err(err);
}
};
Ok(())
}
Err(err) => {
warn!("rejected session settings update: {err}");
Err(err)
}
let previous_cwd = state.session_configuration.cwd.clone();
let sandbox_policy_changed =
state.session_configuration.sandbox_policy != updated.sandbox_policy;
let next_cwd = updated.cwd.clone();
let codex_home = updated.codex_home.clone();
let session_source = updated.session_source.clone();
state.session_configuration = updated;
(
previous_cwd,
sandbox_policy_changed,
next_cwd,
codex_home,
session_source,
)
};
self.maybe_refresh_shell_snapshot_for_cwd(
&previous_cwd,
&next_cwd,
&codex_home,
&session_source,
);
if sandbox_policy_changed {
self.refresh_managed_network_proxy_for_current_sandbox_policy()
.await;
}
Ok(())
}
pub(crate) async fn set_session_startup_prewarm(
+6
View File
@@ -2070,6 +2070,7 @@ async fn set_rate_limits_retains_previous_credits() {
balance: Some("10.00".to_string()),
}),
plan_type: Some(codex_protocol::account::PlanType::Plus),
rate_limit_reached_type: None,
};
state.set_rate_limits(initial.clone());
@@ -2088,6 +2089,7 @@ async fn set_rate_limits_retains_previous_credits() {
}),
credits: None,
plan_type: None,
rate_limit_reached_type: None,
};
state.set_rate_limits(update.clone());
@@ -2100,6 +2102,7 @@ async fn set_rate_limits_retains_previous_credits() {
secondary: update.secondary,
credits: initial.credits,
plan_type: initial.plan_type,
rate_limit_reached_type: None,
})
);
}
@@ -2176,6 +2179,7 @@ async fn set_rate_limits_updates_plan_type_when_present() {
balance: Some("15.00".to_string()),
}),
plan_type: Some(codex_protocol::account::PlanType::Plus),
rate_limit_reached_type: None,
};
state.set_rate_limits(initial.clone());
@@ -2190,6 +2194,7 @@ async fn set_rate_limits_updates_plan_type_when_present() {
secondary: None,
credits: None,
plan_type: Some(codex_protocol::account::PlanType::Pro),
rate_limit_reached_type: None,
};
state.set_rate_limits(update.clone());
@@ -2202,6 +2207,7 @@ async fn set_rate_limits_updates_plan_type_when_present() {
secondary: update.secondary,
credits: initial.credits,
plan_type: update.plan_type,
rate_limit_reached_type: None,
})
);
}
+25 -21
View File
@@ -446,13 +446,7 @@ impl Session {
sub_id: String,
updates: SessionSettingsUpdate,
) -> ConstraintResult<Arc<TurnContext>> {
let (
session_configuration,
sandbox_policy_changed,
previous_cwd,
codex_home,
session_source,
) = {
let update_result = {
let mut state = self.state.lock().await;
match state.session_configuration.clone().apply(&updates) {
Ok(next) => {
@@ -462,26 +456,36 @@ impl Session {
let codex_home = next.codex_home.clone();
let session_source = next.session_source.clone();
state.session_configuration = next.clone();
(
Ok((
next,
sandbox_policy_changed,
previous_cwd,
codex_home,
session_source,
)
}
Err(err) => {
drop(state);
self.send_event_raw(Event {
id: sub_id.clone(),
msg: EventMsg::Error(ErrorEvent {
message: err.to_string(),
codex_error_info: Some(CodexErrorInfo::BadRequest),
}),
})
.await;
return Err(err);
))
}
Err(err) => Err(err),
}
};
let (
session_configuration,
sandbox_policy_changed,
previous_cwd,
codex_home,
session_source,
) = match update_result {
Ok(update) => update,
Err(err) => {
self.send_event_raw(Event {
id: sub_id.clone(),
msg: EventMsg::Error(ErrorEvent {
message: err.to_string(),
codex_error_info: Some(CodexErrorInfo::BadRequest),
}),
})
.await;
return Err(err);
}
};
+6
View File
@@ -87,6 +87,7 @@ async fn set_rate_limits_defaults_limit_id_to_codex_when_missing() {
secondary: None,
credits: None,
plan_type: None,
rate_limit_reached_type: None,
});
assert_eq!(
@@ -114,6 +115,7 @@ async fn set_rate_limits_defaults_to_codex_when_limit_id_missing_after_other_buc
secondary: None,
credits: None,
plan_type: None,
rate_limit_reached_type: None,
});
state.set_rate_limits(RateLimitSnapshot {
limit_id: None,
@@ -126,6 +128,7 @@ async fn set_rate_limits_defaults_to_codex_when_limit_id_missing_after_other_buc
secondary: None,
credits: None,
plan_type: None,
rate_limit_reached_type: None,
});
assert_eq!(
@@ -157,6 +160,7 @@ async fn set_rate_limits_carries_credits_and_plan_type_from_codex_to_codex_other
balance: Some("50".to_string()),
}),
plan_type: Some(codex_protocol::account::PlanType::Plus),
rate_limit_reached_type: None,
});
state.set_rate_limits(RateLimitSnapshot {
@@ -170,6 +174,7 @@ async fn set_rate_limits_carries_credits_and_plan_type_from_codex_to_codex_other
secondary: None,
credits: None,
plan_type: None,
rate_limit_reached_type: None,
});
assert_eq!(
@@ -189,6 +194,7 @@ async fn set_rate_limits_carries_credits_and_plan_type_from_codex_to_codex_other
balance: Some("50".to_string()),
}),
plan_type: Some(codex_protocol::account::PlanType::Plus),
rate_limit_reached_type: None,
})
);
}
+40 -40
View File
@@ -884,9 +884,8 @@ impl JsReplManager {
let (req_id, rx) = {
let req_id = Uuid::new_v4().to_string();
let mut pending = pending_execs.lock().await;
let (tx, rx) = tokio::sync::oneshot::channel();
pending.insert(req_id.clone(), tx);
pending_execs.lock().await.insert(req_id.clone(), tx);
exec_contexts.lock().await.insert(
req_id.clone(),
ExecContext {
@@ -956,9 +955,7 @@ impl JsReplManager {
let response = match tokio::time::timeout(Duration::from_millis(timeout_ms), rx).await {
Ok(Ok(msg)) => msg,
Ok(Err(_)) => {
let mut pending = pending_execs.lock().await;
let removed = pending.remove(&req_id).is_some();
drop(pending);
let removed = pending_execs.lock().await.remove(&req_id).is_some();
if removed {
self.clear_top_level_exec_if_matches(&req_id).await;
}
@@ -1340,40 +1337,40 @@ impl JsReplManager {
KernelToHost::EmitImage(req) => {
let exec_id = req.exec_id.clone();
let emit_id = req.id.clone();
let response =
if let Some(ctx) = exec_contexts.lock().await.get(&exec_id).cloned() {
match validate_emitted_image_url(&req.image_url) {
Ok(()) => {
let content_item = emitted_image_content_item(
ctx.turn.as_ref(),
req.image_url,
req.detail,
);
JsReplManager::record_exec_content_item(
&exec_tool_calls,
&exec_id,
content_item,
)
.await;
HostToKernel::EmitImageResult(EmitImageResult {
id: emit_id,
ok: true,
error: None,
})
}
Err(error) => HostToKernel::EmitImageResult(EmitImageResult {
let context = exec_contexts.lock().await.get(&exec_id).cloned();
let response = if let Some(ctx) = context {
match validate_emitted_image_url(&req.image_url) {
Ok(()) => {
let content_item = emitted_image_content_item(
ctx.turn.as_ref(),
req.image_url,
req.detail,
);
JsReplManager::record_exec_content_item(
&exec_tool_calls,
&exec_id,
content_item,
)
.await;
HostToKernel::EmitImageResult(EmitImageResult {
id: emit_id,
ok: false,
error: Some(error),
}),
ok: true,
error: None,
})
}
} else {
HostToKernel::EmitImageResult(EmitImageResult {
Err(error) => HostToKernel::EmitImageResult(EmitImageResult {
id: emit_id,
ok: false,
error: Some("js_repl exec context not found".to_string()),
})
};
error: Some(error),
}),
}
} else {
HostToKernel::EmitImageResult(EmitImageResult {
id: emit_id,
ok: false,
error: Some("js_repl exec context not found".to_string()),
})
};
if let Err(err) = JsReplManager::write_message(&stdin, &response).await {
let snapshot =
@@ -1424,7 +1421,7 @@ impl JsReplManager {
let exec_id = req.exec_id.clone();
let tool_call_id = req.id.clone();
let tool_name = req.tool_name.clone();
let context = { exec_contexts.lock().await.get(&exec_id).cloned() };
let context = exec_contexts.lock().await.get(&exec_id).cloned();
let result = match context {
Some(ctx) => {
tokio::select! {
@@ -1502,14 +1499,17 @@ impl JsReplManager {
}
}
let mut pending = pending_execs.lock().await;
let pending_exec_ids = pending.keys().cloned().collect::<Vec<_>>();
for (_id, tx) in pending.drain() {
let pending_execs_to_notify = {
let mut pending = pending_execs.lock().await;
pending.drain().collect::<Vec<_>>()
};
let mut pending_exec_ids = Vec::with_capacity(pending_execs_to_notify.len());
for (id, tx) in pending_execs_to_notify {
pending_exec_ids.push(id);
let _ = tx.send(ExecResultMessage::Err {
message: kernel_exit_message.clone(),
});
}
drop(pending);
if !pending_exec_ids.is_empty() {
Self::clear_top_level_exec_if_matches_any_map(&manager_kernel, &pending_exec_ids).await;
}
+5 -10
View File
@@ -223,10 +223,8 @@ impl NetworkApprovalService {
}
pub(crate) async fn unregister_call(&self, registration_id: &str) {
let mut active_calls = self.active_calls.lock().await;
active_calls.shift_remove(registration_id);
let mut call_outcomes = self.call_outcomes.lock().await;
call_outcomes.remove(registration_id);
self.active_calls.lock().await.shift_remove(registration_id);
self.call_outcomes.lock().await.remove(registration_id);
}
async fn resolve_single_active_call(&self) -> Option<Arc<ActiveNetworkApprovalCall>> {
@@ -344,8 +342,7 @@ impl NetworkApprovalService {
let Some(turn_context) = Self::active_turn_context(session.as_ref()).await else {
pending.set_decision(PendingApprovalDecision::Deny).await;
let mut pending_approvals = self.pending_host_approvals.lock().await;
pending_approvals.remove(&key);
self.pending_host_approvals.lock().await.remove(&key);
self.record_outcome_for_single_active_call(NetworkApprovalOutcome::DeniedByPolicy(
policy_denial_message,
))
@@ -354,8 +351,7 @@ impl NetworkApprovalService {
};
if !sandbox_policy_allows_network_approval_flow(turn_context.sandbox_policy.get()) {
pending.set_decision(PendingApprovalDecision::Deny).await;
let mut pending_approvals = self.pending_host_approvals.lock().await;
pending_approvals.remove(&key);
self.pending_host_approvals.lock().await.remove(&key);
self.record_outcome_for_single_active_call(NetworkApprovalOutcome::DeniedByPolicy(
policy_denial_message,
))
@@ -364,8 +360,7 @@ impl NetworkApprovalService {
}
if !allows_network_approval_flow(turn_context.approval_policy.value()) {
pending.set_decision(PendingApprovalDecision::Deny).await;
let mut pending_approvals = self.pending_host_approvals.lock().await;
pending_approvals.remove(&key);
self.pending_host_approvals.lock().await.remove(&key);
self.record_outcome_for_single_active_call(NetworkApprovalOutcome::DeniedByPolicy(
policy_denial_message,
))
@@ -2,6 +2,7 @@ use super::process::UnifiedExecProcess;
use crate::unified_exec::UnifiedExecError;
use async_trait::async_trait;
use codex_exec_server::ExecProcess;
use codex_exec_server::ExecProcessEventReceiver;
use codex_exec_server::ExecServerError;
use codex_exec_server::ProcessId;
use codex_exec_server::ReadResponse;
@@ -33,6 +34,10 @@ impl ExecProcess for MockExecProcess {
self.wake_tx.subscribe()
}
fn subscribe_events(&self) -> ExecProcessEventReceiver {
ExecProcessEventReceiver::empty()
}
async fn read(
&self,
_after_seq: Option<u64>,
+6 -3
View File
@@ -2375,7 +2375,8 @@ async fn token_count_includes_rate_limits_snapshot() {
"resets_at": 1704074400
},
"credits": null,
"plan_type": null
"plan_type": null,
"rate_limit_reached_type": null
}
})
);
@@ -2426,7 +2427,8 @@ async fn token_count_includes_rate_limits_snapshot() {
"resets_at": 1704074400
},
"credits": null,
"plan_type": null
"plan_type": null,
"rate_limit_reached_type": null
}
})
);
@@ -2500,7 +2502,8 @@ async fn usage_limit_error_emits_rate_limit_event() -> anyhow::Result<()> {
"resets_at": null
},
"credits": null,
"plan_type": null
"plan_type": null,
"rate_limit_reached_type": null
});
let submission_id = codex
@@ -1033,7 +1033,8 @@ async fn responses_websocket_usage_limit_error_emits_rate_limit_event() {
"resets_at": null
},
"credits": null,
"plan_type": null
"plan_type": null,
"rate_limit_reached_type": null
}
})
);
+254 -6
View File
@@ -1,5 +1,7 @@
use std::collections::BTreeMap;
use std::collections::HashMap;
use std::sync::Arc;
use std::sync::Mutex as StdMutex;
use std::time::Duration;
use arc_swap::ArcSwap;
@@ -16,6 +18,9 @@ use crate::ProcessId;
use crate::client_api::ExecServerClientConnectOptions;
use crate::client_api::RemoteExecServerConnectArgs;
use crate::connection::JsonRpcConnection;
use crate::process::ExecProcessEvent;
use crate::process::ExecProcessEventLog;
use crate::process::ExecProcessEventReceiver;
use crate::protocol::EXEC_CLOSED_METHOD;
use crate::protocol::EXEC_EXITED_METHOD;
use crate::protocol::EXEC_METHOD;
@@ -53,6 +58,7 @@ use crate::protocol::INITIALIZE_METHOD;
use crate::protocol::INITIALIZED_METHOD;
use crate::protocol::InitializeParams;
use crate::protocol::InitializeResponse;
use crate::protocol::ProcessOutputChunk;
use crate::protocol::ReadParams;
use crate::protocol::ReadResponse;
use crate::protocol::TerminateParams;
@@ -65,6 +71,8 @@ use crate::rpc::RpcClientEvent;
const CONNECT_TIMEOUT: Duration = Duration::from_secs(10);
const INITIALIZE_TIMEOUT: Duration = Duration::from_secs(10);
const PROCESS_EVENT_CHANNEL_CAPACITY: usize = 256;
const PROCESS_EVENT_RETAINED_BYTES: usize = 1024 * 1024;
impl Default for ExecServerClientConnectOptions {
fn default() -> Self {
@@ -100,9 +108,20 @@ impl RemoteExecServerConnectArgs {
pub(crate) struct SessionState {
wake_tx: watch::Sender<u64>,
events: ExecProcessEventLog,
ordered_events: StdMutex<OrderedSessionEvents>,
failure: Mutex<Option<String>>,
}
#[derive(Default)]
struct OrderedSessionEvents {
last_published_seq: u64,
// Server-side output, exit, and closed notifications are emitted by
// different tasks and can reach the client out of order. Keep future events
// here until all lower sequence numbers have been published.
pending: BTreeMap<u64, ExecProcessEvent>,
}
#[derive(Clone)]
pub(crate) struct Session {
client: ExecServerClient,
@@ -452,6 +471,11 @@ impl SessionState {
let (wake_tx, _wake_rx) = watch::channel(0);
Self {
wake_tx,
events: ExecProcessEventLog::new(
PROCESS_EVENT_CHANNEL_CAPACITY,
PROCESS_EVENT_RETAINED_BYTES,
),
ordered_events: StdMutex::new(OrderedSessionEvents::default()),
failure: Mutex::new(None),
}
}
@@ -460,19 +484,71 @@ impl SessionState {
self.wake_tx.subscribe()
}
pub(crate) fn subscribe_events(&self) -> ExecProcessEventReceiver {
self.events.subscribe()
}
fn note_change(&self, seq: u64) {
let next = (*self.wake_tx.borrow()).max(seq);
let _ = self.wake_tx.send(next);
}
/// Publishes a process event only when all earlier sequenced events have
/// already been published.
///
/// Returns `true` only when this call actually publishes the ordered
/// `Closed` event. The caller uses that signal to remove the session route
/// after the terminal event is visible to subscribers, rather than when a
/// possibly-early closed notification first arrives.
fn publish_ordered_event(&self, event: ExecProcessEvent) -> bool {
let Some(seq) = event.seq() else {
self.events.publish(event);
return false;
};
let mut ready = Vec::new();
{
let mut ordered_events = self
.ordered_events
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
// We have already delivered this sequence number or moved past it,
// so accepting it again would duplicate output or lifecycle events.
if seq <= ordered_events.last_published_seq {
return false;
}
ordered_events.pending.entry(seq).or_insert(event);
loop {
let next_seq = ordered_events.last_published_seq + 1;
let Some(event) = ordered_events.pending.remove(&next_seq) else {
break;
};
ordered_events.last_published_seq += 1;
ready.push(event);
}
}
let mut published_closed = false;
for event in ready {
published_closed |= matches!(&event, ExecProcessEvent::Closed { .. });
self.events.publish(event);
}
published_closed
}
async fn set_failure(&self, message: String) {
let mut failure = self.failure.lock().await;
if failure.is_none() {
*failure = Some(message);
let should_publish = failure.is_none();
if should_publish {
*failure = Some(message.clone());
}
drop(failure);
let next = (*self.wake_tx.borrow()).saturating_add(1);
let _ = self.wake_tx.send(next);
if should_publish {
let _ = self.publish_ordered_event(ExecProcessEvent::Failed(message));
}
}
async fn failed_response(&self) -> Option<ReadResponse> {
@@ -505,6 +581,10 @@ impl Session {
self.state.subscribe()
}
pub(crate) fn subscribe_events(&self) -> ExecProcessEventReceiver {
self.state.subscribe_events()
}
pub(crate) async fn read(
&self,
after_seq: Option<u64>,
@@ -628,6 +708,15 @@ async fn handle_server_notification(
serde_json::from_value(notification.params.unwrap_or(Value::Null))?;
if let Some(session) = inner.get_session(&params.process_id) {
session.note_change(params.seq);
let published_closed =
session.publish_ordered_event(ExecProcessEvent::Output(ProcessOutputChunk {
seq: params.seq,
stream: params.stream,
chunk: params.chunk,
}));
if published_closed {
inner.remove_session(&params.process_id).await;
}
}
}
EXEC_EXITED_METHOD => {
@@ -635,16 +724,28 @@ async fn handle_server_notification(
serde_json::from_value(notification.params.unwrap_or(Value::Null))?;
if let Some(session) = inner.get_session(&params.process_id) {
session.note_change(params.seq);
let published_closed = session.publish_ordered_event(ExecProcessEvent::Exited {
seq: params.seq,
exit_code: params.exit_code,
});
if published_closed {
inner.remove_session(&params.process_id).await;
}
}
}
EXEC_CLOSED_METHOD => {
let params: ExecClosedNotification =
serde_json::from_value(notification.params.unwrap_or(Value::Null))?;
// Closed is the terminal lifecycle event for this process, so drop
// the routing entry before forwarding it.
let session = inner.remove_session(&params.process_id).await;
if let Some(session) = session {
if let Some(session) = inner.get_session(&params.process_id) {
session.note_change(params.seq);
// Closed is terminal, but it can arrive before tail output or
// exited. Keep routing this process until the ordered publisher
// says Closed has actually been delivered.
let published_closed =
session.publish_ordered_event(ExecProcessEvent::Closed { seq: params.seq });
if published_closed {
inner.remove_session(&params.process_id).await;
}
}
}
other => {
@@ -673,14 +774,18 @@ mod tests {
use super::ExecServerClientConnectOptions;
use crate::ProcessId;
use crate::connection::JsonRpcConnection;
use crate::process::ExecProcessEvent;
use crate::protocol::EXEC_CLOSED_METHOD;
use crate::protocol::EXEC_EXITED_METHOD;
use crate::protocol::EXEC_OUTPUT_DELTA_METHOD;
use crate::protocol::ExecClosedNotification;
use crate::protocol::ExecExitedNotification;
use crate::protocol::ExecOutputDeltaNotification;
use crate::protocol::ExecOutputStream;
use crate::protocol::INITIALIZE_METHOD;
use crate::protocol::INITIALIZED_METHOD;
use crate::protocol::InitializeResponse;
use crate::protocol::ProcessOutputChunk;
async fn read_jsonrpc_line<R>(lines: &mut tokio::io::Lines<BufReader<R>>) -> JSONRPCMessage
where
@@ -705,6 +810,149 @@ mod tests {
.expect("json-rpc line should write");
}
#[tokio::test]
async fn process_events_are_delivered_in_seq_order_when_notifications_are_reordered() {
let (client_stdin, server_reader) = duplex(1 << 20);
let (mut server_writer, client_stdout) = duplex(1 << 20);
let (notifications_tx, mut notifications_rx) = mpsc::channel(16);
let server = tokio::spawn(async move {
let mut lines = BufReader::new(server_reader).lines();
let initialize = read_jsonrpc_line(&mut lines).await;
let request = match initialize {
JSONRPCMessage::Request(request) if request.method == INITIALIZE_METHOD => request,
other => panic!("expected initialize request, got {other:?}"),
};
write_jsonrpc_line(
&mut server_writer,
JSONRPCMessage::Response(JSONRPCResponse {
id: request.id,
result: serde_json::to_value(InitializeResponse {
session_id: "session-1".to_string(),
})
.expect("initialize response should serialize"),
}),
)
.await;
let initialized = read_jsonrpc_line(&mut lines).await;
match initialized {
JSONRPCMessage::Notification(notification)
if notification.method == INITIALIZED_METHOD => {}
other => panic!("expected initialized notification, got {other:?}"),
}
while let Some(message) = notifications_rx.recv().await {
write_jsonrpc_line(&mut server_writer, message).await;
}
});
let client = ExecServerClient::connect(
JsonRpcConnection::from_stdio(
client_stdout,
client_stdin,
"test-exec-server-client".to_string(),
),
ExecServerClientConnectOptions::default(),
)
.await
.expect("client should connect");
let process_id = ProcessId::from("reordered");
let session = client
.register_session(&process_id)
.await
.expect("session should register");
let mut events = session.subscribe_events();
for message in [
JSONRPCMessage::Notification(JSONRPCNotification {
method: EXEC_CLOSED_METHOD.to_string(),
params: Some(
serde_json::to_value(ExecClosedNotification {
process_id: process_id.clone(),
seq: 4,
})
.expect("closed notification should serialize"),
),
}),
JSONRPCMessage::Notification(JSONRPCNotification {
method: EXEC_OUTPUT_DELTA_METHOD.to_string(),
params: Some(
serde_json::to_value(ExecOutputDeltaNotification {
process_id: process_id.clone(),
seq: 1,
stream: ExecOutputStream::Stdout,
chunk: b"one".to_vec().into(),
})
.expect("output notification should serialize"),
),
}),
JSONRPCMessage::Notification(JSONRPCNotification {
method: EXEC_EXITED_METHOD.to_string(),
params: Some(
serde_json::to_value(ExecExitedNotification {
process_id: process_id.clone(),
seq: 3,
exit_code: 0,
})
.expect("exit notification should serialize"),
),
}),
JSONRPCMessage::Notification(JSONRPCNotification {
method: EXEC_OUTPUT_DELTA_METHOD.to_string(),
params: Some(
serde_json::to_value(ExecOutputDeltaNotification {
process_id: process_id.clone(),
seq: 2,
stream: ExecOutputStream::Stderr,
chunk: b"two".to_vec().into(),
})
.expect("output notification should serialize"),
),
}),
] {
notifications_tx
.send(message)
.await
.expect("notification should queue");
}
let mut delivered = Vec::new();
for _ in 0..4 {
delivered.push(
timeout(Duration::from_secs(1), events.recv())
.await
.expect("process event should not time out")
.expect("process event stream should stay open"),
);
}
assert_eq!(
delivered,
vec![
ExecProcessEvent::Output(ProcessOutputChunk {
seq: 1,
stream: ExecOutputStream::Stdout,
chunk: b"one".to_vec().into(),
}),
ExecProcessEvent::Output(ProcessOutputChunk {
seq: 2,
stream: ExecOutputStream::Stderr,
chunk: b"two".to_vec().into(),
}),
ExecProcessEvent::Exited {
seq: 3,
exit_code: 0,
},
ExecProcessEvent::Closed { seq: 4 },
]
);
drop(notifications_tx);
drop(client);
server.await.expect("server task should finish");
}
#[tokio::test]
async fn wake_notifications_do_not_block_other_sessions() {
let (client_stdin, server_reader) = duplex(1 << 20);
+129 -2
View File
@@ -30,14 +30,20 @@ use crate::local_file_system::current_sandbox_cwd;
use crate::rpc::internal_error;
use crate::rpc::invalid_request;
const FS_HELPER_ENV_ALLOWLIST: &[&str] = &["PATH", "TMPDIR", "TMP", "TEMP"];
#[derive(Clone, Debug)]
pub(crate) struct FileSystemSandboxRunner {
runtime_paths: ExecServerRuntimePaths,
helper_env: HashMap<String, String>,
}
impl FileSystemSandboxRunner {
pub(crate) fn new(runtime_paths: ExecServerRuntimePaths) -> Self {
Self { runtime_paths }
Self {
runtime_paths,
helper_env: helper_env(),
}
}
pub(crate) async fn run(
@@ -88,7 +94,7 @@ impl FileSystemSandboxRunner {
program: helper.as_path().as_os_str().to_owned(),
args: vec![CODEX_FS_HELPER_ARG1.to_string()],
cwd: cwd.clone(),
env: HashMap::new(),
env: self.helper_env.clone(),
additional_permissions: Some(
self.helper_permissions(sandbox_context.additional_permissions.as_ref()),
),
@@ -205,6 +211,26 @@ fn normalize_top_level_alias(path: AbsolutePathBuf) -> AbsolutePathBuf {
path
}
fn helper_env() -> HashMap<String, String> {
helper_env_from_vars(std::env::vars_os())
}
fn helper_env_from_vars(
vars: impl IntoIterator<Item = (std::ffi::OsString, std::ffi::OsString)>,
) -> HashMap<String, String> {
vars.into_iter()
.filter_map(|(key, value)| {
let key = key.to_string_lossy();
helper_env_key_is_allowed(&key)
.then(|| (key.into_owned(), value.to_string_lossy().into_owned()))
})
.collect()
}
fn helper_env_key_is_allowed(key: &str) -> bool {
FS_HELPER_ENV_ALLOWLIST.contains(&key) || (cfg!(windows) && key.eq_ignore_ascii_case("PATH"))
}
async fn run_command(
command: SandboxExecRequest,
request_json: Vec<u8>,
@@ -296,9 +322,14 @@ fn json_error(err: serde_json::Error) -> JSONRPCErrorError {
#[cfg(test)]
mod tests {
use std::collections::HashMap;
use std::ffi::OsString;
use codex_protocol::models::FileSystemPermissions;
use codex_protocol::models::NetworkPermissions;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::ReadOnlyAccess;
use codex_protocol::protocol::SandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
@@ -307,6 +338,9 @@ mod tests {
use crate::ExecServerRuntimePaths;
use super::FileSystemSandboxRunner;
use super::helper_env;
use super::helper_env_from_vars;
use super::helper_env_key_is_allowed;
use super::sandbox_policy_with_helper_runtime_defaults;
#[test]
@@ -399,6 +433,99 @@ mod tests {
assert_eq!(read, Some(vec![readable]));
}
#[test]
fn helper_env_carries_only_allowlisted_runtime_vars() {
let env = helper_env();
let expected = std::env::vars_os()
.filter_map(|(key, value)| {
let key = key.to_string_lossy();
helper_env_key_is_allowed(&key)
.then(|| (key.into_owned(), value.to_string_lossy().into_owned()))
})
.collect::<HashMap<_, _>>();
assert_eq!(env, expected);
}
#[test]
fn helper_env_preserves_path_for_system_bwrap_discovery_without_leaking_secrets() {
let env = helper_env_from_vars(
[
("PATH", "/usr/bin:/bin"),
("TMPDIR", "/tmp/codex"),
("TMP", "/tmp"),
("TEMP", "/tmp"),
("HOME", "/home/user"),
("OPENAI_API_KEY", "secret"),
("HTTPS_PROXY", "http://proxy.example"),
]
.map(|(key, value)| (OsString::from(key), OsString::from(value))),
);
assert_eq!(
env,
HashMap::from([
("PATH".to_string(), "/usr/bin:/bin".to_string()),
("TMPDIR".to_string(), "/tmp/codex".to_string()),
("TMP".to_string(), "/tmp".to_string()),
("TEMP".to_string(), "/tmp".to_string()),
])
);
}
#[cfg(windows)]
#[test]
fn helper_env_preserves_windows_path_key_for_system_bwrap_discovery() {
let env = helper_env_from_vars(
[
("Path", r"C:\Windows\System32"),
("PATH_INJECTION", "bad"),
("OPENAI_API_KEY", "secret"),
]
.map(|(key, value)| (OsString::from(key), OsString::from(value))),
);
assert_eq!(
env,
HashMap::from([("Path".to_string(), r"C:\Windows\System32".to_string())])
);
}
#[test]
fn sandbox_exec_request_carries_helper_env() {
let Some((path_key, path)) = std::env::vars_os().find(|(key, _)| {
let key = key.to_string_lossy();
key == "PATH" || (cfg!(windows) && key.eq_ignore_ascii_case("PATH"))
}) else {
return;
};
let path_key = path_key.to_string_lossy().into_owned();
let path = path.to_string_lossy().into_owned();
let codex_self_exe = std::env::current_exe().expect("current exe");
let runtime_paths =
ExecServerRuntimePaths::new(codex_self_exe.clone(), Some(codex_self_exe))
.expect("runtime paths");
let runner = FileSystemSandboxRunner::new(runtime_paths);
let cwd = AbsolutePathBuf::current_dir().expect("cwd");
let sandbox_policy = SandboxPolicy::new_workspace_write_policy();
let file_system_policy =
FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, cwd.as_path());
let sandbox_context = crate::FileSystemSandboxContext::new(sandbox_policy.clone());
let request = runner
.sandbox_exec_request(
&sandbox_policy,
&file_system_policy,
NetworkSandboxPolicy::Restricted,
&cwd,
&sandbox_context,
)
.expect("sandbox exec request");
assert_eq!(request.env.get(&path_key), Some(&path));
}
#[test]
fn helper_permissions_include_helper_read_root_without_additional_permissions() {
let codex_self_exe = std::env::current_exe().expect("current exe");
+3
View File
@@ -39,6 +39,8 @@ pub use local_file_system::LOCAL_FS;
pub use local_file_system::LocalFileSystem;
pub use process::ExecBackend;
pub use process::ExecProcess;
pub use process::ExecProcessEvent;
pub use process::ExecProcessEventReceiver;
pub use process::StartedExecProcess;
pub use process_id::ProcessId;
pub use protocol::ExecClosedNotification;
@@ -65,6 +67,7 @@ pub use protocol::FsWriteFileParams;
pub use protocol::FsWriteFileResponse;
pub use protocol::InitializeParams;
pub use protocol::InitializeResponse;
pub use protocol::ProcessOutputChunk;
pub use protocol::ReadParams;
pub use protocol::ReadResponse;
pub use protocol::TerminateParams;
+33 -5
View File
@@ -17,9 +17,12 @@ use tokio::sync::watch;
use crate::ExecBackend;
use crate::ExecProcess;
use crate::ExecProcessEvent;
use crate::ExecProcessEventReceiver;
use crate::ExecServerError;
use crate::ProcessId;
use crate::StartedExecProcess;
use crate::process::ExecProcessEventLog;
use crate::protocol::EXEC_CLOSED_METHOD;
use crate::protocol::ExecClosedNotification;
use crate::protocol::ExecEnvPolicy;
@@ -44,6 +47,7 @@ use crate::rpc::invalid_request;
const RETAINED_OUTPUT_BYTES_PER_PROCESS: usize = 1024 * 1024;
const NOTIFICATION_CHANNEL_CAPACITY: usize = 256;
const PROCESS_EVENT_CHANNEL_CAPACITY: usize = 256;
#[cfg(test)]
const EXITED_PROCESS_RETENTION: Duration = Duration::from_millis(25);
#[cfg(not(test))]
@@ -65,6 +69,7 @@ struct RunningProcess {
next_seq: u64,
exit_code: Option<i32>,
wake_tx: watch::Sender<u64>,
events: ExecProcessEventLog,
output_notify: Arc<Notify>,
open_streams: usize,
closed: bool,
@@ -89,6 +94,7 @@ struct LocalExecProcess {
process_id: ProcessId,
backend: LocalProcess,
wake_tx: watch::Sender<u64>,
events: ExecProcessEventLog,
}
impl Default for LocalProcess {
@@ -138,7 +144,7 @@ impl LocalProcess {
async fn start_process(
&self,
params: ExecParams,
) -> Result<(ExecResponse, watch::Sender<u64>), JSONRPCErrorError> {
) -> Result<(ExecResponse, watch::Sender<u64>, ExecProcessEventLog), JSONRPCErrorError> {
let process_id = params.process_id.clone();
let (program, args) = params
.argv
@@ -198,6 +204,10 @@ impl LocalProcess {
let output_notify = Arc::new(Notify::new());
let (wake_tx, _wake_rx) = watch::channel(0);
let events = ExecProcessEventLog::new(
PROCESS_EVENT_CHANNEL_CAPACITY,
RETAINED_OUTPUT_BYTES_PER_PROCESS,
);
{
let mut process_map = self.inner.processes.lock().await;
process_map.insert(
@@ -211,6 +221,7 @@ impl LocalProcess {
next_seq: 1,
exit_code: None,
wake_tx: wake_tx.clone(),
events: events.clone(),
output_notify: Arc::clone(&output_notify),
open_streams: 2,
closed: false,
@@ -247,13 +258,13 @@ impl LocalProcess {
output_notify,
));
Ok((ExecResponse { process_id }, wake_tx))
Ok((ExecResponse { process_id }, wake_tx, events))
}
pub(crate) async fn exec(&self, params: ExecParams) -> Result<ExecResponse, JSONRPCErrorError> {
self.start_process(params)
.await
.map(|(response, _)| response)
.map(|(response, _, _)| response)
}
pub(crate) async fn exec_read(
@@ -424,7 +435,7 @@ fn shell_environment_policy(env_policy: &ExecEnvPolicy) -> ShellEnvironmentPolic
#[async_trait]
impl ExecBackend for LocalProcess {
async fn start(&self, params: ExecParams) -> Result<StartedExecProcess, ExecServerError> {
let (response, wake_tx) = self
let (response, wake_tx, events) = self
.start_process(params)
.await
.map_err(map_handler_error)?;
@@ -433,6 +444,7 @@ impl ExecBackend for LocalProcess {
process_id: response.process_id,
backend: self.clone(),
wake_tx,
events,
}),
})
}
@@ -448,6 +460,10 @@ impl ExecProcess for LocalExecProcess {
self.wake_tx.subscribe()
}
fn subscribe_events(&self) -> ExecProcessEventReceiver {
self.events.subscribe()
}
async fn read(
&self,
after_seq: Option<u64>,
@@ -548,11 +564,19 @@ async fn stream_output(
process.retained_bytes = process.retained_bytes.saturating_sub(evicted.chunk.len());
}
let _ = process.wake_tx.send(seq);
let output = ProcessOutputChunk {
seq,
stream,
chunk: chunk.into(),
};
process
.events
.publish(ExecProcessEvent::Output(output.clone()));
ExecOutputDeltaNotification {
process_id: process_id.clone(),
seq,
stream,
chunk: chunk.into(),
chunk: output.chunk,
}
};
output_notify.notify_waiters();
@@ -580,6 +604,9 @@ async fn watch_exit(
process.next_seq += 1;
process.exit_code = Some(exit_code);
let _ = process.wake_tx.send(seq);
process
.events
.publish(ExecProcessEvent::Exited { seq, exit_code });
Some(ExecExitedNotification {
process_id: process_id.clone(),
seq,
@@ -640,6 +667,7 @@ async fn maybe_emit_closed(process_id: ProcessId, inner: Arc<Inner>) {
let seq = process.next_seq;
process.next_seq += 1;
let _ = process.wake_tx.send(seq);
process.events.publish(ExecProcessEvent::Closed { seq });
Some(ExecClosedNotification {
process_id: process_id.clone(),
seq,
+201
View File
@@ -1,11 +1,15 @@
use std::collections::VecDeque;
use std::sync::Arc;
use std::sync::Mutex as StdMutex;
use async_trait::async_trait;
use tokio::sync::broadcast;
use tokio::sync::watch;
use crate::ExecServerError;
use crate::ProcessId;
use crate::protocol::ExecParams;
use crate::protocol::ProcessOutputChunk;
use crate::protocol::ReadResponse;
use crate::protocol::WriteResponse;
@@ -13,12 +17,158 @@ pub struct StartedExecProcess {
pub process: Arc<dyn ExecProcess>,
}
/// Pushed process events for consumers that want to follow process output as it
/// arrives instead of polling retained output with [`ExecProcess::read`].
///
/// The stream is scoped to one [`ExecProcess`] handle. `Output` events carry
/// stdout, stderr, or pty bytes. `Exited` reports the process exit status, while
/// `Closed` means all output streams have ended and no more output events will
/// arrive. `Failed` is used when the process session cannot continue, for
/// example because the remote executor connection disconnected.
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum ExecProcessEvent {
Output(ProcessOutputChunk),
Exited { seq: u64, exit_code: i32 },
Closed { seq: u64 },
Failed(String),
}
/// Replay buffer plus live fan-out for pushed process events.
///
/// New subscribers first drain a bounded replay history, then continue on the
/// live broadcast channel. The history is bounded by event count and retained
/// output bytes: count protects against many tiny events, while bytes protects
/// against a few very large output chunks.
#[derive(Clone)]
pub(crate) struct ExecProcessEventLog {
inner: Arc<ExecProcessEventLogInner>,
}
struct ExecProcessEventLogInner {
history: StdMutex<ExecProcessEventHistory>,
live_tx: broadcast::Sender<ExecProcessEvent>,
event_capacity: usize,
byte_capacity: usize,
}
#[derive(Default)]
struct ExecProcessEventHistory {
events: VecDeque<ExecProcessEvent>,
retained_bytes: usize,
}
impl ExecProcessEvent {
/// Sequence number used to order process-owned events.
///
/// `Failed` is intentionally unsequenced because it is synthesized by the
/// client when the session or transport fails, not emitted by the process.
pub(crate) fn seq(&self) -> Option<u64> {
match self {
ExecProcessEvent::Output(chunk) => Some(chunk.seq),
ExecProcessEvent::Exited { seq, .. } | ExecProcessEvent::Closed { seq } => Some(*seq),
ExecProcessEvent::Failed(_) => None,
}
}
fn retained_len(&self) -> usize {
match self {
ExecProcessEvent::Output(chunk) => chunk.chunk.0.len(),
ExecProcessEvent::Failed(message) => message.len(),
ExecProcessEvent::Exited { .. } | ExecProcessEvent::Closed { .. } => 0,
}
}
}
impl ExecProcessEventLog {
pub(crate) fn new(event_capacity: usize, byte_capacity: usize) -> Self {
let (live_tx, _live_rx) = broadcast::channel(event_capacity);
Self {
inner: Arc::new(ExecProcessEventLogInner {
history: StdMutex::new(ExecProcessEventHistory::default()),
live_tx,
event_capacity,
byte_capacity,
}),
}
}
pub(crate) fn publish(&self, event: ExecProcessEvent) {
let mut history = self
.inner
.history
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
history.retained_bytes += event.retained_len();
history.events.push_back(event.clone());
while history.events.len() > self.inner.event_capacity
|| history.retained_bytes > self.inner.byte_capacity
{
let Some(evicted) = history.events.pop_front() else {
break;
};
history.retained_bytes = history
.retained_bytes
.saturating_sub(evicted.retained_len());
}
let _ = self.inner.live_tx.send(event);
}
pub(crate) fn subscribe(&self) -> ExecProcessEventReceiver {
let history = self
.inner
.history
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let live_rx = self.inner.live_tx.subscribe();
let replay = history.events.iter().cloned().collect();
ExecProcessEventReceiver { replay, live_rx }
}
}
pub struct ExecProcessEventReceiver {
replay: VecDeque<ExecProcessEvent>,
live_rx: broadcast::Receiver<ExecProcessEvent>,
}
impl ExecProcessEventReceiver {
pub fn empty() -> Self {
let (_live_tx, live_rx) = broadcast::channel(1);
Self {
replay: VecDeque::new(),
live_rx,
}
}
/// Returns the next replayed or live event.
///
/// `Lagged` means this receiver fell behind the bounded live channel. The
/// caller should recover through [`ExecProcess::read`] using the last
/// delivered sequence number, then continue receiving pushed events.
pub async fn recv(&mut self) -> Result<ExecProcessEvent, broadcast::error::RecvError> {
if let Some(event) = self.replay.pop_front() {
return Ok(event);
}
self.live_rx.recv().await
}
}
/// Handle for an executor-managed process.
///
/// Implementations must support both retained-output reads and pushed events:
/// `read` is the request/response API for callers that want to page through
/// buffered output, while `subscribe_events` is the streaming API for callers
/// that want output and lifecycle changes delivered as they happen.
#[async_trait]
pub trait ExecProcess: Send + Sync {
fn process_id(&self) -> &ProcessId;
fn subscribe_wake(&self) -> watch::Receiver<u64>;
fn subscribe_events(&self) -> ExecProcessEventReceiver;
async fn read(
&self,
after_seq: Option<u64>,
@@ -35,3 +185,54 @@ pub trait ExecProcess: Send + Sync {
pub trait ExecBackend: Send + Sync {
async fn start(&self, params: ExecParams) -> Result<StartedExecProcess, ExecServerError>;
}
#[cfg(test)]
mod tests {
use pretty_assertions::assert_eq;
use tokio::time::Duration;
use tokio::time::timeout;
use super::ExecProcessEvent;
use super::ExecProcessEventLog;
use crate::protocol::ExecOutputStream;
use crate::protocol::ProcessOutputChunk;
#[tokio::test]
async fn event_history_replay_is_bounded_by_retained_bytes() {
let log = ExecProcessEventLog::new(/*event_capacity*/ 8, /*byte_capacity*/ 3);
log.publish(ExecProcessEvent::Output(ProcessOutputChunk {
seq: 1,
stream: ExecOutputStream::Stdout,
chunk: b"large".to_vec().into(),
}));
log.publish(ExecProcessEvent::Exited {
seq: 2,
exit_code: 0,
});
log.publish(ExecProcessEvent::Closed { seq: 3 });
let mut events = log.subscribe();
let replay = vec![
timeout(Duration::from_secs(1), events.recv())
.await
.expect("exit event replay should not time out")
.expect("exit event replay should be available"),
timeout(Duration::from_secs(1), events.recv())
.await
.expect("closed event replay should not time out")
.expect("closed event replay should be available"),
];
assert_eq!(
replay,
vec![
ExecProcessEvent::Exited {
seq: 2,
exit_code: 0,
},
ExecProcessEvent::Closed { seq: 3 },
]
);
}
}
@@ -6,6 +6,7 @@ use tracing::trace;
use crate::ExecBackend;
use crate::ExecProcess;
use crate::ExecProcessEventReceiver;
use crate::ExecServerError;
use crate::StartedExecProcess;
use crate::client::ExecServerClient;
@@ -56,6 +57,10 @@ impl ExecProcess for RemoteExecProcess {
self.session.subscribe_wake()
}
fn subscribe_events(&self) -> ExecProcessEventReceiver {
self.session.subscribe_events()
}
async fn read(
&self,
after_seq: Option<u64>,
@@ -57,6 +57,15 @@ pub(crate) fn test_codex_helper_paths() -> anyhow::Result<TestCodexHelperPaths>
}
pub(crate) async fn exec_server() -> anyhow::Result<ExecServerHarness> {
exec_server_with_env(std::iter::empty::<(&str, &str)>()).await
}
pub(crate) async fn exec_server_with_env<I, K, V>(env: I) -> anyhow::Result<ExecServerHarness>
where
I: IntoIterator<Item = (K, V)>,
K: AsRef<std::ffi::OsStr>,
V: AsRef<std::ffi::OsStr>,
{
let helper_paths = test_codex_helper_paths()?;
let codex_home = TempDir::new()?;
let mut child = Command::new(&helper_paths.codex_exe);
@@ -66,6 +75,7 @@ pub(crate) async fn exec_server() -> anyhow::Result<ExecServerHarness> {
child.stderr(Stdio::inherit());
child.kill_on_drop(true);
child.env("CODEX_HOME", codex_home.path());
child.envs(env);
let mut child = child.spawn()?;
let websocket_url = read_listen_url_from_stdout(&mut child).await?;
+202 -3
View File
@@ -7,8 +7,10 @@ use std::sync::Arc;
use anyhow::Result;
use codex_exec_server::Environment;
use codex_exec_server::ExecBackend;
use codex_exec_server::ExecOutputStream;
use codex_exec_server::ExecParams;
use codex_exec_server::ExecProcess;
use codex_exec_server::ExecProcessEvent;
use codex_exec_server::ProcessId;
use codex_exec_server::ReadResponse;
use codex_exec_server::StartedExecProcess;
@@ -27,6 +29,22 @@ struct ProcessContext {
server: Option<ExecServerHarness>,
}
#[derive(Debug, PartialEq, Eq)]
enum ProcessEventSnapshot {
Output {
seq: u64,
stream: ExecOutputStream,
text: String,
},
Exited {
seq: u64,
exit_code: i32,
},
Closed {
seq: u64,
},
}
async fn create_process_context(use_remote: bool) -> Result<ProcessContext> {
if use_remote {
let server = exec_server().await?;
@@ -117,6 +135,69 @@ async fn collect_process_output_from_reads(
Ok((output, exit_code, true))
}
async fn collect_process_output_from_events(
session: Arc<dyn ExecProcess>,
) -> Result<(String, String, Option<i32>, bool)> {
let mut events = session.subscribe_events();
let mut stdout = String::new();
let mut stderr = String::new();
let mut exit_code = None;
loop {
match timeout(Duration::from_secs(2), events.recv()).await?? {
ExecProcessEvent::Output(chunk) => match chunk.stream {
ExecOutputStream::Stdout | ExecOutputStream::Pty => {
stdout.push_str(&String::from_utf8_lossy(&chunk.chunk.into_inner()));
}
ExecOutputStream::Stderr => {
stderr.push_str(&String::from_utf8_lossy(&chunk.chunk.into_inner()));
}
},
ExecProcessEvent::Exited {
seq: _,
exit_code: code,
} => {
exit_code = Some(code);
}
ExecProcessEvent::Closed { seq: _ } => {
drop(session);
return Ok((stdout, stderr, exit_code, true));
}
ExecProcessEvent::Failed(message) => {
anyhow::bail!("process failed before closed state: {message}");
}
}
}
}
async fn collect_process_event_snapshots(
session: Arc<dyn ExecProcess>,
) -> Result<Vec<ProcessEventSnapshot>> {
let mut events = session.subscribe_events();
let mut snapshots = Vec::new();
loop {
let snapshot = match timeout(Duration::from_secs(2), events.recv()).await?? {
ExecProcessEvent::Output(chunk) => ProcessEventSnapshot::Output {
seq: chunk.seq,
stream: chunk.stream,
text: String::from_utf8_lossy(&chunk.chunk.into_inner()).into_owned(),
},
ExecProcessEvent::Exited { seq, exit_code } => {
ProcessEventSnapshot::Exited { seq, exit_code }
}
ExecProcessEvent::Closed { seq } => ProcessEventSnapshot::Closed { seq },
ExecProcessEvent::Failed(message) => {
anyhow::bail!("process failed before closed state: {message}");
}
};
let closed = matches!(snapshot, ProcessEventSnapshot::Closed { .. });
snapshots.push(snapshot);
if closed {
drop(session);
return Ok(snapshots);
}
}
}
async fn assert_exec_process_streams_output(use_remote: bool) -> Result<()> {
let context = create_process_context(use_remote).await?;
let process_id = "proc-stream".to_string();
@@ -148,6 +229,96 @@ async fn assert_exec_process_streams_output(use_remote: bool) -> Result<()> {
Ok(())
}
async fn assert_exec_process_pushes_events(use_remote: bool) -> Result<()> {
let context = create_process_context(use_remote).await?;
let process_id = "proc-events".to_string();
let session = context
.backend
.start(ExecParams {
process_id: process_id.clone().into(),
argv: vec![
"/bin/sh".to_string(),
"-c".to_string(),
"printf 'event output\\n'; sleep 0.1; printf 'event err\\n' >&2; sleep 0.1; exit 7".to_string(),
],
cwd: std::env::current_dir()?,
env_policy: /*env_policy*/ None,
env: Default::default(),
tty: false,
pipe_stdin: false,
arg0: None,
})
.await?;
assert_eq!(session.process.process_id().as_str(), process_id);
let StartedExecProcess { process } = session;
let actual = collect_process_event_snapshots(process).await?;
assert_eq!(
actual,
vec![
ProcessEventSnapshot::Output {
seq: 1,
stream: ExecOutputStream::Stdout,
text: "event output\n".to_string(),
},
ProcessEventSnapshot::Output {
seq: 2,
stream: ExecOutputStream::Stderr,
text: "event err\n".to_string(),
},
ProcessEventSnapshot::Exited {
seq: 3,
exit_code: 7,
},
ProcessEventSnapshot::Closed { seq: 4 },
]
);
Ok(())
}
async fn assert_exec_process_replays_events_after_close(use_remote: bool) -> Result<()> {
let context = create_process_context(use_remote).await?;
let process_id = "proc-events-late".to_string();
let session = context
.backend
.start(ExecParams {
process_id: process_id.clone().into(),
argv: vec![
"/bin/sh".to_string(),
"-c".to_string(),
"printf 'late one\\n'; printf 'late two\\n'".to_string(),
],
cwd: std::env::current_dir()?,
env_policy: /*env_policy*/ None,
env: Default::default(),
tty: false,
pipe_stdin: false,
arg0: None,
})
.await?;
assert_eq!(session.process.process_id().as_str(), process_id);
let StartedExecProcess { process } = session;
let wake_rx = process.subscribe_wake();
let read_result = collect_process_output_from_reads(Arc::clone(&process), wake_rx).await?;
assert_eq!(
read_result,
("late one\nlate two\n".to_string(), Some(0), true)
);
let event_result = collect_process_output_from_events(process).await?;
assert_eq!(
event_result,
(
"late one\nlate two\n".to_string(),
String::new(),
Some(0),
true
)
);
Ok(())
}
async fn assert_exec_process_write_then_read(use_remote: bool) -> Result<()> {
let context = create_process_context(use_remote).await?;
let process_id = "proc-stdin".to_string();
@@ -311,15 +482,25 @@ async fn remote_exec_process_reports_transport_disconnect() -> Result<()> {
})
.await?;
let process = Arc::clone(&session.process);
let mut events = process.subscribe_events();
let server = context
.server
.as_mut()
.expect("remote context should include exec-server harness");
server.shutdown().await?;
let mut wake_rx = session.process.subscribe_wake();
let response =
read_process_until_change(session.process, &mut wake_rx, /*after_seq*/ None).await?;
let event = timeout(Duration::from_secs(2), events.recv()).await??;
let ExecProcessEvent::Failed(event_message) = event else {
anyhow::bail!("expected process failure event, got {event:?}");
};
assert!(
event_message.starts_with("exec-server transport disconnected"),
"unexpected failure event: {event_message}"
);
let mut wake_rx = process.subscribe_wake();
let response = read_process_until_change(process, &mut wake_rx, /*after_seq*/ None).await?;
let message = response
.failure
.expect("disconnect should surface as a failure");
@@ -353,6 +534,24 @@ async fn exec_process_streams_output(use_remote: bool) -> Result<()> {
assert_exec_process_streams_output(use_remote).await
}
#[test_case(false ; "local")]
#[test_case(true ; "remote")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
// Serialize tests that launch a real exec-server process through the full CLI.
#[serial_test::serial(remote_exec_server)]
async fn exec_process_pushes_events(use_remote: bool) -> Result<()> {
assert_exec_process_pushes_events(use_remote).await
}
#[test_case(false ; "local")]
#[test_case(true ; "remote")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
// Serialize tests that launch a real exec-server process through the full CLI.
#[serial_test::serial(remote_exec_server)]
async fn exec_process_replays_events_after_close(use_remote: bool) -> Result<()> {
assert_exec_process_replays_events_after_close(use_remote).await
}
#[test_case(false ; "local")]
#[test_case(true ; "remote")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+94
View File
@@ -2,6 +2,8 @@
mod common;
#[cfg(target_os = "linux")]
use std::os::unix::fs::PermissionsExt;
use std::os::unix::fs::symlink;
use std::path::Path;
use std::path::PathBuf;
@@ -31,6 +33,8 @@ use test_case::test_case;
use common::exec_server::ExecServerHarness;
use common::exec_server::TestCodexHelperPaths;
use common::exec_server::exec_server;
#[cfg(target_os = "linux")]
use common::exec_server::exec_server_with_env;
use common::exec_server::test_codex_helper_paths;
struct FileSystemContext {
@@ -148,6 +152,96 @@ fn alias_root_candidate() -> Result<Option<PathBuf>> {
Ok(None)
}
#[cfg(target_os = "linux")]
fn write_fake_bwrap(bin_dir: &Path) -> Result<PathBuf> {
std::fs::create_dir_all(bin_dir)?;
let fake_bwrap = bin_dir.join("bwrap");
std::fs::write(
&fake_bwrap,
r#"#!/bin/bash
set -euo pipefail
for arg in "$@"; do
if [[ "${arg}" == "--help" ]]; then
echo "Usage: bwrap --argv0"
exit 0
fi
done
printf '%s\n' "$*" >> "${0}.log"
args=("$@")
argv0=""
command_start=-1
for i in "${!args[@]}"; do
if [[ "${args[$i]}" == "--argv0" && $((i + 1)) -lt ${#args[@]} ]]; then
argv0="${args[$((i + 1))]}"
fi
if [[ "${args[$i]}" == "--" ]]; then
command_start=$((i + 1))
break
fi
done
if [[ "${command_start}" -lt 0 || "${command_start}" -ge "${#args[@]}" ]]; then
echo "fake bwrap did not find an inner command" >&2
exit 125
fi
cmd=("${args[@]:$command_start}")
if [[ -n "${argv0}" ]]; then
exec -a "${argv0}" "${cmd[@]}"
fi
exec "${cmd[@]}"
"#,
)?;
let mut permissions = std::fs::metadata(&fake_bwrap)?.permissions();
permissions.set_mode(0o755);
std::fs::set_permissions(&fake_bwrap, permissions)?;
Ok(fake_bwrap)
}
#[cfg(target_os = "linux")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn sandboxed_file_system_helper_finds_bwrap_on_preserved_path() -> Result<()> {
let tmp = TempDir::new()?;
let fake_bin_dir = tmp.path().join("bin");
let fake_bwrap = write_fake_bwrap(&fake_bin_dir)?;
let mut path_entries = vec![fake_bin_dir];
if let Some(path) = std::env::var_os("PATH") {
path_entries.extend(std::env::split_paths(&path));
}
let helper_path = std::env::join_paths(path_entries)?;
let server = exec_server_with_env([("PATH", helper_path.as_os_str())]).await?;
let environment = Environment::create(Some(server.websocket_url().to_string())).await?;
let file_system = environment.get_filesystem();
let workspace = tmp.path().join("workspace");
std::fs::create_dir_all(&workspace)?;
let file_path = workspace.join("created.txt");
let sandbox = workspace_write_sandbox(workspace);
file_system
.write_file(
&absolute_path(file_path.clone()),
b"written through fs helper".to_vec(),
Some(&sandbox),
)
.await?;
assert_eq!(std::fs::read(&file_path)?, b"written through fs helper");
let bwrap_log = fake_bwrap.with_file_name("bwrap.log");
let log = std::fs::read_to_string(&bwrap_log)
.with_context(|| format!("expected fake bwrap log at {}", bwrap_log.display()))?;
assert!(
log.contains("--argv0"),
"expected fs helper sandbox path to invoke PATH bwrap with --argv0, got: {log}"
);
Ok(())
}
#[test_case(false ; "local")]
#[test_case(true ; "remote")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+1
View File
@@ -36,6 +36,7 @@ fn rate_limit_snapshot() -> RateLimitSnapshot {
}),
credits: None,
plan_type: None,
rate_limit_reached_type: None,
}
}
+12
View File
@@ -2200,6 +2200,18 @@ pub struct RateLimitSnapshot {
pub secondary: Option<RateLimitWindow>,
pub credits: Option<CreditsSnapshot>,
pub plan_type: Option<crate::account::PlanType>,
pub rate_limit_reached_type: Option<RateLimitReachedType>,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize, JsonSchema, TS)]
#[serde(rename_all = "snake_case")]
#[ts(rename_all = "snake_case")]
pub enum RateLimitReachedType {
RateLimitReached,
WorkspaceOwnerCreditsDepleted,
WorkspaceMemberCreditsDepleted,
WorkspaceOwnerUsageLimitReached,
WorkspaceMemberUsageLimitReached,
}
#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema, TS)]
@@ -418,22 +418,23 @@ async fn fail_session_post_when_armed(
return next.run(request).await;
}
let mut armed_failure = state.armed_failure.lock().await;
if let Some(failure) = armed_failure.as_mut()
&& failure.remaining > 0
{
failure.remaining -= 1;
let status = failure.status;
if failure.remaining == 0 {
*armed_failure = None;
let mut armed_failure = state.armed_failure.lock().await;
if let Some(failure) = armed_failure.as_mut()
&& failure.remaining > 0
{
failure.remaining -= 1;
let status = failure.status;
if failure.remaining == 0 {
*armed_failure = None;
}
let mut response = Response::new(Body::from(format!(
"forced session failure with status {status}"
)));
*response.status_mut() = status;
return response;
}
let mut response = Response::new(Body::from(format!(
"forced session failure with status {status}"
)));
*response.status_mut() = status;
return response;
}
drop(armed_failure);
next.run(request).await
}
+3
View File
@@ -5,6 +5,7 @@ mod oauth;
mod perform_oauth_login;
mod program_resolver;
mod rmcp_client;
mod stdio_server_launcher;
mod utils;
pub use auth_status::StreamableHttpOAuthDiscovery;
@@ -29,3 +30,5 @@ pub use rmcp_client::ListToolsWithConnectorIdResult;
pub use rmcp_client::RmcpClient;
pub use rmcp_client::SendElicitation;
pub use rmcp_client::ToolWithConnectorId;
pub use stdio_server_launcher::LocalStdioServerLauncher;
pub use stdio_server_launcher::StdioServerLauncher;
+18 -149
View File
@@ -4,7 +4,6 @@ use std::ffi::OsString;
use std::future::Future;
use std::io;
use std::path::PathBuf;
use std::process::Stdio;
use std::sync::Arc;
use std::sync::atomic::AtomicUsize;
use std::sync::atomic::Ordering;
@@ -52,7 +51,6 @@ use rmcp::transport::StreamableHttpClientTransport;
use rmcp::transport::auth::AuthClient;
use rmcp::transport::auth::AuthError;
use rmcp::transport::auth::OAuthState;
use rmcp::transport::child_process::TokioChildProcess;
use rmcp::transport::streamable_http_client::AuthRequiredError;
use rmcp::transport::streamable_http_client::StreamableHttpClient;
use rmcp::transport::streamable_http_client::StreamableHttpClientTransportConfig;
@@ -63,23 +61,20 @@ use serde::Serialize;
use serde_json::Value;
use sse_stream::Sse;
use sse_stream::SseStream;
use tokio::io::AsyncBufReadExt;
use tokio::io::BufReader;
use tokio::process::Command;
use tokio::sync::Mutex;
use tokio::sync::watch;
use tokio::time;
use tracing::info;
use tracing::warn;
use crate::elicitation_client_service::ElicitationClientService;
use crate::load_oauth_tokens;
use crate::oauth::OAuthPersistor;
use crate::oauth::StoredOAuthTokens;
use crate::program_resolver;
use crate::stdio_server_launcher::StdioServerCommand;
use crate::stdio_server_launcher::StdioServerLauncher;
use crate::stdio_server_launcher::StdioServerTransport;
use crate::utils::apply_default_headers;
use crate::utils::build_default_headers;
use crate::utils::create_env_for_mcp_server;
use codex_config::types::OAuthCredentialsStoreMode;
const EVENT_STREAM_MIME_TYPE: &str = "text/event-stream";
@@ -307,9 +302,8 @@ impl StreamableHttpClient for StreamableHttpResponseClient {
}
enum PendingTransport {
ChildProcess {
transport: TokioChildProcess,
process_group_guard: Option<ProcessGroupGuard>,
Stdio {
transport: StdioServerTransport,
},
StreamableHttp {
transport: StreamableHttpClientTransport<StreamableHttpResponseClient>,
@@ -325,79 +319,16 @@ enum ClientState {
transport: Option<PendingTransport>,
},
Ready {
_process_group_guard: Option<ProcessGroupGuard>,
service: Arc<RunningService<RoleClient, ElicitationClientService>>,
oauth: Option<OAuthPersistor>,
},
}
#[cfg(unix)]
const PROCESS_GROUP_TERM_GRACE_PERIOD: Duration = Duration::from_secs(2);
#[cfg(unix)]
struct ProcessGroupGuard {
process_group_id: u32,
}
#[cfg(not(unix))]
struct ProcessGroupGuard;
impl ProcessGroupGuard {
fn new(process_group_id: u32) -> Self {
#[cfg(unix)]
{
Self { process_group_id }
}
#[cfg(not(unix))]
{
let _ = process_group_id;
Self
}
}
#[cfg(unix)]
fn maybe_terminate_process_group(&self) {
let process_group_id = self.process_group_id;
let should_escalate =
match codex_utils_pty::process_group::terminate_process_group(process_group_id) {
Ok(exists) => exists,
Err(error) => {
warn!("Failed to terminate MCP process group {process_group_id}: {error}");
false
}
};
if should_escalate {
std::thread::spawn(move || {
std::thread::sleep(PROCESS_GROUP_TERM_GRACE_PERIOD);
if let Err(error) =
codex_utils_pty::process_group::kill_process_group(process_group_id)
{
warn!("Failed to kill MCP process group {process_group_id}: {error}");
}
});
}
}
#[cfg(not(unix))]
fn maybe_terminate_process_group(&self) {}
}
impl Drop for ProcessGroupGuard {
fn drop(&mut self) {
if cfg!(unix) {
self.maybe_terminate_process_group();
}
}
}
#[derive(Clone)]
enum TransportRecipe {
Stdio {
program: OsString,
args: Vec<OsString>,
env: Option<HashMap<OsString, OsString>>,
env_vars: Vec<String>,
cwd: Option<PathBuf>,
command: StdioServerCommand,
launcher: Arc<dyn StdioServerLauncher>,
},
StreamableHttp {
server_name: String,
@@ -574,13 +505,11 @@ impl RmcpClient {
env: Option<HashMap<OsString, OsString>>,
env_vars: &[String],
cwd: Option<PathBuf>,
launcher: Arc<dyn StdioServerLauncher>,
) -> io::Result<Self> {
let transport_recipe = TransportRecipe::Stdio {
program,
args,
env,
env_vars: env_vars.to_vec(),
cwd,
command: StdioServerCommand::new(program, args, env, env_vars.to_vec(), cwd),
launcher,
};
let transport = Self::create_pending_transport(&transport_recipe)
.await
@@ -650,7 +579,7 @@ impl RmcpClient {
}
};
let (service, oauth_persistor, process_group_guard) =
let (service, oauth_persistor) =
Self::connect_pending_transport(pending_transport, client_service.clone(), timeout)
.await?;
@@ -671,7 +600,6 @@ impl RmcpClient {
{
let mut guard = self.state.lock().await;
*guard = ClientState::Ready {
_process_group_guard: process_group_guard,
service,
oauth: oauth_persistor.clone(),
};
@@ -954,60 +882,9 @@ impl RmcpClient {
transport_recipe: &TransportRecipe,
) -> Result<PendingTransport> {
match transport_recipe {
TransportRecipe::Stdio {
program,
args,
env,
env_vars,
cwd,
} => {
let program_name = program.to_string_lossy().into_owned();
let envs = create_env_for_mcp_server(env.clone(), env_vars);
let resolved_program = program_resolver::resolve(program.clone(), &envs)?;
let mut command = Command::new(resolved_program);
command
.kill_on_drop(true)
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.env_clear()
.envs(envs)
.args(args);
#[cfg(unix)]
command.process_group(0);
if let Some(cwd) = cwd {
command.current_dir(cwd);
}
let (transport, stderr) = TokioChildProcess::builder(command)
.stderr(Stdio::piped())
.spawn()?;
let process_group_guard = transport.id().map(ProcessGroupGuard::new);
if let Some(stderr) = stderr {
tokio::spawn(async move {
let mut reader = BufReader::new(stderr).lines();
loop {
match reader.next_line().await {
Ok(Some(line)) => {
info!("MCP server stderr ({program_name}): {line}");
}
Ok(None) => break,
Err(error) => {
warn!(
"Failed to read MCP server stderr ({program_name}): {error}"
);
break;
}
}
}
});
}
Ok(PendingTransport::ChildProcess {
transport,
process_group_guard,
})
TransportRecipe::Stdio { command, launcher } => {
let transport = launcher.launch(command.clone()).await?;
Ok(PendingTransport::Stdio { transport })
}
TransportRecipe::StreamableHttp {
server_name,
@@ -1101,21 +978,15 @@ impl RmcpClient {
) -> Result<(
Arc<RunningService<RoleClient, ElicitationClientService>>,
Option<OAuthPersistor>,
Option<ProcessGroupGuard>,
)> {
let (transport, oauth_persistor, process_group_guard) = match pending_transport {
PendingTransport::ChildProcess {
transport,
process_group_guard,
} => (
let (transport, oauth_persistor) = match pending_transport {
PendingTransport::Stdio { transport } => (
service::serve_client(client_service, transport).boxed(),
None,
process_group_guard,
),
PendingTransport::StreamableHttp { transport } => (
service::serve_client(client_service, transport).boxed(),
None,
None,
),
PendingTransport::StreamableHttpWithOAuth {
transport,
@@ -1123,7 +994,6 @@ impl RmcpClient {
} => (
service::serve_client(client_service, transport).boxed(),
Some(oauth_persistor),
None,
),
};
@@ -1137,7 +1007,7 @@ impl RmcpClient {
.map_err(|err| anyhow!("handshaking with MCP server failed: {err}"))?,
};
Ok((Arc::new(service), oauth_persistor, process_group_guard))
Ok((Arc::new(service), oauth_persistor))
}
async fn run_service_operation<T, F, Fut>(
@@ -1249,7 +1119,7 @@ impl RmcpClient {
.clone()
.ok_or_else(|| anyhow!("MCP client cannot recover before initialize succeeds"))?;
let pending_transport = Self::create_pending_transport(&self.transport_recipe).await?;
let (service, oauth_persistor, process_group_guard) = Self::connect_pending_transport(
let (service, oauth_persistor) = Self::connect_pending_transport(
pending_transport,
initialize_context.client_service,
initialize_context.timeout,
@@ -1259,7 +1129,6 @@ impl RmcpClient {
{
let mut guard = self.state.lock().await;
*guard = ClientState::Ready {
_process_group_guard: process_group_guard,
service,
oauth: oauth_persistor.clone(),
};
@@ -0,0 +1,268 @@
//! Launch MCP stdio servers and return the transport rmcp should use.
//!
//! This module owns the "where does the server process run?" boundary for
//! stdio MCP servers. In this PR there is only the local launcher, which keeps
//! the existing behavior: the orchestrator starts the configured command and
//! rmcp talks to the child process through local stdin/stdout pipes.
//!
//! Later stack entries add an executor-backed launcher without changing
//! `RmcpClient`'s MCP lifecycle code.
use std::collections::HashMap;
use std::ffi::OsString;
use std::future::Future;
use std::io;
use std::path::PathBuf;
use std::process::Stdio;
#[cfg(unix)]
use std::thread::sleep;
#[cfg(unix)]
use std::thread::spawn;
#[cfg(unix)]
use std::time::Duration;
#[cfg(unix)]
use codex_utils_pty::process_group::kill_process_group;
#[cfg(unix)]
use codex_utils_pty::process_group::terminate_process_group;
use futures::FutureExt;
use futures::future::BoxFuture;
use rmcp::service::RoleClient;
use rmcp::service::RxJsonRpcMessage;
use rmcp::service::TxJsonRpcMessage;
use rmcp::transport::Transport;
use rmcp::transport::child_process::TokioChildProcess;
use tokio::io::AsyncBufReadExt;
use tokio::io::BufReader;
use tokio::process::Command;
use tracing::info;
use tracing::warn;
use crate::program_resolver;
use crate::utils::create_env_for_mcp_server;
// General purpose public code.
/// Launches an MCP stdio server and returns the transport for rmcp.
///
/// This trait is the boundary between MCP lifecycle code and process placement.
/// `RmcpClient` owns MCP operations such as `initialize` and `tools/list`; the
/// launcher owns starting the configured command and producing an rmcp
/// [`Transport`] over the server's stdin/stdout bytes.
pub trait StdioServerLauncher: private::Sealed + Send + Sync {
/// Start the configured stdio server and return its rmcp-facing transport.
fn launch(
&self,
command: StdioServerCommand,
) -> BoxFuture<'static, io::Result<StdioServerTransport>>;
}
/// Command-line process shape shared by stdio server launchers.
#[derive(Clone)]
pub struct StdioServerCommand {
program: OsString,
args: Vec<OsString>,
env: Option<HashMap<OsString, OsString>>,
env_vars: Vec<String>,
cwd: Option<PathBuf>,
}
/// Client-side rmcp transport for a launched MCP stdio server.
///
/// The concrete process placement stays private to this module. `RmcpClient`
/// only sees the standard rmcp transport abstraction and can pass this value
/// directly to `rmcp::service::serve_client`.
pub struct StdioServerTransport {
inner: StdioServerTransportInner,
_process_group_guard: Option<ProcessGroupGuard>,
}
enum StdioServerTransportInner {
Local(TokioChildProcess),
}
impl Transport<RoleClient> for StdioServerTransport {
type Error = io::Error;
fn send(
&mut self,
item: TxJsonRpcMessage<RoleClient>,
) -> impl Future<Output = std::result::Result<(), Self::Error>> + Send + 'static {
match &mut self.inner {
StdioServerTransportInner::Local(transport) => transport.send(item).boxed(),
}
}
fn receive(&mut self) -> impl Future<Output = Option<RxJsonRpcMessage<RoleClient>>> + Send {
match &mut self.inner {
StdioServerTransportInner::Local(transport) => transport.receive().boxed(),
}
}
async fn close(&mut self) -> std::result::Result<(), Self::Error> {
match &mut self.inner {
StdioServerTransportInner::Local(transport) => transport.close().await,
}
}
}
impl StdioServerCommand {
/// Build the stdio process parameters before choosing where the process
/// runs.
pub(super) fn new(
program: OsString,
args: Vec<OsString>,
env: Option<HashMap<OsString, OsString>>,
env_vars: Vec<String>,
cwd: Option<PathBuf>,
) -> Self {
Self {
program,
args,
env,
env_vars,
cwd,
}
}
}
// Local public implementation.
/// Starts MCP stdio servers as local child processes.
///
/// This is the existing behavior for local MCP servers: the orchestrator
/// process spawns the configured command and rmcp talks to the child's local
/// stdin/stdout pipes directly.
#[derive(Clone)]
pub struct LocalStdioServerLauncher;
impl StdioServerLauncher for LocalStdioServerLauncher {
fn launch(
&self,
command: StdioServerCommand,
) -> BoxFuture<'static, io::Result<StdioServerTransport>> {
async move { Self::launch_server(command) }.boxed()
}
}
// Local private implementation.
#[cfg(unix)]
const PROCESS_GROUP_TERM_GRACE_PERIOD: Duration = Duration::from_secs(2);
#[cfg(unix)]
struct ProcessGroupGuard {
process_group_id: u32,
}
#[cfg(not(unix))]
struct ProcessGroupGuard;
mod private {
pub trait Sealed {}
}
impl private::Sealed for LocalStdioServerLauncher {}
impl LocalStdioServerLauncher {
fn launch_server(command: StdioServerCommand) -> io::Result<StdioServerTransport> {
let StdioServerCommand {
program,
args,
env,
env_vars,
cwd,
} = command;
let program_name = program.to_string_lossy().into_owned();
let envs = create_env_for_mcp_server(env, &env_vars);
let resolved_program =
program_resolver::resolve(program, &envs).map_err(io::Error::other)?;
let mut command = Command::new(resolved_program);
command
.kill_on_drop(true)
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.env_clear()
.envs(envs)
.args(args);
#[cfg(unix)]
command.process_group(0);
if let Some(cwd) = cwd {
command.current_dir(cwd);
}
let (transport, stderr) = TokioChildProcess::builder(command)
.stderr(Stdio::piped())
.spawn()?;
let process_group_guard = transport.id().map(ProcessGroupGuard::new);
if let Some(stderr) = stderr {
tokio::spawn(async move {
let mut reader = BufReader::new(stderr).lines();
loop {
match reader.next_line().await {
Ok(Some(line)) => {
info!("MCP server stderr ({program_name}): {line}");
}
Ok(None) => break,
Err(error) => {
warn!("Failed to read MCP server stderr ({program_name}): {error}");
break;
}
}
}
});
}
Ok(StdioServerTransport {
inner: StdioServerTransportInner::Local(transport),
_process_group_guard: process_group_guard,
})
}
}
impl ProcessGroupGuard {
fn new(process_group_id: u32) -> Self {
#[cfg(unix)]
{
Self { process_group_id }
}
#[cfg(not(unix))]
{
let _ = process_group_id;
Self
}
}
#[cfg(unix)]
fn maybe_terminate_process_group(&self) {
let process_group_id = self.process_group_id;
let should_escalate = match terminate_process_group(process_group_id) {
Ok(exists) => exists,
Err(error) => {
warn!("Failed to terminate MCP process group {process_group_id}: {error}");
false
}
};
if should_escalate {
spawn(move || {
sleep(PROCESS_GROUP_TERM_GRACE_PERIOD);
if let Err(error) = kill_process_group(process_group_id) {
warn!("Failed to kill MCP process group {process_group_id}: {error}");
}
});
}
}
#[cfg(not(unix))]
fn maybe_terminate_process_group(&self) {}
}
impl Drop for ProcessGroupGuard {
fn drop(&mut self) {
if cfg!(unix) {
self.maybe_terminate_process_group();
}
}
}
@@ -4,10 +4,12 @@ use std::collections::HashMap;
use std::ffi::OsString;
use std::fs;
use std::path::Path;
use std::sync::Arc;
use std::time::Duration;
use anyhow::Context;
use anyhow::Result;
use codex_rmcp_client::LocalStdioServerLauncher;
use codex_rmcp_client::RmcpClient;
fn process_exists(pid: u32) -> bool {
@@ -78,6 +80,7 @@ async fn drop_kills_wrapper_process_group() -> Result<()> {
)])),
&[],
/*cwd*/ None,
Arc::new(LocalStdioServerLauncher),
)
.await?;
+3
View File
@@ -1,9 +1,11 @@
use std::ffi::OsString;
use std::path::PathBuf;
use std::sync::Arc;
use std::time::Duration;
use codex_rmcp_client::ElicitationAction;
use codex_rmcp_client::ElicitationResponse;
use codex_rmcp_client::LocalStdioServerLauncher;
use codex_rmcp_client::RmcpClient;
use codex_utils_cargo_bin::CargoBinError;
use futures::FutureExt as _;
@@ -61,6 +63,7 @@ async fn rmcp_client_can_list_and_read_resources() -> anyhow::Result<()> {
/*env*/ None,
&[],
/*cwd*/ None,
Arc::new(LocalStdioServerLauncher),
)
.await?;
@@ -391,11 +391,11 @@ mod tests {
use std::sync::atomic::AtomicBool;
use std::sync::atomic::Ordering;
use tempfile::TempDir;
use tokio::sync::Semaphore;
use tokio::time::Instant;
use tokio::time::sleep;
static ESCALATE_SERVER_TEST_LOCK: LazyLock<tokio::sync::Mutex<()>> =
LazyLock::new(|| tokio::sync::Mutex::new(()));
static ESCALATE_SERVER_TEST_LOCK: LazyLock<Semaphore> = LazyLock::new(|| Semaphore::new(1));
struct DeterministicEscalationPolicy {
decision: EscalationDecision,
@@ -596,7 +596,7 @@ mod tests {
/// until `close_client_socket()` is called.
#[tokio::test]
async fn start_session_exposes_wrapper_env_overlay() -> anyhow::Result<()> {
let _guard = ESCALATE_SERVER_TEST_LOCK.lock().await;
let _guard = ESCALATE_SERVER_TEST_LOCK.acquire().await?;
let execve_wrapper = PathBuf::from("/tmp/codex-execve-wrapper");
let execve_wrapper_str = execve_wrapper.to_string_lossy().to_string();
let server = EscalateServer::new(
@@ -638,7 +638,7 @@ mod tests {
#[tokio::test]
async fn exec_closes_parent_socket_after_shell_spawn() -> anyhow::Result<()> {
let _guard = ESCALATE_SERVER_TEST_LOCK.lock().await;
let _guard = ESCALATE_SERVER_TEST_LOCK.acquire().await?;
let after_spawn_invoked = Arc::new(AtomicBool::new(false));
let server = EscalateServer::new(
PathBuf::from("/bin/bash"),
@@ -672,7 +672,7 @@ mod tests {
#[tokio::test]
async fn handle_escalate_session_respects_run_in_sandbox_decision() -> anyhow::Result<()> {
let _guard = ESCALATE_SERVER_TEST_LOCK.lock().await;
let _guard = ESCALATE_SERVER_TEST_LOCK.acquire().await?;
let (server, client) = AsyncSocket::pair()?;
let server_task = tokio::spawn(handle_escalate_session_with_policy(
server,
@@ -712,7 +712,7 @@ mod tests {
#[tokio::test]
async fn handle_escalate_session_resolves_relative_file_against_request_workdir()
-> anyhow::Result<()> {
let _guard = ESCALATE_SERVER_TEST_LOCK.lock().await;
let _guard = ESCALATE_SERVER_TEST_LOCK.acquire().await?;
let (server, client) = AsyncSocket::pair()?;
let tmp = tempfile::TempDir::new()?;
let workdir = tmp.path().join("workspace");
@@ -751,7 +751,7 @@ mod tests {
#[tokio::test]
async fn handle_escalate_session_executes_escalated_command() -> anyhow::Result<()> {
let _guard = ESCALATE_SERVER_TEST_LOCK.lock().await;
let _guard = ESCALATE_SERVER_TEST_LOCK.acquire().await?;
let (server, client) = AsyncSocket::pair()?;
let server_task = tokio::spawn(handle_escalate_session_with_policy(
server,
@@ -844,7 +844,7 @@ mod tests {
#[tokio::test]
async fn handle_escalate_session_accepts_received_fds_that_overlap_destinations()
-> anyhow::Result<()> {
let _guard = ESCALATE_SERVER_TEST_LOCK.lock().await;
let _guard = ESCALATE_SERVER_TEST_LOCK.acquire().await?;
let mut pipe_fds = [0; 2];
if unsafe { libc::pipe(pipe_fds.as_mut_ptr()) } == -1 {
return Err(std::io::Error::last_os_error().into());
@@ -916,7 +916,7 @@ mod tests {
#[tokio::test]
async fn handle_escalate_session_passes_permissions_to_executor() -> anyhow::Result<()> {
let _guard = ESCALATE_SERVER_TEST_LOCK.lock().await;
let _guard = ESCALATE_SERVER_TEST_LOCK.acquire().await?;
let (server, client) = AsyncSocket::pair()?;
let server_task = tokio::spawn(handle_escalate_session_with_policy(
server,
@@ -972,7 +972,7 @@ mod tests {
#[tokio::test]
async fn dropping_session_aborts_intercept_workers_and_kills_spawned_child()
-> anyhow::Result<()> {
let _guard = ESCALATE_SERVER_TEST_LOCK.lock().await;
let _guard = ESCALATE_SERVER_TEST_LOCK.acquire().await?;
let tmp = TempDir::new()?;
let pid_file = tmp.path().join("escalated-child.pid");
let pid_file_display = pid_file.display().to_string();
+47 -10
View File
@@ -93,6 +93,7 @@ use codex_app_server_protocol::PluginUninstallResponse;
use codex_app_server_protocol::RequestId;
use codex_app_server_protocol::ServerNotification;
use codex_app_server_protocol::ServerRequest;
use codex_app_server_protocol::SkillsListParams;
use codex_app_server_protocol::SkillsListResponse;
use codex_app_server_protocol::ThreadItem;
use codex_app_server_protocol::ThreadLoadedListParams;
@@ -2072,6 +2073,25 @@ impl App {
});
}
/// Starts the initial skills refresh without delaying the first interactive frame.
///
/// Startup only needs skill metadata to populate skill mentions and the skills UI; the prompt can be
/// rendered before that metadata arrives. The result is routed through the normal app event queue so
/// the same response handler updates the chat widget and emits invalid `SKILL.md` warnings once the
/// app-server RPC finishes. User-initiated skills refreshes still use the blocking app command path so
/// callers that explicitly asked for fresh skill state do not race ahead of their own refresh.
fn refresh_startup_skills(&mut self, app_server: &AppServerSession) {
let request_handle = app_server.request_handle();
let app_event_tx = self.app_event_tx.clone();
let cwd = self.config.cwd.to_path_buf();
tokio::spawn(async move {
let result = fetch_skills_list(request_handle, cwd)
.await
.map_err(|err| err.to_string());
app_event_tx.send(AppEvent::SkillsListLoaded { result });
});
}
fn fetch_plugins_list(&mut self, app_server: &AppServerSession, cwd: PathBuf) {
let request_handle = app_server.request_handle();
let app_event_tx = self.app_event_tx.clone();
@@ -4036,16 +4056,6 @@ impl App {
app.enqueue_primary_thread_session(started.session, started.turns)
.await?;
}
app.handle_skills_list_result(
app_server
.skills_list(codex_app_server_protocol::SkillsListParams {
cwds: vec![app.config.cwd.to_path_buf()],
force_reload: true,
per_cwd_extra_user_roots: None,
})
.await,
"failed to load skills on startup",
);
// On startup, if Agent mode (workspace-write) or ReadOnly is active, warn about world-writable dirs on Windows.
#[cfg(target_os = "windows")]
@@ -4076,6 +4086,7 @@ impl App {
tokio::pin!(tui_events);
tui.frame_requester().schedule_frame();
app.refresh_startup_skills(&app_server);
// Kick off a non-blocking rate-limit prefetch so the first `/status`
// already has data, without delaying the initial frame render.
if requires_openai_auth && has_chatgpt_account {
@@ -4774,6 +4785,12 @@ impl App {
AppEvent::McpInventoryLoaded { result } => {
self.handle_mcp_inventory_result(result);
}
AppEvent::SkillsListLoaded { result } => {
self.handle_skills_list_result(
result.map_err(|err| color_eyre::eyre::eyre!(err)),
"failed to load skills on startup",
);
}
AppEvent::StartFileSearch(query) => {
self.file_search.on_user_query(query);
}
@@ -6454,6 +6471,26 @@ async fn fetch_account_rate_limits(
Ok(app_server_rate_limit_snapshots_to_core(response))
}
async fn fetch_skills_list(
request_handle: AppServerRequestHandle,
cwd: PathBuf,
) -> Result<SkillsListResponse> {
let request_id = RequestId::String(format!("startup-skills-list-{}", Uuid::new_v4()));
// Use the cloneable request handle so startup can issue this RPC from a background task without
// extending a borrow of `AppServerSession` across the first frame render.
request_handle
.request_typed(ClientRequest::SkillsList {
request_id,
params: SkillsListParams {
cwds: vec![cwd],
force_reload: true,
per_cwd_extra_user_roots: None,
},
})
.await
.wrap_err("skills/list failed in TUI")
}
async fn fetch_plugins_list(
request_handle: AppServerRequestHandle,
cwd: PathBuf,
+10
View File
@@ -17,6 +17,7 @@ use codex_app_server_protocol::PluginListResponse;
use codex_app_server_protocol::PluginReadParams;
use codex_app_server_protocol::PluginReadResponse;
use codex_app_server_protocol::PluginUninstallResponse;
use codex_app_server_protocol::SkillsListResponse;
use codex_file_search::FileMatch;
use codex_protocol::ThreadId;
use codex_protocol::openai_models::ModelPreset;
@@ -306,6 +307,15 @@ pub(crate) enum AppEvent {
result: Result<Vec<McpServerStatus>, String>,
},
/// Result of the startup skills refresh that runs after the first frame is scheduled.
///
/// This event is startup-only. Interactive skills refreshes are handled synchronously through the app
/// command path because those callers expect the visible skill state to be current when their command
/// completes.
SkillsListLoaded {
result: Result<SkillsListResponse, String>,
},
InsertHistoryCell(Box<dyn HistoryCell>),
/// Apply rollback semantics to local transcript cells.
+1
View File
@@ -1188,6 +1188,7 @@ pub(crate) fn app_server_rate_limit_snapshot_to_core(
secondary: snapshot.secondary.map(app_server_rate_limit_window_to_core),
credits: snapshot.credits.map(app_server_credits_snapshot_to_core),
plan_type: snapshot.plan_type,
rate_limit_reached_type: snapshot.rate_limit_reached_type.map(Into::into),
}
}
+1 -1
View File
@@ -120,6 +120,7 @@ mod selection_tabs;
mod textarea;
mod unified_exec_footer;
pub(crate) use feedback_view::FeedbackNoteView;
pub(crate) use selection_tabs::SelectionTab;
/// How long the "press again to quit" hint stays visible.
///
@@ -854,7 +855,6 @@ impl BottomPane {
.and_then(|view| view.selected_index())
}
#[allow(dead_code)]
pub(crate) fn active_tab_id_for_active_view(&self, view_id: &'static str) -> Option<&str> {
self.view_stack
.last()
+18 -1
View File
@@ -844,6 +844,7 @@ pub(crate) struct ChatWidget {
plugins_fetch_state: PluginListFetchState,
plugin_install_apps_needing_auth: Vec<AppSummary>,
plugin_install_auth_flow: Option<PluginInstallAuthFlowState>,
plugins_active_tab_id: Option<String>,
// Queue of interruptive UI events deferred during an active write cycle
interrupts: InterruptManager,
// Accumulates the current reasoning block text to extract a header
@@ -4880,6 +4881,7 @@ impl ChatWidget {
plugins_fetch_state: PluginListFetchState::default(),
plugin_install_apps_needing_auth: Vec::new(),
plugin_install_auth_flow: None,
plugins_active_tab_id: None,
interrupts: InterruptManager::new(),
reasoning_buffer: String::new(),
full_reasoning_buffer: String::new(),
@@ -7126,7 +7128,22 @@ impl ChatWidget {
.map(|ti| &ti.total_token_usage)
.unwrap_or(&default_usage);
let collaboration_mode = self.collaboration_mode_label();
let reasoning_effort_override = Some(self.effective_reasoning_effort());
let model = self.current_model().to_string();
let model_default_reasoning_effort =
self.model_catalog
.try_list_models()
.ok()
.and_then(|models| {
models
.into_iter()
.find(|preset| preset.model == model)
.map(|preset| preset.default_reasoning_effort)
});
let reasoning_effort_override = Some(
self.effective_reasoning_effort()
.or(self.config.model_reasoning_effort)
.or(model_default_reasoning_effort),
);
let rate_limit_snapshots: Vec<RateLimitSnapshotDisplay> = self
.rate_limit_snapshots_by_limit_id
.values()
+315 -99
View File
@@ -7,8 +7,10 @@ use crate::app_event::AppEvent;
use crate::bottom_pane::ColumnWidthMode;
use crate::bottom_pane::SelectionItem;
use crate::bottom_pane::SelectionRowDisplay;
use crate::bottom_pane::SelectionTab;
use crate::bottom_pane::SelectionViewParams;
use crate::history_cell;
use crate::legacy_core::plugins::OPENAI_CURATED_MARKETPLACE_NAME;
use crate::onboarding::mark_url_hyperlink;
use crate::render::renderable::ColumnRenderable;
use crate::render::renderable::Renderable;
@@ -35,6 +37,9 @@ use ratatui::widgets::Wrap;
use unicode_width::UnicodeWidthStr;
const PLUGINS_SELECTION_VIEW_ID: &str = "plugins-selection";
const ALL_PLUGINS_TAB_ID: &str = "all-plugins";
const INSTALLED_PLUGINS_TAB_ID: &str = "installed-plugins";
const OPENAI_CURATED_TAB_ID: &str = "marketplace:openai-curated";
const PLUGIN_ROW_PREFIX_WIDTH: usize = 2;
const LOADING_ANIMATION_DELAY: Duration = Duration::from_secs(1);
const LOADING_ANIMATION_INTERVAL: Duration = Duration::from_millis(100);
@@ -141,6 +146,7 @@ impl ChatWidget {
return;
}
self.plugins_active_tab_id = Some(ALL_PLUGINS_TAB_ID.to_string());
self.prefetch_plugins();
match self.plugins_cache_for_current_cwd() {
@@ -226,11 +232,18 @@ impl ChatWidget {
}
fn open_plugins_popup(&mut self, response: &PluginListResponse) {
self.bottom_pane
.show_selection_view(self.plugins_popup_params(response));
self.plugins_active_tab_id = Some(ALL_PLUGINS_TAB_ID.to_string());
self.bottom_pane.show_selection_view(
self.plugins_popup_params(response, self.plugins_active_tab_id.clone()),
);
}
pub(crate) fn open_plugin_detail_loading_popup(&mut self, plugin_display_name: &str) {
self.plugins_active_tab_id = self
.bottom_pane
.active_tab_id_for_active_view(PLUGINS_SELECTION_VIEW_ID)
.map(str::to_string)
.or_else(|| self.plugins_active_tab_id.clone());
let params = self.plugin_detail_loading_popup_params(plugin_display_name);
let _ = self
.bottom_pane
@@ -504,7 +517,7 @@ impl ChatWidget {
Some(SelectionViewParams {
view_id: Some(PLUGINS_SELECTION_VIEW_ID),
header: Box::new(header),
footer_hint: Some(plugins_popup_hint_line()),
footer_hint: Some(plugin_detail_hint_line()),
items,
col_width_mode: ColumnWidthMode::AutoAllRows,
..Default::default()
@@ -547,17 +560,24 @@ impl ChatWidget {
_ => None,
};
if let Some(plugins_response) = plugins_response {
let tab_id = self.plugins_active_tab_id.clone();
let _ = self.bottom_pane.replace_selection_view_if_active(
PLUGINS_SELECTION_VIEW_ID,
self.plugins_popup_params(&plugins_response),
self.plugins_popup_params(&plugins_response, tab_id),
);
}
}
fn refresh_plugins_popup_if_open(&mut self, response: &PluginListResponse) {
let active_tab_id = self
.bottom_pane
.active_tab_id_for_active_view(PLUGINS_SELECTION_VIEW_ID)
.map(str::to_string)
.or_else(|| self.plugins_active_tab_id.clone());
self.plugins_active_tab_id = active_tab_id.clone();
let _ = self.bottom_pane.replace_selection_view_if_active(
PLUGINS_SELECTION_VIEW_ID,
self.plugins_popup_params(response),
self.plugins_popup_params(response, active_tab_id),
);
}
@@ -568,7 +588,7 @@ impl ChatWidget {
self.frame_requester.clone(),
self.config.animations,
"Loading available plugins...".to_string(),
Some("This first pass shows the ChatGPT marketplace only.".to_string()),
Some("This updates when the marketplace list is ready.".to_string()),
)),
items: vec![SelectionItem {
name: "Loading plugins...".to_string(),
@@ -697,13 +717,17 @@ impl ChatWidget {
SelectionViewParams {
view_id: Some(PLUGINS_SELECTION_VIEW_ID),
header: Box::new(header),
footer_hint: Some(plugins_popup_hint_line()),
footer_hint: Some(plugin_detail_hint_line()),
items,
..Default::default()
}
}
fn plugins_popup_params(&self, response: &PluginListResponse) -> SelectionViewParams {
fn plugins_popup_params(
&self,
response: &PluginListResponse,
active_tab_id: Option<String>,
) -> SelectionViewParams {
let marketplaces: Vec<&PluginMarketplaceEntry> = response.marketplaces.iter().collect();
let total: usize = marketplaces
@@ -716,111 +740,131 @@ impl ChatWidget {
.filter(|plugin| plugin.installed)
.count();
let mut header = ColumnRenderable::new();
header.push(Line::from("Plugins".bold()));
header.push(Line::from(
"Browse plugins from available marketplaces.".dim(),
));
header.push(Line::from(
format!("Installed {installed} of {total} available plugins.").dim(),
));
if let Some(remote_sync_error) = response.remote_sync_error.as_deref() {
header.push(Line::from(
format!("Using cached marketplace data: {remote_sync_error}").dim(),
));
}
let mut plugin_entries: Vec<(&PluginMarketplaceEntry, &PluginSummary, String)> =
marketplaces
.iter()
.flat_map(|marketplace| {
marketplace
.plugins
.iter()
.map(move |plugin| (*marketplace, plugin, plugin_display_name(plugin)))
})
.collect();
plugin_entries.sort_by(|left, right| {
right
.1
.installed
.cmp(&left.1.installed)
.then_with(|| {
left.2
.to_ascii_lowercase()
.cmp(&right.2.to_ascii_lowercase())
})
.then_with(|| left.2.cmp(&right.2))
.then_with(|| left.1.name.cmp(&right.1.name))
.then_with(|| left.1.id.cmp(&right.1.id))
});
let status_label_width = plugin_entries
.iter()
.map(|(_, plugin, _)| plugin_status_label(plugin).chars().count())
.max()
.unwrap_or(0);
let name_column_width = plugin_entries
let all_entries = plugin_entries_for_marketplaces(marketplaces.iter().copied());
let name_column_width = all_entries
.iter()
.map(|(_, _, display_name)| {
PLUGIN_ROW_PREFIX_WIDTH + UnicodeWidthStr::width(display_name.as_str())
})
.max();
let installed_entries = all_entries
.iter()
.filter(|(_, plugin, _)| plugin.installed)
.cloned()
.collect();
let mut items: Vec<SelectionItem> = Vec::new();
for (marketplace, plugin, display_name) in plugin_entries {
let marketplace_label = marketplace_display_name(marketplace);
let status_label = plugin_status_label(plugin);
let description =
plugin_brief_description(plugin, &marketplace_label, status_label_width);
let selected_status_label = format!("{status_label:<status_label_width$}");
let selected_description =
format!("{selected_status_label} Press Enter to view plugin details.");
let search_value = format!(
"{display_name} {} {} {}",
plugin.id, plugin.name, marketplace_label
);
let cwd = self.config.cwd.to_path_buf();
let plugin_display_name = display_name.clone();
let marketplace_path = marketplace.path.clone();
let plugin_name = plugin.name.clone();
let mut tabs = Vec::new();
tabs.push(SelectionTab {
id: ALL_PLUGINS_TAB_ID.to_string(),
label: "All Plugins".to_string(),
header: plugins_header(
"Browse plugins from available marketplaces.".to_string(),
format!("Installed {installed} of {total} available plugins."),
response.remote_sync_error.as_deref(),
),
items: self.plugin_selection_items(
all_entries,
/*include_marketplace_names*/ true,
"No marketplace plugins available",
"No plugins are available in the discovered marketplaces.",
),
});
items.push(SelectionItem {
name: display_name,
description: Some(description),
selected_description: Some(selected_description),
search_value: Some(search_value),
actions: vec![Box::new(move |tx| {
tx.send(AppEvent::OpenPluginDetailLoading {
plugin_display_name: plugin_display_name.clone(),
});
tx.send(AppEvent::FetchPluginDetail {
cwd: cwd.clone(),
params: codex_app_server_protocol::PluginReadParams {
marketplace_path: marketplace_path.clone(),
plugin_name: plugin_name.clone(),
},
});
})],
..Default::default()
});
}
tabs.push(SelectionTab {
id: INSTALLED_PLUGINS_TAB_ID.to_string(),
label: format!("Installed ({installed})"),
header: plugins_header(
"Installed plugins.".to_string(),
format!("Showing {installed} installed plugins."),
response.remote_sync_error.as_deref(),
),
items: self.plugin_selection_items(
installed_entries,
/*include_marketplace_names*/ true,
"No installed plugins",
"No installed plugins.",
),
});
if items.is_empty() {
items.push(SelectionItem {
name: "No marketplace plugins available".to_string(),
description: Some(
"No plugins are available in the discovered marketplaces.".to_string(),
let curated_marketplace = marketplaces
.iter()
.find(|marketplace| marketplace.name == OPENAI_CURATED_MARKETPLACE_NAME)
.copied();
let curated_entries = curated_marketplace
.map(|marketplace| plugin_entries_for_marketplaces([marketplace]))
.unwrap_or_default();
let curated_total = curated_entries.len();
let curated_installed = curated_entries
.iter()
.filter(|(_, plugin, _)| plugin.installed)
.count();
tabs.push(SelectionTab {
id: OPENAI_CURATED_TAB_ID.to_string(),
label: "OpenAI Curated".to_string(),
header: plugins_header(
"OpenAI Curated marketplace.".to_string(),
format!("Installed {curated_installed} of {curated_total} OpenAI Curated plugins."),
response.remote_sync_error.as_deref(),
),
items: self.plugin_selection_items(
curated_entries,
/*include_marketplace_names*/ false,
"No OpenAI Curated plugins available",
"No OpenAI Curated plugins available.",
),
});
let mut additional_marketplaces: Vec<&PluginMarketplaceEntry> = marketplaces
.iter()
.copied()
.filter(|marketplace| marketplace.name != OPENAI_CURATED_MARKETPLACE_NAME)
.collect();
additional_marketplaces.sort_by(|left, right| {
marketplace_display_name(left)
.to_ascii_lowercase()
.cmp(&marketplace_display_name(right).to_ascii_lowercase())
.then_with(|| marketplace_display_name(left).cmp(&marketplace_display_name(right)))
.then_with(|| left.name.cmp(&right.name))
});
let labels = disambiguate_duplicate_tab_labels(
additional_marketplaces
.iter()
.map(|marketplace| marketplace_display_name(marketplace))
.collect(),
);
for (marketplace, label) in additional_marketplaces.into_iter().zip(labels) {
let entries = plugin_entries_for_marketplaces([marketplace]);
let marketplace_total = entries.len();
let marketplace_installed = entries
.iter()
.filter(|(_, plugin, _)| plugin.installed)
.count();
tabs.push(SelectionTab {
id: marketplace_tab_id(marketplace),
label: label.clone(),
header: plugins_header(
format!("{label}."),
format!(
"Installed {marketplace_installed} of {marketplace_total} {label} plugins."
),
response.remote_sync_error.as_deref(),
),
items: self.plugin_selection_items(
entries,
/*include_marketplace_names*/ false,
"No plugins available in this marketplace",
"No plugins available in this marketplace.",
),
is_disabled: true,
..Default::default()
});
}
SelectionViewParams {
view_id: Some(PLUGINS_SELECTION_VIEW_ID),
header: Box::new(header),
header: Box::new(()),
footer_hint: Some(plugins_popup_hint_line()),
items,
tabs,
initial_tab_id: active_tab_id,
is_searchable: true,
search_placeholder: Some("Type to search plugins".to_string()),
col_width_mode: ColumnWidthMode::AutoAllRows,
@@ -962,18 +1006,178 @@ impl ChatWidget {
SelectionViewParams {
view_id: Some(PLUGINS_SELECTION_VIEW_ID),
header: Box::new(header),
footer_hint: Some(plugins_popup_hint_line()),
footer_hint: Some(plugin_detail_hint_line()),
items,
col_width_mode: ColumnWidthMode::AutoAllRows,
..Default::default()
}
}
fn plugin_selection_items<'a>(
&self,
mut plugin_entries: Vec<(&'a PluginMarketplaceEntry, &'a PluginSummary, String)>,
include_marketplace_names: bool,
empty_name: &str,
empty_description: &str,
) -> Vec<SelectionItem> {
sort_plugin_entries(&mut plugin_entries);
let status_label_width = plugin_entries
.iter()
.map(|(_, plugin, _)| plugin_status_label(plugin).chars().count())
.max()
.unwrap_or(0);
let mut items: Vec<SelectionItem> = Vec::new();
for (marketplace, plugin, display_name) in plugin_entries {
let marketplace_label = marketplace_display_name(marketplace);
let status_label = plugin_status_label(plugin);
let description = if include_marketplace_names {
plugin_brief_description(plugin, &marketplace_label, status_label_width)
} else {
plugin_brief_description_without_marketplace(plugin, status_label_width)
};
let selected_status_label = format!("{status_label:<status_label_width$}");
let selected_description =
format!("{selected_status_label} Press Enter to view plugin details.");
let search_value = format!(
"{display_name} {} {} {}",
plugin.id, plugin.name, marketplace_label
);
let cwd = self.config.cwd.to_path_buf();
let plugin_display_name = display_name.clone();
let marketplace_path = marketplace.path.clone();
let plugin_name = plugin.name.clone();
items.push(SelectionItem {
name: display_name,
description: Some(description),
selected_description: Some(selected_description),
search_value: Some(search_value),
actions: vec![Box::new(move |tx| {
tx.send(AppEvent::OpenPluginDetailLoading {
plugin_display_name: plugin_display_name.clone(),
});
tx.send(AppEvent::FetchPluginDetail {
cwd: cwd.clone(),
params: codex_app_server_protocol::PluginReadParams {
marketplace_path: marketplace_path.clone(),
plugin_name: plugin_name.clone(),
},
});
})],
..Default::default()
});
}
if items.is_empty() {
items.push(SelectionItem {
name: empty_name.to_string(),
description: Some(empty_description.to_string()),
is_disabled: true,
..Default::default()
});
}
items
}
}
fn plugins_popup_hint_line() -> Line<'static> {
Line::from("←/→ select marketplace · enter view details · esc close")
}
fn plugin_detail_hint_line() -> Line<'static> {
Line::from("Press esc to close.")
}
fn plugins_header(
subtitle: String,
count_line: String,
remote_sync_error: Option<&str>,
) -> Box<dyn Renderable> {
let mut header = ColumnRenderable::new();
header.push(Line::from("Plugins".bold()));
header.push(Line::from(subtitle.dim()));
header.push(Line::from(count_line.dim()));
if let Some(remote_sync_error) = remote_sync_error {
header.push(Line::from(
format!("Using cached marketplace data: {remote_sync_error}").dim(),
));
}
Box::new(header)
}
fn plugin_entries_for_marketplaces<'a>(
marketplaces: impl IntoIterator<Item = &'a PluginMarketplaceEntry>,
) -> Vec<(&'a PluginMarketplaceEntry, &'a PluginSummary, String)> {
marketplaces
.into_iter()
.flat_map(|marketplace| {
marketplace
.plugins
.iter()
.map(move |plugin| (marketplace, plugin, plugin_display_name(plugin)))
})
.collect()
}
fn sort_plugin_entries(entries: &mut [(&PluginMarketplaceEntry, &PluginSummary, String)]) {
entries.sort_by(|left, right| {
right
.1
.installed
.cmp(&left.1.installed)
.then_with(|| {
left.2
.to_ascii_lowercase()
.cmp(&right.2.to_ascii_lowercase())
})
.then_with(|| left.2.cmp(&right.2))
.then_with(|| left.1.name.cmp(&right.1.name))
.then_with(|| left.1.id.cmp(&right.1.id))
});
}
fn marketplace_tab_id(marketplace: &PluginMarketplaceEntry) -> String {
format!("marketplace:{}", marketplace.path.display())
}
fn disambiguate_duplicate_tab_labels(labels: Vec<String>) -> Vec<String> {
let mut counts: Vec<(String, usize)> = Vec::new();
for label in &labels {
if let Some((_, count)) = counts.iter_mut().find(|(existing, _)| existing == label) {
*count += 1;
} else {
counts.push((label.clone(), 1));
}
}
let mut seen: Vec<(String, usize)> = Vec::new();
labels
.into_iter()
.map(|label| {
let total = counts
.iter()
.find(|(existing, _)| existing == &label)
.map(|(_, count)| *count)
.unwrap_or(1);
if total == 1 {
return label;
}
let current = if let Some((_, seen_count)) =
seen.iter_mut().find(|(existing, _)| existing == &label)
{
*seen_count += 1;
*seen_count
} else {
seen.push((label.clone(), 1));
1
};
format!("{label} ({current}/{total})")
})
.collect()
}
fn marketplace_display_name(marketplace: &PluginMarketplaceEntry) -> String {
marketplace
.interface
@@ -1009,6 +1213,18 @@ fn plugin_brief_description(
}
}
fn plugin_brief_description_without_marketplace(
plugin: &PluginSummary,
status_label_width: usize,
) -> String {
let status_label = plugin_status_label(plugin);
let status_label = format!("{status_label:<status_label_width$}");
match plugin_description(plugin) {
Some(description) => format!("{status_label} · {description}"),
None => status_label,
}
}
fn plugin_status_label(plugin: &PluginSummary) -> &'static str {
if plugin.installed {
if plugin.enabled {
@@ -7,10 +7,12 @@ expression: popup
Installed 1 of 4 available plugins.
Using cached marketplace data: remote sync timed out
[All Plugins] Installed (1) OpenAI Curated Repo Marketplace
Type to search plugins
› Alpha Sync Disabled Press Enter to view plugin details.
Bravo Search Available · ChatGPT Marketplace · Search docs and tickets.
Hidden Repo Plugin Available · Repo Marketplace · Should not be shown in /plugins.
Starter Available by default · ChatGPT Marketplace · Included by default.
Press esc to close.
←/→ select marketplace · enter view details · esc close
@@ -4,6 +4,6 @@ expression: popup
---
Plugins
Loading available plugins...
This first pass shows the ChatGPT marketplace only.
This updates when the marketplace list is ready.
› Loading plugins... This updates when the marketplace list is ready.
@@ -6,7 +6,9 @@ expression: popup
Browse plugins from available marketplaces.
Installed 0 of 3 available plugins.
[All Plugins] Installed (0) OpenAI Curated
sla
› Slack Available Press Enter to view plugin details.
Press esc to close.
←/→ select marketplace · enter view details · esc close
@@ -106,6 +106,7 @@ pub(super) fn snapshot(percent: f64) -> RateLimitSnapshot {
secondary: None,
credits: None,
plan_type: None,
rate_limit_reached_type: None,
}
}
@@ -233,6 +234,7 @@ pub(super) async fn make_chatwidget_manual(
connectors_partial_snapshot: None,
plugin_install_apps_needing_auth: Vec::new(),
plugin_install_auth_flow: None,
plugins_active_tab_id: None,
connectors_prefetch_in_flight: false,
connectors_force_refetch_pending: false,
plugins_cache: PluginsCacheState::default(),
@@ -442,6 +442,160 @@ async fn plugins_popup_search_filters_visible_rows_snapshot() {
);
}
#[tokio::test]
async fn plugins_popup_installed_tab_filters_rows_and_clears_search() {
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
chat.set_feature_enabled(Feature::Plugins, /*enabled*/ true);
render_loaded_plugins_popup(
&mut chat,
plugins_test_response(vec![plugins_test_curated_marketplace(vec![
plugins_test_summary(
"plugin-calendar",
"calendar",
Some("Calendar"),
Some("Schedule management."),
/*installed*/ true,
/*enabled*/ true,
PluginInstallPolicy::Available,
),
plugins_test_summary(
"plugin-slack",
"slack",
Some("Slack"),
Some("Team chat."),
/*installed*/ false,
/*enabled*/ true,
PluginInstallPolicy::Available,
),
])]),
);
type_plugins_search_query(&mut chat, "sla");
chat.handle_key_event(KeyEvent::from(KeyCode::Right));
let popup = render_bottom_popup(&chat, /*width*/ 100);
assert!(
popup.contains("Installed plugins.") && popup.contains("Showing 1 installed plugins."),
"expected Installed tab header, got:\n{popup}"
);
assert!(
popup.contains("Calendar") && !popup.contains("Slack"),
"expected Installed tab to show only installed plugins, got:\n{popup}"
);
assert!(
!popup.contains("sla"),
"expected tab switch to clear search query, got:\n{popup}"
);
}
#[tokio::test]
async fn plugins_popup_openai_curated_tab_omits_marketplace_in_rows() {
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
chat.set_feature_enabled(Feature::Plugins, /*enabled*/ true);
render_loaded_plugins_popup(
&mut chat,
plugins_test_response(vec![
plugins_test_curated_marketplace(vec![plugins_test_summary(
"plugin-calendar",
"calendar",
Some("Calendar"),
Some("Schedule management."),
/*installed*/ false,
/*enabled*/ true,
PluginInstallPolicy::Available,
)]),
plugins_test_repo_marketplace(vec![plugins_test_summary(
"plugin-repo",
"repo",
Some("Repo Plugin"),
Some("Repo-only plugin."),
/*installed*/ false,
/*enabled*/ true,
PluginInstallPolicy::Available,
)]),
]),
);
chat.handle_key_event(KeyEvent::from(KeyCode::Right));
chat.handle_key_event(KeyEvent::from(KeyCode::Right));
let popup = render_bottom_popup(&chat, /*width*/ 100);
assert!(
popup.contains("OpenAI Curated marketplace."),
"expected OpenAI Curated tab header, got:\n{popup}"
);
assert!(
popup.contains("Calendar") && !popup.contains("Repo Plugin"),
"expected OpenAI Curated tab to show only official marketplace plugins, got:\n{popup}"
);
assert!(
!popup.contains("ChatGPT Marketplace ·"),
"expected marketplace-specific rows to omit marketplace labels, got:\n{popup}"
);
}
#[tokio::test]
async fn plugins_popup_refresh_preserves_duplicate_marketplace_tab_by_path() {
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
chat.set_feature_enabled(Feature::Plugins, /*enabled*/ true);
let response = plugins_test_response(vec![
PluginMarketplaceEntry {
name: "duplicate".to_string(),
path: plugins_test_absolute_path("marketplaces/home/marketplace.json"),
interface: Some(MarketplaceInterface {
display_name: Some("Duplicate Marketplace".to_string()),
}),
plugins: vec![plugins_test_summary(
"plugin-home",
"home",
Some("Home Plugin"),
Some("Home marketplace plugin."),
/*installed*/ false,
/*enabled*/ true,
PluginInstallPolicy::Available,
)],
},
PluginMarketplaceEntry {
name: "duplicate".to_string(),
path: plugins_test_absolute_path("marketplaces/repo/marketplace.json"),
interface: Some(MarketplaceInterface {
display_name: Some("Duplicate Marketplace".to_string()),
}),
plugins: vec![plugins_test_summary(
"plugin-repo",
"repo",
Some("Repo Plugin"),
Some("Repo marketplace plugin."),
/*installed*/ false,
/*enabled*/ true,
PluginInstallPolicy::Available,
)],
},
]);
let cwd = chat.config.cwd.to_path_buf();
chat.on_plugins_loaded(cwd.clone(), Ok(response.clone()));
chat.add_plugins_output();
for _ in 0..4 {
chat.handle_key_event(KeyEvent::from(KeyCode::Right));
}
chat.on_plugins_loaded(cwd, Ok(response));
let popup = render_bottom_popup(&chat, /*width*/ 100);
assert!(
popup.contains("Duplicate Marketplace (2/2)."),
"expected refresh to preserve the second duplicate marketplace tab, got:\n{popup}"
);
assert!(
popup.contains("Repo Plugin") && !popup.contains("Home Plugin"),
"expected second duplicate marketplace rows after refresh, got:\n{popup}"
);
}
#[tokio::test]
async fn plugins_popup_search_no_matches_and_backspace_restores_results() {
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
@@ -235,6 +235,7 @@ async fn rate_limit_snapshot_keeps_prior_credits_when_missing_from_headers() {
balance: Some("17.5".to_string()),
}),
plan_type: None,
rate_limit_reached_type: None,
}));
let initial_balance = chat
.rate_limit_snapshots_by_limit_id
@@ -254,6 +255,7 @@ async fn rate_limit_snapshot_keeps_prior_credits_when_missing_from_headers() {
secondary: None,
credits: None,
plan_type: None,
rate_limit_reached_type: None,
}));
let display = chat
@@ -292,6 +294,7 @@ async fn rate_limit_snapshot_updates_and_retains_plan_type() {
}),
credits: None,
plan_type: Some(PlanType::Plus),
rate_limit_reached_type: None,
}));
assert_eq!(chat.plan_type, Some(PlanType::Plus));
@@ -310,6 +313,7 @@ async fn rate_limit_snapshot_updates_and_retains_plan_type() {
}),
credits: None,
plan_type: Some(PlanType::Pro),
rate_limit_reached_type: None,
}));
assert_eq!(chat.plan_type, Some(PlanType::Pro));
@@ -328,6 +332,7 @@ async fn rate_limit_snapshot_updates_and_retains_plan_type() {
}),
credits: None,
plan_type: None,
rate_limit_reached_type: None,
}));
assert_eq!(chat.plan_type, Some(PlanType::Pro));
}
@@ -351,6 +356,7 @@ async fn rate_limit_snapshots_keep_separate_entries_per_limit_id() {
balance: Some("5.00".to_string()),
}),
plan_type: Some(PlanType::Pro),
rate_limit_reached_type: None,
}));
chat.on_rate_limit_snapshot(Some(RateLimitSnapshot {
@@ -364,6 +370,7 @@ async fn rate_limit_snapshots_keep_separate_entries_per_limit_id() {
secondary: None,
credits: None,
plan_type: Some(PlanType::Pro),
rate_limit_reached_type: None,
}));
let codex = chat
@@ -416,6 +423,7 @@ async fn rate_limit_switch_prompt_skips_non_codex_limit() {
secondary: None,
credits: None,
plan_type: None,
rate_limit_reached_type: None,
}));
assert!(matches!(
@@ -76,6 +76,25 @@ async fn status_command_renders_immediately_without_rate_limit_refresh() {
);
}
#[tokio::test]
async fn status_command_uses_catalog_default_reasoning_when_config_empty() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1-codex-max")).await;
chat.config.model_reasoning_effort = None;
chat.dispatch_command(SlashCommand::Status);
let rendered = match rx.try_recv() {
Ok(AppEvent::InsertHistoryCell(cell)) => {
lines_to_single_string(&cell.display_lines(/*width*/ 80))
}
other => panic!("expected status output, got {other:?}"),
};
assert!(
rendered.contains("gpt-5.1-codex-max (reasoning medium, summaries auto)"),
"expected /status to render the catalog default reasoning effort, got: {rendered}"
);
}
#[tokio::test]
async fn status_command_renders_instruction_sources_from_thread_session() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
+1 -1
View File
@@ -259,7 +259,7 @@ impl StatusHistoryCell {
];
if config.model_provider.wire_api == WireApi::Responses {
let effort_value = reasoning_effort_override
.unwrap_or(None)
.unwrap_or(config.model_reasoning_effort)
.map(|effort| effort.to_string())
.unwrap_or_else(|| "none".to_string());
config_entries.push(("reasoning effort", effort_value));
@@ -0,0 +1,21 @@
---
source: tui/src/status/tests.rs
expression: sanitized
---
/status
╭─────────────────────────────────────────────────────────────────────────╮
│ >_ OpenAI Codex (v0.0.0) │
│ │
│ Visit https://chatgpt.com/codex/settings/usage for up-to-date │
│ information on rate limits and credits │
│ │
│ Model: gpt-5.1-codex-max (reasoning medium, summaries auto) │
│ Directory: [[workspace]] │
│ Permissions: Custom (read-only, on-request) │
│ Agents.md: <none> │
│ │
│ Token usage: 750 total (500 input + 250 output) │
│ Context window: 100% left (750 used / 272K) │
│ Limits: data not available yet │
╰─────────────────────────────────────────────────────────────────────────╯
+64
View File
@@ -1,5 +1,6 @@
use super::new_status_output;
use super::new_status_output_with_rate_limits;
use super::new_status_output_with_rate_limits_handle;
use super::rate_limit_snapshot_display;
use crate::history_cell::HistoryCell;
use crate::legacy_core::config::Config;
@@ -138,6 +139,7 @@ async fn status_snapshot_includes_reasoning_details() {
}),
credits: None,
plan_type: None,
rate_limit_reached_type: None,
};
let rate_display = rate_limit_snapshot_display(&snapshot, captured_at);
@@ -321,6 +323,7 @@ async fn status_snapshot_includes_monthly_limit() {
secondary: None,
credits: None,
plan_type: None,
rate_limit_reached_type: None,
};
let rate_display = rate_limit_snapshot_display(&snapshot, captured_at);
@@ -372,6 +375,7 @@ async fn status_snapshot_shows_unlimited_credits() {
balance: None,
}),
plan_type: None,
rate_limit_reached_type: None,
};
let rate_display = rate_limit_snapshot_display(&snapshot, captured_at);
let model_slug = crate::legacy_core::test_support::get_model_offline(config.model.as_deref());
@@ -421,6 +425,7 @@ async fn status_snapshot_shows_positive_credits() {
balance: Some("12.5".to_string()),
}),
plan_type: None,
rate_limit_reached_type: None,
};
let rate_display = rate_limit_snapshot_display(&snapshot, captured_at);
let model_slug = crate::legacy_core::test_support::get_model_offline(config.model.as_deref());
@@ -470,6 +475,7 @@ async fn status_snapshot_hides_zero_credits() {
balance: Some("0".to_string()),
}),
plan_type: None,
rate_limit_reached_type: None,
};
let rate_display = rate_limit_snapshot_display(&snapshot, captured_at);
let model_slug = crate::legacy_core::test_support::get_model_offline(config.model.as_deref());
@@ -517,6 +523,7 @@ async fn status_snapshot_hides_when_has_no_credits_flag() {
balance: None,
}),
plan_type: None,
rate_limit_reached_type: None,
};
let rate_display = rate_limit_snapshot_display(&snapshot, captured_at);
let model_slug = crate::legacy_core::test_support::get_model_offline(config.model.as_deref());
@@ -622,6 +629,7 @@ async fn status_snapshot_truncates_in_narrow_terminal() {
secondary: None,
credits: None,
plan_type: None,
rate_limit_reached_type: None,
};
let rate_display = rate_limit_snapshot_display(&snapshot, captured_at);
@@ -702,6 +710,56 @@ async fn status_snapshot_shows_missing_limits_message() {
assert_snapshot!(sanitized);
}
#[tokio::test]
async fn status_snapshot_uses_default_reasoning_when_config_empty() {
let temp_home = TempDir::new().expect("temp home");
let mut config = test_config(&temp_home).await;
config.model = Some("gpt-5.1-codex-max".to_string());
config.cwd = test_path_buf("/workspace/tests").abs();
let account_display = test_status_account_display();
let usage = TokenUsage {
input_tokens: 500,
cached_input_tokens: 0,
output_tokens: 250,
reasoning_output_tokens: 0,
total_tokens: 750,
};
let now = chrono::Local
.with_ymd_and_hms(2024, 2, 3, 4, 5, 6)
.single()
.expect("timestamp");
let model_slug = crate::legacy_core::test_support::get_model_offline(config.model.as_deref());
let token_info = token_info_for(&model_slug, &config, &usage);
let (composite, _) = new_status_output_with_rate_limits_handle(
&config,
account_display.as_ref(),
Some(&token_info),
&usage,
&None,
/*thread_name*/ None,
/*forked_from*/ None,
&[],
None,
now,
&model_slug,
/*collaboration_mode*/ None,
/*reasoning_effort_override*/ Some(Some(ReasoningEffort::Medium)),
"<none>".to_string(),
/*refreshing_rate_limits*/ false,
);
let mut rendered_lines = render_lines(&composite.display_lines(/*width*/ 80));
if cfg!(windows) {
for line in &mut rendered_lines {
*line = line.replace('\\', "/");
}
}
let sanitized = sanitize_directory(rendered_lines).join("\n");
assert_snapshot!(sanitized);
}
#[tokio::test]
async fn status_snapshot_shows_refreshing_limits_notice() {
let temp_home = TempDir::new().expect("temp home");
@@ -735,6 +793,7 @@ async fn status_snapshot_shows_refreshing_limits_notice() {
}),
credits: None,
plan_type: None,
rate_limit_reached_type: None,
};
let rate_display = rate_limit_snapshot_display(&snapshot, captured_at);
@@ -805,6 +864,7 @@ async fn status_snapshot_includes_credits_and_limits() {
balance: Some("37.5".to_string()),
}),
plan_type: None,
rate_limit_reached_type: None,
};
let rate_display = rate_limit_snapshot_display(&snapshot, captured_at);
@@ -858,6 +918,7 @@ async fn status_snapshot_shows_unavailable_limits_message() {
secondary: None,
credits: None,
plan_type: None,
rate_limit_reached_type: None,
};
let captured_at = chrono::Local
.with_ymd_and_hms(2024, 6, 7, 8, 9, 10)
@@ -914,6 +975,7 @@ async fn status_snapshot_treats_refreshing_empty_limits_as_unavailable() {
secondary: None,
credits: None,
plan_type: None,
rate_limit_reached_type: None,
};
let captured_at = chrono::Local
.with_ymd_and_hms(2024, 6, 7, 8, 9, 10)
@@ -984,6 +1046,7 @@ async fn status_snapshot_shows_stale_limits_message() {
}),
credits: None,
plan_type: None,
rate_limit_reached_type: None,
};
let rate_display = rate_limit_snapshot_display(&snapshot, captured_at);
let now = captured_at + ChronoDuration::minutes(20);
@@ -1054,6 +1117,7 @@ async fn status_snapshot_cached_limits_hide_credits_without_flag() {
balance: Some("80".to_string()),
}),
plan_type: None,
rate_limit_reached_type: None,
};
let rate_display = rate_limit_snapshot_display(&snapshot, captured_at);
let now = captured_at + ChronoDuration::minutes(20);
+3 -4
View File
@@ -162,12 +162,11 @@ async fn spawn_process_with_stdin_mode(
let (stdout_tx, stdout_rx) = mpsc::channel::<Vec<u8>>(128);
let (stderr_tx, stderr_rx) = mpsc::channel::<Vec<u8>>(128);
let writer_handle = if let Some(stdin) = stdin {
let writer = Arc::new(tokio::sync::Mutex::new(stdin));
tokio::spawn(async move {
let mut writer = stdin;
while let Some(bytes) = writer_rx.recv().await {
let mut guard = writer.lock().await;
let _ = guard.write_all(&bytes).await;
let _ = guard.flush().await;
let _ = writer.write_all(&bytes).await;
let _ = writer.flush().await;
}
})
} else {
+24 -5
View File
@@ -277,17 +277,36 @@ mod tests {
#[tokio::test]
async fn subscribe_returns_error_when_lock_is_held() {
let flag = ReadinessFlag::new();
let _guard = flag
.tokens
.try_lock()
.expect("initial lock acquisition should succeed");
let flag = Arc::new(ReadinessFlag::new());
let (locked_tx, locked_rx) = std::sync::mpsc::channel();
let (release_tx, release_rx) = std::sync::mpsc::channel();
let lock_thread = {
let flag = Arc::clone(&flag);
std::thread::spawn(move || {
let _guard = flag.tokens.blocking_lock();
locked_tx
.send(())
.expect("test should receive lock acquisition notification");
release_rx
.recv()
.expect("test should release held readiness lock");
})
};
locked_rx
.recv()
.expect("test should observe held readiness lock");
let err = flag
.subscribe()
.await
.expect_err("contended subscribe should report a lock failure");
assert_matches!(err, ReadinessError::TokenLockFailed);
release_tx
.send(())
.expect("test should release readiness lock thread");
lock_thread
.join()
.expect("readiness lock thread should not panic");
}
#[tokio::test]