Run CI once per pull request - #35
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: 54229c2149
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
Updates the CI workflow triggers so CI runs once per pull request (via pull_request) and only runs on push for main, avoiding duplicate check suites for same-repo PR branches.
Changes:
- Restricts
pushworkflow runs tomainonly. - Runs CI for PRs via
pull_requestand simplifies job-levelif:gating. - Removes the
testjob’s previous fork/push-only condition so it runs for PRs as well.
Comments suppressed due to low confidence (2)
.github/workflows/ci.yml:53
- With CI now running on
pull_requestfor same-repo PRs,github.ref_nameresolves to a PR ref (e.g.<pr_number>/merge) rather than the branch name. If the staging-link behavior is intended for PR runs, usegithub.head_ref(fallback toref_namefor push).
runs-on: ${{ startsWith(github.repository, 'stainless-sdks/') && 'depot-ubuntu-24.04' || 'ubuntu-latest' }}
if: github.event_name != 'push' || github.event.head_commit.message != 'codegen metadata'
steps:
.github/workflows/ci.yml:107
- On
pull_requestruns,github.ref_nameis a PR ref like<pr_number>/merge, not the source branch name. Since this workflow now runs CI on same-repo PRs, the staging-link will no longer work as intended unless it usesgithub.head_reffor PRs (fallback toref_namefor pushes).
test:
timeout-minutes: 10
name: test
runs-on: ${{ startsWith(github.repository, 'stainless-sdks/') && 'depot-ubuntu-24.04' || 'ubuntu-latest' }}
steps:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
HAYDEN-OAI
left a comment
HAYDEN-OAI
left a comment
There was a problem hiding this comment.
Found two event-context regressions from moving same-repository PR CI from push to pull_request. The workflow needs a PR-head-aware branch identity before this is safe.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 431cd629d6
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e1f740d94
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
.github/workflows/ci.yml:56
- The
buildjob grantsid-token: writewhile the job itself runs forpull_requestevents (including forks). Even though the OIDC/upload steps are gated onIS_TRUSTED_SOURCE, any earlier step that executes PR-controlled code (including./scripts/*or local actions) could potentially request an OIDC token if the platform allows it, which is a least-privilege/security concern.
Consider moving the OIDC token + tarball upload into a separate job that only runs when IS_TRUSTED_SOURCE == 'true' (and give that job id-token: write), while keeping the main build job at contents: read for all PRs.
permissions:
contents: read
id-token: write
runs-on: ${{ startsWith(github.repository, 'stainless-sdks/') && 'depot-ubuntu-24.04' || 'ubuntu-latest' }}
if: github.event_name != 'push' || github.event.head_commit.message != 'codegen metadata'
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a098727bb
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
.github/workflows/ci.yml:175
- The workflow still skips
lint/build-artifacts(and thusbuild) onpushcommits with messagecodegen metadata, buttestwill still run on those pushes since its job-levelif:was removed. If the intent is to suppress CI for these codegen-metadata pushes,testshould use the same predicate to avoid running a partial CI set and consuming runner time.
test:
timeout-minutes: 10
name: test
runs-on: ${{ startsWith(github.repository, 'stainless-sdks/') && 'depot-ubuntu-24.04' || 'ubuntu-latest' }}
steps:
There was a problem hiding this comment.
Reviewed current head b639ce8 end-to-end, including prior threads, pull-request versus push SHA/ref semantics, fork gating, OIDC isolation, the tar-wrapped artifact handoff, required build aggregation, and the final metadata-push alignment. I found no substantive issues.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
.github/actions/setup-go/action.yml:22
- This composite action runs
./scripts/bootstrap(later in the file) while the CI workflow also runs./scripts/bootstrapafter optional./scripts/linkadjustments. Since this action is only used by.github/workflows/ci.ymlright now, keeping bootstrap in both places causes redundant (and potentially expensive) bootstrap runs on every job. Consider removing the bootstrap step from this action and letting the workflow own when bootstrap happens (especially since it needs to occur after any./scripts/linkreplacement).
if: steps.get_token.outputs.github_access_token != ''
shell: bash
env:
GITHUB_ACCESS_TOKEN: ${{ steps.get_token.outputs.github_access_token }}
run: |
HAYDEN-OAI
left a comment
HAYDEN-OAI
left a comment
There was a problem hiding this comment.
Re-reviewed current head 5a8fe77, including the full PR and the delta since the prior approval. The setup-go follow-up cleanly skips private token retrieval and Git rewriting when fork PRs receive no secret, while preserving the trusted publish boundary and required build aggregation. I found no substantive issues.
Summary
pull_requesteventpushCI tomain, eliminating duplicate push and pull-request runs for the same PR commitlint,build, andtestWhy
Same-repository PR branches previously triggered both
pushandpull_requestworkflow runs. The pull-request jobs were skipped, but still appeared as duplicate checks and could ambiguously satisfy required-check contexts.After this change, a PR commit produces one CI run. The merged commit gets a separate post-merge run on
main. Branches without an open PR no longer run this workflow until a PR is opened.Validation
./scripts/lint./scripts/test