Skip to content

ci: add Brev launchable shellcheck and provider compatibility matrix#829

Merged
elookpotts-nvidia merged 5 commits intomainfrom
elookpotts/brev-test
Apr 10, 2026
Merged

ci: add Brev launchable shellcheck and provider compatibility matrix#829
elookpotts-nvidia merged 5 commits intomainfrom
elookpotts/brev-test

Conversation

@elookpotts-nvidia
Copy link
Copy Markdown
Contributor

@elookpotts-nvidia elookpotts-nvidia commented Apr 7, 2026

Description

Adds CI for the Brev launchable (deployments/brev/).

Shellcheck runs on every PR touching deployments/brev/** via bazel test //deployments/brev/... (hermetic shellcheck binary, required check in pr-checks.yml).

Provider compatibility (provider-compat job) provisions L40/L40S instances across all Brev providers and runs hello world, disk fill (nvcr.io/nvidia/nemo:24.12, ~40 GB), and GPU workload tests, then commits results to the compatibility matrix in deployments/brev/README.md. The weekly cron is disabled until Brev supports long-lived API tokens.

setup.sh: fixes Docker data-root detection to skip read-only filesystems (resolves Nebius failures where /mnt/cloud-metadata was incorrectly selected).

Issue #None

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

Summary by CodeRabbit

  • New Features
    • Manual "Brev Launchable" workflow to run automated provider compatibility checks and emit PASS/FAIL results.
  • Tests
    • Added disk-fill validation and platform-aware shellcheck test.
    • PR checks now conditionally run brev-related compatibility tests.
  • Documentation
    • Compatibility Matrix with standardized test definitions and automatic update support.
  • Chores
    • Improved setup script robustness, tooling installation, and CI cleanup/teardown behavior.

@elookpotts-nvidia elookpotts-nvidia requested a review from a team as a code owner April 7, 2026 21:59
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 7, 2026

📝 Walkthrough

Walkthrough

Adds Brev compatibility automation: GitHub Actions workflows and Bazel test infra, a Claude-driven Brev provider compatibility runner (prompt/spec, setup script, tests), a disk-fill task, shellcheck integration, README compatibility matrix updates, and CI guardrails and cleanup steps.

Changes

Cohort / File(s) Summary
GitHub Actions: Brev workflow
.github/workflows/brev.yml
New manual workflow to install Brev/Node, login, install Claude Code skills, render deployments/brev/prompt.md, run npx @anthropic-ai/claude-code``, enforce git guardrail (only deployments/brev/README.md allowed), push README updates, validate `compat-result.txt`, and best-effort delete created Brev instances.
PR gating
.github/workflows/pr-checks.yaml
Adds brev paths filter/output and a conditional brev job that runs bazel test //deployments/brev/... when relevant.
Bazel module: shellcheck binaries
MODULE.bazel
Adds http_archive entries for hermetic ShellCheck v0.10.0 binaries (shellcheck_linux_x86_64, shellcheck_darwin_arm64) for use by Bazel tests.
Bazel targets & shellcheck test
deployments/brev/BUILD, deployments/brev/shellcheck_test.sh
Adds config_setting platform selectors and an sh_test(name = "shellcheck") that selects platform-specific shellcheck binary; new shellcheck_test.sh runs $SHELLCHECK --severity=warning against SETUP_SH.
Brev deployment spec & prompt
deployments/brev/prompt.md, deployments/brev/README.md
Adds an automated, no-interaction Brev compatibility prompt/spec (prompt.md) describing discovery, parallel creation, per-instance tests, teardown, and README matrix replacement; adds a Compatibility Matrix and update template to README.md.
Instance setup script
deployments/brev/setup.sh
Updates quoting for user/group and apt repo handling, probes docker data-root mounts for writability, fixes GOPATH/bin and OSMO PATH persistence with quoted exports and shellcheck directives.
Disk-fill test workflow
deployments/brev/disk-fill-test.yaml
New disk-fill-test workflow defining large-image task that runs nvcr.io/nvidia/nemo:24.12 and executes python3 -c "import nemo; print(nemo.__version__)".

Sequence Diagram(s)

sequenceDiagram
    participant GH as GitHub Actions
    participant Repo as Repository
    participant Brev as Brev CLI/Service
    participant Claude as Claude Code
    participant Instance as Remote Instance / K8s

    GH->>Repo: checkout, read `deployments/brev/prompt.md`
    GH->>Brev: install/login, install Claude skills
    GH->>Claude: run rendered prompt via `npx `@anthropic-ai/claude-code``
    Claude-->>GH: plan (instances, tests, README update)
    GH->>Brev: create instances (parallel)
    Brev-->>GH: instance IDs / endpoints
    GH->>Brev: `brev exec` -> run `deployments/brev/setup.sh` on instances
    Brev-->>Instance: deploy OSMO components
    loop per-instance polling
      GH->>Instance: poll pod/workflow status
      Instance-->>GH: status updates
    end
    GH->>Repo: validate git diff (guardrail)
    alt README changed
      GH->>Repo: commit & push `deployments/brev/README.md`
    end
    GH->>Repo: verify `compat-result.txt` (PASS/FAIL)
    GH->>Brev: delete created instances (cleanup)
    Brev-->>GH: deletion results
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I hopped through YAML, scripts, and care,
I nudged the CLI and seeded the air.
Instances rose, tests danced in a spin —
The README learned what each had been.
I nibbled logs, then swept the lawn; carrot win. 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main changes: adding CI infrastructure (ShellCheck via Bazel and a provider compatibility workflow) for the Brev launchable.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ 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 elookpotts/brev-test

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

Base automatically changed from elookpotts/brev-mount to main April 7, 2026 23:14
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.

Actionable comments posted: 2

🤖 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/brev.yml:
- Around line 69-72: The workflow step named "Install Brev CLI" currently pipes
a script from the mutable main branch into bash (curl | bash); replace that with
downloading the published GitHub Release tarball for the desired Brev version,
verify the release checksum/signature, extract the binary into $HOME/.brev/bin
(create the directory if needed), and then append $HOME/.brev/bin to
GITHUB_PATH; ensure the run block references a fixed release tag/version rather
than main and includes explicit checksum verification before extraction.
- Around line 111-116: The readiness check assigns NOT_READY from a pipeline
using grep -vcE which can exit non-zero when there are no matches, causing the
current "|| echo \"error\"" fallback to inject "error" and break the equality
check against "0". Change the pipeline that sets NOT_READY (the
ssh/kubectl/awk/grep pipeline) so it always returns a numeric count on success
or fallback to "0" on failure — for example replace the grep-count approach with
a robust alternative (e.g., pipe through wc -l or append "|| true"/"|| echo 0"
to the pipeline) so NOT_READY is reliably a number and the if [ "$NOT_READY" =
"0" ] check (and the readiness loop) works correctly.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 435ac92f-5aed-4b15-a42e-8e405805a0b7

📥 Commits

Reviewing files that changed from the base of the PR and between df4bc8d and 0791e78.

📒 Files selected for processing (2)
  • .github/workflows/brev.yml
  • deployments/brev/disk-fill-test.yaml

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 42.82%. Comparing base (f6535b9) to head (0eeee45).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #829   +/-   ##
=======================================
  Coverage   42.82%   42.82%           
=======================================
  Files         203      203           
  Lines       27123    27123           
  Branches     7759     7759           
=======================================
  Hits        11616    11616           
  Misses      15398    15398           
  Partials      109      109           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 9, 2026

📖 Docs preview: https://d3in15bfzp49i0.cloudfront.net/829/index.html

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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@deployments/brev/prompt.md`:
- Around line 54-56: The markdown has unlabeled fenced code blocks causing
markdownlint MD040; update each fence to include an appropriate language
identifier: change the block containing "brev exec <instance>
`@deployments/brev/setup.sh`" to use ```bash, change the block with "Workflow ID -
<id>" to use ```text, and change the table block beginning "| Provider |
Instance Type | GPU | Hello World | Disk Fill | GPU Workload | Notes |" to use
```text (or ```markdown if you prefer table syntax highlighting); keep the
existing fence markers and only add the language tokens so the three snippets
are recognized by linters.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: dcc43dd7-6e43-43f2-97b0-e59dacc3020a

📥 Commits

Reviewing files that changed from the base of the PR and between 0791e78 and a6904a6.

📒 Files selected for processing (8)
  • .github/workflows/brev.yml
  • .github/workflows/pr-checks.yaml
  • MODULE.bazel
  • deployments/brev/BUILD
  • deployments/brev/README.md
  • deployments/brev/prompt.md
  • deployments/brev/setup.sh
  • deployments/brev/shellcheck_test.sh
✅ Files skipped from review due to trivial changes (1)
  • deployments/brev/setup.sh
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/brev.yml

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.

Actionable comments posted: 2

♻️ Duplicate comments (1)
deployments/brev/prompt.md (1)

54-56: ⚠️ Potential issue | 🟡 Minor

Add language identifiers to fenced code blocks (MD040).

These three fenced blocks are unlabeled and will keep triggering markdownlint.

🛠️ Proposed fix
-```
+```bash
 brev exec <instance> `@deployments/brev/setup.sh`

- +text
Workflow ID -


-```
+```text
| Provider | Instance Type | GPU | Hello World | Disk Fill | GPU Workload | Notes |
</details>


Also applies to: 79-81, 121-123

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @deployments/brev/prompt.md around lines 54 - 56, The three unlabeled fenced
code blocks need language identifiers to satisfy markdownlint MD040: update the
block containing "brev exec @deployments/brev/setup.sh" to use
bash, the block with "Workflow ID - <id>" to use text (or text/plain), and the block with the table header starting "| Provider | Instance Type | GPU | Hello World | Disk Fill | GPU Workload | Notes |" to use text (or

to locate the blocks and add the appropriate language tag to the opening fence
(also apply the same change to the other occurrences noted around the same
sections).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@deployments/brev/prompt.md`:
- Around line 39-41: The example instance name uses an a100 GPU but the prompt
scope targets L40/L40S; update the example to match the selected GPU families by
replacing the a100 example with an L40-family slug (e.g., change
`osmo-compat-{{GITHUB_RUN_ID}}-hyperstack-a100-2g` to something like
`osmo-compat-{{GITHUB_RUN_ID}}-hyperstack-l40-2g` or `...-hyperstack-l40s-2g`)
so the example aligns with the naming pattern
`osmo-compat-{{GITHUB_RUN_ID}}-<provider>-<gpu-slug>`.

In `@deployments/brev/README.md`:
- Around line 59-63: The HTML comment block in the README contains a stray
double-quote just before the comment terminator (the line with Note: run all
brev commands...), which breaks the comment; open the block around the snippet
containing the cla ude/Note lines and remove the extra `"` so the comment ends
with a proper -->; verify the comment now reads as a well-formed HTML comment
and no other characters were altered.

---

Duplicate comments:
In `@deployments/brev/prompt.md`:
- Around line 54-56: The three unlabeled fenced code blocks need language
identifiers to satisfy markdownlint MD040: update the block containing "brev
exec <instance> `@deployments/brev/setup.sh`" to use ```bash, the block with
"Workflow ID - <id>" to use ```text (or ```text/plain), and the block with the
table header starting "| Provider | Instance Type | GPU | Hello World | Disk
Fill | GPU Workload | Notes |" to use ```text (or ```md/table) so all fenced
blocks are labeled; search for those exact snippets to locate the blocks and add
the appropriate language tag to the opening fence (also apply the same change to
the other occurrences noted around the same sections).
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 90be0525-bd1b-4bc6-b837-731c494e0dad

📥 Commits

Reviewing files that changed from the base of the PR and between a6904a6 and 38426c0.

📒 Files selected for processing (9)
  • .github/workflows/brev.yml
  • .github/workflows/pr-checks.yaml
  • MODULE.bazel
  • deployments/brev/BUILD
  • deployments/brev/README.md
  • deployments/brev/disk-fill-test.yaml
  • deployments/brev/prompt.md
  • deployments/brev/setup.sh
  • deployments/brev/shellcheck_test.sh
✅ Files skipped from review due to trivial changes (6)
  • deployments/brev/shellcheck_test.sh
  • deployments/brev/disk-fill-test.yaml
  • deployments/brev/setup.sh
  • .github/workflows/pr-checks.yaml
  • deployments/brev/BUILD
  • .github/workflows/brev.yml

@elookpotts-nvidia elookpotts-nvidia changed the title ci: add brev launchable shellcheck and weekly e2e workflow ci: add Brev launchable shellcheck and provider compatibility matrix Apr 9, 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.

♻️ Duplicate comments (4)
deployments/brev/prompt.md (4)

121-123: ⚠️ Potential issue | 🟡 Minor

Add language identifier to fenced code block.

The code block lacks a language identifier, triggering markdownlint MD040.

🛠️ Proposed fix
-```
+```text
 | Provider | Instance Type | GPU | Hello World | Disk Fill | GPU Workload | Notes |

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @deployments/brev/prompt.md around lines 121 - 123, The fenced code block
containing the table header line "| Provider | Instance Type | GPU | Hello World
| Disk Fill | GPU Workload | Notes |" is missing a language identifier; add a
language tag (e.g., text or markdown) immediately after the opening
backticks so the block becomes text (or markdown) to satisfy markdownlint
MD040 and avoid the lint warning.


</details>

---

`54-56`: _⚠️ Potential issue_ | _🟡 Minor_

**Add language identifier to fenced code block.**

The code block lacks a language identifier, triggering markdownlint MD040.




<details>
<summary>🛠️ Proposed fix</summary>

```diff
-```
+```bash
 brev exec <instance> `@deployments/brev/setup.sh`
 ```
```

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @deployments/brev/prompt.md around lines 54 - 56, The fenced code block
containing the command "brev exec @deployments/brev/setup.sh" is
missing a language identifier; update that markdown block to use a bash language
tag (e.g., change the opening tobash) so the snippet is properly
recognized and MD040 is resolved.


</details>

---

`41-41`: _⚠️ Potential issue_ | _🟡 Minor_

**Update the instance-name example to match the GPU count.**

The example shows `2g` (2 GPUs) but Phase 1 (lines 30-31) specifies "1 GPU each". The example should use `1g` to align with the spec.




<details>
<summary>🛠️ Proposed fix</summary>

```diff
-(e.g. `osmo-compat-{{GITHUB_RUN_ID}}-hyperstack-a100-2g`)
+(e.g. `osmo-compat-{{GITHUB_RUN_ID}}-hyperstack-l40-1g`)
```

</details>

*Note: This also addresses the past review comment about using `a100` instead of L40/L40S.*

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In `@deployments/brev/prompt.md` at line 41, Update the instance-name example
string in deployments/brev/prompt.md to match Phase 1's "1 GPU each" by changing
the suffix from `-2g`/`2g` to `-1g` (i.e., use
`osmo-compat-{{GITHUB_RUN_ID}}-hyperstack-l40-1g` →
`...-hyperstack-<flavor>-1g`), and also apply the earlier hardware naming change
by using `a100` instead of `l40`/`l40s` in the example so the final example
reads like `osmo-compat-{{GITHUB_RUN_ID}}-hyperstack-a100-1g`.
```

</details>

---

`79-81`: _⚠️ Potential issue_ | _🟡 Minor_

**Add language identifier to fenced code block.**

The code block lacks a language identifier, triggering markdownlint MD040.




<details>
<summary>🛠️ Proposed fix</summary>

```diff
-```
+```text
 Workflow ID - <id>
 ```
```

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @deployments/brev/prompt.md around lines 79 - 81, Update the fenced code
block that contains the literal "Workflow ID - " to include a language
identifier (e.g., change totext) so markdownlint MD040 is satisfied;
locate the block containing the exact string "Workflow ID - " in prompt.md
and replace the opening fence with ```text.


</details>

</blockquote></details>

</blockquote></details>

<details>
<summary>🤖 Prompt for all review comments with AI agents</summary>

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In @deployments/brev/prompt.md:

  • Around line 121-123: The fenced code block containing the table header line "|
    Provider | Instance Type | GPU | Hello World | Disk Fill | GPU Workload | Notes
    |" is missing a language identifier; add a language tag (e.g., ```text or
```text (or ```markdown) to satisfy markdownlint MD040 and avoid the lint
warning.
- Around line 54-56: The fenced code block containing the command "brev exec
<instance> `@deployments/brev/setup.sh`" is missing a language identifier; update
that markdown block to use a bash language tag (e.g., change the opening ``` to
```bash) so the snippet is properly recognized and MD040 is resolved.
- Line 41: Update the instance-name example string in deployments/brev/prompt.md
to match Phase 1's "1 GPU each" by changing the suffix from `-2g`/`2g` to `-1g`
(i.e., use `osmo-compat-{{GITHUB_RUN_ID}}-hyperstack-l40-1g` →
`...-hyperstack-<flavor>-1g`), and also apply the earlier hardware naming change
by using `a100` instead of `l40`/`l40s` in the example so the final example
reads like `osmo-compat-{{GITHUB_RUN_ID}}-hyperstack-a100-1g`.
- Around line 79-81: Update the fenced code block that contains the literal
"Workflow ID - <id>" to include a language identifier (e.g., change ``` to
```text) so markdownlint MD040 is satisfied; locate the block containing the
exact string "Workflow ID - <id>" in prompt.md and replace the opening fence
with ```text.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9f4688fd-0434-44f5-9664-dde5ba8e9aa0

📥 Commits

Reviewing files that changed from the base of the PR and between 38426c0 and 0eeee45.

📒 Files selected for processing (1)
  • deployments/brev/prompt.md

@elookpotts-nvidia elookpotts-nvidia merged commit 0cca61f into main Apr 10, 2026
16 of 19 checks passed
@elookpotts-nvidia elookpotts-nvidia deleted the elookpotts/brev-test branch April 10, 2026 17:08
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.

4 participants