Skip to content

Add script/gh_auth_check to verify gh CLI installation and auth#1122

Open
kitcommerce wants to merge 2 commits intonextfrom
issue-926-gh-auth-check
Open

Add script/gh_auth_check to verify gh CLI installation and auth#1122
kitcommerce wants to merge 2 commits intonextfrom
issue-926-gh-auth-check

Conversation

@kitcommerce
Copy link
Contributor

Fixes #926

Adds script/gh_auth_check, a shell script that:

  • Verifies gh CLI is installed (exits non-zero with install instructions if not)
  • Runs gh auth status and exits non-zero with login guidance if not authenticated
  • Prints a success message when both checks pass

Client impact

None (developer tooling only).

Verification Plan

./script/gh_auth_check

Kit (OpenClaw) added 2 commits March 13, 2026 17:37
Closes #925

Adds a POSIX-compatible shell script at script/pr_body_check that
verifies a pull request body contains both a 'Client impact' section
and a 'Verification Plan' section.

Features:
- Accepts a bare PR number or a full GitHub PR URL
- Uses the gh CLI to fetch the PR body (JSON + jq)
- Case-insensitive section detection via grep -i
- Helpful error messages for missing gh, unauthenticated state, or
  unrecognisable input
- Read-only: performs no repository mutations
- POSIX sh with macOS-friendly tooling (no GNU-only extensions)
Exits non-zero with actionable guidance if gh is not installed or
not authenticated. Closes #926.
@kitcommerce
Copy link
Contributor Author

Architecture Review

Verdict: NEEDS_REVISION

Finding (severity: medium): This PR ships a second script/pr_body_check — a 133-line POSIX sh version — that directly conflicts with the 46-line bash version introduced in PR #1120. If both are merged, the repository will have one canonical script with two incompatible histories depending on merge order, or a merge conflict that gets resolved in an ad hoc way.

The right approach is one of:

  1. Supersede — Mark PR Add script/pr_body_check to validate PR body sections #1120 as superseded by this one; this PR's pr_body_check becomes the canonical version.
  2. Compose — Have pr_body_check call gh_auth_check as a preflight (sourced or subprocess) rather than re-implementing auth checks inline.
  3. Defer URL parsing — Keep PR Add script/pr_body_check to validate PR body sections #1120's simple version canonical, and defer URL parsing + auth preflight to a follow-on PR that explicitly replaces it.

gh_auth_check as a standalone script is well-scoped and has no overlap concern — that part is clean.

Action required: Coordinate with PR #1120 and choose one of the above approaches before merge.

@kitcommerce
Copy link
Contributor Author

Simplicity Review

Verdict: NEEDS_REVISION

The 133-line pr_body_check re-implements gh auth checking logic that also lives in gh_auth_check. This duplication violates the single-responsibility principle and increases maintenance surface.

Specifically: the auth preflight block inside pr_body_check should be a call to gh_auth_check (or a sourced helper) rather than copied logic. If gh_auth_check's error messages or behavior change, pr_body_check won't stay in sync.\n\ngh_auth_check itself (31 lines) is appropriately lean — no simplicity concerns there.

@kitcommerce
Copy link
Contributor Author

Security Review

Verdict: PASS

gh_auth_check: command -v gh and gh auth status are safe. No user input interpolated. Exit messages are hardcoded strings. Clean.

pr_body_check (133-line version): URL parsing uses shell parameter expansion and sed on the input argument — no eval, no unsafe substitution patterns observed. The --repo flag is passed as a positional argument to gh, not interpolated into a command string. PR body content is piped through grep, not executed. Auth preflight runs before any gh calls.

No significant security concerns in either script.

@kitcommerce
Copy link
Contributor Author

Rails Conventions Review

Verdict: PASS (N/A)

Shell scripts only — no Ruby code. Rails conventions do not apply. Confirmed.

@kitcommerce kitcommerce added review:architecture-done Review complete review:simplicity-done Review complete review:security-done Review complete review:rails-conventions-done Rails conventions review complete and removed review:architecture-pending Review in progress review:simplicity-pending Review in progress review:security-pending Review in progress review:rails-conventions-pending Rails conventions review in progress labels Mar 17, 2026
@kitcommerce kitcommerce added review:wave1-complete review:rails-security-pending Rails security review in progress review:database-pending Database review in progress review:test-quality-pending Review in progress labels Mar 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gate:build-passed Build gate passed review:architecture-done Review complete review:database-pending Database review in progress review:rails-conventions-done Rails conventions review complete review:rails-security-pending Rails security review in progress review:security-done Review complete review:simplicity-done Review complete review:test-quality-pending Review in progress review:wave1-complete

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant