Repository navigation
ci(standards): add version pin compliance check [OMN-4806] - #663
jonahgabriel merged 7 commits into
Conversation
Wire check_version_pins.py into Phase 1 CI as a new version-pin-check job. Checks out omni_home standards and validates omnibase_core's dependency pins against the shared version matrix (OMN-4803). Job is added to quality-gate needs so it gates the full test suite.
📝 WalkthroughWalkthroughA new GitHub Actions job Changes
Sequence Diagram(s)sequenceDiagram
participant GH as "GitHub Actions"
participant Runner as "Actions Runner"
participant Repo as "Repository (omnibase_core)"
participant OmniHome as "omni_home standards"
participant Script as "check_version_pins.py"
GH->>Runner: start `version-pin-check` job
Runner->>Repo: checkout repository
Runner->>OmniHome: sparse-checkout `standards/`
Runner->>Runner: setup Python 3.12
Runner->>Script: run `python omni_home/standards/check_version_pins.py --repo omnibase_core --root omni_home`
Script->>OmniHome: read version standards
Script->>Repo: scan dependency version pins
Script-->>Runner: exit status (0 or non-zero)
Runner-->>GH: report job result
GH->>GH: include `version-pin-check` in `quality-gate` needs
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
…sion-matrix-checker-into-ci
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/test.yml (1)
652-693:⚠️ Potential issue | 🔴 Critical
Quality Gatedoes not enforceversion-pin-checkresult, allowing it to pass when the check fails.
version-pin-checkis listed in theneedsat line 652, but the gate logic (lines 680–693) never reads or checksneeds.version-pin-check.result. Withif: always(), the job runs regardless of upstream failures, soversion-pin-checkfailures are silently ignored.Suggested fix
enum_gov="${{ needs.enum-governance.result }}" secrets="${{ needs.detect-secrets.result }}" + version_pin="${{ needs.version-pin-check.result }}" @@ echo "| Enum Governance | $enum_gov |" >> $GITHUB_STEP_SUMMARY echo "| Detect Secrets | $secrets |" >> $GITHUB_STEP_SUMMARY + echo "| Version Pin Compliance | $version_pin |" >> $GITHUB_STEP_SUMMARY @@ [[ "$docs" == "success" ]] && \ [[ "$enum_gov" == "success" ]] && \ - [[ "$secrets" == "success" ]]; then + [[ "$secrets" == "success" ]] && \ + [[ "$version_pin" == "success" ]]; then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test.yml around lines 652 - 693, The Quality Gate currently omits the version-pin-check result: add a variable (e.g., version_pin="${{ needs.version-pin-check.result }}"), echo it into the summary table (a row like "| Version Pin Check | $version_pin |"), and include it in the gate condition alongside the other checks (add && [[ "$version_pin" == "success" ]] to the if that evaluates all results) so failures in version-pin-check will fail the gate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/test.yml:
- Around line 617-624: The checkout step using actions/checkout@v6 for
repository OmniNode-ai/omni_home (path: omni_home, sparse-checkout: standards/)
should be pinned to a specific ref; add a ref: <commit-sha-or-tag> entry to that
checkout step so the actions/checkout invocation for omni_home always uses a
deterministic commit/tag rather than the upstream default branch.
---
Outside diff comments:
In @.github/workflows/test.yml:
- Around line 652-693: The Quality Gate currently omits the version-pin-check
result: add a variable (e.g., version_pin="${{ needs.version-pin-check.result
}}"), echo it into the summary table (a row like "| Version Pin Check |
$version_pin |"), and include it in the gate condition alongside the other
checks (add && [[ "$version_pin" == "success" ]] to the if that evaluates all
results) so failures in version-pin-check will fail the gate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 26a7cce2-c646-486d-aa01-873b93c9008f
📒 Files selected for processing (1)
.github/workflows/test.yml
…ersion-pin-check The Checkout omni_home standards step fails with 422 because omni_home is a private repo and the default GITHUB_TOKEN does not have cross-repo read access. Use CROSS_REPO_PAT (org-level secret available to all repos) to authenticate.
…sion-matrix-checker-into-ci
…sion-matrix-checker-into-ci
Add continue-on-error to omni_home checkout step and wrap script execution in file-existence check. Until OMN-4803 merges the check_version_pins.py script into omni_home main, the job emits a warning and exits 0 instead of failing CI.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/test.yml (1)
617-626:⚠️ Potential issue | 🟠 MajorPin
omni_homecheckout ref and reconsidercontinue-on-errorplacement.Two concerns with this checkout step:
Missing ref pin: The checkout fetches
OmniNode-ai/omni_homewithout a pinnedref, making CI non-deterministic and vulnerable to upstream drift.
continue-on-error: truemasks failures: If the checkout fails (e.g., invalid PAT, network issue, repo access revoked), the step silently succeeds. The subsequent script-existence check (line 636) will then exit 0 with a misleading "script not found" warning, masking the real cause.Consider pinning the ref and moving the error tolerance to the script execution step instead:
Suggested improvement
- name: Checkout omni_home standards uses: actions/checkout@v6 - continue-on-error: true with: repository: OmniNode-ai/omni_home token: ${{ secrets.CROSS_REPO_PAT }} path: omni_home + ref: main # or pin to a specific SHA/tag for full reproducibility sparse-checkout: | standards/ sparse-checkout-cone-mode: false - name: Set up Python uses: actions/setup-python@v6 with: python-version: "3.12" - name: Check version pin compliance + continue-on-error: true # Tolerate missing script until OMN-4803 merges run: | SCRIPT="omni_home/standards/check_version_pins.py" if [ ! -f "$SCRIPT" ]; then echo "::warning::$SCRIPT not found (OMN-4803 not yet merged into omni_home main). Skipping." exit 0 fi python "$SCRIPT" --repo omnibase_core --root omni_home,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test.yml around lines 617 - 626, The checkout step labeled "Checkout omni_home standards" must be pinned to a stable ref and should not silently ignore failures: add a concrete ref (branch/tag/commit) to the actions/checkout@v6 invocation (e.g., ref: 'main' or a specific tag/sha) and remove continue-on-error: true from that step; instead, apply error tolerance to the later "script-existence" check step (the step that verifies the presence of the omni_home script) so that checkout failures surface as CI failures while only the optional script-check gracefully handles a missing script.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/test.yml:
- Line 660: The quality-gate evaluation currently lists version-pin-check in the
job dependencies but doesn't capture its outcome, show it in the summary table,
or include it in the pass/fail condition; update the quality-gate job script to
(1) capture version-pin-check's outcome into a variable (e.g.,
version_pin_check_result) alongside the existing result variables, (2) append a
row for version-pin-check in the generated Markdown summary table, and (3)
include version_pin_check_result in the final pass/fail boolean expression that
decides the job outcome (or, if omission is intentional like node-purity-check,
add a clear comment explaining why version-pin-check is excluded).
---
Duplicate comments:
In @.github/workflows/test.yml:
- Around line 617-626: The checkout step labeled "Checkout omni_home standards"
must be pinned to a stable ref and should not silently ignore failures: add a
concrete ref (branch/tag/commit) to the actions/checkout@v6 invocation (e.g.,
ref: 'main' or a specific tag/sha) and remove continue-on-error: true from that
step; instead, apply error tolerance to the later "script-existence" check step
(the step that verifies the presence of the omni_home script) so that checkout
failures surface as CI failures while only the optional script-check gracefully
handles a missing script.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 660ccb28-72f9-4527-8bd8-1a20bb763d49
📒 Files selected for processing (1)
.github/workflows/test.yml
…sion-matrix-checker-into-ci
Summary
Wires
check_version_pins.pyfrom omni_home into omnibase_core CI as a Phase 1version-pin-checkjob (OMN-4806).OmniNode-ai/omni_home(sparse:standards/) on each CI runpython omni_home/standards/check_version_pins.py --repo omnibase_core --root omni_homequality-gateneeds — gates the full test suiteDependencies
Depends on OMN-4803 (omni_home PR #33) which creates
standards/check_version_pins.py. This PR will pass CI as soon as that merges.Definition of Done
Linear: OMN-4806
Summary by CodeRabbit