fix(github): eliminate bare gh invocations, harden rate limiter quota tracking, and optimize project reconciliation (#235)

[dan-petty]

Summary

• Eliminates all remaining bare gh CLI invocations project-wide (branch_protection.py, release.py, auth.py, commands/config.py), routing all GitHub CLI subprocesses through rate-managed run_gh with token-bucket pacing and automatic exponential backoff.
• Hardens rate limiter quota tracking: QuotaState metrics default to None instead of 0 so unknown states never produce false zero quota, and errors during rate limit evaluation are explicitly propagated rather than masked.
• Adds CONST_GH_NON_API_COMMANDS (auth, version, help) to constants.py and marks them rate-limit exempt in _is_rate_limit_exempt to avoid triggering unauthenticated /rate_limit queries during initial auth setup.
• Guards extract_json_payload against non-string/mock inputs and preserves run_subprocess namespace in rate_limiter.py for clean test mocking.
• Closes fix(github): project reconcile rate quota exhaustion and unbounded candidate mutations #235.

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate findings affect mutation-budget enforcement, rate-limit accounting, and request handling.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR centralizes GitHub CLI execution through run_gh, strengthens quota-aware pacing and retries, and updates project reconciliation, output handling, tests, dependencies, and documentation.

Changes:

• Adds quota tracking, caching, mutation detection, pagination controls, and rate-managed GitHub operations.
• Updates project reconciliation, PR commands, output utilities, and authentication flows.
• Refreshes tests, documentation, roadmap entries, and task metadata.

File summaries

File	Reviewed changes and final findings
uv.lock	Locks rate-limiting dependencies.
tests/test_pr_cmd.py	Updates PR command mocks and readiness coverage.
tests/test_github_secrets.py	Updates secret-operation mocks.
tests/test_github_rate_limiter.py	Tests quota and parsing behavior.
tests/test_github_pr_monitor.py	Updates monitor subprocess mocks.
tests/test_github_pages.py	Updates Pages command mocks.
tests/test_github_milestones.py	Updates milestone synchronization setup.
tests/test_github_issues.py	Updates issue command mocks.
tests/test_github_client.py	Updates client command mocks.
tests/test_github_branch_protection.py	Updates protection command mocks.
tests/test_config_commands.py	Updates authentication command mocks.
src/devops_cli/output/formatters/panels.py	Renders raw reviews as Markdown payloads.
src/devops_cli/output/file_writer.py	Adds path validation and serialization helpers.
src/devops_cli/lang/en/messages.py	Adds terminal PR messages.
src/devops_cli/github/secrets.py	Routes secret operations through run_gh.
src/devops_cli/github/rate_limiter.py	Adds pacing, caching, quota, retry, async, and pagination logic. L1576 — Moderate (3 votes): Effective request cost can be decremented twice. L1453 — Moderate (1 vote): Invalid utilization input is silently ignored. L1557 — Moderate (1 vote): Retry delay is not capped by max_backoff. L619 — Moderate (1 vote): quota_max_age does not affect cached quota validity. L1398 — Moderate (1 vote): Reaching the page cap does not report truncation. L177 — Moderate (2 votes): Non-dictionary payloads raise an unstructured TypeError. L1196 — Critical (1 vote): Non-POST mutations may be incorrectly cached. L619 — Moderate (2 votes): max_backoff and quota_max_age are stored without affecting behavior. L508 — Moderate (1 vote): Malformed resetAt data raises an unstructured ValueError. L495 — Moderate (1 vote): GraphQL mutations are not detected for mutation pacing and tracking.
src/devops_cli/github/projects.py	Adds owner caching and reconciliation controls. L804 — Moderate (2 votes): Issue synchronization lacks a shared mutation budget. L1110 — Critical (3 votes): Mutation limits reset and count items rather than individual API mutations. L1213 — Moderate (1 vote): Non-open states can be provisioned and reconciled. L1213 — Moderate (1 vote): Closed existing board items are excluded from metadata reconciliation.
src/devops_cli/github/pr_monitor.py	Routes monitoring requests through run_gh.
src/devops_cli/github/pages.py	Routes Pages requests through run_gh.
src/devops_cli/github/issues.py	Routes issue requests through run_gh.
src/devops_cli/github/client.py	Routes label and milestone requests through run_gh.
src/devops_cli/github/branch_protection.py	Routes protection calls through run_gh. L132 — Moderate (1 vote): The project-wide bare-gh elimination claim remains false because release PR creation still uses run_subprocess.
src/devops_cli/config/defaults.py	Adds GitHub limiter defaults.
src/devops_cli/config/constants.py	Adds GitHub command and mutation constants.
src/devops_cli/commands/pr.py	Uses rate-managed PR commands and readiness handling. L731 — Moderate (2 votes): Removing --milestone is an undocumented breaking change. L20 — Moderate (1 vote): Release automation still bypasses the new rate-managed execution.
src/devops_cli/commands/config.py	Routes GitHub authentication through run_gh.
README.md	Updates PR edit documentation.
pyproject.toml	Adds rate-limiting dependencies.
docs/ROADMAP.md	Adds API budget protection roadmap work.
docs/commands/pr.md	Updates PR edit reference.
docs/commands/gh.md	Updates GitHub command reference.
docs/CLI_REFERENCE.md	Synchronizes CLI documentation.
docs/agent/tasks/task-235-project-reconcile-rate-quota-exhaustion.md	Adds task tracking details. L4 — Nit (2 votes): The task still has PR: TBD and Status: In Progress instead of linking this PR and using In Review.
docs/agent/tasks/task-233-paginated-url-dry-run-reconciliation.md	Updates the prior task status.

Review details

Suppressed comments (10)

src/devops_cli/commands/pr.py:20

• The project-wide elimination claim is incomplete: src/devops_cli/commands/release.py still builds gh pr create and _execute_release_pr invokes _get("run_subprocess") for both the primary and label-fallback commands. Release automation therefore bypasses the new pacing and quota tracking; route both calls through run_gh or narrow the PR description.

from devops_cli.github.rate_limiter import run_gh

src/devops_cli/github/branch_protection.py:132

• The PR summary claims all bare gh invocations are now routed through run_gh, but src/devops_cli/commands/release.py:866-873 still executes gh pr create through run_subprocess. The project-wide guarantee is therefore not true; include that path in the change or narrow the claim.

        res = run_gh(cmd, check=False, quiet=True)

src/devops_cli/github/projects.py:1215

• Although the default is open, exposing state and passing it directly to both candidate fetches allows callers to request all or closed; those records are then provisioned and reconciled. That bypasses the issue's requirement to restrict candidate provisioning to open issues/PRs. Keep reconciliation fetches fixed to open or validate/reject non-open states.

    issues = _fetch_repository_issues(repo, state=state)
    prs = _fetch_repository_prs(repo, state=state)
    candidates = issues + prs

src/devops_cli/github/projects.py:1214

• Open-only filtering is applied to both candidate provisioning and field reconciliation. Items that are already on the board but now closed are absent from these API results, so their existing custom fields are never reconciled; keep provisioning limited to open items while separately evaluating metadata for all existing board URLs.

    issues = _fetch_repository_issues(repo, state=state)
    prs = _fetch_repository_prs(repo, state=state)

src/devops_cli/github/rate_limiter.py:1456

• The record_utilization exception handler discards TypeError/ValueError, so malformed quota state or invalid utilization input is silently ignored. That contradicts the new propagation behavior and can leave the limiter believing it has quota when its accounting failed; let the exception propagate (or convert it to the typed rate-limit error) instead of using pass.

        try:
            limiter.record_utilization(resource, cost=cost)
        except TypeError, ValueError:
            pass

src/devops_cli/github/rate_limiter.py:1559

• max_backoff is configurable on the limiter, but this retry path uses the uncapped value from calculate_backoff_delay. A future reset timestamp can therefore make a retry sleep far longer than the configured 60-second bound, defeating the bounded-backoff circuit. Cap the computed delay with limiter.max_backoff before sleeping.

    backoff = limiter.calculate_backoff_delay(
        combined_err, attempt=attempt + 1, subcommand=target_resource
    )

src/devops_cli/github/rate_limiter.py:620

• The new quota_max_age value is only stored; quota validity still checks only reset_epoch, never last_updated. A persisted response can therefore remain valid for hours until reset despite the 300-second maximum age, so pacing and the project safety check may rely on stale quota data. Apply this age when resolving/validating cached state, or remove the unused option.

        max_backoff: float = DEFAULT_GH_MAX_BACKOFF_SECONDS,
        quota_max_age: float = DEFAULT_GH_QUOTA_MAX_AGE_SECONDS,

src/devops_cli/github/rate_limiter.py:1398

• When the loop receives max_pages full pages, it exits normally and returns a successful combined response without indicating that more pages exist. Paginated issue/project callers can therefore silently lose records beyond the 100-page cap and reconcile an incomplete candidate set. Return an explicit truncation failure/diagnostic when the cap is reached.

    while page <= max_pages:

src/devops_cli/github/rate_limiter.py:509

• Malformed resetAt data now escapes as a bare ValueError, so a bad GitHub response can bypass the structured CLI error taxonomy and surface as an unhandled built-in exception. Raise GitHubRateLimitError (with bounded structured details) instead, consistent with AGENTS.md:349.

    except (ValueError, TypeError) as err:
        raise ValueError(f"Failed to parse resetAt timestamp '{reset_at}': {err}") from err

src/devops_cli/github/rate_limiter.py:496

• GraphQL mutations sent as gh api graphql -f query=mutation ... are not recognized here: _is_api_mutation only checks HTTP methods, while these commands use the default POST with the mutation in the query body. Project mutations such as _create_project_view therefore skip mutation_min_interval and mutation tracking; detect the GraphQL operation type or pass explicit mutation metadata.

    if clean[0] == "api":
        return _is_api_mutation(clean[1:])

• Files reviewed: 33/34 changed files
• Comments generated: 8
• Review effort level: Lite

---

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