diff --git a/sdk/python/scripts/update_sdk_artifacts.py b/sdk/python/scripts/update_sdk_artifacts.py index 9053091cea..1889026a01 100755 --- a/sdk/python/scripts/update_sdk_artifacts.py +++ b/sdk/python/scripts/update_sdk_artifacts.py @@ -881,16 +881,23 @@ def _kw_signature_lines(fields: list[PublicFieldSpec]) -> list[str]: return lines -def _approval_mode_signature_lines() -> list[str]: - """Return the public approval mode kwarg emitted on start helpers.""" +def _approval_mode_start_signature_lines() -> list[str]: + """Return the approval mode kwarg for new threads.""" return [" approval_mode: ApprovalMode = ApprovalMode.auto_review,"] -def _approval_mode_assignment_line(*, indent: str = " ") -> str: +def _approval_mode_override_signature_lines() -> list[str]: + """Return the optional approval mode kwarg for override-style helpers.""" + return [" approval_mode: ApprovalMode | None = None,"] + + +def _approval_mode_assignment_line( + helper_name: str, *, indent: str = " " +) -> str: """Return the local mapping from public mode to app-server params.""" return ( f"{indent}approval_policy, approvals_reviewer = " - "_approval_mode_settings(approval_mode)" + f"{helper_name}(approval_mode)" ) @@ -929,10 +936,10 @@ def _render_codex_block( " def thread_start(", " self,", " *,", - *_approval_mode_signature_lines(), + *_approval_mode_start_signature_lines(), *_kw_signature_lines(thread_start_fields), " ) -> Thread:", - _approval_mode_assignment_line(), + _approval_mode_assignment_line("_approval_mode_settings"), " params = ThreadStartParams(", *_approval_mode_model_arg_lines(), *_model_arg_lines(thread_start_fields), @@ -954,10 +961,10 @@ def _render_codex_block( " self,", " thread_id: str,", " *,", - *_approval_mode_signature_lines(), + *_approval_mode_override_signature_lines(), *_kw_signature_lines(resume_fields), " ) -> Thread:", - _approval_mode_assignment_line(), + _approval_mode_assignment_line("_approval_mode_override_settings"), " params = ThreadResumeParams(", " thread_id=thread_id,", *_approval_mode_model_arg_lines(), @@ -970,10 +977,10 @@ def _render_codex_block( " self,", " thread_id: str,", " *,", - *_approval_mode_signature_lines(), + *_approval_mode_override_signature_lines(), *_kw_signature_lines(fork_fields), " ) -> Thread:", - _approval_mode_assignment_line(), + _approval_mode_assignment_line("_approval_mode_override_settings"), " params = ThreadForkParams(", " thread_id=thread_id,", *_approval_mode_model_arg_lines(), @@ -1002,11 +1009,11 @@ def _render_async_codex_block( " async def thread_start(", " self,", " *,", - *_approval_mode_signature_lines(), + *_approval_mode_start_signature_lines(), *_kw_signature_lines(thread_start_fields), " ) -> AsyncThread:", " await self._ensure_initialized()", - _approval_mode_assignment_line(), + _approval_mode_assignment_line("_approval_mode_settings"), " params = ThreadStartParams(", *_approval_mode_model_arg_lines(), *_model_arg_lines(thread_start_fields), @@ -1029,11 +1036,11 @@ def _render_async_codex_block( " self,", " thread_id: str,", " *,", - *_approval_mode_signature_lines(), + *_approval_mode_override_signature_lines(), *_kw_signature_lines(resume_fields), " ) -> AsyncThread:", " await self._ensure_initialized()", - _approval_mode_assignment_line(), + _approval_mode_assignment_line("_approval_mode_override_settings"), " params = ThreadResumeParams(", " thread_id=thread_id,", *_approval_mode_model_arg_lines(), @@ -1046,11 +1053,11 @@ def _render_async_codex_block( " self,", " thread_id: str,", " *,", - *_approval_mode_signature_lines(), + *_approval_mode_override_signature_lines(), *_kw_signature_lines(fork_fields), " ) -> AsyncThread:", " await self._ensure_initialized()", - _approval_mode_assignment_line(), + _approval_mode_assignment_line("_approval_mode_override_settings"), " params = ThreadForkParams(", " thread_id=thread_id,", *_approval_mode_model_arg_lines(), @@ -1079,11 +1086,11 @@ def _render_thread_block( " self,", " input: Input,", " *,", - *_approval_mode_signature_lines(), + *_approval_mode_override_signature_lines(), *_kw_signature_lines(turn_fields), " ) -> TurnHandle:", " wire_input = _to_wire_input(input)", - _approval_mode_assignment_line(), + _approval_mode_assignment_line("_approval_mode_override_settings"), " params = TurnStartParams(", " thread_id=self.id,", " input=wire_input,", @@ -1104,12 +1111,12 @@ def _render_async_thread_block( " self,", " input: Input,", " *,", - *_approval_mode_signature_lines(), + *_approval_mode_override_signature_lines(), *_kw_signature_lines(turn_fields), " ) -> AsyncTurnHandle:", " await self._codex._ensure_initialized()", " wire_input = _to_wire_input(input)", - _approval_mode_assignment_line(), + _approval_mode_assignment_line("_approval_mode_override_settings"), " params = TurnStartParams(", " thread_id=self.id,", " input=wire_input,", diff --git a/sdk/python/src/openai_codex/api.py b/sdk/python/src/openai_codex/api.py index ce41f16211..683575c7e2 100644 --- a/sdk/python/src/openai_codex/api.py +++ b/sdk/python/src/openai_codex/api.py @@ -103,6 +103,15 @@ def _assert_never_approval_mode(approval_mode: NoReturn) -> NoReturn: 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) + + class Codex: """Minimal typed SDK surface for app-server v2.""" @@ -242,7 +251,7 @@ class Codex: self, thread_id: str, *, - approval_mode: ApprovalMode = ApprovalMode.auto_review, + approval_mode: ApprovalMode | None = None, base_instructions: str | None = None, config: JsonObject | None = None, cwd: str | None = None, @@ -253,7 +262,9 @@ class Codex: sandbox: SandboxMode | None = None, service_tier: str | None = None, ) -> Thread: - approval_policy, approvals_reviewer = _approval_mode_settings(approval_mode) + approval_policy, approvals_reviewer = _approval_mode_override_settings( + approval_mode + ) params = ThreadResumeParams( thread_id=thread_id, approval_policy=approval_policy, @@ -275,7 +286,7 @@ class Codex: self, thread_id: str, *, - approval_mode: ApprovalMode = ApprovalMode.auto_review, + approval_mode: ApprovalMode | None = None, base_instructions: str | None = None, config: JsonObject | None = None, cwd: str | None = None, @@ -287,7 +298,9 @@ class Codex: service_tier: str | None = None, thread_source: ThreadSource | None = None, ) -> Thread: - approval_policy, approvals_reviewer = _approval_mode_settings(approval_mode) + approval_policy, approvals_reviewer = _approval_mode_override_settings( + approval_mode + ) params = ThreadForkParams( thread_id=thread_id, approval_policy=approval_policy, @@ -445,7 +458,7 @@ class AsyncCodex: self, thread_id: str, *, - approval_mode: ApprovalMode = ApprovalMode.auto_review, + approval_mode: ApprovalMode | None = None, base_instructions: str | None = None, config: JsonObject | None = None, cwd: str | None = None, @@ -457,7 +470,9 @@ class AsyncCodex: service_tier: str | None = None, ) -> AsyncThread: await self._ensure_initialized() - approval_policy, approvals_reviewer = _approval_mode_settings(approval_mode) + approval_policy, approvals_reviewer = _approval_mode_override_settings( + approval_mode + ) params = ThreadResumeParams( thread_id=thread_id, approval_policy=approval_policy, @@ -479,7 +494,7 @@ class AsyncCodex: self, thread_id: str, *, - approval_mode: ApprovalMode = ApprovalMode.auto_review, + approval_mode: ApprovalMode | None = None, base_instructions: str | None = None, config: JsonObject | None = None, cwd: str | None = None, @@ -492,7 +507,9 @@ class AsyncCodex: thread_source: ThreadSource | None = None, ) -> AsyncThread: await self._ensure_initialized() - approval_policy, approvals_reviewer = _approval_mode_settings(approval_mode) + approval_policy, approvals_reviewer = _approval_mode_override_settings( + approval_mode + ) params = ThreadForkParams( thread_id=thread_id, approval_policy=approval_policy, @@ -536,7 +553,7 @@ class Thread: self, input: RunInput, *, - approval_mode: ApprovalMode = ApprovalMode.auto_review, + approval_mode: ApprovalMode | None = None, cwd: str | None = None, effort: ReasoningEffort | None = None, model: str | None = None, @@ -569,7 +586,7 @@ class Thread: self, input: Input, *, - approval_mode: ApprovalMode = ApprovalMode.auto_review, + approval_mode: ApprovalMode | None = None, cwd: str | None = None, effort: ReasoningEffort | None = None, model: str | None = None, @@ -580,7 +597,9 @@ class Thread: summary: ReasoningSummary | None = None, ) -> TurnHandle: wire_input = _to_wire_input(input) - approval_policy, approvals_reviewer = _approval_mode_settings(approval_mode) + approval_policy, approvals_reviewer = _approval_mode_override_settings( + approval_mode + ) params = TurnStartParams( thread_id=self.id, input=wire_input, @@ -619,7 +638,7 @@ class AsyncThread: self, input: RunInput, *, - approval_mode: ApprovalMode = ApprovalMode.auto_review, + approval_mode: ApprovalMode | None = None, cwd: str | None = None, effort: ReasoningEffort | None = None, model: str | None = None, @@ -652,7 +671,7 @@ class AsyncThread: self, input: Input, *, - approval_mode: ApprovalMode = ApprovalMode.auto_review, + approval_mode: ApprovalMode | None = None, cwd: str | None = None, effort: ReasoningEffort | None = None, model: str | None = None, @@ -664,7 +683,9 @@ class AsyncThread: ) -> AsyncTurnHandle: await self._codex._ensure_initialized() wire_input = _to_wire_input(input) - approval_policy, approvals_reviewer = _approval_mode_settings(approval_mode) + approval_policy, approvals_reviewer = _approval_mode_override_settings( + approval_mode + ) params = TurnStartParams( thread_id=self.id, input=wire_input, diff --git a/sdk/python/tests/test_public_api_runtime_behavior.py b/sdk/python/tests/test_public_api_runtime_behavior.py index 5b44c3eb2f..e30e9dcf9b 100644 --- a/sdk/python/tests/test_public_api_runtime_behavior.py +++ b/sdk/python/tests/test_public_api_runtime_behavior.py @@ -261,6 +261,64 @@ def _approval_mode_turn_params(approval_mode: ApprovalMode) -> TurnStartParams: ) +class CapturingApprovalClient: + """Collect wrapper params at the app-server client boundary.""" + + def __init__(self) -> None: + self.params: list[Any] = [] + + def thread_start(self, params: Any) -> SimpleNamespace: + self.params.append(params) + return SimpleNamespace(thread=SimpleNamespace(id="thread-1")) + + def thread_resume(self, thread_id: str, params: Any) -> SimpleNamespace: + self.params.append(params) + return SimpleNamespace(thread=SimpleNamespace(id=thread_id)) + + def thread_fork(self, thread_id: str, params: Any) -> SimpleNamespace: + self.params.append(params) + return SimpleNamespace(thread=SimpleNamespace(id=f"{thread_id}-fork")) + + def turn_start( + self, + thread_id: str, + input: object, # noqa: A002 + *, + params: Any, + ) -> SimpleNamespace: + self.params.append(params) + return SimpleNamespace(turn=SimpleNamespace(id=f"{thread_id}-turn")) + + +class CapturingAsyncApprovalClient: + """Async mirror of CapturingApprovalClient for public async wrappers.""" + + def __init__(self) -> None: + self.params: list[Any] = [] + + async def thread_start(self, params: Any) -> SimpleNamespace: + self.params.append(params) + return SimpleNamespace(thread=SimpleNamespace(id="thread-1")) + + async def thread_resume(self, thread_id: str, params: Any) -> SimpleNamespace: + self.params.append(params) + return SimpleNamespace(thread=SimpleNamespace(id=thread_id)) + + async def thread_fork(self, thread_id: str, params: Any) -> SimpleNamespace: + self.params.append(params) + return SimpleNamespace(thread=SimpleNamespace(id=f"{thread_id}-fork")) + + async def turn_start( + self, + thread_id: str, + input: object, # noqa: A002 + *, + params: Any, + ) -> SimpleNamespace: + self.params.append(params) + return SimpleNamespace(turn=SimpleNamespace(id=f"{thread_id}-turn")) + + def test_approval_modes_serialize_to_expected_start_params() -> None: """ApprovalMode should map to the app-server params sent for new work.""" assert { @@ -281,6 +339,59 @@ def test_unknown_approval_mode_is_rejected() -> None: public_api_module._approval_mode_settings("allow_all") # type: ignore[arg-type] +def test_approval_defaults_preserve_existing_sync_thread_settings() -> None: + """Only thread creation should write approval defaults unless callers override.""" + client = CapturingApprovalClient() + codex = Codex.__new__(Codex) + codex._client = client + + started = codex.thread_start(approval_mode=ApprovalMode.deny_all) + started.turn([]) + codex.thread_resume("existing-thread") + codex.thread_fork("existing-thread") + started.turn([], approval_mode=ApprovalMode.auto_review) + + assert _approval_settings(client.params) == [ + {"approvalPolicy": "never"}, + {}, + {}, + {}, + { + "approvalPolicy": "on-request", + "approvalsReviewer": "auto_review", + }, + ] + + +def test_approval_defaults_preserve_existing_async_thread_settings() -> None: + """Async wrappers should follow the same approval override semantics.""" + + async def scenario() -> None: + client = CapturingAsyncApprovalClient() + codex = AsyncCodex() + codex._client = client # type: ignore[assignment] + codex._initialized = True + + started = await codex.thread_start(approval_mode=ApprovalMode.deny_all) + await started.turn([]) + await codex.thread_resume("existing-thread") + await codex.thread_fork("existing-thread") + await started.turn([], approval_mode=ApprovalMode.auto_review) + + assert _approval_settings(client.params) == [ + {"approvalPolicy": "never"}, + {}, + {}, + {}, + { + "approvalPolicy": "on-request", + "approvalsReviewer": "auto_review", + }, + ] + + asyncio.run(scenario()) + + def test_turn_streams_can_consume_multiple_turns_on_one_client() -> None: """Two sync TurnHandle streams should advance independently on one client.""" client = AppServerClient() diff --git a/sdk/python/tests/test_public_api_signatures.py b/sdk/python/tests/test_public_api_signatures.py index 6d540bd30f..f17ec5d4fc 100644 --- a/sdk/python/tests/test_public_api_signatures.py +++ b/sdk/python/tests/test_public_api_signatures.py @@ -383,15 +383,25 @@ def test_generated_public_signatures_are_snake_case_and_typed() -> None: _assert_no_any_annotations(fn) -def test_generated_public_methods_default_to_auto_review() -> None: - """Thread and turn starts should use auto-review unless callers opt out.""" +def test_new_thread_methods_default_to_auto_review() -> None: + """New threads should start with auto-review unless callers opt out.""" funcs = [ Codex.thread_start, + AsyncCodex.thread_start, + ] + + assert {fn: _keyword_default(fn, "approval_mode") for fn in funcs} == { + fn: ApprovalMode.auto_review for fn in funcs + } + + +def test_existing_thread_methods_default_to_preserving_approval_settings() -> None: + """Existing thread operations should not serialize approval overrides by default.""" + funcs = [ Codex.thread_resume, Codex.thread_fork, Thread.turn, Thread.run, - AsyncCodex.thread_start, AsyncCodex.thread_resume, AsyncCodex.thread_fork, AsyncThread.turn, @@ -399,7 +409,7 @@ def test_generated_public_methods_default_to_auto_review() -> None: ] assert {fn: _keyword_default(fn, "approval_mode") for fn in funcs} == { - fn: ApprovalMode.auto_review for fn in funcs + fn: None for fn in funcs }