Skip to content

ci: gate full research image smoke - #217

Merged
proerror77 merged 1 commit into
mainfrom
codex/ci-research-image-smoke
Jul 22, 2026
Merged

ci: gate full research image smoke#217
proerror77 merged 1 commit into
mainfrom
codex/ci-research-image-smoke

Conversation

@proerror77

@proerror77 proerror77 commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Change contract

On pull requests, run the full unified research-image smoke only when image construction inputs change; keep image-affecting PR and main paths aligned, while ordinary Rust source PRs continue through targeted Rust CI without the 20–25 minute image build.

Closes #216. A PRD is unnecessary because this is one already-specified CI scheduling behavior with a repository-owned selector seam.

Out of scope

ACR release-binary precompilation, Dockerfile packaging changes, cross-workflow artifact reuse, self-hosted runners, unrelated monorepo main paths, Rust/research behavior, collector deployment, and runtime changes.

Dependency or merge order

None. This is an independent CI scheduling and rollback unit.

Focused validation

  • .github/scripts/test-select-prediction-image-smoke.sh
  • .github/scripts/test-select-rust-ci-scope.sh
  • bash -n .github/scripts/select-prediction-image-smoke.sh .github/scripts/test-select-prediction-image-smoke.sh
  • actionlint .github/workflows/ploy-ci.yml
  • git diff --check
  • Matt Standards review: 0 findings
  • Matt Spec review: 0 findings

Counterexamples cover ordinary prediction .rs changes (skip), Dockerfile, .dockerignore, root and nested Cargo manifests/lockfiles, the actual nested Rust toolchain, Cargo config, workflow/selector changes (run), and main/manual events (run).

Rollout/rollback impact

Rollout changes PR image-smoke scheduling and preserves fail-closed full image smoke for related main-push inputs. This PR intentionally runs the full image smoke once because it changes the workflow and selector. Rollback reverts this commit and restores the full image build on every prediction-market PR.

Summary by CodeRabbit

  • CI Improvements
    • Improved selection of research image smoke checks based on event type and relevant file changes.
    • Expanded CI triggers to cover Rust workspace, toolchain, Docker, and smoke-test configuration changes.
    • Added validation scenarios to verify smoke-check selection across pull requests, pushes, and manual runs.

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a Bash selector for research image smoke scope, repository-owned selector tests, expanded workflow path filters, and a gating job that runs the full research image smoke only when the selector emits true.

Changes

Prediction image smoke selection

Layer / File(s) Summary
Selector logic
.github/scripts/select-prediction-image-smoke.sh
Parses event, commit, changed-file, and output arguments; selects all non-pull_request events and matches pull-request changes against image-related paths.
Selector validation
.github/scripts/test-select-prediction-image-smoke.sh
Tests ordinary Rust pull requests, image-related paths, push events, and manual dispatch, asserting the emitted selection value.
Workflow scope and gating
.github/workflows/ploy-ci.yml
Expands path filters, adds image-smoke-scope, and runs research-image-smoke only when its selector output is true.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GitHubActions
  participant image_smoke_scope
  participant Selector
  participant research_image_smoke
  GitHubActions->>image_smoke_scope: start workflow
  image_smoke_scope->>Selector: evaluate event and changed paths
  Selector-->>image_smoke_scope: return research_image_smoke
  image_smoke_scope-->>research_image_smoke: provide gating output
  research_image_smoke->>research_image_smoke: run when output is true
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main CI gating change.
Description check ✅ Passed The description covers the required contract, scope, validation, and rollout details, with only minor heading wording differences.
Linked Issues check ✅ Passed The workflow and selector changes match #216 by gating PR image smoke, preserving targeted Rust CI, and covering main/manual smoke paths.
Out of Scope Changes check ✅ Passed The changes stay focused on the selector, its tests, and workflow gating without introducing unrelated scope.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/ci-research-image-smoke

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Checkov (3.3.8)
.github/workflows/ploy-ci.yml

Traceback (most recent call last):
File "/usr/local/bin/checkov", line 2, in
from checkov.main import Checkov
ModuleNotFoundError: No module named 'checkov'


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

research_image_smoke: ${{ steps.scope.outputs.research_image_smoke }}
steps:
- name: Checkout code
uses: actions/checkout@v6

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ploy-ci.yml:
- Around line 68-80: Update the “Test image smoke selector” and “Select full
image smoke” workflow steps to invoke the scripts from the workspace root using
.github/scripts/... paths instead of ../../.github/scripts/..., preserving the
existing arguments and scope output behavior.
- Around line 63-66: Add persist-credentials: false to the with configuration of
the actions/checkout step in the PR checkout job, while preserving fetch-depth:
0 and the existing checkout behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 39be3f6b-f47f-4caf-b613-b67b291a6df1

📥 Commits

Reviewing files that changed from the base of the PR and between eb07cf9 and fb1523c.

📒 Files selected for processing (3)
  • .github/scripts/select-prediction-image-smoke.sh
  • .github/scripts/test-select-prediction-image-smoke.sh
  • .github/workflows/ploy-ci.yml

Comment on lines +63 to +66
- name: Checkout code
uses: actions/checkout@v6
with:
fetch-depth: 0

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

sed -n '1,220p' .github/workflows/ploy-ci.yml | cat -n

Repository: proerror77/monday

Length of output: 8537


🏁 Script executed:

#!/bin/bash
set -euo pipefail
sed -n '1,220p' .github/workflows/ploy-ci.yml | cat -n

Repository: proerror77/monday

Length of output: 8537


🏁 Script executed:

pwd

Repository: proerror77/monday

Length of output: 174


🏁 Script executed:

python3 - <<'PY'
from pathlib import Path
p = Path('.github/workflows/ploy-ci.yml')
print(p.exists())
print(p.read_text()[:4000])
PY

Repository: proerror77/monday

Length of output: 4161


Disable persisted credentials on the PR checkout.
This job runs checked-out PR scripts and only needs read access to git history, so set persist-credentials: false to avoid leaving the token in git config.

Proposed fix
      - name: Checkout code
        uses: actions/checkout@v6
        with:
          fetch-depth: 0
+          persist-credentials: false
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Checkout code
uses: actions/checkout@v6
with:
fetch-depth: 0
- name: Checkout code
uses: actions/checkout@v6
with:
fetch-depth: 0
persist-credentials: false
🧰 Tools
🪛 zizmor (1.26.1)

[warning] 63-66: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ploy-ci.yml around lines 63 - 66, Add persist-credentials:
false to the with configuration of the actions/checkout step in the PR checkout
job, while preserving fetch-depth: 0 and the existing checkout behavior.

Source: Linters/SAST tools

Comment on lines +68 to +80
- name: Test image smoke selector
run: ../../.github/scripts/test-select-prediction-image-smoke.sh

- name: Select full image smoke
id: scope
env:
BASE_SHA: ${{ github.event.pull_request.base.sha }}
HEAD_SHA: ${{ github.event.pull_request.head.sha || github.sha }}
run: >-
../../.github/scripts/select-prediction-image-smoke.sh
--event "${GITHUB_EVENT_NAME}"
--base "${BASE_SHA}"
--head "${HEAD_SHA}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Invoke the scripts from the workspace root.

Both ../../.github/... paths resolve outside the checked-out repository. The scope job will fail before emitting its output, so research-image-smoke is skipped even for image-affecting changes. Use .github/scripts/... (optionally via bash) instead.

Proposed fix
       - name: Test image smoke selector
-        run: ../../.github/scripts/test-select-prediction-image-smoke.sh
+        run: bash .github/scripts/test-select-prediction-image-smoke.sh

       - name: Select full image smoke
         id: scope
         env:
           BASE_SHA: ${{ github.event.pull_request.base.sha }}
           HEAD_SHA: ${{ github.event.pull_request.head.sha || github.sha }}
         run: >-
-          ../../.github/scripts/select-prediction-image-smoke.sh
+          bash .github/scripts/select-prediction-image-smoke.sh
           --event "${GITHUB_EVENT_NAME}"
           --base "${BASE_SHA}"
           --head "${HEAD_SHA}"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Test image smoke selector
run: ../../.github/scripts/test-select-prediction-image-smoke.sh
- name: Select full image smoke
id: scope
env:
BASE_SHA: ${{ github.event.pull_request.base.sha }}
HEAD_SHA: ${{ github.event.pull_request.head.sha || github.sha }}
run: >-
../../.github/scripts/select-prediction-image-smoke.sh
--event "${GITHUB_EVENT_NAME}"
--base "${BASE_SHA}"
--head "${HEAD_SHA}"
- name: Test image smoke selector
run: bash .github/scripts/test-select-prediction-image-smoke.sh
- name: Select full image smoke
id: scope
env:
BASE_SHA: ${{ github.event.pull_request.base.sha }}
HEAD_SHA: ${{ github.event.pull_request.head.sha || github.sha }}
run: >-
bash .github/scripts/select-prediction-image-smoke.sh
--event "${GITHUB_EVENT_NAME}"
--base "${BASE_SHA}"
--head "${HEAD_SHA}"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ploy-ci.yml around lines 68 - 80, Update the “Test image
smoke selector” and “Select full image smoke” workflow steps to invoke the
scripts from the workspace root using .github/scripts/... paths instead of
../../.github/scripts/..., preserving the existing arguments and scope output
behavior.

@proerror77
proerror77 merged commit 19c3611 into main Jul 22, 2026
37 checks passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

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.

Reviewed commit: fb1523c211

ℹ️ 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".

Comment on lines +36 to +37
while IFS= read -r -d '' path; do paths+=("$path"); done \
< <(git diff --no-renames --name-only --diff-filter=ACMRD -z "$base...$head")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Propagate failures from the changed-file diff

In ploy-ci.yml's image-smoke-scope PR path, an unavailable base/head revision makes git diff fail inside the process substitution, but the surrounding while still succeeds; verified with a missing base ref, the selector exited 0 and emitted research_image_smoke=false. An incomplete checkout or unresolvable event SHA can therefore turn a selector error into a skipped image build instead of failing closed. Capture and validate the diff command's status before emitting the decision, and add an invalid-ref counterexample test.

AGENTS.md reference: AGENTS.md:L93-L96

Useful? React with 👍 / 👎.

Comment on lines +36 to +37
while IFS= read -r -d '' path; do paths+=("$path"); done \
< <(git diff --no-renames --name-only --diff-filter=ACMRD -z "$base...$head")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include type changes in the smoke-scope diff

When a PR converts an image-affecting path such as Dockerfile.research or .dockerignore between a regular file and a symlink, Git reports status T, but --diff-filter=ACMRD excludes that path (git diff -h lists T as a separate selectable diff type). Verified with a regular-file-to-symlink change to Dockerfile.research, the selector emitted research_image_smoke=false, so the build-impacting change bypasses the image smoke; include T or avoid restricting the diff statuses, and add this counterexample to the selector tests.

AGENTS.md reference: AGENTS.md:L93-L96

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gate full research image smoke to image-affecting PR changes

2 participants