Skip to content

Mask development command updates#756

Merged
forstmeier merged 5 commits intomasterfrom
mask-development-command-updates
Feb 15, 2026
Merged

Mask development command updates#756
forstmeier merged 5 commits intomasterfrom
mask-development-command-updates

Conversation

@forstmeier
Copy link
Collaborator

@forstmeier forstmeier commented Feb 13, 2026

Overview

Changes

  • add explicit non-zero error code propagation to mask development commands

Context

The GitHub workflow was erroring in inaccurate locations so this should fix that.

Summary by CodeRabbit

  • Chores
    • Renamed Rust check to reflect package-level checks and switched to package-wide checking.
    • Re-enabled and reordered an additional Rust verification step to run alongside format, lint, and test stages.
    • Added an explicit success message for Python type checks.
    • Hardened Python test flow so coverage report generation failures now cause the pipeline to fail.

@forstmeier forstmeier self-assigned this Feb 13, 2026
@forstmeier forstmeier added the markdown Markdown code updates label Feb 13, 2026
@github-project-automation github-project-automation bot moved this to To Do in Overview Feb 13, 2026
@github-project-automation github-project-automation bot moved this to To Do in Overview Feb 13, 2026
@forstmeier forstmeier moved this from To Do to In Progress in Overview Feb 13, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 13, 2026

📝 Walkthrough

Walkthrough

Rename and reorder Rust checks to use cargo check --workspace, re-enable an additional Rust check in the aggregate dev target, add explicit success messaging for Python type checks, and enforce fail-fast behavior (add || exit on coverage/generation steps). All edits are script-level in maskfile.md; no public APIs changed.

Changes

Cohort / File(s) Summary
CI / developer checklist
maskfile.md
Renamed Rust section wording to "Rust packages" and switched to cargo check --workspace; reintroduced an extra Rust check in the consolidated dev target and reordered steps; added explicit Python type-check success message; strengthened fail-fast behavior by appending `

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Mask development command updates' is related to the changeset, which modifies mask development commands to improve error code propagation and execution flow in maskfile.md.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into master

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

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch mask-development-command-updates

No actionable comments were generated in the recent review. 🎉


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.

@forstmeier forstmeier changed the base branch from master to infrastructure February 13, 2026 00:52
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 13, 2026

Greptile Overview

Greptile Summary

This PR adds explicit non-zero error code propagation (|| exit 1) to mask development commands in maskfile.md to ensure CI workflows fail at the correct location when errors occur. The PR also includes significant infrastructure updates that appear to be from a previous merge that were committed together.

Key changes in maskfile.md:

  • Added || exit 1 to Python commands: ruff format, ruff check, uvx vulture, uvx ty check, and pytest coverage pipeline
  • Added || exit 1 to Rust commands: cargo clippy and cargo test (when coverage tools unavailable)
  • Reordered Rust all task: format now runs before check (check doesn't modify files)
  • Updated service names from oscmcompany-* to oscm-* and ECR repository paths
  • Changed pulumi whoami to pulumi org get-default and stack references from oscmcompany to fund

Additional infrastructure updates included:

  • Flox manifest now includes [profile] section for environment variable loading
  • Infrastructure code adds GitHub OIDC provider, secrets management, budget alerts, S3 encryption, and IAM policies
  • New test suite validates infrastructure security requirements
  • New runbook documentation for GitHub environment setup

Confidence Score: 5/5

  • This PR is safe to merge with proper error propagation that improves CI reliability
  • The main changes add explicit error code propagation (|| exit 1) to development commands, which is a best practice that prevents silent failures in CI. The implementation is straightforward and consistent. The infrastructure changes appear to be from a previous merge (dbb07c4) and include security improvements with corresponding test coverage.
  • No files require special attention

Important Files Changed

Filename Overview
maskfile.md Added `
infrastructure/main.py Major infrastructure refactor: added GitHub OIDC, secrets management, budget alerts, and security hardening
libraries/python/tests/test_infrastructure_configuration.py New test suite validating infrastructure security configuration requirements

Last reviewed commit: e535159

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request adds explicit non-zero error code propagation to mask development commands and includes significant infrastructure refactoring. The changes rename the Pulumi project from "oscm" to "fund", consolidate secrets management, add comprehensive IAM policies for GitHub Actions, and introduce infrastructure testing and documentation.

Changes:

  • Added || exit 1 to development commands in maskfile.md for proper error propagation in CI/CD workflows
  • Refactored infrastructure/main.py with consolidated secrets management, GitHub OIDC provider setup, and least-privilege IAM policies
  • Created infrastructure tests and GitHub environment runbook documentation

