[jonahgabriel]

Summary

• Cross-repo pre-commit hook (no-bare-feature-flags) that rejects bare os.getenv("ENABLE_*"), os.environ.get/["ENABLE_*"], process.env.ENABLE_*, and *_ENABLED suffix env var reads
• Lines with # ONEX_FLAG_EXEMPT: <reason> are exempt; exemptions without a reason token are themselves violations
• Approved basenames (feature_flag_resolver.py, contract.yaml, check_bare_feature_flags.py, model_contract_feature_flag.py) and path segments (/tests/, /capabilities/, /config_discovery/) are never flagged
• 21 unit tests covering all detection, exemption, and approval cases

Test plan

• 21 unit tests pass (uv run pytest tests/unit/scripts/test_check_bare_feature_flags.py -v)
• Full test suite passes (452 tests)
• pre-commit run --all-files passes
• Entry point registered in pyproject.toml
• Hook entry added to .pre-commit-hooks.yaml

Closes OMN-5573

Summary by CodeRabbit

• New Features

  • Introduced a feature flag validation tool that automatically detects and reports improper environment variable usage patterns in Python, TypeScript, and JavaScript source files, helping maintain consistent feature flag practices across the codebase.

• Chores

  • Added pre-commit hook configuration enabling automated validation during development commits, with support for exemptions through inline code markers for intentional exceptions.

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request introduces a new pre-commit hook named no-bare-feature-flags that scans Python, TypeScript, and JavaScript files for bare feature-flag environment variable reads. The implementation includes a CLI script with support for approved files/paths, exemption markers, and comprehensive test coverage.

Changes

Cohort / File(s)	Summary
Pre-commit Hook Configuration .pre-commit-hooks.yaml, pyproject.toml	Added no-bare-feature-flags hook definition and registered check-bare-feature-flags as a CLI entry point script.
Feature-Flag Scanner Implementation src/onex_change_control/scripts/check_bare_feature_flags.py	Implemented regex-based scanner for bare environment variable reads matching ENABLE_* and *_ENABLED patterns in Python and TypeScript/JavaScript, with support for approved basenames/paths and ONEX_FLAG_EXEMPT: exemption markers.
Unit Tests tests/unit/scripts/test_check_bare_feature_flags.py	Comprehensive test suite covering Python and TypeScript pattern detection, approved file/path exclusions, comment filtering, exemption handling, and CLI exit codes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Bare flags wander wild and free,
But now I hunt them carefully!
With regex sharp and paths approved,
Feature gates are gently grooved—
No naked reads shall pass my sight,
Pre-commit hooks make code just right! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 17.86% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: adding a pre-commit validator for bare feature flags. It is specific, concise, and clearly conveys the primary purpose of the changeset.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch jonahgabriel/omn-5573-check-bare-feature-flags

📝 Coding Plan

• Generate coding plan for human review comments

---

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

[coderabbitai]

🧹 Nitpick comments (1)

src/onex_change_control/scripts/check_bare_feature_flags.py (1)

71-74: Consider making _ScanResult public or documenting its semi-public status.

check_file_full is a public function that returns _ScanResult, but the underscore prefix suggests it's a private type. This creates a minor API inconsistency. For a pre-commit hook with limited external consumers, this is acceptable, but consider renaming to ScanResult if external callers are expected.

🤖 Prompt for AI Agents

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

In `@src/onex_change_control/scripts/check_bare_feature_flags.py` around lines 71
- 74, The public function check_file_full returns a dataclass named _ScanResult
whose leading underscore implies private scope; either make the type public by
renaming _ScanResult to ScanResult or explicitly document its semi-public API
surface. Locate the dataclass definition named _ScanResult and either change its
identifier to ScanResult and update all references (e.g., return annotations and
any instantiations in check_file_full) or add a module-level docstring/comment
stating that _ScanResult is intentionally exported for callers of
check_file_full, and update __all__ if present to include "_ScanResult" (or
"ScanResult" if renamed).

🤖 Prompt for all review comments with AI agents

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

Nitpick comments:
In `@src/onex_change_control/scripts/check_bare_feature_flags.py`:
- Around line 71-74: The public function check_file_full returns a dataclass
named _ScanResult whose leading underscore implies private scope; either make
the type public by renaming _ScanResult to ScanResult or explicitly document its
semi-public API surface. Locate the dataclass definition named _ScanResult and
either change its identifier to ScanResult and update all references (e.g.,
return annotations and any instantiations in check_file_full) or add a
module-level docstring/comment stating that _ScanResult is intentionally
exported for callers of check_file_full, and update __all__ if present to
include "_ScanResult" (or "ScanResult" if renamed).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 249bc879-8c5a-447d-99fc-faa6e06f94c8

📥 Commits

Reviewing files that changed from the base of the PR and between bc2b338 and bdd09d6.

📒 Files selected for processing (4)

• .pre-commit-hooks.yaml
• pyproject.toml
• src/onex_change_control/scripts/check_bare_feature_flags.py
• tests/unit/scripts/test_check_bare_feature_flags.py