mirror of
https://github.com/openai/codex.git
synced 2026-05-20 11:12:43 +00:00
[codex] Split Python SDK helper logic (#22939)
## Summary - Move approval-mode mapping into `sdk/python/src/openai_codex/_approval_mode.py`. - Move initialize metadata parsing and normalization into `sdk/python/src/openai_codex/_initialize_metadata.py`. - Keep the public `ApprovalMode` export stable and retarget direct metadata helper coverage. ## Integration coverage - Add an app-server harness smoke that exercises sync and async SDK initialization plus thread creation. ## Validation - Local tests were not run per repo guidance. CI should validate this branch once the PR is online.
This commit is contained in:
51
sdk/python/src/openai_codex/_approval_mode.py
Normal file
51
sdk/python/src/openai_codex/_approval_mode.py
Normal file
@@ -0,0 +1,51 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from enum import Enum
|
||||
from typing import NoReturn
|
||||
|
||||
from .generated.v2_all import (
|
||||
ApprovalsReviewer,
|
||||
AskForApproval,
|
||||
AskForApprovalValue,
|
||||
)
|
||||
|
||||
|
||||
class ApprovalMode(str, Enum):
|
||||
"""High-level approval behavior for escalated permission requests."""
|
||||
|
||||
deny_all = "deny_all"
|
||||
auto_review = "auto_review"
|
||||
|
||||
|
||||
def _approval_mode_settings(
|
||||
approval_mode: ApprovalMode,
|
||||
) -> tuple[AskForApproval, ApprovalsReviewer | None]:
|
||||
"""Map the public approval mode to generated app-server start params."""
|
||||
if not isinstance(approval_mode, ApprovalMode):
|
||||
supported = ", ".join(mode.value for mode in ApprovalMode)
|
||||
raise ValueError(f"approval_mode must be one of: {supported}")
|
||||
|
||||
match approval_mode:
|
||||
case ApprovalMode.auto_review:
|
||||
return (
|
||||
AskForApproval(root=AskForApprovalValue.on_request),
|
||||
ApprovalsReviewer.auto_review,
|
||||
)
|
||||
case ApprovalMode.deny_all:
|
||||
return AskForApproval(root=AskForApprovalValue.never), None
|
||||
case _:
|
||||
return _assert_never_approval_mode(approval_mode)
|
||||
|
||||
|
||||
def _assert_never_approval_mode(approval_mode: NoReturn) -> NoReturn:
|
||||
"""Make approval mode mapping exhaustive for static type checkers."""
|
||||
raise AssertionError(f"Unhandled approval mode: {approval_mode!r}")
|
||||
|
||||
|
||||
def _approval_mode_override_settings(
|
||||
approval_mode: ApprovalMode | None,
|
||||
) -> tuple[AskForApproval | None, ApprovalsReviewer | None]:
|
||||
"""Map an optional public approval mode to app-server override params."""
|
||||
if approval_mode is None:
|
||||
return None, None
|
||||
return _approval_mode_settings(approval_mode)
|
||||
54
sdk/python/src/openai_codex/_initialize_metadata.py
Normal file
54
sdk/python/src/openai_codex/_initialize_metadata.py
Normal file
@@ -0,0 +1,54 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from .models import InitializeResponse, ServerInfo
|
||||
|
||||
|
||||
def _split_user_agent(user_agent: str) -> tuple[str | None, str | None]:
|
||||
raw = user_agent.strip()
|
||||
if not raw:
|
||||
return None, None
|
||||
if "/" in raw:
|
||||
name, version = raw.split("/", 1)
|
||||
return (name or None), (version or None)
|
||||
parts = raw.split(maxsplit=1)
|
||||
if len(parts) == 2:
|
||||
return parts[0], parts[1]
|
||||
return raw, None
|
||||
|
||||
|
||||
def validate_initialize_metadata(payload: InitializeResponse) -> InitializeResponse:
|
||||
user_agent = (payload.userAgent or "").strip()
|
||||
server = payload.serverInfo
|
||||
|
||||
server_name: str | None = None
|
||||
server_version: str | None = None
|
||||
|
||||
if server is not None:
|
||||
server_name = (server.name or "").strip() or None
|
||||
server_version = (server.version or "").strip() or None
|
||||
|
||||
if (server_name is None or server_version is None) and user_agent:
|
||||
parsed_name, parsed_version = _split_user_agent(user_agent)
|
||||
if server_name is None:
|
||||
server_name = parsed_name
|
||||
if server_version is None:
|
||||
server_version = parsed_version
|
||||
|
||||
normalized_server_name = (server_name or "").strip()
|
||||
normalized_server_version = (server_version or "").strip()
|
||||
if not user_agent or not normalized_server_name or not normalized_server_version:
|
||||
raise RuntimeError(
|
||||
"initialize response missing required metadata "
|
||||
f"(user_agent={user_agent!r}, server_name={normalized_server_name!r}, server_version={normalized_server_version!r})"
|
||||
)
|
||||
|
||||
if server is None:
|
||||
payload.serverInfo = ServerInfo(
|
||||
name=normalized_server_name,
|
||||
version=normalized_server_version,
|
||||
)
|
||||
else:
|
||||
server.name = normalized_server_name
|
||||
server.version = normalized_server_version
|
||||
|
||||
return payload
|
||||
@@ -2,9 +2,14 @@ from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
from dataclasses import dataclass
|
||||
from enum import Enum
|
||||
from typing import AsyncIterator, Iterator, NoReturn
|
||||
from typing import AsyncIterator, Iterator
|
||||
|
||||
from ._approval_mode import (
|
||||
ApprovalMode as ApprovalMode,
|
||||
_approval_mode_override_settings,
|
||||
_approval_mode_settings,
|
||||
)
|
||||
from ._initialize_metadata import validate_initialize_metadata
|
||||
from ._inputs import (
|
||||
ImageInput as ImageInput,
|
||||
Input,
|
||||
@@ -25,9 +30,6 @@ from ._run import (
|
||||
from .async_client import AsyncAppServerClient
|
||||
from .client import AppServerClient, AppServerConfig
|
||||
from .generated.v2_all import (
|
||||
ApprovalsReviewer,
|
||||
AskForApproval,
|
||||
AskForApprovalValue,
|
||||
ModelListResponse,
|
||||
Personality,
|
||||
ReasoningEffort,
|
||||
@@ -55,61 +57,7 @@ from .generated.v2_all import (
|
||||
TurnStartParams,
|
||||
TurnSteerResponse,
|
||||
)
|
||||
from .models import InitializeResponse, JsonObject, Notification, ServerInfo
|
||||
|
||||
|
||||
def _split_user_agent(user_agent: str) -> tuple[str | None, str | None]:
|
||||
raw = user_agent.strip()
|
||||
if not raw:
|
||||
return None, None
|
||||
if "/" in raw:
|
||||
name, version = raw.split("/", 1)
|
||||
return (name or None), (version or None)
|
||||
parts = raw.split(maxsplit=1)
|
||||
if len(parts) == 2:
|
||||
return parts[0], parts[1]
|
||||
return raw, None
|
||||
|
||||
|
||||
class ApprovalMode(str, Enum):
|
||||
"""High-level approval behavior for escalated permission requests."""
|
||||
|
||||
deny_all = "deny_all"
|
||||
auto_review = "auto_review"
|
||||
|
||||
|
||||
def _approval_mode_settings(
|
||||
approval_mode: ApprovalMode,
|
||||
) -> tuple[AskForApproval, ApprovalsReviewer | None]:
|
||||
"""Map the public approval mode to generated app-server start params."""
|
||||
if not isinstance(approval_mode, ApprovalMode):
|
||||
supported = ", ".join(mode.value for mode in ApprovalMode)
|
||||
raise ValueError(f"approval_mode must be one of: {supported}")
|
||||
|
||||
match approval_mode:
|
||||
case ApprovalMode.auto_review:
|
||||
return (
|
||||
AskForApproval(root=AskForApprovalValue.on_request),
|
||||
ApprovalsReviewer.auto_review,
|
||||
)
|
||||
case ApprovalMode.deny_all:
|
||||
return AskForApproval(root=AskForApprovalValue.never), None
|
||||
case _:
|
||||
return _assert_never_approval_mode(approval_mode)
|
||||
|
||||
|
||||
def _assert_never_approval_mode(approval_mode: NoReturn) -> NoReturn:
|
||||
"""Make approval mode mapping exhaustive for static type checkers."""
|
||||
raise AssertionError(f"Unhandled approval mode: {approval_mode!r}")
|
||||
|
||||
|
||||
def _approval_mode_override_settings(
|
||||
approval_mode: ApprovalMode | None,
|
||||
) -> tuple[AskForApproval | None, ApprovalsReviewer | None]:
|
||||
"""Map an optional public approval mode to app-server override params."""
|
||||
if approval_mode is None:
|
||||
return None, None
|
||||
return _approval_mode_settings(approval_mode)
|
||||
from .models import InitializeResponse, JsonObject, Notification
|
||||
|
||||
|
||||
class Codex:
|
||||
@@ -119,7 +67,7 @@ class Codex:
|
||||
self._client = AppServerClient(config=config)
|
||||
try:
|
||||
self._client.start()
|
||||
self._init = self._validate_initialize(self._client.initialize())
|
||||
self._init = validate_initialize_metadata(self._client.initialize())
|
||||
except Exception:
|
||||
self._client.close()
|
||||
raise
|
||||
@@ -130,44 +78,6 @@ class Codex:
|
||||
def __exit__(self, _exc_type, _exc, _tb) -> None:
|
||||
self.close()
|
||||
|
||||
@staticmethod
|
||||
def _validate_initialize(payload: InitializeResponse) -> InitializeResponse:
|
||||
user_agent = (payload.userAgent or "").strip()
|
||||
server = payload.serverInfo
|
||||
|
||||
server_name: str | None = None
|
||||
server_version: str | None = None
|
||||
|
||||
if server is not None:
|
||||
server_name = (server.name or "").strip() or None
|
||||
server_version = (server.version or "").strip() or None
|
||||
|
||||
if (server_name is None or server_version is None) and user_agent:
|
||||
parsed_name, parsed_version = _split_user_agent(user_agent)
|
||||
if server_name is None:
|
||||
server_name = parsed_name
|
||||
if server_version is None:
|
||||
server_version = parsed_version
|
||||
|
||||
normalized_server_name = (server_name or "").strip()
|
||||
normalized_server_version = (server_version or "").strip()
|
||||
if not user_agent or not normalized_server_name or not normalized_server_version:
|
||||
raise RuntimeError(
|
||||
"initialize response missing required metadata "
|
||||
f"(user_agent={user_agent!r}, server_name={normalized_server_name!r}, server_version={normalized_server_version!r})"
|
||||
)
|
||||
|
||||
if server is None:
|
||||
payload.serverInfo = ServerInfo(
|
||||
name=normalized_server_name,
|
||||
version=normalized_server_version,
|
||||
)
|
||||
else:
|
||||
server.name = normalized_server_name
|
||||
server.version = normalized_server_version
|
||||
|
||||
return payload
|
||||
|
||||
@property
|
||||
def metadata(self) -> InitializeResponse:
|
||||
return self._init
|
||||
@@ -354,7 +264,7 @@ class AsyncCodex:
|
||||
try:
|
||||
await self._client.start()
|
||||
payload = await self._client.initialize()
|
||||
self._init = Codex._validate_initialize(payload)
|
||||
self._init = validate_initialize_metadata(payload)
|
||||
self._initialized = True
|
||||
except Exception:
|
||||
await self._client.close()
|
||||
|
||||
@@ -39,6 +39,61 @@ def test_thread_set_name_and_read(tmp_path) -> None:
|
||||
}
|
||||
|
||||
|
||||
def test_sync_and_async_initialization_round_trip_metadata(tmp_path) -> None:
|
||||
"""Public clients should initialize and start threads through app-server."""
|
||||
|
||||
async def async_scenario(harness: AppServerHarness) -> dict[str, object]:
|
||||
async with AsyncCodex(config=harness.app_server_config()) as codex:
|
||||
thread = await codex.thread_start()
|
||||
server = codex.metadata.serverInfo
|
||||
return {
|
||||
"thread_id": thread.id,
|
||||
"user_agent": codex.metadata.userAgent,
|
||||
"server_name": None if server is None else server.name,
|
||||
"server_version": None if server is None else server.version,
|
||||
}
|
||||
|
||||
with AppServerHarness(tmp_path) as harness:
|
||||
with Codex(config=harness.app_server_config()) as codex:
|
||||
thread = codex.thread_start()
|
||||
server = codex.metadata.serverInfo
|
||||
sync_summary = {
|
||||
"thread_id": thread.id,
|
||||
"user_agent": codex.metadata.userAgent,
|
||||
"server_name": None if server is None else server.name,
|
||||
"server_version": None if server is None else server.version,
|
||||
}
|
||||
async_summary = asyncio.run(async_scenario(harness))
|
||||
|
||||
assert {
|
||||
"sync": {
|
||||
"thread_id_present": bool(sync_summary["thread_id"]),
|
||||
"user_agent_present": bool(sync_summary["user_agent"]),
|
||||
"server_name_present": bool(sync_summary["server_name"]),
|
||||
"server_version_present": bool(sync_summary["server_version"]),
|
||||
},
|
||||
"async": {
|
||||
"thread_id_present": bool(async_summary["thread_id"]),
|
||||
"user_agent_present": bool(async_summary["user_agent"]),
|
||||
"server_name_present": bool(async_summary["server_name"]),
|
||||
"server_version_present": bool(async_summary["server_version"]),
|
||||
},
|
||||
} == {
|
||||
"sync": {
|
||||
"thread_id_present": True,
|
||||
"user_agent_present": True,
|
||||
"server_name_present": True,
|
||||
"server_version_present": True,
|
||||
},
|
||||
"async": {
|
||||
"thread_id_present": True,
|
||||
"user_agent_present": True,
|
||||
"server_name_present": True,
|
||||
"server_version_present": True,
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
def test_thread_list_filters_archived_threads(tmp_path) -> None:
|
||||
"""Thread listing should reflect archive state through app-server."""
|
||||
with AppServerHarness(tmp_path) as harness:
|
||||
|
||||
@@ -18,6 +18,7 @@ from openai_codex import (
|
||||
RunResult,
|
||||
Thread,
|
||||
)
|
||||
from openai_codex._initialize_metadata import validate_initialize_metadata
|
||||
from openai_codex.types import InitializeResponse
|
||||
|
||||
EXPECTED_ROOT_EXPORTS = [
|
||||
@@ -444,7 +445,7 @@ def test_lifecycle_methods_are_codex_scoped() -> None:
|
||||
def test_initialize_metadata_parses_user_agent_shape() -> None:
|
||||
"""Initialize metadata should accept the legacy user-agent-only payload shape."""
|
||||
payload = InitializeResponse.model_validate({"userAgent": "codex-cli/1.2.3"})
|
||||
parsed = Codex._validate_initialize(payload)
|
||||
parsed = validate_initialize_metadata(payload)
|
||||
assert parsed is payload
|
||||
assert parsed.userAgent == "codex-cli/1.2.3"
|
||||
assert parsed.serverInfo is not None
|
||||
@@ -455,7 +456,7 @@ def test_initialize_metadata_parses_user_agent_shape() -> None:
|
||||
def test_initialize_metadata_requires_non_empty_information() -> None:
|
||||
"""Initialize metadata should fail when the runtime gives no identity signal."""
|
||||
try:
|
||||
Codex._validate_initialize(InitializeResponse.model_validate({}))
|
||||
validate_initialize_metadata(InitializeResponse.model_validate({}))
|
||||
except RuntimeError as exc:
|
||||
assert "missing required metadata" in str(exc)
|
||||
else:
|
||||
|
||||
Reference in New Issue
Block a user