Reviewed changes

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Show a summary per file
File Description
maskfile.md Added explicit error code propagation to Rust and Python development commands; updated organization/project naming from oscmcompany to fund/oscm
libraries/python/tests/test_infrastructure_configuration.py New test file for infrastructure configuration validation
infrastructure/github_environment_runbook.md New runbook documenting GitHub environment setup for Pulumi operations
infrastructure/main.py Major refactoring with consolidated secrets, GitHub OIDC provider, and least-privilege IAM policies
infrastructure/Pulumi.yaml Renamed project from "oscm" to "fund"
infrastructure/Pulumi.production.yaml Added comprehensive encrypted configuration for secrets, GitHub integration, and budget alerts
.flox/env/manifest.toml Added profile section for .env file sourcing
.flox/env/manifest.lock Updated lock file with new profile configuration

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 13, 2026

Greptile Overview

Greptile Summary

This PR adds explicit error code propagation to development commands in the maskfile and updates test assertions to reflect infrastructure naming changes. The changes ensure that when development commands fail (linting, type checking, tests, etc.), the failure is properly propagated to CI workflows with non-zero exit codes using || exit 1, preventing GitHub Actions from reporting success when commands fail. The test file updates correct assertion strings from oscmcompany: to fund: to match the actual Pulumi configuration namespace, and fix the OIDC capitalization from OIDC to match the infrastructure code.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The changes are straightforward improvements to error handling in development commands and test assertion corrections. All modifications follow proper bash error propagation patterns with || exit 1, and the test updates accurately reflect the actual infrastructure configuration (verified against Pulumi.production.yaml and infrastructure/__main__.py). The changes are isolated to development tooling and tests with no impact on production code.
  • No files require special attention

Important Files Changed

Filename Overview
maskfile.md Added `
libraries/python/tests/test_infrastructure_configuration.py Updated test assertions to reflect project name change from oscmcompany to fund and corrected OIDC capitalization

Last reviewed commit: 4da577b

chrisaddy
chrisaddy previously approved these changes Feb 14, 2026
@github-project-automation github-project-automation bot moved this from To Do to In Progress in Overview Feb 14, 2026
Base automatically changed from infrastructure to master February 14, 2026 03:20
@forstmeier forstmeier dismissed chrisaddy’s stale review February 14, 2026 03:20

The base branch was changed.

Copilot AI review requested due to automatic review settings February 14, 2026 03:23
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 14, 2026

Greptile Overview

Greptile Summary

Added explicit non-zero error code propagation (|| exit 1) to development commands in the maskfile to ensure that failures in linting, formatting, type checking, and testing are properly detected and reported in CI/CD workflows.

Key changes:

  • Added || exit 1 to cargo clippy (Rust lint)
  • Added || exit 1 to cargo test fallback paths when coverage tools unavailable
  • Added || exit 1 to ruff format (Python formatter)
  • Added || exit 1 to ruff check (Python linter)
  • Added || exit 1 to uvx vulture (Python dead code checker)
  • Added || exit 1 to uvx ty check (Python type checker)
  • Added || exit 1 to Python test coverage chain
  • Reordered Rust all task to run format before check (cosmetic improvement)
  • Updated echo messages for clarity ("Check Rust packages" instead of "Check Rust compilation")
  • Added success message to Python type check task

All changes are safe and improve error detection reliability without altering core functionality.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The changes are straightforward and improve error handling without modifying any core logic. All additions follow bash best practices for explicit error propagation, which aligns with the existing set -euo pipefail usage throughout the file. The changes address a real issue where CI/CD workflows were not properly detecting failures.
  • No files require special attention

Important Files Changed

Filename Overview
maskfile.md Added explicit error code propagation (`

Last reviewed commit: 4ca66fa

Copy link
Contributor

@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

🤖 Fix all issues with AI agents
In `@maskfile.md`:
- Around line 357-361: The "cargo check --workspace" command does not propagate
failure like other validation steps; update the command in maskfile.md (the
cargo check invocation) to append "|| exit 1" so a non-zero exit from cargo
check causes the script to exit with a failure status, matching the pattern used
by commands such as the cargo clippy invocation.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 9 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@coveralls
Copy link
Collaborator

coveralls commented Feb 14, 2026

Coverage Status

coverage: 86.212%. remained the same
when pulling d3d7328 on mask-development-command-updates
into 066efb8 on master.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 14, 2026

Greptile Overview

Greptile Summary

This PR improves error propagation in mask development commands by adding explicit || exit 1 to all individual command scripts. This ensures that when any command fails (cargo check, cargo fmt, ruff format, etc.), the shell script will immediately exit with a non-zero status code, allowing the GitHub Actions CI pipeline to correctly detect and report failures.

Key improvements:

  • Added || exit 1 to all Rust commands: cargo update, cargo check --workspace, cargo fmt --all, cargo clippy, and cargo test --workspace --verbose
  • Added || exit 1 to all Python commands: uv sync, ruff format, vulture, ruff check, uvx ty check, and coverage test chain
  • Fixed message in rust check command from "Check Rust compilation" to "Check Rust packages" (more accurate)
  • Added missing success message for Python type-check command
  • Reordered rust all workflow: format now runs before check (potentially intentional to auto-fix formatting before checking)

The changes align with bash best practices when set -euo pipefail is used, as some command failures in subshells or specific contexts may not propagate correctly. The explicit || exit 1 ensures proper error handling for CI workflows that depend on exit codes.

Confidence Score: 5/5

  • This PR is safe to merge with no risk - adds defensive error handling without changing logic
  • All changes add explicit error propagation (|| exit 1) to development commands, ensuring CI properly detects failures. No logic changes, only defensive error handling. The reordering of format before check in rust workflow is a logical improvement. Changes are straightforward, well-scoped, and directly address the stated problem of GitHub workflows erroring in inaccurate locations.
  • No files require special attention

Important Files Changed

Filename Overview
maskfile.md Added explicit `

Last reviewed commit: 3001ea5

coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 14, 2026
This commit addresses all 8 review threads from copilot-pull-request-reviewer by removing redundant || exit 1 guards that were overriding the set -euo pipefail directive.

Key changes:
- Removed || exit 1 from standalone commands (cargo clippy, cargo test, cargo update, cargo fmt, ruff format, ruff check, uvx vulture, uvx ty check, uv sync)
- Changed || exit 1 to || exit on the Python coverage chain to preserve actual exit codes (set -e doesn't reliably exit in && chains, so this guard is necessary)
- Updated rust check task description from "Check Rust compilation" to "Check Rust packages" to match the workspace-level check behavior

Rationale:
The set -euo pipefail directive at the start of each script already ensures that standalone commands exit immediately on failure. Adding || exit 1 after these commands was redundant and had the negative effect of replacing the tool's actual exit code with a generic 1, making debugging harder. The Python coverage && chain is the exception where the guard is necessary, but we now use || exit (without the 1) to ensure failure propagation while preserving the original exit status.

Verification:
- All local checks passing: mask development rust all
- All local checks passing: mask development python all
- All 8 review threads responded to and resolved
- Verified with test scripts that set -e handles standalone commands but not && chains

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 14, 2026

Greptile Overview

Greptile Summary

This PR fixes a CI reliability issue in maskfile.md where the Python test/coverage && chain could fail silently. In bash, set -e does not apply to commands within &&/|| lists (per POSIX spec), so a failing pytest or coverage step would not cause the script to exit — CI would report success even when tests failed. The || exit addition correctly propagates the failure.

Additional changes:

  • Reorder Rust all command: Moves format before check, which is the conventional order — format code first, then verify it compiles.
  • Add --workspace to cargo check: Aligns with other workspace-level commands (cargo test --workspace, cargo fmt --all) to ensure all crates are checked.
  • Add success echo to Python type-check: Adds a missing completion message for consistency with all other mask commands.
  • Update check description: Renames from "Check Rust compilation" to "Check Rust packages" to better reflect the --workspace scope.

Confidence Score: 4/5

  • This PR is safe to merge — it fixes a real CI bug and makes minor consistency improvements to build scripts.
  • Score of 4 reflects that the changes are correct and fix a real issue (silent test failures in CI). The || exit fix is verified to be necessary due to bash's set -e exemption for AND-OR lists. The other changes (reordering, --workspace, echo message) are low-risk improvements. Deducting 1 point because || exit without an explicit exit code relies on $? implicitly, which while correct, is slightly less explicit than || { exit $?; }.
  • No files require special attention — all changes in maskfile.md are straightforward and correct.

Important Files Changed

Filename Overview
maskfile.md Fixes silent failure in Python test coverage pipeline by adding `

Last reviewed commit: d3d7328

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@forstmeier forstmeier merged commit 17d5629 into master Feb 15, 2026
12 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Overview Feb 15, 2026
@github-project-automation github-project-automation bot moved this from In Progress to Done in Overview Feb 15, 2026
@forstmeier forstmeier deleted the mask-development-command-updates branch February 15, 2026 02:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

markdown Markdown code updates

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants