mirror of
https://github.com/openai/codex.git
synced 2026-05-03 10:56:37 +00:00
17ae906048
## Why This keeps the partial Guardian subagent -> Auto-review rename forward-compatible across mixed Codex installations. Newer binaries need to understand the new `auto_review` spelling, but they cannot write it to shared `~/.codex/config.toml` yet because older CLI/app-server bundles only know `user` and `guardian_subagent` and can fail during config load before recovering. The Python SDK had the opposite compatibility gap: app-server responses can contain `approvalsReviewer: "auto_review"`, but the checked-in generated SDK enum did not accept that value. ## What Changed - Keep `ApprovalsReviewer::AutoReview` readable from both `guardian_subagent` and `auto_review`, while serializing it as `guardian_subagent` in both protocol crates. - Update TUI Auto-review persistence tests so enabling Auto-review writes `approvals_reviewer = "guardian_subagent"` while UI copy still says Auto-review. - Map managed/cloud `feature_requirements.auto_review` to the existing `Feature::GuardianApproval` gate without adding a broad local `[features].auto_review` key or changing config writes. - Add `auto_review` to the Python SDK `ApprovalsReviewer` enum and cover `ThreadResumeResponse` validation. ## Testing - `cargo test -p codex-protocol approvals_reviewer` - `cargo test -p codex-app-server-protocol approvals_reviewer` - `cargo test -p codex-tui update_feature_flags_enabling_guardian_selects_auto_review` - `cargo test -p codex-tui update_feature_flags_enabling_guardian_in_profile_sets_profile_auto_review_policy` - `cargo test -p codex-core feature_requirements_auto_review_disables_guardian_approval` - `pytest sdk/python/tests/test_client_rpc_methods.py::test_thread_resume_response_accepts_auto_review_reviewer` - `git diff --check`
131 lines
4.1 KiB
Python
131 lines
4.1 KiB
Python
from __future__ import annotations
|
|
|
|
from pathlib import Path
|
|
from typing import Any
|
|
|
|
from codex_app_server.client import AppServerClient, _params_dict
|
|
from codex_app_server.generated.v2_all import (
|
|
ApprovalsReviewer,
|
|
ThreadListParams,
|
|
ThreadResumeResponse,
|
|
ThreadTokenUsageUpdatedNotification,
|
|
)
|
|
from codex_app_server.models import UnknownNotification
|
|
|
|
ROOT = Path(__file__).resolve().parents[1]
|
|
|
|
|
|
def test_thread_set_name_and_compact_use_current_rpc_methods() -> None:
|
|
client = AppServerClient()
|
|
calls: list[tuple[str, dict[str, Any] | None]] = []
|
|
|
|
def fake_request(method: str, params, *, response_model): # type: ignore[no-untyped-def]
|
|
calls.append((method, params))
|
|
return response_model.model_validate({})
|
|
|
|
client.request = fake_request # type: ignore[method-assign]
|
|
|
|
client.thread_set_name("thread-1", "sdk-name")
|
|
client.thread_compact("thread-1")
|
|
|
|
assert calls[0][0] == "thread/name/set"
|
|
assert calls[1][0] == "thread/compact/start"
|
|
|
|
|
|
def test_generated_params_models_are_snake_case_and_dump_by_alias() -> None:
|
|
params = ThreadListParams(search_term="needle", limit=5)
|
|
|
|
assert "search_term" in ThreadListParams.model_fields
|
|
dumped = _params_dict(params)
|
|
assert dumped == {"searchTerm": "needle", "limit": 5}
|
|
|
|
|
|
def test_generated_v2_bundle_has_single_shared_plan_type_definition() -> None:
|
|
source = (ROOT / "src" / "codex_app_server" / "generated" / "v2_all.py").read_text()
|
|
assert source.count("class PlanType(") == 1
|
|
|
|
|
|
def test_thread_resume_response_accepts_auto_review_reviewer() -> None:
|
|
response = ThreadResumeResponse.model_validate(
|
|
{
|
|
"approvalPolicy": "on-request",
|
|
"approvalsReviewer": "auto_review",
|
|
"cwd": "/tmp",
|
|
"model": "gpt-5",
|
|
"modelProvider": "openai",
|
|
"sandbox": {"type": "dangerFullAccess"},
|
|
"thread": {
|
|
"cliVersion": "1.0.0",
|
|
"createdAt": 1,
|
|
"cwd": "/tmp",
|
|
"ephemeral": False,
|
|
"id": "thread-1",
|
|
"modelProvider": "openai",
|
|
"preview": "",
|
|
"source": "cli",
|
|
"status": {"type": "idle"},
|
|
"turns": [],
|
|
"updatedAt": 1,
|
|
},
|
|
}
|
|
)
|
|
|
|
assert response.approvals_reviewer is ApprovalsReviewer.auto_review
|
|
|
|
|
|
def test_notifications_are_typed_with_canonical_v2_methods() -> None:
|
|
client = AppServerClient()
|
|
event = client._coerce_notification(
|
|
"thread/tokenUsage/updated",
|
|
{
|
|
"threadId": "thread-1",
|
|
"turnId": "turn-1",
|
|
"tokenUsage": {
|
|
"last": {
|
|
"cachedInputTokens": 0,
|
|
"inputTokens": 1,
|
|
"outputTokens": 2,
|
|
"reasoningOutputTokens": 0,
|
|
"totalTokens": 3,
|
|
},
|
|
"total": {
|
|
"cachedInputTokens": 0,
|
|
"inputTokens": 1,
|
|
"outputTokens": 2,
|
|
"reasoningOutputTokens": 0,
|
|
"totalTokens": 3,
|
|
},
|
|
},
|
|
},
|
|
)
|
|
|
|
assert event.method == "thread/tokenUsage/updated"
|
|
assert isinstance(event.payload, ThreadTokenUsageUpdatedNotification)
|
|
assert event.payload.turn_id == "turn-1"
|
|
|
|
|
|
def test_unknown_notifications_fall_back_to_unknown_payloads() -> None:
|
|
client = AppServerClient()
|
|
event = client._coerce_notification(
|
|
"unknown/notification",
|
|
{
|
|
"id": "evt-1",
|
|
"conversationId": "thread-1",
|
|
"msg": {"type": "turn_aborted"},
|
|
},
|
|
)
|
|
|
|
assert event.method == "unknown/notification"
|
|
assert isinstance(event.payload, UnknownNotification)
|
|
assert event.payload.params["msg"] == {"type": "turn_aborted"}
|
|
|
|
|
|
def test_invalid_notification_payload_falls_back_to_unknown() -> None:
|
|
client = AppServerClient()
|
|
event = client._coerce_notification(
|
|
"thread/tokenUsage/updated", {"threadId": "missing"}
|
|
)
|
|
|
|
assert event.method == "thread/tokenUsage/updated"
|
|
assert isinstance(event.payload, UnknownNotification)
|