Merge branch 'main' into raymond/fix-empty-mcp-notification

This commit is contained in:
raymond-openai
2026-07-07 10:26:08 -07:00
committed by GitHub
31 changed files with 803 additions and 243 deletions
+1 -1
View File
@@ -190,7 +190,7 @@ common:ci-windows-cross --strategy=TestRunner=local
# V8 embeds IsolateData offsets in snapshot builtins; Windows snapshots must be
# generated by a Windows mksnapshot binary rather than the Linux RBE host tool.
common:ci-windows-cross --strategy=V8Mksnapshot=local
common:ci-windows-cross --local_test_jobs=4
common:ci-windows-cross --local_test_jobs=8
common:ci-windows-cross --test_env=RUST_TEST_THREADS=1
# Native Windows CI still covers the PowerShell parser-process tests. The
# cross-built gnullvm binaries currently hang in those tests when run on the
@@ -0,0 +1,10 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>com.apple.security.cs.allow-jit</key>
<true/>
<key>com.apple.security.cs.allow-unsigned-executable-memory</key>
<true/>
</dict>
</plist>
@@ -0,0 +1,10 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>com.apple.security.cs.allow-jit</key>
<true/>
<key>com.apple.security.cs.allow-unsigned-executable-memory</key>
<true/>
</dict>
</plist>
@@ -0,0 +1,8 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>com.apple.security.cs.allow-jit</key>
<true/>
</dict>
</plist>
@@ -4,5 +4,7 @@
<dict>
<key>com.apple.security.cs.allow-jit</key>
<true/>
<key>com.apple.security.cs.allow-unsigned-executable-memory</key>
<true/>
</dict>
</plist>
@@ -0,0 +1,37 @@
import plistlib
import unittest
from pathlib import Path
ROOT = Path(__file__).resolve().parents[2]
SIGNING_DIR = ROOT / ".github" / "scripts" / "macos-signing"
ALLOW_JIT = "com.apple.security.cs.allow-jit"
ALLOW_UNSIGNED_EXECUTABLE_MEMORY = (
"com.apple.security.cs.allow-unsigned-executable-memory"
)
class MacosSigningEntitlementsTest(unittest.TestCase):
def load(self, binary: str) -> dict[str, bool]:
path = SIGNING_DIR / f"{binary}.entitlements.plist"
with path.open("rb") as file:
return plistlib.load(file)
def test_v8_binaries_allow_unsigned_executable_memory(self) -> None:
expected = {
ALLOW_JIT: True,
ALLOW_UNSIGNED_EXECUTABLE_MEMORY: True,
}
for binary in ["codex", "codex-app-server", "codex-code-mode-host"]:
with self.subTest(binary=binary):
self.assertEqual(self.load(binary), expected)
def test_responses_proxy_keeps_existing_entitlements(self) -> None:
self.assertEqual(
self.load("codex-responses-api-proxy"),
{ALLOW_JIT: True},
)
if __name__ == "__main__":
unittest.main()
+29 -9
View File
@@ -573,6 +573,12 @@ jobs:
zstd -d --stdout "$unsigned_path" >"$signed_path"
chmod 0755 "$signed_path"
entitlements="${GITHUB_WORKSPACE}/.github/scripts/macos-signing/${binary}.entitlements.plist"
if [[ ! -f "$entitlements" ]]; then
echo "Entitlements file $entitlements not found"
exit 1
fi
.github/scripts/macos-signing/sign_macos_code.sh \
--target "$signed_path" \
--identity unused \
@@ -580,7 +586,7 @@ jobs:
--identifier "$binary" \
--options runtime \
--timestamp true \
--entitlements .github/scripts/macos-signing/codex.entitlements.plist
--entitlements "$entitlements"
mkdir -p "${report_dir}/${binary}"
rcodesign print-signature-info "$signed_path" \
@@ -974,15 +980,29 @@ jobs:
target="${{ matrix.target }}"
packaged_dir="dist/${target}"
expected_entitlements="${GITHUB_WORKSPACE}/.github/scripts/macos-signing/codex.entitlements.plist"
case "$target" in
aarch64-apple-darwin) expected_arch="arm64" ;;
x86_64-apple-darwin) expected_arch="x86_64" ;;
*)
echo "Unexpected macOS target: $target"
exit 1
;;
esac
verify_signed_binary() {
local path="$1"
local actual_entitlements normalized_actual normalized_expected
local binary="$2"
local actual_entitlements expected_entitlements normalized_actual normalized_expected
chmod 0755 "$path"
lipo "$path" -verify_arch "$expected_arch"
codesign --verify --strict --verbose=2 "$path"
expected_entitlements="${GITHUB_WORKSPACE}/.github/scripts/macos-signing/${binary}.entitlements.plist"
if [[ ! -f "$expected_entitlements" ]]; then
echo "Expected entitlements file $expected_entitlements not found"
exit 1
fi
actual_entitlements="$(mktemp)"
normalized_actual="$(mktemp)"
normalized_expected="$(mktemp)"
@@ -995,7 +1015,7 @@ jobs:
for binary in ${{ matrix.binaries }}; do
binary_path="${RUNNER_TEMP}/signed-binaries/${binary}"
verify_signed_binary "$binary_path"
verify_signed_binary "$binary_path" "$binary"
# The app-server package contains the host, but its standalone archives
# are omitted above to avoid duplicate release asset names.
@@ -1007,11 +1027,11 @@ jobs:
rm -rf "$direct_archive_dir"
mkdir -p "$direct_archive_dir"
tar -xzf "${packaged_dir}/${binary}-${target}.tar.gz" -C "$direct_archive_dir"
verify_signed_binary "${direct_archive_dir}/${binary}-${target}"
verify_signed_binary "${direct_archive_dir}/${binary}-${target}" "$binary"
direct_zstd_path="${RUNNER_TEMP}/${binary}-${target}-from-zstd"
zstd -d --stdout "${packaged_dir}/${binary}-${target}.zst" >"$direct_zstd_path"
verify_signed_binary "$direct_zstd_path"
verify_signed_binary "$direct_zstd_path" "$binary"
done
case "${{ matrix.bundle }}" in
@@ -1033,8 +1053,8 @@ jobs:
rm -rf "$package_dir"
mkdir -p "$package_dir"
tar -xzf "${packaged_dir}/${package_stem}-${target}.tar.gz" -C "$package_dir"
verify_signed_binary "${package_dir}/bin/${package_entrypoint}"
verify_signed_binary "${package_dir}/bin/codex-code-mode-host"
verify_signed_binary "${package_dir}/bin/${package_entrypoint}" "$package_entrypoint"
verify_signed_binary "${package_dir}/bin/codex-code-mode-host" "codex-code-mode-host"
if [[ "${{ matrix.verify_dmg }}" != "true" ]]; then
exit 0
@@ -1060,7 +1080,7 @@ jobs:
trap cleanup_mount EXIT
for binary in ${{ matrix.binaries }}; do
verify_signed_binary "${mount_dir}/${binary}"
verify_signed_binary "${mount_dir}/${binary}" "$binary"
done
cleanup_mount
@@ -6728,6 +6728,7 @@
"enum": [
"auto",
"prompt",
"writes",
"approve"
],
"type": "string"
@@ -854,6 +854,7 @@
"enum": [
"auto",
"prompt",
"writes",
"approve"
],
"type": "string"
@@ -78,6 +78,7 @@
"enum": [
"auto",
"prompt",
"writes",
"approve"
],
"type": "string"
@@ -2,4 +2,4 @@
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
export type AppToolApproval = "auto" | "prompt" | "approve";
export type AppToolApproval = "auto" | "prompt" | "writes" | "approve";
@@ -163,6 +163,7 @@ pub struct AnalyticsConfig {
pub enum AppToolApproval {
Auto,
Prompt,
Writes,
Approve,
}
+3 -1
View File
@@ -1879,7 +1879,9 @@ review when allowed by configuration requirements.
Use `apps._default.default_tools_approval_mode` to set the approval mode for
tools without a per-app or per-tool override. Supported values are `"auto"`,
`"prompt"`, and `"approve"`. Tool-level `approval_mode` takes precedence over
`"prompt"`, `"writes"`, and `"approve"`. The `"writes"` mode prompts for tools
that do not advertise `readOnlyHint = true` and skips declared read-only tools.
Tool-level `approval_mode` takes precedence over
the per-app `default_tools_approval_mode`, which takes precedence over the
`apps._default` value. Managed tool requirements take precedence over all of
these settings. When none are configured, the mode defaults to `"auto"`.
+116 -42
View File
@@ -22,7 +22,6 @@ use codex_app_server_protocol::CommandExecutionSource;
use codex_app_server_protocol::CommandExecutionStatus;
use codex_app_server_protocol::DeprecationNoticeNotification;
use codex_app_server_protocol::DynamicToolCallParams;
use codex_app_server_protocol::DynamicToolCallStatus;
use codex_app_server_protocol::ErrorNotification;
use codex_app_server_protocol::ExecPolicyAmendment as V2ExecPolicyAmendment;
use codex_app_server_protocol::FileChangeApprovalDecision;
@@ -823,52 +822,16 @@ pub(crate) async fn apply_bespoke_event_handling(
on_request_permissions_response(pending_response, conversation, thread_state).await;
});
}
EventMsg::DynamicToolCallRequest(request) => {
let call_id = request.call_id;
let turn_id = request.turn_id;
let namespace = request.namespace;
let tool = request.tool;
let arguments = request.arguments;
let item = ThreadItem::DynamicToolCall {
id: call_id.clone(),
namespace: namespace.clone(),
tool: tool.clone(),
arguments: arguments.clone(),
status: DynamicToolCallStatus::InProgress,
content_items: None,
success: None,
duration_ms: None,
};
let notification = ItemStartedNotification {
thread_id: conversation_id.to_string(),
turn_id: turn_id.clone(),
started_at_ms: request.started_at_ms,
item,
};
outgoing
.send_server_notification(ServerNotification::ItemStarted(notification))
.await;
let params = DynamicToolCallParams {
thread_id: conversation_id.to_string(),
turn_id: turn_id.clone(),
call_id: call_id.clone(),
namespace,
tool: tool.clone(),
arguments: arguments.clone(),
};
let (_pending_request_id, rx) = outgoing
.send_request(ServerRequestPayload::DynamicToolCall(params))
.await;
tokio::spawn(async move {
crate::dynamic_tools::on_call_response(call_id, rx, conversation).await;
});
EventMsg::DynamicToolCallRequest(_) | EventMsg::DynamicToolCallResponse(_) => {
// Deprecated dynamic-tool events are still fanned out for raw-event and rollout
// compatibility consumers. App-server v2 receives the canonical DynamicToolCall
// item lifecycle and dispatches client requests from canonical starts instead.
}
EventMsg::McpToolCallBegin(_) | EventMsg::McpToolCallEnd(_) => {
// Deprecated MCP tool-call events are still fanned out for legacy clients.
// App-server v2 receives the canonical TurnItem::McpToolCall lifecycle instead.
}
msg @ (EventMsg::DynamicToolCallResponse(_)
| EventMsg::CollabAgentSpawnBegin(_)
msg @ (EventMsg::CollabAgentSpawnBegin(_)
| EventMsg::CollabAgentSpawnEnd(_)
| EventMsg::CollabAgentInteractionBegin(_)
| EventMsg::CollabAgentInteractionEnd(_)
@@ -1040,6 +1003,17 @@ pub(crate) async fn apply_bespoke_event_handling(
.insert(item.id.clone()),
_ => true,
};
let dynamic_tool_call_params = match &event.item {
CoreTurnItem::DynamicToolCall(item) => Some(DynamicToolCallParams {
thread_id: conversation_id.to_string(),
turn_id: event.turn_id.clone(),
call_id: item.id.clone(),
namespace: item.namespace.clone(),
tool: item.tool.clone(),
arguments: item.arguments.clone(),
}),
_ => None,
};
if should_emit {
let notification = item_event_to_server_notification(
EventMsg::ItemStarted(event),
@@ -1048,6 +1022,15 @@ pub(crate) async fn apply_bespoke_event_handling(
);
outgoing.send_server_notification(notification).await;
}
if let Some(params) = dynamic_tool_call_params {
let call_id = params.call_id.clone();
let (_pending_request_id, rx) = outgoing
.send_request(ServerRequestPayload::DynamicToolCall(params))
.await;
tokio::spawn(async move {
crate::dynamic_tools::on_call_response(call_id, rx, conversation).await;
});
}
}
EventMsg::ItemCompleted(event) => {
apply_canonical_item_completed_side_effects(&thread_state, &event.item).await;
@@ -2185,10 +2168,14 @@ mod tests {
use codex_app_server_protocol::AutoReviewDecisionSource;
use codex_app_server_protocol::GuardianApprovalReviewStatus;
use codex_app_server_protocol::JSONRPCErrorError;
use codex_app_server_protocol::ServerRequest;
use codex_app_server_protocol::TurnPlanStepStatus;
use codex_login::CodexAuth;
use codex_protocol::AgentPath;
use codex_protocol::items::DynamicToolCallItem;
use codex_protocol::items::DynamicToolCallStatus as CoreDynamicToolCallStatus;
use codex_protocol::items::HookPromptFragment;
use codex_protocol::items::TurnItem as CoreTurnItem;
use codex_protocol::items::build_hook_prompt_message;
use codex_protocol::models::FileSystemPermissions as CoreFileSystemPermissions;
use codex_protocol::models::NetworkPermissions as CoreNetworkPermissions;
@@ -2205,6 +2192,7 @@ mod tests {
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::GuardianAssessmentEvent;
use codex_protocol::protocol::GuardianAssessmentStatus;
use codex_protocol::protocol::ItemStartedEvent;
use codex_protocol::protocol::RateLimitSnapshot;
use codex_protocol::protocol::RateLimitWindow;
use codex_protocol::protocol::RolloutItem;
@@ -3482,6 +3470,92 @@ mod tests {
Ok(())
}
#[tokio::test]
async fn canonical_dynamic_tool_start_emits_item_and_requests_client() -> Result<()> {
let codex_home = TempDir::new()?;
let config = load_default_config_for_test(&codex_home).await;
let thread_manager = Arc::new(
codex_core::test_support::thread_manager_with_models_provider_and_home(
CodexAuth::create_dummy_chatgpt_auth_for_testing(),
config.model_provider.clone(),
config.codex_home.to_path_buf(),
Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()),
),
);
let codex_core::NewThread {
thread_id: conversation_id,
thread: conversation,
..
} = thread_manager.start_thread(config).await?;
let (tx, mut rx) = mpsc::channel(CHANNEL_CAPACITY);
let outgoing = Arc::new(OutgoingMessageSender::new(
tx,
codex_analytics::AnalyticsEventsClient::disabled(),
));
let outgoing = ThreadScopedOutgoingMessageSender::new(
outgoing,
vec![ConnectionId(1)],
conversation_id,
);
apply_bespoke_event_handling(
Event {
id: "turn-1".to_string(),
msg: EventMsg::ItemStarted(ItemStartedEvent {
thread_id: conversation_id,
turn_id: "turn-1".to_string(),
item: CoreTurnItem::DynamicToolCall(DynamicToolCallItem {
id: "dynamic-1".to_string(),
namespace: Some("apps".to_string()),
tool: "lookup".to_string(),
arguments: json!({"id": "123"}),
status: CoreDynamicToolCallStatus::InProgress,
content_items: None,
success: None,
error: None,
duration: None,
}),
started_at_ms: 42,
}),
},
conversation_id,
conversation,
thread_manager,
outgoing,
new_thread_state(),
ThreadWatchManager::new(),
Arc::new(tokio::sync::Semaphore::new(/*permits*/ 1)),
"test-provider".to_string(),
)
.await;
let item_started = recv_broadcast_message(&mut rx).await?;
let OutgoingMessage::AppServerNotification(ServerNotification::ItemStarted(payload)) =
item_started
else {
bail!("unexpected message: {item_started:?}");
};
assert_eq!(payload.item.id(), "dynamic-1");
let request = recv_broadcast_message(&mut rx).await?;
let OutgoingMessage::Request(ServerRequest::DynamicToolCall { params, .. }) = request
else {
bail!("unexpected message: {request:?}");
};
assert_eq!(
params,
DynamicToolCallParams {
thread_id: conversation_id.to_string(),
turn_id: "turn-1".to_string(),
call_id: "dynamic-1".to_string(),
namespace: Some("apps".to_string()),
tool: "lookup".to_string(),
arguments: json!({"id": "123"}),
}
);
Ok(())
}
#[tokio::test]
async fn test_handle_turn_complete_emits_completed_without_error() -> Result<()> {
let conversation_id = ThreadId::new();
+8 -11
View File
@@ -74,10 +74,10 @@ use codex_feedback::CodexFeedback;
use codex_goal_extension::GoalService;
use codex_home::CodexHomeUserInstructionsProvider;
use codex_login::AuthManager;
use codex_login::CodexAuth;
use codex_login::auth::ExternalAuth;
use codex_login::auth::ExternalAuthRefreshContext;
use codex_login::auth::ExternalAuthRefreshReason;
use codex_login::auth::ExternalAuthTokens;
use codex_protocol::ThreadId;
use codex_protocol::auth::AuthMode as LoginAuthMode;
use codex_protocol::protocol::SessionSource;
@@ -121,10 +121,7 @@ impl ExternalAuthRefreshBridge {
}
}
async fn refresh(
&self,
context: ExternalAuthRefreshContext,
) -> std::io::Result<ExternalAuthTokens> {
async fn refresh(&self, context: ExternalAuthRefreshContext) -> std::io::Result<CodexAuth> {
let params = ChatgptAuthTokensRefreshParams {
reason: Self::map_reason(context.reason),
previous_account_id: context.previous_account_id,
@@ -162,11 +159,11 @@ impl ExternalAuthRefreshBridge {
let response: ChatgptAuthTokensRefreshResponse =
serde_json::from_value(result).map_err(std::io::Error::other)?;
Ok(ExternalAuthTokens::chatgpt(
response.access_token,
response.chatgpt_account_id,
response.chatgpt_plan_type,
))
CodexAuth::from_external_chatgpt_tokens(
response.access_token.as_str(),
response.chatgpt_account_id.as_str(),
response.chatgpt_plan_type.as_deref(),
)
}
}
@@ -178,7 +175,7 @@ impl ExternalAuth for ExternalAuthRefreshBridge {
fn refresh(
&self,
context: ExternalAuthRefreshContext,
) -> codex_login::ExternalAuthFuture<'_, ExternalAuthTokens> {
) -> codex_login::ExternalAuthFuture<'_, CodexAuth> {
Box::pin(ExternalAuthRefreshBridge::refresh(self, context))
}
}
@@ -400,7 +400,7 @@ async fn config_read_includes_apps() -> Result<()> {
r#"
[apps._default]
approvals_reviewer = "auto_review"
default_tools_approval_mode = "approve"
default_tools_approval_mode = "writes"
[apps.app1]
enabled = false
@@ -440,7 +440,7 @@ default_tools_approval_mode = "prompt"
approvals_reviewer: Some(ApprovalsReviewer::AutoReview),
destructive_enabled: true,
open_world_enabled: true,
default_tools_approval_mode: Some(AppToolApproval::Approve),
default_tools_approval_mode: Some(AppToolApproval::Writes),
}),
apps: std::collections::HashMap::from([(
"app1".to_string(),
+1
View File
@@ -963,6 +963,7 @@ async fn run_get(config_overrides: &CliConfigOverrides, get_args: GetArgs) -> Re
let approval_mode = match approval_mode {
AppToolApproval::Auto => "auto",
AppToolApproval::Prompt => "prompt",
AppToolApproval::Writes => "writes",
AppToolApproval::Approve => "approve",
};
println!(" default_tools_approval_mode: {approval_mode}");
+2
View File
@@ -198,6 +198,7 @@ fn serialize_mcp_server(config: &McpServerConfig) -> TomlItem {
entry["default_tools_approval_mode"] = value(match approval_mode {
AppToolApproval::Auto => "auto",
AppToolApproval::Prompt => "prompt",
AppToolApproval::Writes => "writes",
AppToolApproval::Approve => "approve",
});
}
@@ -242,6 +243,7 @@ fn serialize_mcp_server(config: &McpServerConfig) -> TomlItem {
tool_entry["approval_mode"] = value(match approval_mode {
AppToolApproval::Auto => "auto",
AppToolApproval::Prompt => "prompt",
AppToolApproval::Writes => "writes",
AppToolApproval::Approve => "approve",
});
}
+1
View File
@@ -22,6 +22,7 @@ pub enum AppToolApproval {
#[default]
Auto,
Prompt,
Writes,
Approve,
}
+275
View File
@@ -388,6 +388,41 @@ fn write_skill_interface_at(skill_dir: &Path, contents: &str) -> PathBuf {
write_skill_metadata_at(skill_dir, contents)
}
fn write_plugin_manifest(plugin_root: &Path, contents: &str) {
let manifest_path = plugin_root.join(".codex-plugin/plugin.json");
fs::create_dir_all(manifest_path.parent().expect("manifest parent")).unwrap();
fs::write(manifest_path, contents).unwrap();
}
async fn load_user_skills_root(root: &Path) -> SkillLoadOutcome {
load_skills_from_roots(
[SkillRoot {
path: root.abs(),
scope: SkillScope::User,
file_system: Arc::clone(&LOCAL_FS),
plugin_id: None,
plugin_namespace: None,
plugin_root: None,
}],
/*plugin_skill_snapshots*/ None,
)
.await
}
fn expected_user_skill(path: &Path, name: &str, description: &str) -> SkillMetadata {
SkillMetadata {
name: name.to_string(),
description: description.to_string(),
short_description: None,
interface: None,
dependencies: None,
policy: None,
path_to_skills_md: normalized(path),
scope: SkillScope::User,
plugin_id: None,
}
}
#[tokio::test]
async fn loads_skill_dependencies_metadata_from_yaml() {
let codex_home = tempfile::tempdir().expect("tempdir");
@@ -1326,6 +1361,246 @@ async fn namespaces_plugin_skills_using_provided_namespace() {
);
}
#[tokio::test]
async fn namespaces_nested_plugin_skills_without_namespacing_plain_siblings() {
let root = tempfile::tempdir().expect("tempdir");
let skills_root = root.path().join("skills");
let plain_skill_path =
write_skill_at(&skills_root, "plain", "plain-skill", "plain description");
let plugin_root = skills_root.join("nested-plugin");
write_plugin_manifest(&plugin_root, r#"{"name":"nested"}"#);
let plugin_skill_path = write_skill_at(
&plugin_root.join("skills"),
"search",
"plugin-skill",
"plugin description",
);
let outcome = load_user_skills_root(&skills_root).await;
assert!(
outcome.errors.is_empty(),
"unexpected errors: {:?}",
outcome.errors
);
assert_eq!(
outcome.skills,
vec![
expected_user_skill(
&plugin_skill_path,
"nested:plugin-skill",
"plugin description"
),
expected_user_skill(&plain_skill_path, "plain-skill", "plain description"),
]
);
}
#[tokio::test]
async fn inherits_plugin_namespace_from_above_scanned_skills_root() {
let root = tempfile::tempdir().expect("tempdir");
let plugin_root = root.path().join("plugin");
write_plugin_manifest(&plugin_root, r#"{"name":"outer"}"#);
let skills_root = plugin_root.join("skills");
let skill_path = write_skill_at(&skills_root, "search", "search-skill", "search description");
let outcome = load_user_skills_root(&skills_root).await;
assert!(
outcome.errors.is_empty(),
"unexpected errors: {:?}",
outcome.errors
);
assert_eq!(
outcome.skills,
vec![expected_user_skill(
&skill_path,
"outer:search-skill",
"search description",
)]
);
}
#[tokio::test]
async fn nearest_valid_nested_plugin_namespace_overrides_outer_namespace() {
let root = tempfile::tempdir().expect("tempdir");
let outer_plugin_root = root.path().join("outer-plugin");
write_plugin_manifest(&outer_plugin_root, r#"{"name":"outer"}"#);
let skills_root = outer_plugin_root.join("skills");
let nested_plugin_root = skills_root.join("nested-plugin");
write_plugin_manifest(&nested_plugin_root, r#"{"name":"nested"}"#);
let skill_path = write_skill_at(
&nested_plugin_root.join("skills"),
"search",
"search-skill",
"search description",
);
let outcome = load_user_skills_root(&skills_root).await;
assert!(
outcome.errors.is_empty(),
"unexpected errors: {:?}",
outcome.errors
);
assert_eq!(
outcome.skills,
vec![expected_user_skill(
&skill_path,
"nested:search-skill",
"search description",
)]
);
}
#[tokio::test]
async fn invalid_nested_plugin_manifest_falls_back_to_outer_namespace() {
let root = tempfile::tempdir().expect("tempdir");
let outer_plugin_root = root.path().join("outer-plugin");
write_plugin_manifest(&outer_plugin_root, r#"{"name":"outer"}"#);
let skills_root = outer_plugin_root.join("skills");
let nested_plugin_root = skills_root.join("nested-plugin");
write_plugin_manifest(&nested_plugin_root, "not json");
let skill_path = write_skill_at(
&nested_plugin_root.join("skills"),
"search",
"search-skill",
"search description",
);
let outcome = load_user_skills_root(&skills_root).await;
assert!(
outcome.errors.is_empty(),
"unexpected errors: {:?}",
outcome.errors
);
assert_eq!(
outcome.skills,
vec![expected_user_skill(
&skill_path,
"outer:search-skill",
"search description",
)]
);
}
// Directory symlinks on Windows can require Developer Mode or administrator privileges.
#[cfg(unix)]
#[tokio::test]
async fn namespaces_skills_in_symlinked_plugin_skills_dir() {
// skills/
// └── linked-plugin -> shared-plugin/skills/
// shared-plugin/
// ├── .codex-plugin/plugin.json
// └── skills/search/SKILL.md
let root = tempfile::tempdir().expect("tempdir");
let shared_plugin_root = tempfile::tempdir().expect("tempdir");
write_plugin_manifest(shared_plugin_root.path(), r#"{"name":"linked"}"#);
let skill_path = write_skill_at(
&shared_plugin_root.path().join("skills"),
"search",
"search-skill",
"search description",
);
let skills_root = root.path().join("skills");
fs::create_dir_all(&skills_root).unwrap();
symlink_dir(
&shared_plugin_root.path().join("skills"),
&skills_root.join("linked-plugin"),
);
let outcome = load_user_skills_root(&skills_root).await;
assert!(
outcome.errors.is_empty(),
"unexpected errors: {:?}",
outcome.errors
);
assert_eq!(
outcome.skills,
vec![expected_user_skill(
&skill_path,
"linked:search-skill",
"search description",
)]
);
}
// Directory symlinks on Windows can require Developer Mode or administrator privileges.
#[cfg(unix)]
#[tokio::test]
async fn does_not_inherit_namespace_for_skills_in_symlinked_plain_dir() {
// outer-plugin/
// ├── .codex-plugin/plugin.json
// └── skills/linked-plain -> plain-root/
// plain-root/
// └── search/SKILL.md
let root = tempfile::tempdir().expect("tempdir");
let plugin_root = root.path().join("outer-plugin");
write_plugin_manifest(&plugin_root, r#"{"name":"outer"}"#);
let skills_root = plugin_root.join("skills");
let plain_root = tempfile::tempdir().expect("tempdir");
let skill_path = write_skill_at(
plain_root.path(),
"search",
"plain-skill",
"plain description",
);
fs::create_dir_all(&skills_root).unwrap();
symlink_dir(plain_root.path(), &skills_root.join("linked-plain"));
let outcome = load_user_skills_root(&skills_root).await;
assert!(
outcome.errors.is_empty(),
"unexpected errors: {:?}",
outcome.errors
);
assert_eq!(
outcome.skills,
vec![expected_user_skill(
&skill_path,
"plain-skill",
"plain description",
)]
);
}
// Directory symlinks on Windows can require Developer Mode or administrator privileges.
#[cfg(unix)]
#[tokio::test]
async fn keeps_inherited_namespace_when_symlink_target_is_scan_root_ancestor() {
// temp-root/
// └── a/b/c/d/e/f/outer-plugin/
// ├── .codex-plugin/plugin.json
// └── skills/
// ├── root/SKILL.md
// └── link -> temp-root/
let root = tempfile::tempdir().expect("tempdir");
let plugin_root = root.path().join("a/b/c/d/e/f/outer-plugin");
write_plugin_manifest(&plugin_root, r#"{"name":"outer"}"#);
let skills_root = plugin_root.join("skills");
let skill_path = write_skill_at(&skills_root, "root", "root-skill", "root description");
symlink_dir(root.path(), &skills_root.join("link"));
let outcome = load_user_skills_root(&skills_root).await;
assert!(
outcome.errors.is_empty(),
"unexpected errors: {:?}",
outcome.errors
);
assert_eq!(
outcome.skills,
vec![expected_user_skill(
&skill_path,
"outer:root-skill",
"root description",
)]
);
}
#[tokio::test]
async fn plugin_skill_name_length_limit_allows_max_qualified_name() {
let root = tempfile::tempdir().expect("tempdir");
+1
View File
@@ -150,6 +150,7 @@
"enum": [
"auto",
"prompt",
"writes",
"approve"
],
"type": "string"
@@ -121,6 +121,7 @@ fn serialize_mcp_server_table(config: &McpServerConfig) -> TomlTable {
entry["default_tools_approval_mode"] = value(match approval_mode {
AppToolApproval::Auto => "auto",
AppToolApproval::Prompt => "prompt",
AppToolApproval::Writes => "writes",
AppToolApproval::Approve => "approve",
});
}
@@ -173,6 +174,7 @@ fn serialize_mcp_server_tool(config: &McpServerToolConfig) -> TomlItem {
entry["approval_mode"] = value(match approval_mode {
AppToolApproval::Auto => "auto",
AppToolApproval::Prompt => "prompt",
AppToolApproval::Writes => "writes",
AppToolApproval::Approve => "approve",
});
}
+22 -8
View File
@@ -1223,8 +1223,7 @@ async fn maybe_request_mcp_tool_approval(
}
let annotations = metadata.and_then(|metadata| metadata.annotations.as_ref());
let approval_required = requires_mcp_tool_approval(annotations);
if !approval_required && approval_mode != AppToolApproval::Prompt {
if !requires_mcp_tool_approval_for_mode(annotations, approval_mode) {
return None;
}
@@ -1927,12 +1926,13 @@ fn normalize_approval_decision_for_mode(
decision: McpToolApprovalDecision,
approval_mode: AppToolApproval,
) -> McpToolApprovalDecision {
if approval_mode == AppToolApproval::Prompt
&& matches!(
decision,
McpToolApprovalDecision::AcceptForSession | McpToolApprovalDecision::AcceptAndRemember
)
{
if matches!(
approval_mode,
AppToolApproval::Prompt | AppToolApproval::Writes
) && matches!(
decision,
McpToolApprovalDecision::AcceptForSession | McpToolApprovalDecision::AcceptAndRemember
) {
McpToolApprovalDecision::Accept
} else {
decision
@@ -2179,6 +2179,20 @@ fn requires_mcp_tool_approval(annotations: Option<&ToolAnnotations>) -> bool {
.unwrap_or(true)
}
fn requires_mcp_tool_approval_for_mode(
annotations: Option<&ToolAnnotations>,
approval_mode: AppToolApproval,
) -> bool {
match approval_mode {
AppToolApproval::Auto => requires_mcp_tool_approval(annotations),
AppToolApproval::Prompt => true,
AppToolApproval::Writes => !annotations
.and_then(|annotations| annotations.read_only_hint)
.unwrap_or(false),
AppToolApproval::Approve => false,
}
}
async fn notify_mcp_tool_call_skip(
sess: &Session,
turn_context: &TurnContext,
+35 -11
View File
@@ -398,23 +398,47 @@ fn approval_not_required_when_read_only_and_other_hints_are_absent() {
}
#[test]
fn prompt_mode_does_not_allow_persistent_remember() {
fn writes_mode_requires_approval_for_non_read_only_tools() {
let annotations = annotations(Some(false), Some(false), Some(false));
assert_eq!(
normalize_approval_decision_for_mode(
McpToolApprovalDecision::AcceptForSession,
AppToolApproval::Prompt,
),
McpToolApprovalDecision::Accept
requires_mcp_tool_approval_for_mode(Some(&annotations), AppToolApproval::Writes),
true
);
assert_eq!(
normalize_approval_decision_for_mode(
McpToolApprovalDecision::AcceptAndRemember,
AppToolApproval::Prompt,
),
McpToolApprovalDecision::Accept
requires_mcp_tool_approval_for_mode(/*annotations*/ None, AppToolApproval::Writes),
true
);
}
#[test]
fn writes_mode_does_not_require_approval_for_read_only_tools() {
let annotations = annotations(Some(true), Some(true), Some(true));
assert_eq!(
requires_mcp_tool_approval_for_mode(Some(&annotations), AppToolApproval::Writes),
false
);
}
#[test]
fn prompting_modes_do_not_allow_persistent_remember() {
for approval_mode in [AppToolApproval::Prompt, AppToolApproval::Writes] {
assert_eq!(
normalize_approval_decision_for_mode(
McpToolApprovalDecision::AcceptForSession,
approval_mode,
),
McpToolApprovalDecision::Accept
);
assert_eq!(
normalize_approval_decision_for_mode(
McpToolApprovalDecision::AcceptAndRemember,
approval_mode,
),
McpToolApprovalDecision::Accept
);
}
}
#[tokio::test]
async fn mcp_tool_call_span_records_expected_fields() {
let buffer: &'static std::sync::Mutex<Vec<u8>> =
+41 -33
View File
@@ -9,14 +9,13 @@ use crate::tools::handlers::parse_arguments;
use crate::tools::registry::CoreToolRuntime;
use crate::tools::registry::ToolExecutor;
use crate::tools::registry::ToolExposure;
use crate::turn_timing::now_unix_timestamp_ms;
use codex_protocol::dynamic_tools::DynamicToolCallRequest;
use codex_protocol::dynamic_tools::DynamicToolFunctionSpec;
use codex_protocol::dynamic_tools::DynamicToolNamespaceSpec;
use codex_protocol::dynamic_tools::DynamicToolResponse;
use codex_protocol::items::DynamicToolCallItem;
use codex_protocol::items::DynamicToolCallStatus;
use codex_protocol::items::TurnItem;
use codex_protocol::models::FunctionCallOutputContentItem;
use codex_protocol::protocol::DynamicToolCallResponseEvent;
use codex_protocol::protocol::EventMsg;
use codex_tools::ResponsesApiNamespace;
use codex_tools::ResponsesApiNamespaceTool;
use codex_tools::ToolName;
@@ -178,7 +177,6 @@ async fn request_dynamic_tool(
) -> Option<DynamicToolResponse> {
let namespace = tool_name.namespace;
let tool = tool_name.name;
let turn_id = turn_context.sub_id.clone();
let (tx_response, rx_response) = oneshot::channel();
let event_id = call_id.clone();
let prev_entry = {
@@ -196,45 +194,55 @@ async fn request_dynamic_tool(
}
let started_at = Instant::now();
let started_at_ms = now_unix_timestamp_ms();
let event = EventMsg::DynamicToolCallRequest(DynamicToolCallRequest {
call_id: call_id.clone(),
turn_id: turn_id.clone(),
started_at_ms,
namespace: namespace.clone(),
tool: tool.clone(),
arguments: arguments.clone(),
});
session.send_event(turn_context, event).await;
session
.emit_turn_item_started(
turn_context,
&TurnItem::DynamicToolCall(DynamicToolCallItem {
id: call_id.clone(),
namespace: namespace.clone(),
tool: tool.clone(),
arguments: arguments.clone(),
status: DynamicToolCallStatus::InProgress,
content_items: None,
success: None,
error: None,
duration: None,
}),
)
.await;
let response = rx_response.await.ok();
let response_event = match &response {
Some(response) => EventMsg::DynamicToolCallResponse(DynamicToolCallResponseEvent {
call_id,
turn_id,
completed_at_ms: now_unix_timestamp_ms(),
let item = match &response {
Some(response) => DynamicToolCallItem {
id: call_id,
namespace,
tool,
arguments,
content_items: response.content_items.clone(),
success: response.success,
status: if response.success {
DynamicToolCallStatus::Completed
} else {
DynamicToolCallStatus::Failed
},
content_items: Some(response.content_items.clone()),
success: Some(response.success),
error: None,
duration: started_at.elapsed(),
}),
None => EventMsg::DynamicToolCallResponse(DynamicToolCallResponseEvent {
call_id,
turn_id,
completed_at_ms: now_unix_timestamp_ms(),
duration: Some(started_at.elapsed()),
},
None => DynamicToolCallItem {
id: call_id,
namespace,
tool,
arguments,
content_items: Vec::new(),
success: false,
status: DynamicToolCallStatus::Failed,
content_items: Some(Vec::new()),
success: Some(false),
error: Some("dynamic tool call was cancelled before receiving a response".to_string()),
duration: started_at.elapsed(),
}),
duration: Some(started_at.elapsed()),
},
};
session.send_event(turn_context, response_event).await;
session
.emit_turn_item_completed(turn_context, TurnItem::DynamicToolCall(item))
.await;
response
}
@@ -20,6 +20,7 @@ use codex_protocol::user_input::UserInput;
use core_test_support::PathExt;
use core_test_support::apps_test_server::AppsTestServer;
use core_test_support::apps_test_server::SEARCH_CALENDAR_CREATE_TOOL;
use core_test_support::apps_test_server::SEARCH_CALENDAR_LIST_TOOL;
use core_test_support::apps_test_server::SEARCH_CALENDAR_NAMESPACE;
use core_test_support::apps_test_server::recorded_apps_tool_call_by_call_id;
use core_test_support::apps_test_server::search_capable_apps_builder;
@@ -45,6 +46,7 @@ fn set_calendar_approval_mode(config: &mut Config, approval_mode: AppToolApprova
let approval_mode = match approval_mode {
AppToolApproval::Auto => "auto",
AppToolApproval::Prompt => "prompt",
AppToolApproval::Writes => "writes",
AppToolApproval::Approve => "approve",
};
let user_config_path = config.codex_home.join("config.toml").abs();
@@ -68,6 +70,7 @@ fn set_default_app_approval_mode_and_reviewer(
let approval_mode = match approval_mode {
AppToolApproval::Auto => "auto",
AppToolApproval::Prompt => "prompt",
AppToolApproval::Writes => "writes",
AppToolApproval::Approve => "approve",
};
let user_config_path = config.codex_home.join("config.toml").abs();
@@ -333,6 +336,127 @@ async fn apps_default_prompt_with_auto_review_routes_actual_mcp_approval_to_guar
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn apps_default_writes_prompts_for_writes_but_not_reads() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let apps_server = AppsTestServer::mount(&server).await?;
let read_call_id = "calendar-read";
let write_call_id = "calendar-write";
let list_args = serde_json::to_string(&json!({}))?;
let create_args = serde_json::to_string(&json!({
"title": "Lunch",
"starts_at": "2026-03-10T12:00:00Z"
}))?;
let responses = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-read"),
ev_function_call_with_namespace(
read_call_id,
SEARCH_CALENDAR_NAMESPACE,
SEARCH_CALENDAR_LIST_TOOL,
&list_args,
),
ev_completed("resp-read"),
]),
sse(vec![
ev_response_created("resp-write"),
ev_function_call_with_namespace(
write_call_id,
SEARCH_CALENDAR_NAMESPACE,
SEARCH_CALENDAR_CREATE_TOOL,
&create_args,
),
ev_completed("resp-write"),
]),
sse(vec![
ev_response_created("resp-done"),
ev_assistant_message("msg-done", "done"),
ev_completed("resp-done"),
]),
],
)
.await;
let mut builder = search_capable_apps_builder(apps_server.chatgpt_base_url.clone())
.with_config(|config| {
config
.features
.enable(Feature::ToolCallMcpElicitation)
.expect("test config should allow feature update");
set_default_app_approval_mode_and_reviewer(
config,
AppToolApproval::Writes,
ApprovalsReviewer::User,
);
});
let test = builder.build(&server).await?;
submit_user_turn(
&test,
"Use [$calendar](app://calendar) to list events, then create one.",
AskForApproval::OnRequest,
/*collaboration_mode*/ None,
)
.await?;
let EventMsg::McpToolCallBegin(read_begin) = wait_for_event(&test.codex, |event| {
matches!(event, EventMsg::McpToolCallBegin(_))
})
.await
else {
unreachable!("event guard guarantees McpToolCallBegin");
};
assert_eq!(read_begin.call_id, read_call_id);
let next_route = wait_for_event(&test.codex, |event| {
matches!(
event,
EventMsg::McpToolCallBegin(_) | EventMsg::ElicitationRequest(_)
)
})
.await;
let EventMsg::McpToolCallBegin(write_begin) = next_route else {
panic!("read-only app action should not prompt in writes mode");
};
assert_eq!(write_begin.call_id, write_call_id);
let EventMsg::ElicitationRequest(request) = wait_for_event(&test.codex, |event| {
matches!(
event,
EventMsg::ElicitationRequest(_) | EventMsg::TurnComplete(_)
)
})
.await
else {
panic!("write app action should prompt in writes mode");
};
test.codex
.submit(Op::ResolveElicitation {
server_name: request.server_name,
request_id: request.id,
decision: ElicitationAction::Accept,
content: None,
meta: None,
})
.await?;
wait_for_event(&test.codex, |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
assert_eq!(responses.requests().len(), 3);
recorded_apps_tool_call_by_call_id(&server, read_call_id).await;
recorded_apps_tool_call_by_call_id(&server, write_call_id).await;
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn mcp_tool_call_metadata_records_prior_request_user_input_tool() -> Result<()> {
skip_if_no_network!(Ok(()));
-13
View File
@@ -1015,19 +1015,6 @@ async fn refresh_failure_is_scoped_to_the_matching_auth_snapshot() {
assert_eq!(manager.refresh_failure_for_auth(&updated_auth), None);
}
#[test]
fn external_auth_tokens_without_chatgpt_metadata_cannot_seed_chatgpt_auth() {
let err = AuthDotJson::from_external_tokens(&ExternalAuthTokens::access_token_only(
"test-access-token",
))
.expect_err("bearer-only external auth should not seed ChatGPT auth");
assert_eq!(
err.to_string(),
"external auth tokens are missing ChatGPT metadata"
);
}
#[tokio::test]
async fn external_bearer_only_auth_manager_uses_cached_provider_token() {
let script = ProviderAuthScript::new(&["provider-token", "next-token"]).unwrap();
+9 -15
View File
@@ -1,7 +1,7 @@
use super::manager::CodexAuth;
use super::manager::ExternalAuth;
use super::manager::ExternalAuthFuture;
use super::manager::ExternalAuthRefreshContext;
use super::manager::ExternalAuthTokens;
use codex_protocol::auth::AuthMode;
use codex_protocol::config_types::ModelProviderAuthInfo;
use std::fmt;
@@ -30,7 +30,7 @@ impl BearerTokenRefresher {
clippy::await_holding_invalid_type,
reason = "external bearer cache misses intentionally hold cached_token across the provider command to avoid duplicate refreshes"
)]
async fn resolve(&self) -> io::Result<Option<ExternalAuthTokens>> {
async fn resolve(&self) -> io::Result<Option<CodexAuth>> {
let access_token = {
let mut cached = self.state.cached_token.lock().await;
if let Some(cached_token) = cached.as_ref() {
@@ -39,8 +39,8 @@ impl BearerTokenRefresher {
None => true,
};
if should_use_cached_token {
return Ok(Some(ExternalAuthTokens::access_token_only(
cached_token.access_token.clone(),
return Ok(Some(CodexAuth::from_api_key(
cached_token.access_token.as_str(),
)));
}
}
@@ -52,20 +52,17 @@ impl BearerTokenRefresher {
});
access_token
};
Ok(Some(ExternalAuthTokens::access_token_only(access_token)))
Ok(Some(CodexAuth::from_api_key(access_token.as_str())))
}
async fn refresh(
&self,
_context: ExternalAuthRefreshContext,
) -> io::Result<ExternalAuthTokens> {
async fn refresh(&self, _context: ExternalAuthRefreshContext) -> io::Result<CodexAuth> {
let access_token = run_provider_auth_command(&self.state.config).await?;
let mut cached = self.state.cached_token.lock().await;
*cached = Some(CachedExternalBearerToken {
access_token: access_token.clone(),
fetched_at: Instant::now(),
});
Ok(ExternalAuthTokens::access_token_only(access_token))
Ok(CodexAuth::from_api_key(access_token.as_str()))
}
}
@@ -74,14 +71,11 @@ impl ExternalAuth for BearerTokenRefresher {
AuthMode::ApiKey
}
fn resolve(&self) -> ExternalAuthFuture<'_, Option<ExternalAuthTokens>> {
fn resolve(&self) -> ExternalAuthFuture<'_, Option<CodexAuth>> {
Box::pin(BearerTokenRefresher::resolve(self))
}
fn refresh(
&self,
context: ExternalAuthRefreshContext,
) -> ExternalAuthFuture<'_, ExternalAuthTokens> {
fn refresh(&self, context: ExternalAuthRefreshContext) -> ExternalAuthFuture<'_, CodexAuth> {
Box::pin(BearerTokenRefresher::refresh(self, context))
}
}
+54 -80
View File
@@ -198,45 +198,6 @@ pub enum RefreshTokenError {
Transient(#[from] std::io::Error),
}
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct ExternalAuthTokens {
pub access_token: String,
pub chatgpt_metadata: Option<ExternalAuthChatgptMetadata>,
}
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct ExternalAuthChatgptMetadata {
pub account_id: String,
pub plan_type: Option<String>,
}
impl ExternalAuthTokens {
pub fn access_token_only(access_token: impl Into<String>) -> Self {
Self {
access_token: access_token.into(),
chatgpt_metadata: None,
}
}
pub fn chatgpt(
access_token: impl Into<String>,
chatgpt_account_id: impl Into<String>,
chatgpt_plan_type: Option<String>,
) -> Self {
Self {
access_token: access_token.into(),
chatgpt_metadata: Some(ExternalAuthChatgptMetadata {
account_id: chatgpt_account_id.into(),
plan_type: chatgpt_plan_type,
}),
}
}
pub fn chatgpt_metadata(&self) -> Option<&ExternalAuthChatgptMetadata> {
self.chatgpt_metadata.as_ref()
}
}
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum ExternalAuthRefreshReason {
Unauthorized,
@@ -251,22 +212,19 @@ pub struct ExternalAuthRefreshContext {
/// Pluggable auth provider used by `AuthManager` for externally managed auth flows.
///
/// Implementations may either resolve auth eagerly via `resolve()` or provide refreshed
/// credentials on demand via `refresh()`.
/// auth on demand via `refresh()`.
pub trait ExternalAuth: Send + Sync {
/// Indicates which top-level auth mode this external provider supplies.
fn auth_mode(&self) -> AuthMode;
/// Returns cached or immediately available auth, if this provider can resolve it synchronously
/// from the caller's perspective.
fn resolve(&self) -> ExternalAuthFuture<'_, Option<ExternalAuthTokens>> {
fn resolve(&self) -> ExternalAuthFuture<'_, Option<CodexAuth>> {
Box::pin(async { Ok(None) })
}
/// Refreshes auth in response to a manager-driven refresh attempt.
fn refresh(
&self,
context: ExternalAuthRefreshContext,
) -> ExternalAuthFuture<'_, ExternalAuthTokens>;
fn refresh(&self, context: ExternalAuthRefreshContext) -> ExternalAuthFuture<'_, CodexAuth>;
}
pub type ExternalAuthFuture<'a, T> = Pin<Box<dyn Future<Output = std::io::Result<T>> + Send + 'a>>;
@@ -747,6 +705,24 @@ impl CodexAuth {
Self::Chatgpt(ChatgptAuth { state, storage })
}
/// Constructs in-memory ChatGPT auth from externally managed tokens.
pub fn from_external_chatgpt_tokens(
access_token: &str,
chatgpt_account_id: &str,
chatgpt_plan_type: Option<&str>,
) -> std::io::Result<Self> {
let auth_dot_json = AuthDotJson::from_external_access_token(
access_token,
chatgpt_account_id,
chatgpt_plan_type,
)?;
let state = ChatgptAuthState {
auth_dot_json: Arc::new(Mutex::new(Some(auth_dot_json))),
client: create_client(),
};
Ok(Self::ChatgptAuthTokens(ChatgptAuthTokens { state }))
}
pub fn from_api_key(api_key: &str) -> Self {
Self::ApiKey(ApiKeyAuth {
api_key: api_key.to_owned(),
@@ -1456,26 +1432,23 @@ fn refresh_token_endpoint() -> String {
}
impl AuthDotJson {
fn from_external_tokens(external: &ExternalAuthTokens) -> std::io::Result<Self> {
let Some(chatgpt_metadata) = external.chatgpt_metadata() else {
return Err(std::io::Error::other(
"external auth tokens are missing ChatGPT metadata",
));
};
fn from_external_access_token(
access_token: &str,
chatgpt_account_id: &str,
chatgpt_plan_type: Option<&str>,
) -> std::io::Result<Self> {
let mut token_info =
parse_chatgpt_jwt_claims(&external.access_token).map_err(std::io::Error::other)?;
token_info.chatgpt_account_id = Some(chatgpt_metadata.account_id.clone());
token_info.chatgpt_plan_type = chatgpt_metadata
.plan_type
.as_deref()
parse_chatgpt_jwt_claims(access_token).map_err(std::io::Error::other)?;
token_info.chatgpt_account_id = Some(chatgpt_account_id.to_string());
token_info.chatgpt_plan_type = chatgpt_plan_type
.map(InternalPlanType::from_raw_value)
.or(token_info.chatgpt_plan_type)
.or(Some(InternalPlanType::Unknown("unknown".to_string())));
let tokens = TokenData {
id_token: token_info,
access_token: external.access_token.clone(),
access_token: access_token.to_string(),
refresh_token: String::new(),
account_id: Some(chatgpt_metadata.account_id.clone()),
account_id: Some(chatgpt_account_id.to_string()),
};
Ok(Self {
@@ -1489,19 +1462,6 @@ impl AuthDotJson {
})
}
fn from_external_access_token(
access_token: &str,
chatgpt_account_id: &str,
chatgpt_plan_type: Option<&str>,
) -> std::io::Result<Self> {
let external = ExternalAuthTokens::chatgpt(
access_token,
chatgpt_account_id,
chatgpt_plan_type.map(str::to_string),
);
Self::from_external_tokens(&external)
}
pub(super) fn resolved_mode(&self) -> AuthMode {
if let Some(mode) = self.auth_mode {
return mode;
@@ -2361,7 +2321,14 @@ impl AuthManager {
let external_auth = self.external_auth()?;
match external_auth.resolve().await {
Ok(Some(tokens)) => Some(CodexAuth::from_api_key(&tokens.access_token)),
Ok(Some(auth)) if auth.api_auth_mode() == AuthMode::ApiKey => Some(auth),
Ok(Some(auth)) => {
tracing::error!(
resolved_mode = ?auth.api_auth_mode(),
"ignoring non-API-key auth from external API key provider"
);
None
}
Ok(None) => None,
Err(err) => {
tracing::error!("Failed to resolve external API key auth: {err}");
@@ -2567,23 +2534,30 @@ impl AuthManager {
if external_auth.auth_mode() == AuthMode::ApiKey {
return Ok(());
}
let Some(chatgpt_metadata) = refreshed.chatgpt_metadata() else {
if refreshed.api_auth_mode() != AuthMode::ChatgptAuthTokens {
return Err(RefreshTokenError::Transient(std::io::Error::other(
"external auth refresh did not return ChatGPT metadata",
"external auth refresh did not return ChatGPT auth tokens",
)));
};
}
let account_id = refreshed.get_account_id().ok_or_else(|| {
RefreshTokenError::Transient(std::io::Error::other(
"external ChatGPT auth tokens are missing an account id",
))
})?;
if let Some(expected_workspace_ids) = forced_chatgpt_workspace_id.as_deref()
&& !expected_workspace_ids.contains(&chatgpt_metadata.account_id)
&& !expected_workspace_ids.contains(&account_id)
{
return Err(RefreshTokenError::Transient(std::io::Error::other(
format!(
"external auth refresh returned workspace {:?}, expected one of {:?}",
chatgpt_metadata.account_id, expected_workspace_ids,
"external auth refresh returned workspace {account_id:?}, expected one of {expected_workspace_ids:?}"
),
)));
}
let auth_dot_json =
AuthDotJson::from_external_tokens(&refreshed).map_err(RefreshTokenError::Transient)?;
let auth_dot_json = refreshed.get_current_auth_json().ok_or_else(|| {
RefreshTokenError::Transient(std::io::Error::other(
"external ChatGPT auth tokens are missing auth state",
))
})?;
save_auth(
&self.codex_home,
&auth_dot_json,
-2
View File
@@ -30,11 +30,9 @@ pub use auth::CODEX_ACCESS_TOKEN_ENV_VAR;
pub use auth::CODEX_API_KEY_ENV_VAR;
pub use auth::CodexAuth;
pub use auth::ExternalAuth;
pub use auth::ExternalAuthChatgptMetadata;
pub use auth::ExternalAuthFuture;
pub use auth::ExternalAuthRefreshContext;
pub use auth::ExternalAuthRefreshReason;
pub use auth::ExternalAuthTokens;
pub use auth::OPENAI_API_KEY_ENV_VAR;
pub use auth::REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR;
pub use auth::REVOKE_TOKEN_URL_OVERRIDE_ENV_VAR;
+5 -14
View File
@@ -7,7 +7,6 @@ use codex_login::AuthManager;
use codex_login::CodexAuth;
use codex_login::ExternalAuth;
use codex_login::ExternalAuthRefreshContext;
use codex_login::ExternalAuthTokens;
use codex_login::TokenData;
use codex_protocol::auth::AuthMode;
use codex_protocol::openai_models::ModelsResponse;
@@ -122,23 +121,15 @@ impl ExternalAuth for TestExternalApiKeyAuth {
AuthMode::ApiKey
}
fn resolve(&self) -> codex_login::ExternalAuthFuture<'_, Option<ExternalAuthTokens>> {
Box::pin(async {
Ok(Some(ExternalAuthTokens::access_token_only(
"test-external-api-key",
)))
})
fn resolve(&self) -> codex_login::ExternalAuthFuture<'_, Option<CodexAuth>> {
Box::pin(async { Ok(Some(CodexAuth::from_api_key("test-external-api-key"))) })
}
fn refresh(
&self,
_context: ExternalAuthRefreshContext,
) -> codex_login::ExternalAuthFuture<'_, ExternalAuthTokens> {
Box::pin(async {
Ok(ExternalAuthTokens::access_token_only(
"test-external-api-key",
))
})
) -> codex_login::ExternalAuthFuture<'_, CodexAuth> {
Box::pin(async { Ok(CodexAuth::from_api_key("test-external-api-key")) })
}
}
@@ -153,7 +144,7 @@ impl ExternalAuth for TestUnresolvedExternalApiKeyAuth {
fn refresh(
&self,
_context: ExternalAuthRefreshContext,
) -> codex_login::ExternalAuthFuture<'_, ExternalAuthTokens> {
) -> codex_login::ExternalAuthFuture<'_, CodexAuth> {
Box::pin(async { Err(std::io::Error::other("unresolved test auth")) })
}
}