-
Notifications
You must be signed in to change notification settings - Fork 1
fix(keepalive): add preflight secrets check before Codex call #107
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -120,12 +120,44 @@ jobs: | |||||
| core.setOutput(key, value); | ||||||
| } | ||||||
|
|
||||||
| run-codex: | ||||||
| name: Keepalive next task | ||||||
| preflight: | ||||||
| name: Verify secrets available | ||||||
| needs: evaluate | ||||||
| if: needs.evaluate.outputs.action == 'run' | ||||||
| runs-on: ubuntu-latest | ||||||
| environment: agent-standard | ||||||
| outputs: | ||||||
| secrets_ok: ${{ steps.check.outputs.secrets_ok }} | ||||||
| steps: | ||||||
| - name: Check secrets | ||||||
| id: check | ||||||
| env: | ||||||
| HAS_CODEX_AUTH: ${{ secrets.CODEX_AUTH_JSON != '' }} | ||||||
| HAS_APP_ID: ${{ secrets.WORKFLOWS_APP_ID != '' }} | ||||||
| HAS_APP_KEY: ${{ secrets.WORKFLOWS_APP_PRIVATE_KEY != '' }} | ||||||
| run: | | ||||||
| echo "CODEX_AUTH_JSON present: $HAS_CODEX_AUTH" | ||||||
| echo "WORKFLOWS_APP_ID present: $HAS_APP_ID" | ||||||
| echo "WORKFLOWS_APP_PRIVATE_KEY present: $HAS_APP_KEY" | ||||||
| if [ "$HAS_CODEX_AUTH" = "true" ] || [ "$HAS_APP_ID" = "true" ]; then | ||||||
| echo "secrets_ok=true" >> $GITHUB_OUTPUT | ||||||
| else | ||||||
| echo "::error::Neither CODEX_AUTH_JSON nor WORKFLOWS_APP_ID is set. Cannot run Codex." | ||||||
| echo "secrets_ok=false" >> $GITHUB_OUTPUT | ||||||
| exit 1 | ||||||
|
||||||
| exit 1 |
Copilot
AI
Dec 24, 2025
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 condition checking secrets_ok == 'true' is redundant because the preflight job already exits with status 1 when secrets are not available. When preflight fails, needs.preflight.result would be 'failure', which would already prevent run-codex from executing due to the default behavior (jobs don't run when their dependencies fail unless you specify if: always() or check needs.X.result).
The current condition needs.preflight.outputs.secrets_ok == 'true' will never be false in practice because preflight will fail before setting secrets_ok to false makes any difference. Consider simplifying the condition to just needs.evaluate.outputs.action == 'run', or if you want to handle a failed preflight explicitly, check needs.preflight.result == 'success' instead.
| if: needs.evaluate.outputs.action == 'run' && needs.preflight.outputs.secrets_ok == 'true' | |
| if: needs.evaluate.outputs.action == 'run' |
Copilot
AI
Dec 24, 2025
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 summary job now depends on the preflight job, but this dependency will cause the summary job to be skipped when the preflight job is skipped (i.e., when needs.evaluate.outputs.action != 'run'). This breaks the intended behavior where the summary should run regardless of whether the preflight job executes. The summary job has if: always() which should allow it to run in all scenarios to update the keepalive status, but the hard dependency on preflight will prevent execution when preflight is skipped.
Consider removing preflight from the needs list in the summary job, since the summary doesn't actually use any outputs from preflight and should run independently to report the final state.
| - preflight | |
| - run-codex |
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 new preflight job returns
secrets_ok=truewhen eitherCODEX_AUTH_JSONorWORKFLOWS_APP_IDis set. However,reusable-codex-run.ymlalways requiresCODEX_AUTH_JSON(theSetup Codex authstep exits if it is empty), so runs where only the GitHub App credentials are available will still proceed torun-codexand then fail later. The preflight gate is supposed to block missing secrets, but with the current OR condition it provides a false green signal and doesn’t prevent the failing Codex invocation.Useful? React with 👍 / 👎.