Files
hermes-agent/website/docs/user-guide/skills/bundled/github/github-github-code-review.md
Teknium 289cc47631 docs: resync reference, user-guide, developer-guide, and messaging pages against code (#17738)
Broad drift audit against origin/main (b52b63396).

Reference pages (most user-visible drift):
- slash-commands: add /busy, /curator, /footer, /indicator, /redraw, /steer
  that were missing; drop non-existent /terminal-setup; fix /q footnote
  (resolves to /queue, not /quit); extend CLI-only list with all 24
  CLI-only commands in the registry
- cli-commands: add dedicated sections for hermes curator / fallback /
  hooks (new subcommands not previously documented); remove stale
  hermes honcho standalone section (the plugin registers dynamically
  via hermes memory); list curator/fallback/hooks in top-level table;
  fix completion to include fish
- toolsets-reference: document the real 52-toolset count; split browser
  vs browser-cdp; add discord / discord_admin / spotify / yuanbao;
  correct hermes-cli tool count from 36 to 38; fix misleading claim
  that hermes-homeassistant adds tools (it's identical to hermes-cli)
- tools-reference: bump tool count 55 -> 68; add 7 Spotify, 5 Yuanbao,
  2 Discord toolsets; move browser_cdp/browser_dialog to their own
  browser-cdp toolset section
- environment-variables: add 40+ user-facing HERMES_* vars that were
  undocumented (--yolo, --accept-hooks, --ignore-*, inference model
  override, agent/stream/checkpoint timeouts, OAuth trace, per-platform
  batch tuning for Telegram/Discord/Matrix/Feishu/WeCom, cron knobs,
  gateway restart/connect timeouts); dedupe the Cron Scheduler section;
  replace stale QQ_SANDBOX with QQ_PORTAL_HOST

User-guide (top level):
- cli.md: compression preserves last 20 turns, not 4 (protect_last_n: 20)
- configuration.md: display.platforms is the canonical per-platform
  override key; tool_progress_overrides is deprecated and auto-migrated
- profiles.md: model.default is the config key, not model.model
- sessions.md: CLI/TUI session IDs use 6-char hex, gateway uses 8
- checkpoints-and-rollback.md: destructive-command list now matches
  _DESTRUCTIVE_PATTERNS (adds rmdir, cp, install, dd)
- docker.md: the container runs as non-root hermes (UID 10000) via
  gosu; fix install command (uv pip); add missing --insecure on the
  dashboard compose example (required for non-loopback bind)
- security.md: systemctl danger pattern also matches 'restart'
- index.md: built-in tool count 47 -> 68
- integrations/index.md: 6 STT providers, 8 memory providers
- integrations/providers.md: drop fictional dashscope/qwen aliases

Features:
- overview.md: 9 image models (not 8), 9 TTS providers (not 5),
  8 memory providers (Supermemory was missing)
- tool-gateway.md: 9 image models
- tools.md: extend common-toolsets list with search / messaging /
  spotify / discord / debugging / safe
- fallback-providers.md: add 6 real providers from PROVIDER_REGISTRY
  (lmstudio, kimi-coding-cn, stepfun, alibaba-coding-plan,
  tencent-tokenhub, azure-foundry)
- plugins.md: Available Hooks table now includes on_session_finalize,
  on_session_reset, subagent_stop
- built-in-plugins.md: add the 7 bundled plugins the page didn't
  mention (spotify, google_meet, three image_gen providers, two
  dashboard examples)
- web-dashboard.md: add --insecure and --tui flags
- cron.md: hermes cron create takes positional schedule/prompt, not
  flags

Messaging:
- telegram.md: TELEGRAM_WEBHOOK_SECRET is now REQUIRED when
  TELEGRAM_WEBHOOK_URL is set (gateway refuses to start without it
  per GHSA-3vpc-7q5r-276h). Biggest user-visible drift in the batch.
- discord.md: HERMES_DISCORD_TEXT_BATCH_SPLIT_DELAY_SECONDS default
  is 2.0, not 0.1
- dingtalk.md: document DINGTALK_REQUIRE_MENTION /
  FREE_RESPONSE_CHATS / MENTION_PATTERNS / HOME_CHANNEL /
  ALLOW_ALL_USERS that the adapter supports
- bluebubbles.md: drop fictional BLUEBUBBLES_SEND_READ_RECEIPTS env
  var; the setting lives in platforms.bluebubbles.extra only
- qqbot.md: drop dead QQ_SANDBOX; add real QQ_PORTAL_HOST and
  QQ_GROUP_ALLOWED_USERS
- wecom-callback.md: replace 'hermes gateway start' (service-only)
  with 'hermes gateway' for first-time setup

Developer-guide:
- architecture.md: refresh tool/toolset counts (61/52), terminal
  backend count (7), line counts for run_agent.py (~13.7k), cli.py
  (~11.5k), main.py (~10.4k), setup.py (~3.5k), gateway/run.py
  (~12.2k), mcp_tool.py (~3.1k); add yuanbao adapter, bump platform
  adapter count 18 -> 20
- agent-loop.md: run_agent.py line count 10.7k -> 13.7k
- tools-runtime.md: add vercel_sandbox backend
- adding-tools.md: remove stale 'Discovery import added to
  model_tools.py' checklist item (registry auto-discovery)
- adding-platform-adapters.md: mark send_typing / get_chat_info as
  concrete base methods; only connect/disconnect/send are abstract
- acp-internals.md: ACP sessions now persist to SessionDB
  (~/.hermes/state.db); acp.run_agent call uses
  use_unstable_protocol=True
- cron-internals.md: gateway runs scheduler in a dedicated background
  thread via _start_cron_ticker, not on a maintenance cycle; locking
  is cross-process via fcntl.flock (Unix) / msvcrt.locking (Windows)
- gateway-internals.md: gateway/run.py ~12k lines
- provider-runtime.md: cron DOES support fallback (run_job reads
  fallback_providers from config)
- session-storage.md: SCHEMA_VERSION = 11 (not 9); add migrations
  10 and 11 (trigram FTS, inline-mode FTS5 re-index); add
  api_call_count column to Sessions DDL; document messages_fts_trigram
  and state_meta in the architecture tree
- context-compression-and-caching.md: remove the obsolete 'context
  pressure warnings' section (warnings were removed for causing
  models to give up early)
- context-engine-plugin.md: compress() signature now includes
  focus_topic param
- extending-the-cli.md: _build_tui_layout_children signature now
  includes model_picker_widget; add to default layout

Also fixed three pre-existing broken links/anchors the build warned
about (docker.md -> api-server.md, yuanbao.md -> cron-jobs.md and
tips#background-tasks, nix-setup.md -> #container-aware-cli).

Regenerated per-skill pages via website/scripts/generate-skill-docs.py
so catalog tables and sidebar are consistent with current SKILL.md
frontmatter.

docusaurus build: clean, no broken links or anchors.
2026-04-29 20:55:59 -07:00

499 lines
14 KiB
Markdown

---
title: "Github Code Review — Review PRs: diffs, inline comments via gh or REST"
sidebar_label: "Github Code Review"
description: "Review PRs: diffs, inline comments via gh or REST"
---
{/* This page is auto-generated from the skill's SKILL.md by website/scripts/generate-skill-docs.py. Edit the source SKILL.md, not this page. */}
# Github Code Review
Review PRs: diffs, inline comments via gh or REST.
## Skill metadata
| | |
|---|---|
| Source | Bundled (installed by default) |
| Path | `skills/github/github-code-review` |
| Version | `1.1.0` |
| Author | Hermes Agent |
| License | MIT |
| Tags | `GitHub`, `Code-Review`, `Pull-Requests`, `Git`, `Quality` |
| Related skills | [`github-auth`](/docs/user-guide/skills/bundled/github/github-github-auth), [`github-pr-workflow`](/docs/user-guide/skills/bundled/github/github-github-pr-workflow) |
## Reference: full SKILL.md
:::info
The following is the complete skill definition that Hermes loads when this skill is triggered. This is what the agent sees as instructions when the skill is active.
:::
# GitHub Code Review
Perform code reviews on local changes before pushing, or review open PRs on GitHub. Most of this skill uses plain `git` — the `gh`/`curl` split only matters for PR-level interactions.
## Prerequisites
- Authenticated with GitHub (see `github-auth` skill)
- Inside a git repository
### Setup (for PR interactions)
```bash
if command -v gh &>/dev/null && gh auth status &>/dev/null; then
AUTH="gh"
else
AUTH="git"
if [ -z "$GITHUB_TOKEN" ]; then
if [ -f ~/.hermes/.env ] && grep -q "^GITHUB_TOKEN=" ~/.hermes/.env; then
GITHUB_TOKEN=$(grep "^GITHUB_TOKEN=" ~/.hermes/.env | head -1 | cut -d= -f2 | tr -d '\n\r')
elif grep -q "github.com" ~/.git-credentials 2>/dev/null; then
GITHUB_TOKEN=$(grep "github.com" ~/.git-credentials 2>/dev/null | head -1 | sed 's|https://[^:]*:\([^@]*\)@.*|\1|')
fi
fi
fi
REMOTE_URL=$(git remote get-url origin)
OWNER_REPO=$(echo "$REMOTE_URL" | sed -E 's|.*github\.com[:/]||; s|\.git$||')
OWNER=$(echo "$OWNER_REPO" | cut -d/ -f1)
REPO=$(echo "$OWNER_REPO" | cut -d/ -f2)
```
---
## 1. Reviewing Local Changes (Pre-Push)
This is pure `git` — works everywhere, no API needed.
### Get the Diff
```bash
# Staged changes (what would be committed)
git diff --staged
# All changes vs main (what a PR would contain)
git diff main...HEAD
# File names only
git diff main...HEAD --name-only
# Stat summary (insertions/deletions per file)
git diff main...HEAD --stat
```
### Review Strategy
1. **Get the big picture first:**
```bash
git diff main...HEAD --stat
git log main..HEAD --oneline
```
2. **Review file by file** — use `read_file` on changed files for full context, and the diff to see what changed:
```bash
git diff main...HEAD -- src/auth/login.py
```
3. **Check for common issues:**
```bash
# Debug statements, TODOs, console.logs left behind
git diff main...HEAD | grep -n "print(\|console\.log\|TODO\|FIXME\|HACK\|XXX\|debugger"
# Large files accidentally staged
git diff main...HEAD --stat | sort -t'|' -k2 -rn | head -10
# Secrets or credential patterns
git diff main...HEAD | grep -in "password\|secret\|api_key\|token.*=\|private_key"
# Merge conflict markers
git diff main...HEAD | grep -n "<<<<<<\|>>>>>>\|======="
```
4. **Present structured feedback** to the user.
### Review Output Format
When reviewing local changes, present findings in this structure:
```
## Code Review Summary
### Critical
- **src/auth.py:45** — SQL injection: user input passed directly to query.
Suggestion: Use parameterized queries.
### Warnings
- **src/models/user.py:23** — Password stored in plaintext. Use bcrypt or argon2.
- **src/api/routes.py:112** — No rate limiting on login endpoint.
### Suggestions
- **src/utils/helpers.py:8** — Duplicates logic in `src/core/utils.py:34`. Consolidate.
- **tests/test_auth.py** — Missing edge case: expired token test.
### Looks Good
- Clean separation of concerns in the middleware layer
- Good test coverage for the happy path
```
---
## 2. Reviewing a Pull Request on GitHub
### View PR Details
**With gh:**
```bash
gh pr view 123
gh pr diff 123
gh pr diff 123 --name-only
```
**With git + curl:**
```bash
PR_NUMBER=123
# Get PR details
curl -s \
-H "Authorization: token $GITHUB_TOKEN" \
https://api.github.com/repos/$OWNER/$REPO/pulls/$PR_NUMBER \
| python3 -c "
import sys, json
pr = json.load(sys.stdin)
print(f\"Title: {pr['title']}\")
print(f\"Author: {pr['user']['login']}\")
print(f\"Branch: {pr['head']['ref']} -> {pr['base']['ref']}\")
print(f\"State: {pr['state']}\")
print(f\"Body:\n{pr['body']}\")"
# List changed files
curl -s \
-H "Authorization: token $GITHUB_TOKEN" \
https://api.github.com/repos/$OWNER/$REPO/pulls/$PR_NUMBER/files \
| python3 -c "
import sys, json
for f in json.load(sys.stdin):
print(f\"{f['status']:10} +{f['additions']:-4} -{f['deletions']:-4} {f['filename']}\")"
```
### Check Out PR Locally for Full Review
This works with plain `git` — no `gh` needed:
```bash
# Fetch the PR branch and check it out
git fetch origin pull/123/head:pr-123
git checkout pr-123
# Now you can use read_file, search_files, run tests, etc.
# View diff against the base branch
git diff main...pr-123
```
**With gh (shortcut):**
```bash
gh pr checkout 123
```
### Leave Comments on a PR
**General PR comment — with gh:**
```bash
gh pr comment 123 --body "Overall looks good, a few suggestions below."
```
**General PR comment — with curl:**
```bash
curl -s -X POST \
-H "Authorization: token $GITHUB_TOKEN" \
https://api.github.com/repos/$OWNER/$REPO/issues/$PR_NUMBER/comments \
-d '{"body": "Overall looks good, a few suggestions below."}'
```
### Leave Inline Review Comments
**Single inline comment — with gh (via API):**
```bash
HEAD_SHA=$(gh pr view 123 --json headRefOid --jq '.headRefOid')
gh api repos/$OWNER/$REPO/pulls/123/comments \
--method POST \
-f body="This could be simplified with a list comprehension." \
-f path="src/auth/login.py" \
-f commit_id="$HEAD_SHA" \
-f line=45 \
-f side="RIGHT"
```
**Single inline comment — with curl:**
```bash
# Get the head commit SHA
HEAD_SHA=$(curl -s \
-H "Authorization: token $GITHUB_TOKEN" \
https://api.github.com/repos/$OWNER/$REPO/pulls/$PR_NUMBER \
| python3 -c "import sys,json; print(json.load(sys.stdin)['head']['sha'])")
curl -s -X POST \
-H "Authorization: token $GITHUB_TOKEN" \
https://api.github.com/repos/$OWNER/$REPO/pulls/$PR_NUMBER/comments \
-d "{
\"body\": \"This could be simplified with a list comprehension.\",
\"path\": \"src/auth/login.py\",
\"commit_id\": \"$HEAD_SHA\",
\"line\": 45,
\"side\": \"RIGHT\"
}"
```
### Submit a Formal Review (Approve / Request Changes)
**With gh:**
```bash
gh pr review 123 --approve --body "LGTM!"
gh pr review 123 --request-changes --body "See inline comments."
gh pr review 123 --comment --body "Some suggestions, nothing blocking."
```
**With curl — multi-comment review submitted atomically:**
```bash
HEAD_SHA=$(curl -s \
-H "Authorization: token $GITHUB_TOKEN" \
https://api.github.com/repos/$OWNER/$REPO/pulls/$PR_NUMBER \
| python3 -c "import sys,json; print(json.load(sys.stdin)['head']['sha'])")
curl -s -X POST \
-H "Authorization: token $GITHUB_TOKEN" \
https://api.github.com/repos/$OWNER/$REPO/pulls/$PR_NUMBER/reviews \
-d "{
\"commit_id\": \"$HEAD_SHA\",
\"event\": \"COMMENT\",
\"body\": \"Code review from Hermes Agent\",
\"comments\": [
{\"path\": \"src/auth.py\", \"line\": 45, \"body\": \"Use parameterized queries to prevent SQL injection.\"},
{\"path\": \"src/models/user.py\", \"line\": 23, \"body\": \"Hash passwords with bcrypt before storing.\"},
{\"path\": \"tests/test_auth.py\", \"line\": 1, \"body\": \"Add test for expired token edge case.\"}
]
}"
```
Event values: `"APPROVE"`, `"REQUEST_CHANGES"`, `"COMMENT"`
The `line` field refers to the line number in the *new* version of the file. For deleted lines, use `"side": "LEFT"`.
---
## 3. Review Checklist
When performing a code review (local or PR), systematically check:
### Correctness
- Does the code do what it claims?
- Edge cases handled (empty inputs, nulls, large data, concurrent access)?
- Error paths handled gracefully?
### Security
- No hardcoded secrets, credentials, or API keys
- Input validation on user-facing inputs
- No SQL injection, XSS, or path traversal
- Auth/authz checks where needed
### Code Quality
- Clear naming (variables, functions, classes)
- No unnecessary complexity or premature abstraction
- DRY — no duplicated logic that should be extracted
- Functions are focused (single responsibility)
### Testing
- New code paths tested?
- Happy path and error cases covered?
- Tests readable and maintainable?
### Performance
- No N+1 queries or unnecessary loops
- Appropriate caching where beneficial
- No blocking operations in async code paths
### Documentation
- Public APIs documented
- Non-obvious logic has comments explaining "why"
- README updated if behavior changed
---
## 4. Pre-Push Review Workflow
When the user asks you to "review the code" or "check before pushing":
1. `git diff main...HEAD --stat` — see scope of changes
2. `git diff main...HEAD` — read the full diff
3. For each changed file, use `read_file` if you need more context
4. Apply the checklist above
5. Present findings in the structured format (Critical / Warnings / Suggestions / Looks Good)
6. If critical issues found, offer to fix them before the user pushes
---
## 5. PR Review Workflow (End-to-End)
When the user asks you to "review PR #N", "look at this PR", or gives you a PR URL, follow this recipe:
### Step 1: Set up environment
```bash
source "${HERMES_HOME:-$HOME/.hermes}/skills/github/github-auth/scripts/gh-env.sh"
# Or run the inline setup block from the top of this skill
```
### Step 2: Gather PR context
Get the PR metadata, description, and list of changed files to understand scope before diving into code.
**With gh:**
```bash
gh pr view 123
gh pr diff 123 --name-only
gh pr checks 123
```
**With curl:**
```bash
PR_NUMBER=123
# PR details (title, author, description, branch)
curl -s -H "Authorization: token $GITHUB_TOKEN" \
https://api.github.com/repos/$GH_OWNER/$GH_REPO/pulls/$PR_NUMBER
# Changed files with line counts
curl -s -H "Authorization: token $GITHUB_TOKEN" \
https://api.github.com/repos/$GH_OWNER/$GH_REPO/pulls/$PR_NUMBER/files
```
### Step 3: Check out the PR locally
This gives you full access to `read_file`, `search_files`, and the ability to run tests.
```bash
git fetch origin pull/$PR_NUMBER/head:pr-$PR_NUMBER
git checkout pr-$PR_NUMBER
```
### Step 4: Read the diff and understand changes
```bash
# Full diff against the base branch
git diff main...HEAD
# Or file-by-file for large PRs
git diff main...HEAD --name-only
# Then for each file:
git diff main...HEAD -- path/to/file.py
```
For each changed file, use `read_file` to see full context around the changes — diffs alone can miss issues visible only with surrounding code.
### Step 5: Run automated checks locally (if applicable)
```bash
# Run tests if there's a test suite
python -m pytest 2>&1 | tail -20
# or: npm test, cargo test, go test ./..., etc.
# Run linter if configured
ruff check . 2>&1 | head -30
# or: eslint, clippy, etc.
```
### Step 6: Apply the review checklist (Section 3)
Go through each category: Correctness, Security, Code Quality, Testing, Performance, Documentation.
### Step 7: Post the review to GitHub
Collect your findings and submit them as a formal review with inline comments.
**With gh:**
```bash
# If no issues — approve
gh pr review $PR_NUMBER --approve --body "Reviewed by Hermes Agent. Code looks clean — good test coverage, no security concerns."
# If issues found — request changes with inline comments
gh pr review $PR_NUMBER --request-changes --body "Found a few issues — see inline comments."
```
**With curl — atomic review with multiple inline comments:**
```bash
HEAD_SHA=$(curl -s -H "Authorization: token $GITHUB_TOKEN" \
https://api.github.com/repos/$GH_OWNER/$GH_REPO/pulls/$PR_NUMBER \
| python3 -c "import sys,json; print(json.load(sys.stdin)['head']['sha'])")
# Build the review JSON — event is APPROVE, REQUEST_CHANGES, or COMMENT
curl -s -X POST \
-H "Authorization: token $GITHUB_TOKEN" \
https://api.github.com/repos/$GH_OWNER/$GH_REPO/pulls/$PR_NUMBER/reviews \
-d "{
\"commit_id\": \"$HEAD_SHA\",
\"event\": \"REQUEST_CHANGES\",
\"body\": \"## Hermes Agent Review\n\nFound 2 issues, 1 suggestion. See inline comments.\",
\"comments\": [
{\"path\": \"src/auth.py\", \"line\": 45, \"body\": \"🔴 **Critical:** User input passed directly to SQL query — use parameterized queries.\"},
{\"path\": \"src/models.py\", \"line\": 23, \"body\": \"⚠️ **Warning:** Password stored without hashing.\"},
{\"path\": \"src/utils.py\", \"line\": 8, \"body\": \"💡 **Suggestion:** This duplicates logic in core/utils.py:34.\"}
]
}"
```
### Step 8: Also post a summary comment
In addition to inline comments, leave a top-level summary so the PR author gets the full picture at a glance. Use the review output format from `references/review-output-template.md`.
**With gh:**
```bash
gh pr comment $PR_NUMBER --body "$(cat <<'EOF'
## Code Review Summary
**Verdict: Changes Requested** (2 issues, 1 suggestion)
### 🔴 Critical
- **src/auth.py:45** — SQL injection vulnerability
### ⚠️ Warnings
- **src/models.py:23** — Plaintext password storage
### 💡 Suggestions
- **src/utils.py:8** — Duplicated logic, consider consolidating
### ✅ Looks Good
- Clean API design
- Good error handling in the middleware layer
---
*Reviewed by Hermes Agent*
EOF
)"
```
### Step 9: Clean up
```bash
git checkout main
git branch -D pr-$PR_NUMBER
```
### Decision: Approve vs Request Changes vs Comment
- **Approve** — no critical or warning-level issues, only minor suggestions or all clear
- **Request Changes** — any critical or warning-level issue that should be fixed before merge
- **Comment** — observations and suggestions, but nothing blocking (use when you're unsure or the PR is a draft)