Skip to content

[CFX-8118] fix(ci): use bash-completion@2 on macOS smoke runners - #910

Open
ajalon1 wants to merge 3 commits into
datarobot-oss:mainfrom
ajalon1:aj/fix-ci-build-failures
Open

[CFX-8118] fix(ci): use bash-completion@2 on macOS smoke runners#910
ajalon1 wants to merge 3 commits into
datarobot-oss:mainfrom
ajalon1:aj/fix-ci-build-failures

Conversation

@ajalon1

@ajalon1 ajalon1 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

RATIONALE

CI smoke jobs after merges to main have been failing before the actual CLI smoke coverage can run. The deterministic issue in this PR is the macOS dependency setup: the smoke action installs bash-completion, while the CLI completion installer detects and recommends Homebrew's bash-completion@2 formula.

Related: CFX-8118

CHANGES

  • Install bash-completion@2 on macOS smoke runners to match CLI completion detection.
  • Leave Linux apt setup unchanged. The available CI metadata only showed apt exit code 100, not a root cause distinct from the existing packages.microsoft.com cleanup.

TESTING

  • ruby -e 'require "yaml"; ... YAML.load_file(...)' for changed YAML files
  • task build
  • expect ./smoke_test_scripts/expect_completion.exp with isolated HOME and XDG_CONFIG_HOME
  • task lint

PR Automation

Comment-Commands: Trigger CI by commenting on the PR:

  • /trigger-smoke-test or /trigger-test-smoke - Run smoke tests
  • /trigger-install-test or /trigger-test-install - Run installation tests

Labels: Apply labels to trigger workflows:

  • run-smoke-tests or go - Run smoke tests on demand (only works for non-forked PRs)

Important

For Forked PRs: The run-smoke-tests label won't work. A required Smoke Tests check will block merge until a maintainer acts:

  • A maintainer uses /approve-smoke-tests to run smoke tests (results will set the check)
  • A maintainer uses /skip-smoke-tests to bypass the check without running tests

Please comment requesting a maintainer review if you need smoke tests to run.


Note

Cursor Bugbot is generating a summary for commit d37612d. Configure here.

ajalon1 and others added 2 commits September 10, 2026 10:32
Retry Linux apt dependency setup before running smoke tests so transient GitHub-hosted mirror failures do not fail main CI runs.

Install bash-completion@2 on macOS to match the CLI completion installer detection and user-facing guidance.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
@ajalon1 ajalon1 changed the title fix(ci): stabilize smoke dependency setup [CFX-8118] fix(ci): stabilize smoke dependency setup Sep 10, 2026
Remove the generic Linux apt retry from the smoke dependency setup. The available CI metadata only showed apt exit 100, not a distinct root cause beyond the existing packages.microsoft.com cleanup.

Keep the deterministic macOS fix: install bash-completion@2, matching the CLI completion installer detection and guidance.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
@ajalon1 ajalon1 changed the title [CFX-8118] fix(ci): stabilize smoke dependency setup [CFX-8118] fix(ci): use bash-completion@2 on macOS smoke runners Sep 10, 2026
@ajalon1
ajalon1 marked this pull request as ready for review September 10, 2026 18:14
@ajalon1
ajalon1 requested a review from a team as a code owner September 10, 2026 18:14
@ajalon1

ajalon1 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

/approve-smoke-tests

@github-actions

Copy link
Copy Markdown
Contributor

🔐 Fork PR smoke tests triggered by @ajalon1

⚠️ Security Notice: This will run tests with access to repository secrets.

What happens next:

  1. Security scans will run automatically (Trivy, gosec)
  2. If security scans pass, smoke tests will run
  3. Results will be posted as PR comments

⚠️ Important: Review the PR code carefully before approving!

Warning

This PR modifies files under .github/, which may alter CI workflows, branch protection rules, or secrets handling. Review these changes carefully before proceeding — malicious workflow changes could exfiltrate secrets.

Changed files:

  • .github/actions/install-deps/action.yaml

shell: bash
run: |
brew install expect bash-completion
brew install expect bash-completion@2

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

on ubuntu, apt get bash-completion automatically get bash-completion v2series. brew does not,bash-completiongives usv1`.

@datarobot-pr-review-router

Copy link
Copy Markdown

Code Ownership

Cli Maintainers

  • .github/actions/install-deps/action.yaml

Review requested from the teams above. Labels will be removed automatically upon approval.

@github-actions

Copy link
Copy Markdown
Contributor

🔐 Fork smoke tests started by maintainer

⏳ Security scans passed. Running smoke tests...

Commit: d37612d9b6ce2e5ccbcce4b5c26cb246fff3af46
View run

Warning

This PR modifies files under .github/, which may alter CI workflows, branch protection rules, or secrets handling. Review these changes carefully before proceeding — malicious workflow changes could exfiltrate secrets.

Changed files:

  • .github/actions/install-deps/action.yaml

@cursor cursor 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.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit d37612d. Configure here.

shell: bash
run: |
brew install expect bash-completion
brew install expect bash-completion@2

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Composite action change skips PR CI

Low Severity

The composite-action CI rule flags this because callers pin install-deps to @main. This PR's smoke jobs still run the old bash-completion install and never exercise bash-completion@2, so the formula change is unvalidated until after merge.

Fix in Cursor Fix in Web

Triggered by project rule: Bugbot Rules for DataRobot CLI

Reviewed by Cursor Bugbot for commit d37612d. Configure here.

@ajalon1 ajalon1 Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I know, I know. Will be monitoring post-merge CI once this goes in

@github-actions

Copy link
Copy Markdown
Contributor

All smoke tests passed! (Fork PR)

✅ Security Scan: success
✅ Linux: success
✅ Windows: success

View run details

@chasdr chasdr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 👍

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants