Add OCI Generative AI model support via request-signing client#3612
Add OCI Generative AI model support via request-signing client#3612fede-kamel wants to merge 5 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5cc66ce73
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if "message" in payload: | ||
| message = payload.get("message") or {} | ||
| content = message.get("content") or [] | ||
| text = "".join(part.get("text", "") for part in content if isinstance(part, dict)) | ||
| raw_tool_calls = message.get("toolCalls") or message.get("tool_calls") or [] | ||
| else: | ||
| text = payload.get("text") or "" | ||
| raw_tool_calls = payload.get("toolCalls") or payload.get("tool_calls") or [] |
There was a problem hiding this comment.
When using the native OCI SDK transport with stream=True, the service streams GenericChatResponse-shaped frames where the generated message and finish reason live under choices[0].message / choices[0].finishReason, not as top-level message or text fields. This parser ignores those choices frames, so real Runner.run_streamed(...) calls over the SDK transport can yield no text/tool-call chunks and finish with an empty output despite the service returning tokens. Please unwrap the first choice before extracting content, tool calls, finish reason, and usage.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch for that revision — this applied to the native SDK transport (oci_sdk_model.py), which was removed when the PR was narrowed to the OpenAI-compatible chat completions and Responses endpoints. The streaming parser no longer exists in the diff. If the native transport comes back as a follow-up, the parser will unwrap choices[0] frames as suggested.
Adds optional-dependency support for Oracle Cloud Infrastructure Generative AI under agents.extensions.models: an httpx auth hook that performs OCI request signing (API key, session token, instance and resource principals, with transparent refresh) plus thin OCIChatCompletionsModel / OCIResponsesModel subclasses and an OCIProvider that route through the service's OpenAI-compatible chat completions and Responses endpoints. Includes unit tests and end-to-end runner tests against mocked transports.
fede-kamel
force-pushed
the
feat/oci-genai-provider
branch
from
d5cc66c to
01c45ef
Compare
fede-kamel
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01c45ef410
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| openai_client = build_signed_openai_client( | ||
| client_config, request_timeout=request_timeout | ||
| ) |
There was a problem hiding this comment.
When callers use the documented direct model construction path without passing openai_client, this creates an AsyncOpenAI/httpx.AsyncClient owned by the model, but neither OCIChatCompletionsModel nor the analogous OCIResponsesModel overrides Model.close(), so even callers that follow the model close contract cannot release the connection pool. In long-running apps that create and retire OCI models, this leaks sockets; track whether the client was created internally and close it from close().
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6018d72. Both model classes now track whether they created the client (openai_client=None path) and override close() to release it, while leaving caller-provided clients untouched. Added tests covering both cases.
The OCI model classes own the AsyncOpenAI client they build when the caller does not pass one; release its connection pool when the model is closed, while leaving caller-provided clients untouched.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6018d72057
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return AsyncOpenAI( | ||
| base_url=oci_openai_base_url(client_config.region), | ||
| api_key="oci-request-signing", | ||
| http_client=http_client, | ||
| ) |
There was a problem hiding this comment.
When callers use the new direct constructors or OCIProvider, this builder creates an AsyncOpenAI client without the OCI Generative AI project OCID. Oracle's OpenAI-compatible examples initialize the OpenAI client with project="ocid1.generativeaiproject..." for both API-key and IAM auth (see https://docs.oracle.com/en-us/iaas/Content/generative-ai/get-started-agents.htm); this code only adds an opc-compartment-id header via the signer, so real /responses and /chat/completions calls made through this path lack the project scoping the endpoint expects unless the caller bypasses the builder with a preconfigured client.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 7586e87. build_signed_openai_client, both model classes, and OCIProvider now accept an optional project_id (ocid1.generativeaiproject...), forwarded as the OpenAI client's project so the OpenAI-Project header reaches the service alongside the signature and opc-compartment-id headers. Kept optional rather than required: per the linked docs, projects scope response/conversation retention and memory settings on the Responses endpoint, and requests verified against the live service succeed without one on tenancies that don't use projects. Added a test asserting the header lands on the wire when set.
Adds an optional project_id (ocid1.generativeaiproject...) to the signed-client builder, both model classes, and OCIProvider, forwarded as the OpenAI client's project so the OpenAI-Project header reaches the service. Projects scope response/conversation retention and memory settings on the Responses endpoint; the parameter stays optional since requests succeed without it on tenancies that do not use projects.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7586e87963
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| client_config: OCIClientConfig, | ||
| *, | ||
| request_timeout: float = DEFAULT_REQUEST_TIMEOUT, | ||
| project_id: str | None = None, |
There was a problem hiding this comment.
With project_id defaulting to None here (and the new model/provider constructors also leaving it optional), the documented/default path such as OCIChatCompletionsModel(..., compartment_id=...) builds an OpenAI client that omits the project header. Oracle's OCI OpenAI-compatible API requires a Generative AI project OCID for calls (see https://docs.oracle.com/en-us/iaas/Content/generative-ai/use-project.htm), so these requests fail unless callers discover and pass the new optional argument. Fresh evidence in this revision is that the code now supports project_id but still defaults it to None, so the default integration path remains unusable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in f5fd275 — the project OCID now also resolves from the OCI_PROJECT_ID environment variable. See the reply on the duplicate thread from the latest review for the full rationale on keeping it optional rather than required.
Replaces the hand-rolled request signer with Oracle's oci-openai package, which provides the AsyncOpenAI-compatible client, all four IAM auth modes with refresh, and compartment routing. The extension now only contributes the thin model subclasses, credential/profile resolution, and the provider. The oci extra becomes oci-openai>=1.1.0.
|
Update: replaced the hand-rolled request signer with Oracle's official |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6131f11be
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| config_file: str | None = None, | ||
| region: str | None = None, | ||
| compartment_id: str | None = None, | ||
| project_id: str | None = None, |
There was a problem hiding this comment.
When project_id is omitted, this builds an OCI OpenAI-compatible client without the OpenAI-Project header, so common calls like OCIProvider(compartment_id=...) or the constructor examples in this file will fail against the real service even though the mocked tests pass. Oracle's current setup docs state that OCI OpenAI-compatible API calls require a project and show project=... in the IAM client examples (https://docs.oracle.com/en-us/iaas/Content/generative-ai/use-project.htm), so this should either require project_id/an env fallback up front or raise a UserError before making the unusable client.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in f5fd275. The project OCID now resolves from the explicit project_id argument or the OCI_PROJECT_ID environment variable (the env-fallback option you suggested), so deployments can supply it without code changes, and the docstrings now state Oracle's requirement up front. I kept it non-fatal rather than raising UserError: requests verified against the live service (chat completions and Responses, non-streaming and streaming, multi-turn tool loops) succeed without a project on tenancies where enforcement isn't active, so hard-failing at client construction would break currently-working setups. If the service starts rejecting project-less requests universally, its error will surface directly and we can tighten this to an upfront error.
Oracle documents that projects are required to call the OCI OpenAI-compatible API, so deployments need a code-free way to supply the project OCID. Resolution order is the explicit project_id argument, then the OCI_PROJECT_ID environment variable. The value stays optional because tenancies without project enforcement accept requests without one.
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
2 participants
Summary
Adds an Oracle Cloud Infrastructure (OCI) Generative AI integration under
agents.extensions.models, as proposed in #3611, following the same optional-extra pattern as the existing LiteLLM integration (pip install 'openai-agents[oci]').OCI Generative AI hosts OpenAI models (
openai.gpt-4o,openai.gpt-5, ...) on OpenAI-compatiblechat/completionsandresponsesendpoints, but behind OCI IAM — which signs each request instead of using bearer tokens. That makes the documentedbase_urlintegration path impossible as-is, and neither bundled adapter covers it (Any-LLM has no OCI backend; LiteLLM's OCI support is a beta subset without the Responses endpoint or credential refresh).The integration is deliberately thin and delegates all OCI-specific transport concerns to Oracle's official
oci-openaipackage (anAsyncOpenAIsubclass that performs request signing for all four IAM auth modes with credential refresh, and attaches the required compartment header — Oracle already tests it againstopenai-agentsin its own dev dependencies):oci_model.py—OCIChatCompletionsModelandOCIResponsesModel: thin subclasses ofOpenAIChatCompletionsModel/OpenAIResponsesModelconstructed over anoci-openaiclient. Convenience credential resolution from~/.oci/configprofiles (auth mode inferred from the profile: session token vs API key; principal modes selectable explicitly), region/compartment resolution from arguments, env vars, or the profile, optionalproject_idfor OCI Generative AI project scoping (sent asOpenAI-Project). All conversion, streaming, tool-calling, andprevious_response_idbehavior is inherited from the existing model implementations, so there is no second conversion layer to maintain. Models close internally created clients fromModel.close().oci_provider.py—OCIProvider: aModelProviderthat hands out chat-completions models by default and Responses models via aresponses:model-name prefix (Responses-only models cannot be detected from the name), sharing one signed client.The
ociextra isoci-openai>=1.1.0, mirroring howlitellm,redis, andboto3are handled. Nothing is imported from core code; a missing dependency raises the same friendly install hint the LiteLLM extension uses.Relative to the proposal in #3611, the native-SDK transport for dedicated AI cluster endpoints is intentionally left out of this PR to keep the surface small; it can be a follow-up if there's interest.
Test plan
tests/models/test_oci_model.py— unit tests for the client builder (base URL, compartment header, project propagation, missing-compartment error), auth-mode selection (session token inferred from profile, explicit overrides), client close-ownership semantics, and provider routing.tests/models/test_oci_integration.py— end-to-endRunnerloops againsthttpx.MockTransport, exercising wire shapes through the real OpenAI client: a multi-turn tool round-trip over chat completions, a Responses-endpoint run, theOpenAI-Projectheader, and a streamed run.make format,make lint,make typecheck(mypy + pyright), andmake tests(full suite) all pass locally.OCIProviderrouting viaRunConfig.Issue number
Closes #3611
Checks
make lintandmake format