Skip to content

fix(ci): close factory determinism gaps in pipeline workflows#500

Merged
castrojo merged 1 commit into
mainfrom
fix/factory-determinism-gaps
Jun 5, 2026
Merged

fix(ci): close factory determinism gaps in pipeline workflows#500
castrojo merged 1 commit into
mainfrom
fix/factory-determinism-gaps

Conversation

@castrojo
Copy link
Copy Markdown
Contributor

@castrojo castrojo commented Jun 5, 2026

Summary

Addresses four architectural gaps found during a factory determinism review.

Changes

1. promotion-candidate-e2e.yml — wire the gate to hive-status

The Tuesday promotion-candidate suite was described as a promotion gate but had no signal path: failures were visible only to someone actively watching the Actions tab. Adds a notify-on-failure job that auto-files a kind/bug issue when the suite fails, so hive-status surfaces the blocker before the promotion window.

2. validate.yml — add the missing submodule drift check

AGENTS.md claimed validate.yml checked submodule drift, but the step didn't exist. A PR could silently move the bluefin-branding submodule pointer to an unreviewed commit and pass all CI gates. Adds a git submodule status check that fails if any submodule is dirty (+) or uninitialized (-).

3. release.yml — idempotent tag creation

Tag is v$(date +'%Y.%m'). A mid-month workflow_dispatch followed by the 1st-of-month cron generates the same tag twice, causing the cron run to fail. Adds allowUpdates: true so duplicate-tag runs succeed silently.


Companion PR for the org-level policy gap: projectbluefin/.github#6

Assisted-by: Claude Sonnet 4.6 via GitHub Copilot
Co-authored-by: Copilot [email protected]

Summary by CodeRabbit

  • Chores
    • Improved E2E test failure detection with automated issue creation for promotion-candidate failures
    • Enhanced release workflow to support updating existing releases
    • Added submodule validation checks to the CI pipeline to ensure repository integrity

- promotion-candidate-e2e: add notify-on-failure job that auto-files
  a kind/bug issue so hive-status surfaces failures before the Tuesday
  promotion window; without this the gate produces signal only if
  someone is actively watching the Actions tab

- validate: add submodule drift check after checkout; AGENTS.md already
  claimed this check existed but the step was missing — closes the gap
  where a PR moving the bluefin-branding pointer could pass all gates

- release: add allowUpdates=true to ncipollo/release-action so mid-month
  workflow_dispatch runs no longer cause the monthly cron to fail with
  a duplicate-tag error on the 1st

Assisted-by: Claude Sonnet 4.6 via GitHub Copilot
Co-authored-by: Copilot <[email protected]>
@dosubot dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. kind/automation Automation and CI/CD labels Jun 5, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 5, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR enhances GitHub Actions workflows across three independent pipelines: adding failure notifications to E2E tests, validating submodule state in the validate job, and updating release action configuration for idempotency handling.

Changes

CI Workflow Improvements

Layer / File(s) Summary
E2E failure notification
.github/workflows/promotion-candidate-e2e.yml
New notify-on-failure job creates a labeled GitHub issue when the e2e job fails, including the workflow run URL and a warning not to promote.
Submodule state validation
.github/workflows/validate.yml
New "Check submodule drift" step verifies that git submodules are not dirty or uninitialized; fails if git submodule status reports violations.
Release workflow robustness
.github/workflows/release.yml
"Create Release" step now passes allowUpdates: true and skipIfReleaseExists: false to the release action, allowing release updates when a tag already exists.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • projectbluefin/common#480: Introduces the e2e job in the promotion-candidate E2E workflow that the new notify-on-failure job depends on.

Suggested labels

lgtm, kind/automation, area/testing, size:XS

Poem

🐰 Three workflows now stand tall and bright,
With messages when tests fail in the night,
Submodules checked, releases made right,
CI pipes humming with automation's might! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly and specifically describes the main change: addressing factory determinism gaps in CI/pipeline workflows, which directly corresponds to all three workflow files modified.
Description check ✅ Passed The PR description is comprehensive, well-structured with clear sections, and thoroughly explains each change with context and rationale. It exceeds the basic template requirements.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/factory-determinism-gaps

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 and usage tips.

@castrojo castrojo enabled auto-merge June 5, 2026 01:24
@coderabbitai coderabbitai Bot added lgtm Maintainer approved — ready to merge. area/testing Testing and QA labels Jun 5, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
.github/workflows/promotion-candidate-e2e.yml (1)

31-34: ⚡ Quick win

Consider using explicit failure condition for robustness.

The current if: failure() will work correctly, but the recommended pattern for failure-notification jobs is if: always() && needs.e2e.result == 'failure'. This is more explicit about the intent and handles edge cases like workflow cancellation more predictably.

♻️ Recommended refactor for clarity and robustness
   notify-on-failure:
     name: "File issue on E2E failure"
     needs: [e2e]
-    if: failure()
+    if: always() && needs.e2e.result == 'failure'
     runs-on: ubuntu-latest
🤖 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/promotion-candidate-e2e.yml around lines 31 - 34, Update
the notify-on-failure job's condition to be explicit about failure: replace the
generic if: failure() with an explicit check using needs.e2e.result (e.g., if:
always() && needs.e2e.result == 'failure') so the notify-on-failure job only
runs when the e2e job actually failed and behaves predictably for cancellations;
modify the job definition named notify-on-failure and its needs entry for e2e
accordingly.
🤖 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.

Nitpick comments:
In @.github/workflows/promotion-candidate-e2e.yml:
- Around line 31-34: Update the notify-on-failure job's condition to be explicit
about failure: replace the generic if: failure() with an explicit check using
needs.e2e.result (e.g., if: always() && needs.e2e.result == 'failure') so the
notify-on-failure job only runs when the e2e job actually failed and behaves
predictably for cancellations; modify the job definition named notify-on-failure
and its needs entry for e2e accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 03fcbec0-4633-4271-a1bb-c1954883c917

📥 Commits

Reviewing files that changed from the base of the PR and between bb840ba and 0b2bcea.

📒 Files selected for processing (3)
  • .github/workflows/promotion-candidate-e2e.yml
  • .github/workflows/release.yml
  • .github/workflows/validate.yml

@castrojo castrojo merged commit 0b2bcea into main Jun 5, 2026
7 checks passed
@castrojo castrojo deleted the fix/factory-determinism-gaps branch June 5, 2026 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/testing Testing and QA kind/automation Automation and CI/CD lgtm Maintainer approved — ready to merge. size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant