fix(github): eliminate bare gh invocations, error masking, and inaccurate quota defaults (#237)

[dan-petty]

Summary of Changes

Closes #237

This pull request addresses rate limit quota accuracy, error masking, and GitHub CLI routing across the codebase:

1. Quota Accuracy & Error Propagation:
   • src/devops_cli/github/rate_limiter.py: In _extract_rate_limit_endpoint_response, eliminated defaulting reset_epoch to 0.0. Added structured GitHubRateLimitError raising on malformed or empty payloads.
   • src/devops_cli/commands/gh.py: In devops gh rate-limit, removed hardcoded 5000 and 0 defaults, rendering missing metrics as "-". Added error handling on JSON parsing failures and missing resources.
2. Branch Protection Error Handling:
   • src/devops_cli/github/branch_protection.py: In get_remote_branch_protection, distinguished true 404 / "branch not protected" states from other API errors (auth failures, rate limits, JSON decoding errors), raising GitHubOperationError on failure rather than returning None.
   • In _apply_remote_protection, raised GitHubOperationError upon non-zero exit codes and handled errors cleanly per branch in sync_branch_protection.
3. GitHub CLI Invocation Hardening:
   • src/devops_cli/commands/release.py: Routed _execute_release_pr through rate-managed run_gh instead of run_subprocess.
   • src/devops_cli/core/process.py: Centralized ambient credential injection (_inject_gh_credentials) across both synchronous and asynchronous subprocess executions when invoking CONST_GH_CLI.
   • Updated documentation in github_cli.md to reference run_gh.
4. Test Suite Alignments:
   • Added unit test cases for branch protection error propagation and JSON parsing failures in tests/test_github_branch_protection.py.
   • Aligned tests/test_release.py with run_gh dispatch for PR creation.

[Copilot]

🟡 Changes recommended

Critical syntax and unresolved error-handling issues remain.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR hardens GitHub quota handling, error propagation, CLI routing, credential injection, tests, and task documentation.

Changes:

• Removes fabricated quota defaults and improves malformed-response handling.
• Propagates branch-protection and synchronization failures.
• Routes release PR creation through run_gh.
• Updates credential handling, tests, documentation, and task tracking.

File summaries

File	Description
tests/test_release.py	Aligns release tests with run_gh.
tests/test_github_branch_protection.py	Adds error-path coverage.
src/devops_cli/github/rate_limiter.py	Validates quota payloads and values.
src/devops_cli/github/branch_protection.py	Propagates API and update failures.
src/devops_cli/core/process.py	Centralizes GitHub credential injection.
src/devops_cli/commands/release.py	Uses rate-managed PR creation.
src/devops_cli/commands/gh.py	Improves quota parsing and display.
src/devops_cli/ai/knowledge_base/it_domains/tools/github_cli.md	Documents managed CLI usage.
docs/agent/tasks/task-237-rate-limit-quota-accuracy-gh-cli-hardening.md	Tracks the PR task.
docs/agent/tasks/task-235-project-reconcile-rate-quota-exhaustion.md	Updates task status.

Review details

Suppressed comments (3)

docs/agent/tasks/task-235-project-reconcile-rate-quota-exhaustion.md:5

• Completed is not one of the repository's task lifecycle states; the documented terminal state is Done (after the associated PR is merged). Please use the canonical status so task tracking remains machine-readable and consistent.

**Status**: Completed

src/devops_cli/commands/gh.py:1161

• Valid but malformed GraphQL JSON can still crash here: json.loads("[]") or a non-object data value reaches .get() and raises AttributeError, while this narrowed handler only catches decode errors. Treat invalid response shapes as an unavailable quota result (or raise the command's structured error) instead of leaking a traceback.

    except json.JSONDecodeError:
        return {}

src/devops_cli/github/rate_limiter.py:1247

• A payload with resources: {} (or resource entries without a usable remaining value) is accepted without updating any quota and without raising, even though this refresh path is supposed to reject empty/malformed payloads. Track whether at least one valid resource was processed and raise GitHubRateLimitError when none was.

    for r_name, r_info in resources.items():
        if isinstance(r_info, dict) and "remaining" in r_info and r_info["remaining"] is not None:

• Files reviewed: 10/10 changed files
• Comments generated: 5
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.