Skip to content

WA-CI-012: Improve CI messaging for Bundler frozen-mode lockfile mismatch#1118

Open
kitcommerce wants to merge 1 commit intonextfrom
kit/869-ci-bundler-frozen-message
Open

WA-CI-012: Improve CI messaging for Bundler frozen-mode lockfile mismatch#1118
kitcommerce wants to merge 1 commit intonextfrom
kit/869-ci-bundler-frozen-message

Conversation

@kitcommerce
Copy link
Contributor

Summary

  • Detect common Bundler frozen/deployment-mode lockfile mismatch failures in CI and emit a high-signal GitHub Actions error annotation with the remediation.

Client impact

None expected.

Verification Plan

  1. In a PR, introduce a Gemfile/Gemfile.lock mismatch (e.g. edit Gemfile without updating Gemfile.lock).
  2. Run CI (or re-run the failed job).
  3. Observe a single ::error:: annotation explaining the frozen-mode lockfile mismatch and instructing to run bundle install and commit Gemfile.lock.

@kitcommerce kitcommerce added gate:build-pending Build gate running gate:build-passed Build gate passed review:architecture-pending Review in progress review:simplicity-pending Review in progress review:security-pending Review in progress review:rails-conventions-pending Rails conventions review in progress and removed gate:build-pending Build gate running labels Mar 17, 2026
@kitcommerce
Copy link
Contributor Author

Architecture Review

Verdict: PASS

The script is appropriately placed in script/ci/ — a good subdirectory convention for CI-specific tooling. Responsibility is well-scoped: detect lockfile mismatch, emit a GitHub Actions ::error:: annotation, exit 1.

The YAML change is architecturally sound: splitting continue-on-error logic into separate experimental vs non-experimental branches makes the intent explicit and prevents swallowing non-experimental failures silently. The indirection (hint script always exits 1) is a clean way to fail the job with enriched messaging.

No coupling concerns. Follows the established script/ pattern.

@kitcommerce
Copy link
Contributor Author

Simplicity Review

Verdict: PASS

The 48-line script does one thing: re-run bundle install, capture output, grep for known lockfile error patterns, and emit an annotation. The log-to-temp-file approach is standard and avoids stdout/stderr entanglement. set -euo pipefail is correct defensive practice.

The pattern of always exiting 1 ensures the hint step never accidentally passes, which keeps the logic simple and explicit. No unnecessary complexity observed.

@kitcommerce
Copy link
Contributor Author

Security Review

Verdict: PASS

No shell injection vectors identified. The script re-runs bundle install in a controlled CI environment — no user-supplied input is interpolated into commands. The ::error:: annotation content is sourced from bundle output; while a malicious gem could theoretically craft error messages to inject into annotations, this is an extremely low-risk scenario in the context of a controlled CI lockfile check.

RUNNER_TEMP fallback to /tmp is safe. set -euo pipefail prevents silent failures. All variable expansions appear properly handled.

@kitcommerce
Copy link
Contributor Author

Rails Conventions Review

Verdict: PASS (N/A)

This PR contains only shell scripts and YAML — no Ruby code. Rails conventions do not apply. Confirmed.

@kitcommerce kitcommerce added review:architecture-done Review complete review:simplicity-done Review complete review:security-done Review complete review:rails-conventions-done Rails conventions review complete review:wave1-complete review:rails-security-pending Rails security review in progress review:database-pending Database review in progress review:test-quality-pending Review in progress and removed review:architecture-pending Review in progress review:simplicity-pending Review in progress review:security-pending Review in progress review:rails-conventions-pending Rails conventions review in progress labels Mar 17, 2026
@kitcommerce
Copy link
Contributor Author

Rails Security Review

Verdict: PASS

Findings

No Rails security findings. This PR modifies only CI workflow configuration (.github/workflows/ci.yml) and adds a diagnostic bash script (script/ci/bundler_lockfile_hint). No Rails application code (controllers, models, views, routes, or Ruby source) is touched — zero Rails security attack surface affected.

Recommendations

None.

@kitcommerce kitcommerce added review:rails-security-done Rails security review complete and removed review:rails-security-pending Rails security review in progress labels Mar 17, 2026
@kitcommerce
Copy link
Contributor Author

