Promote candidate lote1 changes to main (AUD-001/002/003/005/006/041)#20
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
fix: align raw output contract and add regression test (AUD-006)
- type-safe missing-binary detector signature\n- named exit-code constant for root CLI missing\n- pytest monkeypatch/raises simplification in tests\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
fix: enforce non-zero exit when root CLI is missing (AUD-003)
fix: improve generate-plugin CLI error handling (AUD-005)
Apply PR #17 inline review suggestion for robust plugin copy command and clearer remediation steps.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…oting docs: add quickstart troubleshooting blocks (AUD-041)
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
docs: fix AUD-001/AUD-002 command drift
Bring candidate/lote1-audit-2026-03-16 up to date with main to avoid regressing main-only commits during promotion.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
🔍 AI Code ReviewCode Reviewsrc/crawler/formatter.py- _embed_raw(data.get("tree", {}), raw_outputs, cli_map.cli_name)
+ _embed_raw(data.get("commands", {}), raw_outputs)🔵 Info: Changed key from -def _embed_raw(tree: dict, raw_outputs: dict[str, str], cli_name: str) -> None:
+def _embed_raw(tree: dict[str, Any], raw_outputs: dict[str, str]) -> None:🟡 Warning: Type hint improved, but consider using src/crawler/pipeline.py+import sys🔵 Info: Added for +ROOT_CLI_NOT_FOUND_EXIT_CODE = 2
+
+class RootCLIBinaryNotFoundError(RuntimeError):
+ """Raised when the root CLI binary cannot be resolved/executed."""🔵 Info: Good practice to define custom exception and exit code. + if _is_missing_root_cli_binary(cli_name, detection):
+ raise RootCLIBinaryNotFoundError(
+ f"Root CLI binary '{cli_name}' was not found. Ensure it is installed and in PATH."
+ )🔵 Info: Early detection and explicit error raising improves UX. +def _is_missing_root_cli_binary(cli_name: str, detection_result: HelpDetectionResult) -> bool:
+ ...
+ missing_markers = (
+ "command not found",
+ "not recognized as an internal or external command",
+ "no such file or directory",
+ )
+ return any(marker in signal_text for marker in missing_markers)🟡 Warning: Matching error strings is brittle and locale-dependent. Consider checking exit codes or using platform-specific error detection if possible. + except RootCLIBinaryNotFoundError as exc:
+ logger.error("%s", exc)
+ sys.exit(ROOT_CLI_NOT_FOUND_EXIT_CODE)🔵 Info: Proper error handling and exit. src/generator/plugin_generator.py+CLI_USER_ERROR_EXIT_CODE = 2🔵 Info: Consistent exit code definition. + try:
+ cli_map = load_cli_map(args.json_path)
+ except FileNotFoundError:
+ parser.exit(
+ CLI_USER_ERROR_EXIT_CODE,
+ f"Error: input JSON file not found: {args.json_path}\n",
+ )
+ except json.JSONDecodeError as exc:
+ parser.exit(
+ CLI_USER_ERROR_EXIT_CODE,
+ f"Error: invalid JSON in input file {args.json_path}: {exc.msg}\n",
+ )
+ except ValueError as exc:
+ parser.exit(CLI_USER_ERROR_EXIT_CODE, f"Error: {exc}\n")🔵 Info: Good user-friendly error handling with clear exit codes. Tests
🔵 Info: Adding regression tests for raw output embedding is good practice. Summary
No security issues or obvious bugs detected. Code changes improve error handling and robustness. 🤖 gpt-4.1-mini · 2494 tokens · GitHub Models free tier · 0 premium requests |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request promotes a validated batch of changes from the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request promotes a batch of validated changes from a candidate branch to main. The changes are substantial and of high quality, focusing on improving documentation, CLI usability, and robustness. Key improvements include a documentation overhaul to reflect project refactoring and updated commands, enhanced CLI error handling for missing binaries or invalid inputs, and the addition of comprehensive unit tests for the new functionality. The code is clear, the improvements are significant, and I have no specific recommendations for changes as the implementation is solid.
Address PR #20 review concern about locale-dependent missing-binary detection by adding a PATH preflight check in crawl_cli and a focused regression test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🔍 AI Code Review# Code Review Summary
---
## src/crawler/formatter.py
🔵 **Info**
- Changed `_embed_raw` to remove `cli_name` parameter, simplifying the function signature and call sites.
- Changed key from `"tree"` to `"commands"` when embedding raw help text, likely a bug fix.
---
## src/crawler/pipeline.py
🟡 **Warning**
- Added pre-check with `shutil.which(cli_name)` to fast-fail if root CLI binary is missing. Good for UX.
- Also added heuristic `_is_missing_root_cli_binary` to detect missing binary from help detection stderr/stdout.
- Raises `RootCLIBinaryNotFoundError` on missing binary, caught in `main()` to exit with code 2.
- **Consider:** `shutil.which` may not detect binaries in some environments (e.g., non-PATH locations). The heuristic helps but may produce false negatives/positives.
- **Suggestion:** Document this behavior clearly for users.
🔵 **Info**
- Added explicit exit code constant `ROOT_CLI_NOT_FOUND_EXIT_CODE = 2` for clarity and maintainability.
---
## src/generator/plugin_generator.py
🟡 **Warning**
- Added error handling around JSON loading with explicit exit codes and user-friendly messages.
- Good practice to fail fast on invalid input files.
- **Suggestion:** Consider logging the error before exiting for debugging purposes.
---
# Security Considerations
🔵 No direct security issues detected.
- No user input is executed or evaluated unsafely.
- Error handling improves robustness.
---
# Bugs
🔵 The fix in `formatter.py` from `"tree"` to `"commands"` likely fixes a bug where raw help text was not embedded correctly.
---
# Best Practices
🟢 Good use of custom exceptions and exit codes for clearer error handling.
🟢 Improved user feedback on missing binaries and invalid input files.
---
# Summary
This PR improves robustness and user experience by:
- Adding early detection of missing CLI binaries.
- Improving error handling on JSON input.
- Fixing a bug in embedding raw help text.
No critical issues found. Minor suggestions for documentation and logging improvements.
🤖 gpt-4.1-mini · 2036 tokens · GitHub Models free tier · 0 premium requests |
|
Review sweep done on PR #20 (all comments + review notes, including bot feedback).
Validation run:
Inline review threads status: none open or existing on this PR at this moment, so nothing to resolve or close. |
1 participant
Summary
Promote the validated lote1 batch from candidate to main after merging worker PRs and syncing candidate with main.
Included work:
Hygiene and sync performed
.devartifacts from worker PR branches (without touching functional code changes).Validation highlights
tests/unit/test_pipeline_missing_cli_exit.pytests/unit/test_generate_plugin_cli_errors.pytests/unit/test_formatter_raw_contract.pyNotes
All worker PRs are merged into
candidate/lote1-audit-2026-03-16; this PR promotes the consolidated, cleaned candidate state tomain.