feat(api): support expiring service account keys - #178
Conversation
Castiron custom code✅ No new custom-code files detected. 5 mixed files remain; 0 existing customizations changed. Compared 5 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 34317505948 --repo openai/openai-cli \
--name castiron-custom-code-34317505948-1 --dir /tmp/castiron-custom-code-34317505948-1
git apply --stat /tmp/castiron-custom-code-34317505948-1/custom-code.patch
cat /tmp/castiron-custom-code-34317505948-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin f6638de713a15e7d740e7f1bbc74877fbe6ac430 9074fef4e28a1cbeca2828d104958b708b1580dd
python3 scripts/castiron/custom_code_report.py report \
--base f6638de713a15e7d740e7f1bbc74877fbe6ac430 \
--head 9074fef4e28a1cbeca2828d104958b708b1580dd --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-9074fef4e28a
cat /tmp/castiron-custom-code-9074fef4e28a/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
There was a problem hiding this comment.
🟡 Changes recommended
A couple of updated help links are unnecessarily long/brittle and the updated service-account create test drops coverage for the create_service_account_only=true path.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds support for expiring the initial API key when creating project service accounts, and refreshes many CLI help strings to point at the new developers.openai.com API reference / guides URLs.
Changes:
- Add
--expires-in-secondstoadmin:organization:projects:service-accounts createto set an initial key expiry. - Update numerous command help strings to use
developers.openai.comdocumentation links instead ofplatform.openai.com. - Regenerate Castiron generation metadata (
.castiron.stats.yml) and update associated generated tests.
File summaries
| File | Description |
|---|---|
| pkg/cmd/vectorstorefilebatch.go | Update vector store file batch help links to developers.openai.com references. |
| pkg/cmd/vectorstorefile.go | Update vector store file help links (create/delete) to new doc URLs. |
| pkg/cmd/vectorstore.go | Update vector store create help link for files to new doc URL. |
| pkg/cmd/uploadpart.go | Update Upload Part help links and formatting to new doc URLs. |
| pkg/cmd/upload.go | Update Upload create/complete help links to new doc URLs. |
| pkg/cmd/response.go | Refresh Responses command help URLs (guides + streaming reference). |
| pkg/cmd/moderation.go | Update moderation guide link references in help text. |
| pkg/cmd/item.go | Update conversation items include help link references. |
| pkg/cmd/inputtoken.go | Refresh Responses input-token counting help URLs. |
| pkg/cmd/image.go | Refresh image-related help URLs (safety identifiers, guides). |
| pkg/cmd/finetuningjob.go | Update fine-tuning help URLs to model optimization / fine-tuning guides. |
| pkg/cmd/finetuningcheckpointpermission.go | Update admin API key documentation link in help text. |
| pkg/cmd/finetuningalphagrader.go | Update graders guide links in help text. |
| pkg/cmd/embedding.go | Refresh embeddings help URLs (models list + safety identifiers). |
| pkg/cmd/contentprovenancecheck.go | Update content provenance guide link to developers.openai.com. |
| pkg/cmd/completion.go | Refresh completions help URLs (models list, tokenizer link, guides). |
| pkg/cmd/chatcompletion.go | Refresh Chat Completions help URLs (models, tools, streaming, guides). |
| pkg/cmd/call.go | Update prompt template documentation link to developers.openai.com. |
| pkg/cmd/betathreadrunstep.go | Update file-search help links in beta thread run steps. |
| pkg/cmd/betathreadrun.go | Refresh beta thread run help URLs (assistant/model/tools/guides). |
| pkg/cmd/betathread.go | Refresh beta thread help URLs (messages/assistant/model/tools). |
| pkg/cmd/betaresponseinputtoken.go | Refresh beta Responses input-token counting help URLs. |
| pkg/cmd/betaresponse.go | Refresh beta Responses help URLs (guides + streaming reference). |
| pkg/cmd/betaassistant.go | Refresh beta Assistants help URLs (models + reasoning + structured outputs). |
| pkg/cmd/batch.go | Update batch input-file help URLs (files create + batch guide). |
| pkg/cmd/audiotranslation.go | Update speech-to-text prompting guide URL. |
| pkg/cmd/audiotranscription.go | Update speech-to-text prompting + streaming guide URLs. |
| pkg/cmd/audiospeech.go | Update text-to-speech guide URL references in help text. |
| pkg/cmd/adminorganizationprojectserviceaccount.go | Add --expires-in-seconds for initial service-account key expiry. |
| pkg/cmd/adminorganizationprojectserviceaccount_test.go | Update generated test coverage for new expiry flag. |
| pkg/cmd/adminorganizationproject.go | Update data residency controls guide link. |
| pkg/cmd/adminorganizationauditlog.go | Update audit log documentation link. |
| .castiron.stats.yml | Update Castiron generation metadata/hashes for regenerated outputs. |
Review details
Suppressed comments (1)
pkg/cmd/item.go:62
- Same issue as above: the
includehelp link contains a large percent-encoded fragment. Linking to the method page without the encoded anchor is clearer and less fragile.
&requestflag.Flag[[]string]{
Name: "include",
Usage: "Additional fields to include in the response. See the `include`\nparameter for [listing Conversation items above](https://developers.openai.com/api/reference/resources/conversations/subresources/items/methods/list#%28resource%29%20conversations.items%20%3E%20%28method%29%20list%20%3E%20%28params%29%20default%20%3E%20%28param%29%20include%20%3E%20%28schema%29) for more information.\n",
QueryPath: "include",
- Files reviewed: 33/34 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
42c4c97 to
cfaeabb
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The CLI advertises a mutual-exclusion rule between --create-service-account-only=true and a non-null --expires-in-seconds but does not enforce it, so invalid combinations will be sent to the API instead of failing fast with a clear error.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 34/35 changed files
- Comments generated: 1
- Review effort level: Lite
Castiron-Internal-PR: openai/openai-cli-internal#69 Castiron-Source-SHA: 75e3853dc8329c82cf7966bf4e0d4870da00958b Castiron-Public-Base-SHA: f6638de
cfaeabb to
9074fef
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The newly added request-body test can be flaky due to a non-blocking select and should be made deterministic before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
pkg/cmd/serviceaccountcreation_test.go:76
- The non-blocking
selectwith adefaultbranch can make this test flaky: even if the CLI sends the request, the goroutine might not have pushed toreceivedyet when theselectexecutes. Add a small timeout (and fail only after it elapses) to make the assertion deterministic.
select {
case body := <-received:
require.JSONEq(t, test.wantBody, string(body))
default:
t.Fatal("CLI did not send a request")
}
pkg/cmd/adminorganizationprojectserviceaccount.go:42
- The flag help text says a non-null
--expires-in-seconds"cannot be used whencreate_service_account_onlyis true", but the command handler doesn't enforce this. As-is, users can pass both flags and only discover the constraint via a server-side error. Consider adding a client-side validation in the create action (e.g., ifcreate-service-account-onlyis true andexpires-in-secondsis set to a non-null value, return a clear error before making the request).
&requestflag.Flag[*int64]{
Name: "expires-in-seconds",
Usage: "Number of seconds until the initial API key expires. If omitted or null, the key does not expire unless the effective organization or project policy requires an expiration. When a policy sets a maximum lifetime, this value must be provided and must not exceed that limit. A non-null value cannot be used when `create_service_account_only` is true.",
BodyPath: "expires_in_seconds",
},
- Files reviewed: 34/35 changed files
- Comments generated: 1
- Review effort level: Lite
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. |
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Reviewed the exact public promotion and the added request-serialization coverage. The CLI preserves omission for expiry, encodes the valid service-account-only and initial-key-with-expiry cases correctly, keeps the existing server-validation boundary for cross-field constraints, and exact-head lint, test, build, CodeQL, and Castiron checks are green. The synchronous test server queues the body before writing the response, so the non-blocking assertion is deterministic after command.Run returns.
Summary
Add an expiry option when creating project service-account API keys and refresh command documentation.
Changes