Conversation
![chatgpt-codex-connector[bot]](https://cdn.statically.io/img/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: 4172ee10c7
ℹ️ 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".
.github/workflows/sdk.yml
Outdated
| bazel cquery \ | ||
| --ui_event_filters=-info \ | ||
| --noshow_progress \ | ||
| --output=files \ | ||
| //codex-rs/cli:codex \ |
There was a problem hiding this comment.
This bazel cquery call bypasses .github/scripts/run-bazel-ci.sh, which is the only place we apply CI-safe remote settings (including clearing --remote_cache/--remote_executor when BUILDBUDDY_API_KEY is absent). On pull_request runs from forks where secrets are unavailable, the build step can still succeed via the wrapper fallback, but this direct query step can fail against the authenticated remote endpoints from .bazelrc before SDK tests run. Route this query through the same wrapper (or pass equivalent flags/header) to keep fork PR CI green.
Useful? React with 👍 / 👎.
bolinfest
force-pushed
the
pr16046
branch
6 times, most recently
from
576ee3c to
475d54f
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
2 participants
Why
Before this change, the SDK CI job built
codexwith Cargo before running the TypeScript package tests. That step has been getting more expensive as the Rust workspace grows, while the repo already has a Bazel-backed build path for the CLI.The SDK tests also need a normal executable path they can spawn repeatedly. Moving the job to Bazel exposed an extra CI detail: a plain
bazel-bin/...lookup is not reliable under the Linux config because top-level outputs may stay remote and the wrapper emits status lines aroundcqueryoutput.What Changed
sdk/typescript/tests/testCodex.tsto honorCODEX_EXEC_PATHbefore falling back to the local Cargo-styletarget/debug/codexpath--remote-download-toplevelto.github/scripts/run-bazel-ci.shso workflows can force Bazel to materialize top-level outputs on disk after a build.github/workflows/sdk.ymlfromcargo build --bin codexto the shared Bazel CI setup and//codex-rs/cli:codexbuild targetcquery --output=files, stage the binary into${GITHUB_WORKSPACE}/.tmp/sdk-ci/codex, and point the SDK tests at that path viaCODEX_EXEC_PATHVerification
bash -n .github/scripts/run-bazel-ci.sh./.github/scripts/run-bazel-ci.sh -- cquery --output=files -- //codex-rs/cli:codex | grep -E '^(/|bazel-out/)' | tail -n 1./.github/scripts/run-bazel-ci.sh --remote-download-toplevel -- build --build_metadata=TAG_job=sdk -- //codex-rs/cli:codexCODEX_EXEC_PATH="$PWD/.tmp/sdk-ci/codex" pnpm --dir sdk/typescript test --runInBandpnpm --dir sdk/typescript lint