mirror of
https://github.com/openai/codex.git
synced 2026-05-25 05:24:37 +00:00
Preserve approval settings by default
Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -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,",
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user