diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 4c22d8d73..4091c0d24 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2072,6 +2072,7 @@ dependencies = [ "codex-protocol", "codex-skills", "codex-utils-absolute-path", + "codex-utils-output-truncation", "codex-utils-plugins", "dirs", "dunce", diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index 5695877af..b0eca6785 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -3990,6 +3990,25 @@ } ] }, + "WarningNotification": { + "properties": { + "message": { + "description": "Concise warning message for the user.", + "type": "string" + }, + "threadId": { + "description": "Optional thread target when the warning applies to a specific thread.", + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "message" + ], + "type": "object" + }, "WebSearchAction": { "oneOf": [ { @@ -4930,6 +4949,26 @@ "title": "Model/reroutedNotification", "type": "object" }, + { + "properties": { + "method": { + "enum": [ + "warning" + ], + "title": "WarningNotificationMethod", + "type": "string" + }, + "params": { + "$ref": "#/definitions/WarningNotification" + } + }, + "required": [ + "method", + "params" + ], + "title": "WarningNotification", + "type": "object" + }, { "properties": { "method": { diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 9fb9c3e3e..d9f79d920 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -4303,6 +4303,26 @@ "title": "Model/reroutedNotification", "type": "object" }, + { + "properties": { + "method": { + "enum": [ + "warning" + ], + "title": "WarningNotificationMethod", + "type": "string" + }, + "params": { + "$ref": "#/definitions/v2/WarningNotification" + } + }, + "required": [ + "method", + "params" + ], + "title": "WarningNotification", + "type": "object" + }, { "properties": { "method": { @@ -15830,6 +15850,27 @@ ], "type": "string" }, + "WarningNotification": { + "$schema": "http://json-schema.org/draft-07/schema#", + "properties": { + "message": { + "description": "Concise warning message for the user.", + "type": "string" + }, + "threadId": { + "description": "Optional thread target when the warning applies to a specific thread.", + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "message" + ], + "title": "WarningNotification", + "type": "object" + }, "WebSearchAction": { "oneOf": [ { diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index da0381d8b..621903386 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -9787,6 +9787,26 @@ "title": "Model/reroutedNotification", "type": "object" }, + { + "properties": { + "method": { + "enum": [ + "warning" + ], + "title": "WarningNotificationMethod", + "type": "string" + }, + "params": { + "$ref": "#/definitions/WarningNotification" + } + }, + "required": [ + "method", + "params" + ], + "title": "WarningNotification", + "type": "object" + }, { "properties": { "method": { @@ -13674,6 +13694,27 @@ ], "type": "string" }, + "WarningNotification": { + "$schema": "http://json-schema.org/draft-07/schema#", + "properties": { + "message": { + "description": "Concise warning message for the user.", + "type": "string" + }, + "threadId": { + "description": "Optional thread target when the warning applies to a specific thread.", + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "message" + ], + "title": "WarningNotification", + "type": "object" + }, "WebSearchAction": { "oneOf": [ { diff --git a/codex-rs/app-server-protocol/schema/json/v2/WarningNotification.json b/codex-rs/app-server-protocol/schema/json/v2/WarningNotification.json new file mode 100644 index 000000000..460486896 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/json/v2/WarningNotification.json @@ -0,0 +1,21 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "properties": { + "message": { + "description": "Concise warning message for the user.", + "type": "string" + }, + "threadId": { + "description": "Optional thread target when the warning applies to a specific thread.", + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "message" + ], + "title": "WarningNotification", + "type": "object" +} \ No newline at end of file diff --git a/codex-rs/app-server-protocol/schema/typescript/ServerNotification.ts b/codex-rs/app-server-protocol/schema/typescript/ServerNotification.ts index f6df421dc..a43bb8042 100644 --- a/codex-rs/app-server-protocol/schema/typescript/ServerNotification.ts +++ b/codex-rs/app-server-protocol/schema/typescript/ServerNotification.ts @@ -54,10 +54,11 @@ import type { TurnCompletedNotification } from "./v2/TurnCompletedNotification"; import type { TurnDiffUpdatedNotification } from "./v2/TurnDiffUpdatedNotification"; import type { TurnPlanUpdatedNotification } from "./v2/TurnPlanUpdatedNotification"; import type { TurnStartedNotification } from "./v2/TurnStartedNotification"; +import type { WarningNotification } from "./v2/WarningNotification"; import type { WindowsSandboxSetupCompletedNotification } from "./v2/WindowsSandboxSetupCompletedNotification"; import type { WindowsWorldWritableWarningNotification } from "./v2/WindowsWorldWritableWarningNotification"; /** * Notification sent from the server to the client. */ -export type ServerNotification = { "method": "error", "params": ErrorNotification } | { "method": "thread/started", "params": ThreadStartedNotification } | { "method": "thread/status/changed", "params": ThreadStatusChangedNotification } | { "method": "thread/archived", "params": ThreadArchivedNotification } | { "method": "thread/unarchived", "params": ThreadUnarchivedNotification } | { "method": "thread/closed", "params": ThreadClosedNotification } | { "method": "skills/changed", "params": SkillsChangedNotification } | { "method": "thread/name/updated", "params": ThreadNameUpdatedNotification } | { "method": "thread/tokenUsage/updated", "params": ThreadTokenUsageUpdatedNotification } | { "method": "turn/started", "params": TurnStartedNotification } | { "method": "hook/started", "params": HookStartedNotification } | { "method": "turn/completed", "params": TurnCompletedNotification } | { "method": "hook/completed", "params": HookCompletedNotification } | { "method": "turn/diff/updated", "params": TurnDiffUpdatedNotification } | { "method": "turn/plan/updated", "params": TurnPlanUpdatedNotification } | { "method": "item/started", "params": ItemStartedNotification } | { "method": "item/autoApprovalReview/started", "params": ItemGuardianApprovalReviewStartedNotification } | { "method": "item/autoApprovalReview/completed", "params": ItemGuardianApprovalReviewCompletedNotification } | { "method": "item/completed", "params": ItemCompletedNotification } | { "method": "rawResponseItem/completed", "params": RawResponseItemCompletedNotification } | { "method": "item/agentMessage/delta", "params": AgentMessageDeltaNotification } | { "method": "item/plan/delta", "params": PlanDeltaNotification } | { "method": "command/exec/outputDelta", "params": CommandExecOutputDeltaNotification } | { "method": "item/commandExecution/outputDelta", "params": CommandExecutionOutputDeltaNotification } | { "method": "item/commandExecution/terminalInteraction", "params": TerminalInteractionNotification } | { "method": "item/fileChange/outputDelta", "params": FileChangeOutputDeltaNotification } | { "method": "serverRequest/resolved", "params": ServerRequestResolvedNotification } | { "method": "item/mcpToolCall/progress", "params": McpToolCallProgressNotification } | { "method": "mcpServer/oauthLogin/completed", "params": McpServerOauthLoginCompletedNotification } | { "method": "mcpServer/startupStatus/updated", "params": McpServerStatusUpdatedNotification } | { "method": "account/updated", "params": AccountUpdatedNotification } | { "method": "account/rateLimits/updated", "params": AccountRateLimitsUpdatedNotification } | { "method": "app/list/updated", "params": AppListUpdatedNotification } | { "method": "externalAgentConfig/import/completed", "params": ExternalAgentConfigImportCompletedNotification } | { "method": "fs/changed", "params": FsChangedNotification } | { "method": "item/reasoning/summaryTextDelta", "params": ReasoningSummaryTextDeltaNotification } | { "method": "item/reasoning/summaryPartAdded", "params": ReasoningSummaryPartAddedNotification } | { "method": "item/reasoning/textDelta", "params": ReasoningTextDeltaNotification } | { "method": "thread/compacted", "params": ContextCompactedNotification } | { "method": "model/rerouted", "params": ModelReroutedNotification } | { "method": "deprecationNotice", "params": DeprecationNoticeNotification } | { "method": "configWarning", "params": ConfigWarningNotification } | { "method": "fuzzyFileSearch/sessionUpdated", "params": FuzzyFileSearchSessionUpdatedNotification } | { "method": "fuzzyFileSearch/sessionCompleted", "params": FuzzyFileSearchSessionCompletedNotification } | { "method": "thread/realtime/started", "params": ThreadRealtimeStartedNotification } | { "method": "thread/realtime/itemAdded", "params": ThreadRealtimeItemAddedNotification } | { "method": "thread/realtime/transcript/delta", "params": ThreadRealtimeTranscriptDeltaNotification } | { "method": "thread/realtime/transcript/done", "params": ThreadRealtimeTranscriptDoneNotification } | { "method": "thread/realtime/outputAudio/delta", "params": ThreadRealtimeOutputAudioDeltaNotification } | { "method": "thread/realtime/sdp", "params": ThreadRealtimeSdpNotification } | { "method": "thread/realtime/error", "params": ThreadRealtimeErrorNotification } | { "method": "thread/realtime/closed", "params": ThreadRealtimeClosedNotification } | { "method": "windows/worldWritableWarning", "params": WindowsWorldWritableWarningNotification } | { "method": "windowsSandbox/setupCompleted", "params": WindowsSandboxSetupCompletedNotification } | { "method": "account/login/completed", "params": AccountLoginCompletedNotification }; +export type ServerNotification = { "method": "error", "params": ErrorNotification } | { "method": "thread/started", "params": ThreadStartedNotification } | { "method": "thread/status/changed", "params": ThreadStatusChangedNotification } | { "method": "thread/archived", "params": ThreadArchivedNotification } | { "method": "thread/unarchived", "params": ThreadUnarchivedNotification } | { "method": "thread/closed", "params": ThreadClosedNotification } | { "method": "skills/changed", "params": SkillsChangedNotification } | { "method": "thread/name/updated", "params": ThreadNameUpdatedNotification } | { "method": "thread/tokenUsage/updated", "params": ThreadTokenUsageUpdatedNotification } | { "method": "turn/started", "params": TurnStartedNotification } | { "method": "hook/started", "params": HookStartedNotification } | { "method": "turn/completed", "params": TurnCompletedNotification } | { "method": "hook/completed", "params": HookCompletedNotification } | { "method": "turn/diff/updated", "params": TurnDiffUpdatedNotification } | { "method": "turn/plan/updated", "params": TurnPlanUpdatedNotification } | { "method": "item/started", "params": ItemStartedNotification } | { "method": "item/autoApprovalReview/started", "params": ItemGuardianApprovalReviewStartedNotification } | { "method": "item/autoApprovalReview/completed", "params": ItemGuardianApprovalReviewCompletedNotification } | { "method": "item/completed", "params": ItemCompletedNotification } | { "method": "rawResponseItem/completed", "params": RawResponseItemCompletedNotification } | { "method": "item/agentMessage/delta", "params": AgentMessageDeltaNotification } | { "method": "item/plan/delta", "params": PlanDeltaNotification } | { "method": "command/exec/outputDelta", "params": CommandExecOutputDeltaNotification } | { "method": "item/commandExecution/outputDelta", "params": CommandExecutionOutputDeltaNotification } | { "method": "item/commandExecution/terminalInteraction", "params": TerminalInteractionNotification } | { "method": "item/fileChange/outputDelta", "params": FileChangeOutputDeltaNotification } | { "method": "serverRequest/resolved", "params": ServerRequestResolvedNotification } | { "method": "item/mcpToolCall/progress", "params": McpToolCallProgressNotification } | { "method": "mcpServer/oauthLogin/completed", "params": McpServerOauthLoginCompletedNotification } | { "method": "mcpServer/startupStatus/updated", "params": McpServerStatusUpdatedNotification } | { "method": "account/updated", "params": AccountUpdatedNotification } | { "method": "account/rateLimits/updated", "params": AccountRateLimitsUpdatedNotification } | { "method": "app/list/updated", "params": AppListUpdatedNotification } | { "method": "externalAgentConfig/import/completed", "params": ExternalAgentConfigImportCompletedNotification } | { "method": "fs/changed", "params": FsChangedNotification } | { "method": "item/reasoning/summaryTextDelta", "params": ReasoningSummaryTextDeltaNotification } | { "method": "item/reasoning/summaryPartAdded", "params": ReasoningSummaryPartAddedNotification } | { "method": "item/reasoning/textDelta", "params": ReasoningTextDeltaNotification } | { "method": "thread/compacted", "params": ContextCompactedNotification } | { "method": "model/rerouted", "params": ModelReroutedNotification } | { "method": "warning", "params": WarningNotification } | { "method": "deprecationNotice", "params": DeprecationNoticeNotification } | { "method": "configWarning", "params": ConfigWarningNotification } | { "method": "fuzzyFileSearch/sessionUpdated", "params": FuzzyFileSearchSessionUpdatedNotification } | { "method": "fuzzyFileSearch/sessionCompleted", "params": FuzzyFileSearchSessionCompletedNotification } | { "method": "thread/realtime/started", "params": ThreadRealtimeStartedNotification } | { "method": "thread/realtime/itemAdded", "params": ThreadRealtimeItemAddedNotification } | { "method": "thread/realtime/transcript/delta", "params": ThreadRealtimeTranscriptDeltaNotification } | { "method": "thread/realtime/transcript/done", "params": ThreadRealtimeTranscriptDoneNotification } | { "method": "thread/realtime/outputAudio/delta", "params": ThreadRealtimeOutputAudioDeltaNotification } | { "method": "thread/realtime/sdp", "params": ThreadRealtimeSdpNotification } | { "method": "thread/realtime/error", "params": ThreadRealtimeErrorNotification } | { "method": "thread/realtime/closed", "params": ThreadRealtimeClosedNotification } | { "method": "windows/worldWritableWarning", "params": WindowsWorldWritableWarningNotification } | { "method": "windowsSandbox/setupCompleted", "params": WindowsSandboxSetupCompletedNotification } | { "method": "account/login/completed", "params": AccountLoginCompletedNotification }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/WarningNotification.ts b/codex-rs/app-server-protocol/schema/typescript/v2/WarningNotification.ts new file mode 100644 index 000000000..bd3433be4 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/WarningNotification.ts @@ -0,0 +1,13 @@ +// 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 WarningNotification = { +/** + * Optional thread target when the warning applies to a specific thread. + */ +threadId: string | null, +/** + * Concise warning message for the user. + */ +message: string, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts index f9884efb7..6ae7ebb7d 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts @@ -362,6 +362,7 @@ export type { TurnStatus } from "./TurnStatus"; export type { TurnSteerParams } from "./TurnSteerParams"; export type { TurnSteerResponse } from "./TurnSteerResponse"; export type { UserInput } from "./UserInput"; +export type { WarningNotification } from "./WarningNotification"; export type { WebSearchAction } from "./WebSearchAction"; export type { WindowsSandboxSetupCompletedNotification } from "./WindowsSandboxSetupCompletedNotification"; export type { WindowsSandboxSetupMode } from "./WindowsSandboxSetupMode"; diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index c29dbccd5..f4461050e 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -1029,6 +1029,7 @@ server_notification_definitions! { /// Deprecated: Use `ContextCompaction` item type instead. ContextCompacted => "thread/compacted" (v2::ContextCompactedNotification), ModelRerouted => "model/rerouted" (v2::ModelReroutedNotification), + Warning => "warning" (v2::WarningNotification), DeprecationNotice => "deprecationNotice" (v2::DeprecationNoticeNotification), ConfigWarning => "configWarning" (v2::ConfigWarningNotification), FuzzyFileSearchSessionUpdated => "fuzzyFileSearch/sessionUpdated" (FuzzyFileSearchSessionUpdatedNotification), diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 340a80a61..6390b8f10 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -6689,6 +6689,16 @@ pub struct DeprecationNoticeNotification { pub details: Option, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct WarningNotification { + /// Optional thread target when the warning applies to a specific thread. + pub thread_id: Option, + /// Concise warning message for the user. + pub message: String, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index fcc19ce42..2ade95b6a 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -958,6 +958,10 @@ Event notifications are the server-initiated event stream for thread lifecycles, Thread realtime uses a separate thread-scoped notification surface. `thread/realtime/*` notifications are ephemeral transport events, not `ThreadItem`s, and are not returned by `thread/read`, `thread/resume`, or `thread/fork`. +Recoverable configuration and initialization warnings use the existing `configWarning` notification: `{ summary, details?, path?, range? }`. App-server may emit it during initialization for config parsing and related setup diagnostics. + +Generic runtime warnings use the `warning` notification: `{ threadId?, message }`. App-server emits this for non-fatal warnings from the core event stream, including cases where not all enabled skills are included in the model-visible skills list for a session. + ### Notification opt-out Clients can suppress specific notifications per connection by sending exact method names in `initialize.params.capabilities.optOutNotificationMethods`. diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index f41151405..6acc1e2fc 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -101,6 +101,7 @@ use codex_app_server_protocol::TurnPlanStep; use codex_app_server_protocol::TurnPlanUpdatedNotification; use codex_app_server_protocol::TurnStartedNotification; use codex_app_server_protocol::TurnStatus; +use codex_app_server_protocol::WarningNotification; use codex_app_server_protocol::build_command_execution_end_item; use codex_app_server_protocol::build_file_change_approval_request_item; use codex_app_server_protocol::build_file_change_begin_item; @@ -268,7 +269,21 @@ pub(crate) async fn apply_bespoke_event_handling( .await; } } - EventMsg::Warning(_warning_event) => {} + EventMsg::Warning(warning_event) => { + if let ApiVersion::V2 = api_version { + let notification = WarningNotification { + thread_id: Some(conversation_id.to_string()), + message: warning_event.message, + }; + if let Some(analytics_events_client) = analytics_events_client.as_ref() { + analytics_events_client + .track_notification(ServerNotification::Warning(notification.clone())); + } + outgoing + .send_server_notification(ServerNotification::Warning(notification)) + .await; + } + } EventMsg::GuardianAssessment(assessment) => { if let ApiVersion::V2 = api_version { let pending_command_execution = match build_item_from_guardian_event( diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index aa058c583..d2ac241ed 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -4314,6 +4314,7 @@ impl CodexMessageProcessor { thread_id, thread: codex_thread, session_configured, + .. }) => { let SessionConfiguredEvent { rollout_path, .. } = session_configured; let Some(rollout_path) = rollout_path else { diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index e8682d732..9f491b336 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -11,6 +11,7 @@ use app_test_support::create_shell_command_sse_response; use app_test_support::format_with_current_shell_display; use app_test_support::to_response; use app_test_support::write_mock_responses_config_toml_with_chatgpt_base_url; +use app_test_support::write_models_cache; use codex_app_server::INPUT_TOO_LARGE_ERROR_CODE; use codex_app_server::INVALID_PARAMS_ERROR_CODE; use codex_app_server_protocol::ByteRange; @@ -45,6 +46,7 @@ use codex_app_server_protocol::TurnStartResponse; use codex_app_server_protocol::TurnStartedNotification; use codex_app_server_protocol::TurnStatus; use codex_app_server_protocol::UserInput as V2UserInput; +use codex_app_server_protocol::WarningNotification; use codex_config::config_toml::ConfigToml; use codex_core::personality_migration::PERSONALITY_MIGRATION_FILENAME; use codex_features::FEATURES; @@ -244,6 +246,111 @@ async fn turn_start_emits_user_message_item_with_text_elements() -> Result<()> { Ok(()) } +#[tokio::test] +async fn turn_start_emits_thread_scoped_warning_notification_for_trimmed_skills() -> Result<()> { + let responses = vec![create_final_assistant_message_sse_response("Done")?]; + let server = create_mock_responses_server_sequence_unchecked(responses).await; + + let codex_home = TempDir::new()?; + create_config_toml( + codex_home.path(), + &server.uri(), + "never", + &BTreeMap::from([(Feature::Personality, true)]), + )?; + write_models_cache(codex_home.path())?; + let cache_path = codex_home.path().join("models_cache.json"); + let mut cache: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&cache_path)?)?; + let models = cache["models"] + .as_array_mut() + .expect("models_cache.json models should be an array"); + let entry = models + .first_mut() + .expect("models cache should not be empty"); + let model = entry["slug"] + .as_str() + .expect("model slug should be present") + .to_string(); + entry["context_window"] = serde_json::Value::from(100); + std::fs::write(&cache_path, serde_json::to_string_pretty(&cache)?)?; + let config_path = codex_home.path().join("config.toml"); + let config = std::fs::read_to_string(&config_path)?; + std::fs::write( + &config_path, + config.replace("model = \"mock-model\"", &format!("model = \"{model}\"")), + )?; + write_test_skill(codex_home.path(), "alpha-skill")?; + write_test_skill(codex_home.path(), "beta-skill")?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let thread_req = mcp + .send_thread_start_request(ThreadStartParams::default()) + .await?; + let thread_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(thread_req)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response::(thread_resp)?; + + let turn_req = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id.clone(), + input: vec![V2UserInput::Text { + text: "Hello".to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }) + .await?; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_req)), + ) + .await??; + + let notification = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("warning"), + ) + .await??; + let params = notification.params.expect("warning params"); + let warning: WarningNotification = + serde_json::from_value(params).expect("deserialize warning notification"); + assert_eq!(warning.thread_id.as_deref(), Some(thread.id.as_str())); + assert_eq!( + warning.message, + "Some enabled skills were not included in the model-visible skills list for this session. Mention a skill by name or path if you need it." + ); + + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + let requests = server + .received_requests() + .await + .expect("failed to fetch received requests"); + let request = requests + .last() + .expect("expected at least one model request"); + assert!( + body_contains(request, "## Skills"), + "expected outgoing request to include the skills section" + ); + assert!( + !body_contains(request, "- alpha-skill:") && !body_contains(request, "- beta-skill:"), + "expected trimmed skills to be omitted from the outgoing request body" + ); + + Ok(()) +} + #[tokio::test] async fn thread_start_omits_empty_instruction_overrides_from_model_request() -> Result<()> { let server = responses::start_mock_server().await; @@ -2902,3 +3009,12 @@ stream_max_retries = 0 ), ) } + +fn write_test_skill(codex_home: &Path, name: &str) -> std::io::Result<()> { + let skill_dir = codex_home.join("skills").join(name); + std::fs::create_dir_all(&skill_dir)?; + std::fs::write( + skill_dir.join("SKILL.md"), + format!("---\nname: {name}\ndescription: {name} description\n---\n\n# Body\n"), + ) +} diff --git a/codex-rs/core-skills/Cargo.toml b/codex-rs/core-skills/Cargo.toml index bc73909f9..09278c8d0 100644 --- a/codex-rs/core-skills/Cargo.toml +++ b/codex-rs/core-skills/Cargo.toml @@ -24,6 +24,7 @@ codex-otel = { workspace = true } codex-protocol = { workspace = true } codex-skills = { workspace = true } codex-utils-absolute-path = { workspace = true } +codex-utils-output-truncation = { workspace = true } codex-utils-plugins = { workspace = true } dirs = { workspace = true } dunce = { workspace = true } diff --git a/codex-rs/core-skills/src/lib.rs b/codex-rs/core-skills/src/lib.rs index b967e4ddd..6d3ac7a4f 100644 --- a/codex-rs/core-skills/src/lib.rs +++ b/codex-rs/core-skills/src/lib.rs @@ -22,4 +22,8 @@ pub use model::SkillLoadOutcome; pub use model::SkillMetadata; pub use model::SkillPolicy; pub use model::filter_skill_load_outcome_for_product; +pub use render::RenderedSkillsSection; +pub use render::SkillMetadataBudget; +pub use render::SkillRenderReport; +pub use render::default_skill_metadata_budget; pub use render::render_skills_section; diff --git a/codex-rs/core-skills/src/render.rs b/codex-rs/core-skills/src/render.rs index 9089a0543..ad674fbff 100644 --- a/codex-rs/core-skills/src/render.rs +++ b/codex-rs/core-skills/src/render.rs @@ -1,22 +1,104 @@ use crate::model::SkillMetadata; +use codex_otel::SessionTelemetry; +use codex_otel::THREAD_SKILLS_ENABLED_TOTAL_METRIC; +use codex_otel::THREAD_SKILLS_KEPT_TOTAL_METRIC; +use codex_otel::THREAD_SKILLS_TRUNCATED_METRIC; use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG; use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG; +use codex_protocol::protocol::SkillScope; +use codex_utils_output_truncation::approx_token_count; -pub fn render_skills_section(skills: &[SkillMetadata]) -> Option { +const DEFAULT_SKILL_METADATA_CHAR_BUDGET: usize = 8_000; +const SKILL_METADATA_CONTEXT_WINDOW_PERCENT: usize = 2; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SkillMetadataBudget { + Tokens(usize), + Characters(usize), +} + +impl SkillMetadataBudget { + fn limit(self) -> usize { + match self { + Self::Tokens(limit) | Self::Characters(limit) => limit, + } + } + + fn cost(self, text: &str) -> usize { + match self { + Self::Tokens(_) => approx_token_count(text), + Self::Characters(_) => text.chars().count(), + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SkillRenderReport { + pub total_count: usize, + pub included_count: usize, + pub omitted_count: usize, +} + +#[derive(Clone, Copy)] +pub enum SkillRenderSideEffects<'a> { + None, + ThreadStart { + session_telemetry: &'a SessionTelemetry, + }, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct RenderedSkillsSection { + pub text: String, + pub report: SkillRenderReport, + pub emit_warning: bool, +} + +pub fn default_skill_metadata_budget(context_window: Option) -> SkillMetadataBudget { + context_window + .and_then(|window| usize::try_from(window).ok()) + .filter(|window| *window > 0) + .map(|window| { + SkillMetadataBudget::Tokens( + window + .saturating_mul(SKILL_METADATA_CONTEXT_WINDOW_PERCENT) + .saturating_div(100) + .max(1), + ) + }) + .unwrap_or(SkillMetadataBudget::Characters( + DEFAULT_SKILL_METADATA_CHAR_BUDGET, + )) +} + +pub fn render_skills_section( + skills: &[SkillMetadata], + budget: SkillMetadataBudget, + side_effects: SkillRenderSideEffects<'_>, +) -> Option { if skills.is_empty() { + let _ = record_skill_render_side_effects( + side_effects, + /*total_count*/ 0, + /*included_count*/ 0, + /*truncated*/ false, + ); return None; } + let (skill_lines, report) = render_skill_lines(skills, budget); + let emit_warning = record_skill_render_side_effects( + side_effects, + report.total_count, + report.included_count, + report.omitted_count > 0, + ); let mut lines: Vec = Vec::new(); lines.push("## Skills".to_string()); lines.push("A skill is a set of local instructions to follow that is stored in a `SKILL.md` file. Below is the list of skills that can be used. Each entry includes a name, description, and file path so you can open the source for full instructions when using a specific skill.".to_string()); lines.push("### Available skills".to_string()); - - for skill in skills { - let path_str = skill.path_to_skills_md.to_string_lossy().replace('\\', "/"); - let name = skill.name.as_str(); - let description = skill.description.as_str(); - lines.push(format!("- {name}: {description} (file: {path_str})")); + if !skill_lines.is_empty() { + lines.extend(skill_lines); } lines.push("### How to use skills".to_string()); @@ -42,7 +124,189 @@ pub fn render_skills_section(skills: &[SkillMetadata]) -> Option { ); let body = lines.join("\n"); - Some(format!( - "{SKILLS_INSTRUCTIONS_OPEN_TAG}\n{body}\n{SKILLS_INSTRUCTIONS_CLOSE_TAG}" - )) + Some(RenderedSkillsSection { + text: format!("{SKILLS_INSTRUCTIONS_OPEN_TAG}\n{body}\n{SKILLS_INSTRUCTIONS_CLOSE_TAG}"), + report, + emit_warning, + }) +} + +fn record_skill_render_side_effects( + side_effects: SkillRenderSideEffects<'_>, + total_count: usize, + included_count: usize, + truncated: bool, +) -> bool { + match side_effects { + SkillRenderSideEffects::None => false, + SkillRenderSideEffects::ThreadStart { session_telemetry } => { + session_telemetry.histogram( + THREAD_SKILLS_ENABLED_TOTAL_METRIC, + i64::try_from(total_count).unwrap_or(i64::MAX), + &[], + ); + session_telemetry.histogram( + THREAD_SKILLS_KEPT_TOTAL_METRIC, + i64::try_from(included_count).unwrap_or(i64::MAX), + &[], + ); + session_telemetry.histogram( + THREAD_SKILLS_TRUNCATED_METRIC, + if truncated { 1 } else { 0 }, + &[], + ); + truncated + } + } +} + +fn render_skill_lines( + skills: &[SkillMetadata], + budget: SkillMetadataBudget, +) -> (Vec, SkillRenderReport) { + let ordered_skills = ordered_skills_for_budget(skills); + + let mut included = Vec::new(); + let mut used = 0usize; + let mut omitted_count = 0usize; + + for skill in ordered_skills { + let line = render_skill_line(skill); + let line_cost = budget.cost(&format!("{line}\n")); + if used.saturating_add(line_cost) <= budget.limit() { + used = used.saturating_add(line_cost); + included.push(line); + continue; + } + + omitted_count = omitted_count.saturating_add(1); + } + + let report = SkillRenderReport { + total_count: skills.len(), + included_count: included.len(), + omitted_count, + }; + + (included, report) +} + +fn ordered_skills_for_budget(skills: &[SkillMetadata]) -> Vec<&SkillMetadata> { + let mut ordered = skills.iter().collect::>(); + ordered.sort_by(|a, b| { + prompt_scope_rank(a.scope) + .cmp(&prompt_scope_rank(b.scope)) + .then_with(|| a.name.cmp(&b.name)) + .then_with(|| a.path_to_skills_md.cmp(&b.path_to_skills_md)) + }); + ordered +} + +fn prompt_scope_rank(scope: SkillScope) -> u8 { + match scope { + SkillScope::System => 0, + SkillScope::Admin => 1, + SkillScope::Repo => 2, + SkillScope::User => 3, + } +} + +fn render_skill_line(skill: &SkillMetadata) -> String { + let path_str = skill.path_to_skills_md.to_string_lossy().replace('\\', "/"); + let name = skill.name.as_str(); + let description = skill.description.as_str(); + format!("- {name}: {description} (file: {path_str})") +} + +#[cfg(test)] +mod tests { + use super::*; + use codex_utils_absolute_path::test_support::PathBufExt; + use codex_utils_absolute_path::test_support::test_path_buf; + use pretty_assertions::assert_eq; + + fn make_skill(name: &str, scope: SkillScope) -> SkillMetadata { + SkillMetadata { + name: name.to_string(), + description: "desc".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: test_path_buf(&format!("/tmp/{name}/SKILL.md")).abs(), + scope, + } + } + + #[test] + fn default_budget_uses_two_percent_of_full_context_window() { + assert_eq!( + default_skill_metadata_budget(Some(200_000)), + SkillMetadataBudget::Tokens(4_000) + ); + assert_eq!( + default_skill_metadata_budget(Some(99)), + SkillMetadataBudget::Tokens(1) + ); + } + + #[test] + fn default_budget_falls_back_to_characters_without_context_window() { + assert_eq!( + default_skill_metadata_budget(/*context_window*/ None), + SkillMetadataBudget::Characters(DEFAULT_SKILL_METADATA_CHAR_BUDGET) + ); + assert_eq!( + default_skill_metadata_budget(Some(-1)), + SkillMetadataBudget::Characters(DEFAULT_SKILL_METADATA_CHAR_BUDGET) + ); + } + + #[test] + fn budgeted_rendering_preserves_prompt_priority() { + let system = make_skill("system-skill", SkillScope::System); + let user = make_skill("user-skill", SkillScope::User); + let repo = make_skill("repo-skill", SkillScope::Repo); + let admin = make_skill("admin-skill", SkillScope::Admin); + let system_cost = SkillMetadataBudget::Characters(usize::MAX) + .cost(&format!("{}\n", render_skill_line(&system))); + let admin_cost = SkillMetadataBudget::Characters(usize::MAX) + .cost(&format!("{}\n", render_skill_line(&admin))); + let budget = SkillMetadataBudget::Characters(system_cost + admin_cost); + + let rendered = render_skills_section( + &[system, user, repo, admin], + budget, + SkillRenderSideEffects::None, + ) + .expect("skills should render"); + + assert_eq!(rendered.report.included_count, 2); + assert_eq!(rendered.report.omitted_count, 2); + assert!(!rendered.emit_warning); + assert!(rendered.text.contains("- system-skill:")); + assert!(rendered.text.contains("- admin-skill:")); + assert!(!rendered.text.contains("- repo-skill:")); + assert!(!rendered.text.contains("- user-skill:")); + } + + #[test] + fn budgeted_rendering_keeps_scanning_after_oversized_entry() { + let mut oversized = make_skill("oversized-system-skill", SkillScope::System); + oversized.description = "desc ".repeat(100); + let repo = make_skill("repo-skill", SkillScope::Repo); + let repo_cost = SkillMetadataBudget::Characters(usize::MAX) + .cost(&format!("{}\n", render_skill_line(&repo))); + let budget = SkillMetadataBudget::Characters(repo_cost); + + let rendered = + render_skills_section(&[oversized, repo], budget, SkillRenderSideEffects::None) + .expect("skills render"); + + assert_eq!(rendered.report.included_count, 1); + assert_eq!(rendered.report.omitted_count, 1); + assert!(!rendered.emit_warning); + assert!(!rendered.text.contains("- oversized-system-skill:")); + assert!(rendered.text.contains("- repo-skill:")); + } } diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 18d1a3029..1fef33c3f 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -99,6 +99,7 @@ pub(crate) use skills::build_skill_injections; pub(crate) use skills::build_skill_name_counts; pub(crate) use skills::collect_env_var_dependencies; pub(crate) use skills::collect_explicit_skill_mentions; +pub(crate) use skills::default_skill_metadata_budget; pub(crate) use skills::injection; pub(crate) use skills::manager; pub(crate) use skills::maybe_emit_implicit_skill_invocation; diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 766bd04e6..594a2d103 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -20,6 +20,7 @@ use crate::commit_attribution::commit_message_trailer_instruction; use crate::compact; use crate::config::ManagedFeatures; use crate::connectors; +use crate::default_skill_metadata_budget; use crate::exec_policy::ExecPolicyManager; use crate::installation_id::resolve_installation_id; use crate::parse_turn_item; @@ -28,6 +29,7 @@ use crate::realtime_conversation::RealtimeConversationManager; use crate::render_skills_section; use crate::rollout::find_thread_name_by_id; use crate::session_prefix::format_subagent_notification_message; +use crate::skills::SkillRenderSideEffects; use crate::skills_load_input_from_config; use crate::turn_metadata::TurnMetadataState; use async_channel::Receiver; @@ -362,9 +364,10 @@ pub struct Codex { pub(crate) type SessionLoopTermination = Shared>; -/// Wrapper returned by [`Codex::spawn`] containing the spawned [`Codex`], -/// the submission id for the initial `ConfigureSession` request and the -/// unique session id. +pub(crate) const THREAD_START_SKILLS_TRIMMED_WARNING_MESSAGE: &str = "Some enabled skills were not included in the model-visible skills list for this session. Mention a skill by name or path if you need it."; + +/// Wrapper returned by [`Codex::spawn`] containing the spawned [`Codex`] and +/// the unique session id. pub struct CodexSpawnOk { pub codex: Codex, pub thread_id: ThreadId, @@ -396,6 +399,7 @@ pub(crate) const INITIAL_SUBMIT_ID: &str = ""; pub(crate) const SUBMISSION_CHANNEL_CAPACITY: usize = 512; const CYBER_VERIFY_URL: &str = "https://chatgpt.com/cyber"; const CYBER_SAFETY_URL: &str = "https://developers.openai.com/codex/concepts/cyber-safety"; + impl Codex { /// Spawn a new [`Codex`] and initialize the session. pub(crate) async fn spawn(args: CodexSpawnArgs) -> CodexResult { @@ -2422,8 +2426,24 @@ impl Session { .turn_skills .outcome .allowed_skills_for_implicit_invocation(); - if let Some(skills_section) = render_skills_section(&implicit_skills) { - developer_sections.push(skills_section); + let rendered_skills = render_skills_section( + &implicit_skills, + default_skill_metadata_budget(turn_context.model_info.context_window), + SkillRenderSideEffects::ThreadStart { + session_telemetry: &self.services.session_telemetry, + }, + ); + if let Some(rendered_skills) = rendered_skills { + if rendered_skills.emit_warning { + self.send_event_raw(Event { + id: String::new(), + msg: EventMsg::Warning(WarningEvent { + message: THREAD_START_SKILLS_TRIMMED_WARNING_MESSAGE.to_string(), + }), + }) + .await; + } + developer_sections.push(rendered_skills.text); } let loaded_plugins = self .services diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 60183e986..49cacab04 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -12,6 +12,8 @@ use crate::config_loader::project_trust_key; use crate::exec::ExecCapturePolicy; use crate::function_tool::FunctionCallError; use crate::shell::default_user_shell; +use crate::skills::SkillRenderSideEffects; +use crate::skills::render::SkillMetadataBudget; use crate::tools::format_exec_output_str; use codex_features::Feature; @@ -61,6 +63,11 @@ use codex_execpolicy::Decision; use codex_execpolicy::NetworkRuleProtocol; use codex_execpolicy::Policy; use codex_network_proxy::NetworkProxyConfig; +use codex_otel::MetricsClient; +use codex_otel::MetricsConfig; +use codex_otel::THREAD_SKILLS_ENABLED_TOTAL_METRIC; +use codex_otel::THREAD_SKILLS_KEPT_TOTAL_METRIC; +use codex_otel::THREAD_SKILLS_TRUNCATED_METRIC; use codex_otel::TelemetryAuthMode; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; @@ -86,6 +93,7 @@ use codex_protocol::protocol::RealtimeVoice; use codex_protocol::protocol::RealtimeVoicesList; use codex_protocol::protocol::ResumedHistory; use codex_protocol::protocol::RolloutItem; +use codex_protocol::protocol::SkillScope; use codex_protocol::protocol::Submission; use codex_protocol::protocol::ThreadRolledBackEvent; use codex_protocol::protocol::TokenCountEvent; @@ -106,10 +114,16 @@ use core_test_support::responses::mount_sse_once; use core_test_support::responses::sse; use core_test_support::responses::start_mock_server; use core_test_support::test_codex::test_codex; +use core_test_support::test_path_buf; use core_test_support::tracing::install_test_tracing; use core_test_support::wait_for_event; use opentelemetry::trace::TraceContextExt; use opentelemetry::trace::TraceId; +use opentelemetry_sdk::metrics::InMemoryMetricExporter; +use opentelemetry_sdk::metrics::data::AggregatedMetrics; +use opentelemetry_sdk::metrics::data::Metric; +use opentelemetry_sdk::metrics::data::MetricData; +use opentelemetry_sdk::metrics::data::ResourceMetrics; use std::path::Path; use std::time::Duration; use tokio::time::sleep; @@ -155,6 +169,54 @@ fn assistant_message(text: &str) -> ResponseItem { } } +fn test_session_telemetry_without_metadata() -> SessionTelemetry { + let exporter = InMemoryMetricExporter::default(); + let metrics = MetricsClient::new( + MetricsConfig::in_memory("test", "codex-core", env!("CARGO_PKG_VERSION"), exporter) + .with_runtime_reader(), + ) + .expect("in-memory metrics client"); + SessionTelemetry::new( + ThreadId::new(), + "gpt-5.1", + "gpt-5.1", + /*account_id*/ None, + /*account_email*/ None, + /*auth_mode*/ None, + "test_originator".to_string(), + /*log_user_prompts*/ false, + "tty".to_string(), + SessionSource::Cli, + ) + .with_metrics_without_metadata_tags(metrics) +} + +fn find_metric<'a>(resource_metrics: &'a ResourceMetrics, name: &str) -> &'a Metric { + for scope_metrics in resource_metrics.scope_metrics() { + for metric in scope_metrics.metrics() { + if metric.name() == name { + return metric; + } + } + } + panic!("metric {name} missing"); +} + +fn histogram_sum(resource_metrics: &ResourceMetrics, name: &str) -> u64 { + let metric = find_metric(resource_metrics, name); + match metric.data() { + AggregatedMetrics::F64(data) => match data { + MetricData::Histogram(histogram) => { + let points: Vec<_> = histogram.data_points().collect(); + assert_eq!(points.len(), 1); + points[0].sum().round() as u64 + } + _ => panic!("unexpected histogram aggregation"), + }, + _ => panic!("unexpected metric data type"), + } +} + fn skill_message(text: &str) -> ResponseItem { ResponseItem::Message { id: None, @@ -4482,6 +4544,138 @@ async fn build_initial_context_omits_default_image_save_location_without_image_h ); } +#[tokio::test] +async fn build_initial_context_trims_skill_metadata_from_context_window_budget() { + let (session, mut turn_context) = make_session_and_context().await; + let mut outcome = SkillLoadOutcome::default(); + outcome.skills = vec![ + SkillMetadata { + name: "admin-skill".to_string(), + description: "desc".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: test_path_buf("/tmp/admin-skill/SKILL.md").abs(), + scope: SkillScope::Admin, + }, + SkillMetadata { + name: "repo-skill".to_string(), + description: "desc".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: test_path_buf("/tmp/repo-skill/SKILL.md").abs(), + scope: SkillScope::Repo, + }, + ]; + turn_context.model_info.context_window = Some(100); + turn_context.turn_skills = TurnSkillsContext::new(Arc::new(outcome)); + + let initial_context = session.build_initial_context(&turn_context).await; + let developer_texts = developer_input_texts(&initial_context); + + assert!( + developer_texts + .iter() + .all(|text| !text.contains(THREAD_START_SKILLS_TRIMMED_WARNING_MESSAGE)), + "expected skill budget warning to stay out of the initial context, got {developer_texts:?}" + ); + assert!( + developer_texts + .iter() + .all(|text| !text.contains("- admin-skill:") && !text.contains("- repo-skill:")), + "expected no skill metadata entries to fit the tiny budget, got {developer_texts:?}" + ); +} + +#[test] +fn emit_thread_start_skill_metrics_records_enabled_kept_and_truncated_values() { + let session_telemetry = test_session_telemetry_without_metadata(); + let rendered = render_skills_section( + &[SkillMetadata { + name: "repo-skill".to_string(), + description: "desc".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: test_path_buf("/tmp/repo-skill/SKILL.md").abs(), + scope: SkillScope::Repo, + }], + SkillMetadataBudget::Characters(1), + SkillRenderSideEffects::ThreadStart { + session_telemetry: &session_telemetry, + }, + ) + .expect("skills should render"); + + assert!(rendered.emit_warning); + let snapshot = session_telemetry + .snapshot_metrics() + .expect("runtime metrics snapshot"); + assert_eq!( + histogram_sum(&snapshot, THREAD_SKILLS_ENABLED_TOTAL_METRIC), + 1 + ); + assert_eq!(histogram_sum(&snapshot, THREAD_SKILLS_KEPT_TOTAL_METRIC), 0); + assert_eq!(histogram_sum(&snapshot, THREAD_SKILLS_TRUNCATED_METRIC), 1); +} + +#[tokio::test] +async fn build_initial_context_emits_thread_start_skill_warning_on_repeated_builds() { + let (session, turn_context, rx) = make_session_and_context_with_rx().await; + let mut turn_context = Arc::into_inner(turn_context).expect("sole turn context owner"); + let mut outcome = SkillLoadOutcome::default(); + outcome.skills = vec![ + SkillMetadata { + name: "admin-skill".to_string(), + description: "desc".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: test_path_buf("/tmp/admin-skill/SKILL.md").abs(), + scope: SkillScope::Admin, + }, + SkillMetadata { + name: "repo-skill".to_string(), + description: "desc".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: test_path_buf("/tmp/repo-skill/SKILL.md").abs(), + scope: SkillScope::Repo, + }, + ]; + turn_context.model_info.context_window = Some(100); + turn_context.turn_skills = TurnSkillsContext::new(Arc::new(outcome)); + + let _ = session.build_initial_context(&turn_context).await; + let warning_event = timeout(Duration::from_secs(1), rx.recv()) + .await + .expect("warning event should arrive") + .expect("warning event should be readable"); + assert!(matches!( + warning_event.msg, + EventMsg::Warning(WarningEvent { message }) + if message == THREAD_START_SKILLS_TRIMMED_WARNING_MESSAGE + )); + + let _ = session.build_initial_context(&turn_context).await; + let warning_event = timeout(Duration::from_secs(1), rx.recv()) + .await + .expect("warning event should arrive on repeated build") + .expect("warning event should be readable"); + assert!(matches!( + warning_event.msg, + EventMsg::Warning(WarningEvent { message }) + if message == THREAD_START_SKILLS_TRIMMED_WARNING_MESSAGE + )); +} + #[tokio::test] async fn handle_output_item_done_records_image_save_history_message() { let (session, turn_context) = make_session_and_context().await; diff --git a/codex-rs/core/src/skills.rs b/codex-rs/core/src/skills.rs index 84c0af5f3..552587318 100644 --- a/codex-rs/core/src/skills.rs +++ b/codex-rs/core/src/skills.rs @@ -21,11 +21,13 @@ pub use codex_core_skills::SkillError; pub use codex_core_skills::SkillLoadOutcome; pub use codex_core_skills::SkillMetadata; pub use codex_core_skills::SkillPolicy; +pub use codex_core_skills::SkillRenderReport; pub use codex_core_skills::SkillsLoadInput; pub use codex_core_skills::SkillsManager; pub use codex_core_skills::build_skill_name_counts; pub use codex_core_skills::collect_env_var_dependencies; pub use codex_core_skills::config_rules; +pub use codex_core_skills::default_skill_metadata_budget; pub use codex_core_skills::detect_implicit_skill_invocation_for_command; pub use codex_core_skills::filter_skill_load_outcome_for_product; pub use codex_core_skills::injection; @@ -37,6 +39,7 @@ pub use codex_core_skills::manager; pub use codex_core_skills::model; pub use codex_core_skills::remote; pub use codex_core_skills::render; +pub use codex_core_skills::render::SkillRenderSideEffects; pub use codex_core_skills::render_skills_section; pub use codex_core_skills::system; diff --git a/codex-rs/otel/src/metrics/names.rs b/codex-rs/otel/src/metrics/names.rs index fdc03fbaa..9dc718d2d 100644 --- a/codex-rs/otel/src/metrics/names.rs +++ b/codex-rs/otel/src/metrics/names.rs @@ -37,3 +37,6 @@ pub const STARTUP_PREWARM_DURATION_METRIC: &str = "codex.startup_prewarm.duratio pub const STARTUP_PREWARM_AGE_AT_FIRST_TURN_METRIC: &str = "codex.startup_prewarm.age_at_first_turn_ms"; pub const THREAD_STARTED_METRIC: &str = "codex.thread.started"; +pub const THREAD_SKILLS_ENABLED_TOTAL_METRIC: &str = "codex.thread.skills.enabled_total"; +pub const THREAD_SKILLS_KEPT_TOTAL_METRIC: &str = "codex.thread.skills.kept_total"; +pub const THREAD_SKILLS_TRUNCATED_METRIC: &str = "codex.thread.skills.truncated"; diff --git a/codex-rs/tui/src/app/app_server_adapter.rs b/codex-rs/tui/src/app/app_server_adapter.rs index 3dfc9fd23..f0abb5326 100644 --- a/codex-rs/tui/src/app/app_server_adapter.rs +++ b/codex-rs/tui/src/app/app_server_adapter.rs @@ -421,6 +421,7 @@ fn server_notification_thread_target( ServerNotification::ThreadRealtimeClosed(notification) => { Some(notification.thread_id.as_str()) } + ServerNotification::Warning(notification) => notification.thread_id.as_deref(), ServerNotification::SkillsChanged(_) | ServerNotification::McpServerStatusUpdated(_) | ServerNotification::McpServerOauthLoginCompleted(_) @@ -1049,8 +1050,10 @@ fn app_server_codex_error_info_to_core( #[cfg(test)] mod tests { + use super::ServerNotificationThreadTarget; use super::command_execution_started_event; use super::server_notification_thread_events; + use super::server_notification_thread_target; use super::thread_snapshot_events; use super::turn_snapshot_events; use codex_app_server_protocol::AgentMessageDeltaNotification; @@ -1070,6 +1073,7 @@ mod tests { use codex_app_server_protocol::TurnCompletedNotification; use codex_app_server_protocol::TurnError; use codex_app_server_protocol::TurnStatus; + use codex_app_server_protocol::WarningNotification; use codex_protocol::ThreadId; use codex_protocol::items::AgentMessageContent; use codex_protocol::items::AgentMessageItem; @@ -1664,4 +1668,17 @@ mod tests { assert_eq!(raw_reasoning.text, "hidden chain"); assert!(matches!(events[3].msg, EventMsg::TurnComplete(_))); } + + #[test] + fn warning_notifications_route_to_threads_when_thread_id_is_present() { + let thread_id = ThreadId::new(); + let notification = ServerNotification::Warning(WarningNotification { + thread_id: Some(thread_id.to_string()), + message: "warning".to_string(), + }); + + let target = server_notification_thread_target(¬ification); + + assert_eq!(target, ServerNotificationThreadTarget::Thread(thread_id)); + } } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 3fc8df0b5..2c88baf85 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -6196,6 +6196,7 @@ impl ChatWidget { self.refresh_skills_for_current_cwd(/*force_reload*/ true); } ServerNotification::ModelRerouted(_) => {} + ServerNotification::Warning(notification) => self.on_warning(notification.message), ServerNotification::DeprecationNotice(notification) => { self.on_deprecation_notice(DeprecationNoticeEvent { summary: notification.summary, diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index e92a809e8..6067f3a76 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -46,6 +46,7 @@ pub(super) use codex_app_server_protocol::CommandAction as AppServerCommandActio pub(super) use codex_app_server_protocol::CommandExecutionRequestApprovalParams as AppServerCommandExecutionRequestApprovalParams; pub(super) use codex_app_server_protocol::CommandExecutionSource as AppServerCommandExecutionSource; pub(super) use codex_app_server_protocol::CommandExecutionStatus as AppServerCommandExecutionStatus; +pub(super) use codex_app_server_protocol::ConfigWarningNotification; pub(super) use codex_app_server_protocol::ErrorNotification; pub(super) use codex_app_server_protocol::FileUpdateChange; pub(super) use codex_app_server_protocol::GuardianApprovalReview; @@ -94,6 +95,7 @@ pub(super) use codex_app_server_protocol::TurnError as AppServerTurnError; pub(super) use codex_app_server_protocol::TurnStartedNotification; pub(super) use codex_app_server_protocol::TurnStatus as AppServerTurnStatus; pub(super) use codex_app_server_protocol::UserInput as AppServerUserInput; +pub(super) use codex_app_server_protocol::WarningNotification; pub(super) use codex_config::types::ApprovalsReviewer; pub(super) use codex_config::types::Notifications; #[cfg(target_os = "windows")] diff --git a/codex-rs/tui/src/chatwidget/tests/app_server.rs b/codex-rs/tui/src/chatwidget/tests/app_server.rs index acfd358a2..702bd08ef 100644 --- a/codex-rs/tui/src/chatwidget/tests/app_server.rs +++ b/codex-rs/tui/src/chatwidget/tests/app_server.rs @@ -184,6 +184,57 @@ async fn live_app_server_turn_started_sets_feedback_turn_id() { ); } +#[tokio::test] +async fn live_app_server_warning_notification_renders_message() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + + chat.handle_server_notification( + ServerNotification::Warning(WarningNotification { + thread_id: None, + message: "Some enabled skills were not included in the model-visible skills list for this session. Mention a skill by name or path if you need it.".to_string(), + }), + /*replay_kind*/ None, + ); + + let cells = drain_insert_history(&mut rx); + assert_eq!(cells.len(), 1, "expected one warning history cell"); + let rendered = lines_to_single_string(&cells[0]); + let normalized = rendered.split_whitespace().collect::>().join(" "); + assert!( + normalized.contains( + "Some enabled skills were not included in the model-visible skills list for this session." + ), + "expected warning notification message, got {rendered}" + ); + assert!( + normalized.contains("Mention a skill by name or path if you need it."), + "expected warning guidance, got {rendered}" + ); +} + +#[tokio::test] +async fn live_app_server_config_warning_prefixes_summary() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + + chat.handle_server_notification( + ServerNotification::ConfigWarning(ConfigWarningNotification { + summary: "Invalid configuration; using defaults.".to_string(), + details: None, + path: None, + range: None, + }), + /*replay_kind*/ None, + ); + + let cells = drain_insert_history(&mut rx); + assert_eq!(cells.len(), 1, "expected one warning history cell"); + let rendered = lines_to_single_string(&cells[0]); + assert!( + rendered.contains("Invalid configuration; using defaults."), + "expected config warning summary, got {rendered}" + ); +} + #[tokio::test] async fn live_app_server_file_change_item_started_preserves_changes() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;