-
Notifications
You must be signed in to change notification settings - Fork 0
Codex split action into two steps #34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
| - name: Generate patch artifacts | ||
| uses: tensorzero/experimental-ci-bot/generate-pr-patch@main | ||
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| with: | ||
| token: ${{ secrets.GITHUB_TOKEN }} | ||
| tensorzero-base-url: http://localhost:3000 | ||
| # TODO: Remove when agent creates PRs | ||
| tensorzero-diff-patched-successfully-metric-name: tensorzero_github_ci_bot_diff_patched_successfully | ||
| output-artifacts-dir: debug-logs | ||
| clickhouse-url: ${{ secrets.CI_BOT_CLICKHOUSE_URL }} | ||
| clickhouse-table: GitHubBotPullRequestToInferenceMap | ||
| output-artifacts-dir: ci-bot-artifacts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pass GITHUB_TOKEN to generate-pr-patch action
The new implementation of generate-pr-patch now reads the GitHub token exclusively from process.env.GITHUB_TOKEN (src/generate-pr-patch/main.ts, getRequiredGitHubToken). In the workflow step that invokes the action, the previous token input and GH_TOKEN env were removed, but no GITHUB_TOKEN environment variable is supplied. Actions do not automatically expose secrets.GITHUB_TOKEN to the action process, so this step will crash with “GITHUB_TOKEN environment variable is required” before any artifacts are generated. Re‑introduce an env export, e.g. env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}, so the action can authenticate to GitHub APIs and the gh CLI.
Useful? React with 👍 / 👎.
| artifact-directory: pr-artifacts/ci-bot-artifacts | ||
| tensorzero-base-url: http://localhost:3000 | ||
| tensorzero-diff-patched-successfully-metric-name: tensorzero_github_ci_bot_diff_patched_successfully | ||
| clickhouse-url: ${{ secrets.CI_BOT_CLICKHOUSE_URL }} | ||
| clickhouse-table: GitHubBotPullRequestToInferenceMap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need all this here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
believe so bc need to write to CH
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the "proper" way to do this is to spin up a hosted t0 server with auth, have an api key here, and whitelist it so we only accept the github action runners' IP range or something. that's a lot of work though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still worried about repo secrets; let's only run this on trusted code (coming from main repo, not forks)
This splits the original
gather-diff-and-generate-patchjob into 2 steps:This satisfies the least privilege model. We can switch out step 1 to use mini-swe-agent when we're ready.
Fixes #33.