Repository navigation
feat(api): support project API key scope updates - #4082
jbeckwith-oai wants to merge 1 commit into
Conversation
Castiron custom codeEvaluated main: ✅ No new custom-code files detected. 40 mixed files remain; 0 existing customizations changed. Compared 40 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 38685136778 --repo openai/openai-python \
--name castiron-custom-code-38685136778-1 --dir /tmp/castiron-custom-code-38685136778-1
git apply --stat /tmp/castiron-custom-code-38685136778-1/custom-code.patch
cat /tmp/castiron-custom-code-38685136778-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin 70d473892f7c427a02a3b566add9c86072842e25 114246f5d3413e88d6277b57105b80c6e4f75e7c
python3 scripts/castiron/custom_code_report.py report \
--base 70d473892f7c427a02a3b566add9c86072842e25 \
--head 114246f5d3413e88d6277b57105b80c6e4f75e7c --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-114246f5d341
cat /tmp/castiron-custom-code-114246f5d341/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
markstuart-oai
left a comment
markstuart-oai
left a comment
There was a problem hiding this comment.
Reviewed f05bdb4.
The sync and async update paths preserve omitted expiration, explicit empty scopes, existing expiry-only calls, and admin authentication. The generated request type and docs match the shared API contract. No blocking findings.
Validation: immutable source review and current-head hosted CI; all reported native checks passed or were skipped. I did not run local tests.
f05bdb4 to
71e0ff1
Compare
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: 71e0ff1ae0
ℹ️ 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".
| type: object | ||
| required: | ||
| - expires_in_seconds | ||
| required: [] |
There was a problem hiding this comment.
With both properties optional and no object-level constraint, this schema accepts {}, so the generated clients expose update(api_key_id=..., project_id=...) and the new tests incorrectly treat that request as successful. This contradicts the endpoint contract that at least one of scopes or expires_in_seconds must be supplied and causes such calls to send an invalid no-op request that the real API rejects; encode the at-least-one constraint or validate it in both client methods.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed: this SDK currently permits an update with neither field and dispatches a request that the server rejects. The generated mock test does not establish that the real API accepts an empty update. The CLI's local guard does not fix this SDK.
The at-least-one schema/compiler constraint remains deferred while we preserve existing public request types. The smallest proposed behavioral correction is an endpoint-specific check of the effective request body before dispatch, with tests for an empty request and the valid expiry-only, scope-only, combined, and empty-scope-list cases. That proposal must also preserve supported request-body overrides; checking only named arguments would be insufficient.
No correction has been made to this SDK yet. Leaving this finding open and holding this PR from the merge queue pending agreement on that additional runtime validation versus an explicitly accepted server-validation disposition.
Castiron-Internal-PR: openai/openai-python-internal#251 Castiron-Source-SHA: 683a160c9996b947916850f11bf258c613fc9f51 Castiron-Public-Base-SHA: 70d4738
71e0ff1 to
114246f
Compare
|
Updated the baseline in 114246f. The PR base now matches main, and the fresh OkTest run has passed preparation and reached the SDK tests: https://github.com/openai/ok-test/actions/runs/38685094657 . This addresses the stale merge-parent preparation problem; the full test run is still in progress. The empty-update review finding is separate and remains unresolved. This baseline refresh does not add a schema constraint or client-side validation, and this PR remains out of the merge queue pending that decision. Review has been re-requested for the updated commit. |
This branch has not been deployed
3 participants
Summary
Update scope restrictions on an existing project API key, alone or together with its expiration.
Changes