Database Review

Verdict: PASS

Findings

No database changes in this PR. The diff consists entirely of CI workflow YAML () and a new shell script (). There are no migrations, schema changes, model query patterns, or ActiveRecord code to evaluate.

Recommendations

None.

@kitcommerce kitcommerce added review:database-done Database review complete and removed review:database-pending Database review in progress labels Mar 17, 2026
@kitcommerce
Copy link
Contributor Author

Test Quality Review

Verdict: PASS_WITH_NOTES

Summary

This is a CI tooling-only change (bash script + YAML). No Ruby unit tests are added or modified, which is expected — the test quality question here is whether the script's logic is adequately verifiable and whether the verification plan is sound.

Findings

MEDIUM — Regex pattern coverage is untested and brittle
The core detection logic in script/ci/bundler_lockfile_hint relies on a grep -Eq pattern list to identify frozen/deployment-mode lockfile mismatch messages. These patterns are the heart of the feature. If any bundler version changes its error message wording slightly, or if the patterns miss a real-world variant, the hint silently falls through to the generic message — providing no improvement over the baseline. There are no automated tests to verify the patterns match actual bundler output.

LOW — Verification plan is entirely manual
The verification plan (introduce a mismatch, run CI, observe annotation) is operationally reasonable but not repeatable or automated. A bash testing framework like bats-core could mock bundle to emit known error strings and assert on both exit codes and stdout, making the regex patterns and branch logic continuously testable without a full CI run.

What's Done Well

  • set -euo pipefail is present — good defensive practice for CI scripts
  • Each branch of the script has clear inline comments explaining intent
  • The script re-runs bundle install to capture output, rather than trying to parse an upstream log — this is robust and avoids log-file coordination issues
  • The fallback message (non-lockfile-mismatch case) also exits non-zero and emits an annotation — no silent failure
  • CI YAML correctly gates the hint step on steps.bundle.outcome != 'success' && !matrix.experimental so experimental failures won't generate false-positive lockfile annotations

Recommendations

  1. Consider adding a test/ci/bundler_lockfile_hint.bats (or equivalent) with at least three cases: (a) bundle exit 0 re-run, (b) bundle fails with a known lockfile mismatch message, (c) bundle fails with an unrecognized message. This makes the regex patterns auditable and protects against bundler version drift.
  2. Document the source of the pattern strings (e.g., which bundler version / error class they come from) so future maintainers know what to update when patterns drift.

Conclusion

The script is well-structured and non-destructive. The absence of automated tests is a real gap for the regex-based detection logic, but given the narrow scope of this CI tooling change and the low blast radius (worst case: annotation doesn't fire, CI still fails), this does not rise to CHANGES_REQUIRED. Shipping with a follow-up bats test tracked as a low-priority item is acceptable.

@kitcommerce kitcommerce added review:test-quality-done Review complete and removed review:test-quality-pending Review in progress labels Mar 17, 2026
@kitcommerce
Copy link
Contributor Author

Rails Security Review

Verdict: PASS

Findings

No Rails security findings. This PR modifies only CI workflow configuration (.github/workflows/ci.yml) and adds a diagnostic bash script (script/ci/bundler_lockfile_hint). No Rails application code (controllers, models, views, routes, or Ruby source) is touched — zero Rails security attack surface affected.

Recommendations

None.

@kitcommerce
Copy link
Contributor Author

Accessibility Review

Verdict: PASS (N/A)

CI configuration and shell scripts — no user-facing UI or accessibility surface.


🦾 Automated accessibility review — Kit

@kitcommerce
Copy link
Contributor Author

Frontend Review

Verdict: PASS (N/A)

CI YAML and shell scripts — no frontend surface.


🖥️ Automated frontend review — Kit

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

Labels

gate:build-passed Build gate passed review:accessibility-pending Review in progress review:architecture-done Review complete review:database-done Database review complete review:frontend-pending Frontend review in progress review:performance-pending Review in progress review:rails-conventions-done Rails conventions review complete review:rails-security-done Rails security review complete review:security-done Review complete review:simplicity-done Review complete review:test-quality-done Review complete review:wave1-complete review:wave2-complete

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant