fix: safety guardrails false positives, pyyaml error status, set_waves install fallback (#319 #320 #321)#325
Merged
Conversation
|
Warning Review limit reached
Next review available in: 57 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
…r status, set_waves install fallback Bug #321 (safety-guardrails.py): check_file_safety matched dangerous patterns against the full path, causing false positives when directory names contained security-related words (e.g. secrets-injector/values.yaml was blocked because "secret" appeared in the parent directory name). Fix: match only against os.path.basename(path).lower(). Bug #319 (map_step_runner.py): parse_requirements_index caught both ImportError (missing PyYAML) and yaml.YAMLError in a single except-Exception block, returning status='malformed' for both — misleading users into thinking their spec was broken when PyYAML was simply not installed. Fix: split exception handlers; ImportError now returns status='pyyaml_missing' with an actionable "pip install pyyaml" message. validate_blueprint_contract updated to handle the new status. Bug #320 (map_orchestrator.py): set_waves ImportError fallback only searched source-checkout layout paths (src/mapify_cli/ relative to __file__ parents), missing packages installed via 'uv tool install' or 'pipx install' which land in ~/.local/share/uv/tools/mapify-cli/lib/python3.X/site-packages/. Fix: extend the candidate list to include common installed-package locations; improve error message to guide uv-tool install users. Also: skip test_write_project_mcp_json_permission_error when running as root (root bypasses chmod 0o444 restrictions — the test's OSError cannot be triggered). Add regression tests for all three bugs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011dqN4Wq6pEtodBmSTxbyZp
azalio
force-pushed
the
claude/compassionate-cerf-uujxi6
branch
from
01bc834 to
113f1d4
Compare
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes three bugs found in the issue tracker. All changes are in Jinja template sources (
templates_src/) with generated trees re-rendered viamake render-templates. Regression tests added for each bug.Bug #321 — safety guardrails blocked files with safe names in directories with dangerous names
check_file_safetymatched dangerous patterns against the full path (path.lower()), sosecrets-injector/values.yamlwas blocked because "secret" appeared in the directory name rather than the filename. The code comment already stated intent to check file names, not paths.Fix: use
os.path.basename(path).lower()for both the fast-path marker check and the regex loop.File:
src/mapify_cli/templates_src/hooks/safety-guardrails.py.jinjaBug #319 —
parse_requirements_indexreturnedstatus='malformed'when PyYAML was missingA single
except Exceptioncaught bothImportError(PyYAML not installed) andyaml.YAMLError(genuine parse error). Users were told their spec was malformed when the real problem was a missing dependency.Fix: split exception handlers.
ImportErrorreturnsstatus='pyyaml_missing'with"Run: pip install pyyaml"in warnings.validate_blueprint_contractupdated to handle the new status.File:
src/mapify_cli/templates_src/map/scripts/map_step_runner.py.jinjaBug #320 —
set_wavesImportError fallback didn't finddependency_graph.pyin uv tool installsThe fallback path search only walked parent directories for
src/mapify_cli/dependency_graph.py(source-checkout layout). When mapify-cli is installed viauv tool installorpipx install, the package is in~/.local/share/uv/tools/mapify-cli/lib/python3.X/site-packages/.Fix: extend candidate list to include common installed-package locations. Improve error message to guide uv-tool users.
File:
src/mapify_cli/templates_src/map/scripts/map_orchestrator.py.jinjaPre-existing test fix
test_write_project_mcp_json_permission_errorwas always failing when the test suite runs as root (root bypasseschmod 0o444). Added@pytest.mark.skipif(os.getuid() == 0, ...).Test plan
make lint— ruff, mypy, pyright all clean (0 errors)make check-render— generated trees match Jinja sourcespytest tests/hooks/test_safety_guardrails.py::TestDirectoryNameFalsePositives— 11 passed (bug safety-guardrails.py: secrets?\.(json|ya?ml|toml) regex blocks legitimate infra paths containing 'secret'/'secrets' in directory names #321 regression)pytest tests/test_map_step_runner.py::TestST003ParseRequirementsIndex::test_st003_pyyaml_missing_returns_distinct_status— passed (bug map_step_runner.py parse_requirements_index() imports PyYAML → validate_blueprint_contract silently reports 'malformed' Requirements Index without it installed #319 regression)pytest tests/test_map_orchestrator.py::TestSetWavesImportFallback— 2 passed (bug set_waves ImportError: mapify_cli.dependency_graph unimportable from system python3 (uv-tool install layout) #320 regression)pytest tests/ -x -q— 2974 passed, 4 skipped (was 1 failed before)Generated by Claude Code