feat(api): Add service account key expiration support - #689
Conversation
Castiron custom code✅ No new custom-code files detected. 65 mixed files remain; 0 existing customizations changed; 37 generated baselines changed. Compared
28 existing customizations unchanged
A changed generated baseline means this report cannot reliably identify which handwritten lines changed. Inspect the custom-code diffDownload the exact patch produced by this run (requires repository access): gh run download 34364852319 --repo openai/openai-ruby \
--name castiron-custom-code-34364852319-1 --dir /tmp/castiron-custom-code-34364852319-1
git apply --stat /tmp/castiron-custom-code-34364852319-1/custom-code.patch
cat /tmp/castiron-custom-code-34364852319-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin acf5db808c521a1793439fe4e2975c78a9e9e73b 7cd69caf720f3980e5e833aab0e7ef81bf8b1baa
python3 scripts/castiron/custom_code_report.py report \
--base acf5db808c521a1793439fe4e2975c78a9e9e73b \
--head 7cd69caf720f3980e5e833aab0e7ef81bf8b1baa --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-7cd69caf720f
cat /tmp/castiron-custom-code-7cd69caf720f/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 4d27f7a905
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # `create_service_account_only` is true. | ||
| # | ||
| # @return [Integer, nil] | ||
| optional :expires_in_seconds, Integer, nil?: true |
There was a problem hiding this comment.
The existing focused expiration test only exercises service_accounts.api_keys.create; no test sends expires_in_seconds through the newly changed service_accounts.create path or verifies that its nested API-key response preserves expires_at. Consequently, a request-conversion or deserialization regression in this new path could pass the suite, so add a focused request/response test for initial-key expiration.
AGENTS.md reference: AGENTS.md:L56-L60
Useful? React with 👍 / 👎.
markstuart-oai
left a comment
markstuart-oai
left a comment
There was a problem hiding this comment.
Reviewed the expiration models, request serialization, response decoding, type signatures, and generated documentation changes. The new fields use the existing conversion path without adding special-case policy logic. Offline probes passed for omitted/null/bounded lifetimes and omitted/null/integer expiration responses. Two small follow-ups below: retain regression coverage for the initial-create path and correct a generated documentation type name.
Eight model files cross 1,000 lines through documentation expansion alone: beta/beta_tool, evals/run_cancel_response, evals/run_create_response, evals/run_list_response, evals/run_retrieve_response, responses/response_create_params, responses/responses_client_event, and responses/tool. Their non-comment executable tokens are unchanged, and the model and constructor help remains organized around existing fields and types. I am waiving decomposition for these specific crossings: splitting runtime types solely to relocate this documentation would add indirection without removing concepts.
| # | ||
| # @param action [OpenAI::Models::Responses::ResponseFunctionWebSearch::Action::Search, OpenAI::Models::Responses::ResponseFunctionWebSearch::Action::OpenPage, OpenAI::Models::Responses::ResponseFunctionWebSearch::Action::Find] An object describing the specific action taken in this web search call. | ||
| # @param action [OpenAI::Models::Responses::ResponseFunctionWebSearch::Action::Search, OpenAI::Models::Responses::ResponseFunctionWebSearch::Action::OpenPage, OpenAI::Models::Responses::ResponseFunctionWebSearch::Action::FindInPage] |
There was a problem hiding this comment.
[P3] Keep the documented action type aligned with the public class
This changes the constructor documentation to Action::FindInPage, but the runtime union still resolves find_in_page to Action::Find (line 66), and both RBI and RBS expose Find. Action.const_defined?(:FindInPage, false) is false. Please preserve Action::Find in the generated parameter documentation so editor guidance does not point callers to a nonexistent constant.
| # `create_service_account_only` is true. | ||
| # | ||
| # @return [Integer, nil] | ||
| optional :expires_in_seconds, Integer, nil?: true |
There was a problem hiding this comment.
[P3] Cover expiration on the initial service-account creation path
Please extend service_account_api_key_expiration_test.rb with a mocked service_accounts.create request that asserts expires_in_seconds reaches the JSON body and the nested response.api_key.expires_at survives decoding, including omitted/null values. The existing focused test exercises service_accounts.api_keys.create, while the changed resource tests only cover project-key reads. I verified the new path works with offline probes; keeping that coverage in the suite would protect these newly exposed fields and satisfy the repository's focused serialization-test guidance.
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Two in-scope blockers remain on this exact head. The newly added initial service-account key expiration path still has no focused request/response coverage: the existing expiration test exercises service_accounts.api_keys.create, not service_accounts.create (see the existing inline thread). Also, the shared moderation-model documentation now labels a link as “available models” but points it back to the moderation guide in api_reference/openapi.transformed.yml; that link should target the models documentation. Please correct the source and regenerate.
Castiron-Internal-PR: openai/openai-ruby-internal#100 Castiron-Source-SHA: 023a5a940abb3579194a4200c3d7f503c7dcabf0 Castiron-Public-Base-SHA: acf5db8
4d27f7a to
7cd69ca
Compare
The tests are code-generated, and the API reference is the source of truth.
3 participants
Summary
Allow project service account API keys to be created with an expiration period and expose their expiration timestamps in returned models.
Changes
expires_in_secondsparameter when creating a project service account.expires_atfield on project API keys.expires_atfield on API keys returned when creating project service accounts.