Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ADR/phase4_plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ This document outlines potential future improvements for review-roadmap and disc
2. **Development velocity concern is manageable:**
- Use `pip install -e .` locally for development
- Only publish releases for stable milestones
- Users can still install from git for bleeding-edge: `pip install git+https://github.com/jwm4/review-roadmap.git`
- Users can still install from git for bleeding-edge: `pip install git+https://github.com/ambient-code/review-roadmap.git`
3. **GitHub Actions will be much cleaner** with PyPI (one-liner install vs. checkout + setup)
4. **Migration pitch is stronger** with: "Here's a working PyPI package with GitHub Actions integration that's already helping developers"

Expand Down
54 changes: 52 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ cp env.example .env

| Variable | Description |
|----------|-------------|
| `GITHUB_TOKEN` | GitHub personal access token for API access |
| `GITHUB_TOKEN` | GitHub personal access token for API access ([see below](#github-token-setup)) |
| `ANTHROPIC_API_KEY` | Anthropic API key (required if provider is `anthropic`) |
| `OPENAI_API_KEY` | OpenAI API key (required if provider is `openai`) |
| `GOOGLE_API_KEY` | Google API key (required if provider is `google`) |
Expand All @@ -50,6 +50,56 @@ cp env.example .env

> **Note**: The `.env` file values are overridden by any matching environment variables in your shell. If you do not want to have a .env file, you can just skip these steps and set these variables in your environment instead.

### GitHub Token Setup

A GitHub personal access token (PAT) is required to fetch PR data. The type of token and permissions needed depend on your use case:

#### Read-Only Access (default)

For generating roadmaps without posting comments, a token with **read access to repositories** is sufficient.

#### Write Access (for `--post` flag)

To post roadmaps as PR comments using the `--post` flag, your token needs **write access**. The specific requirements differ by token type:

| Token Type | Required Permissions |
|------------|---------------------|
| **Fine-grained PAT** | Repository access → Pull requests: **Read and write** |
| **Classic PAT** | `repo` scope (full repo access) or `public_repo` (public repos only) |

#### Creating a Fine-Grained Personal Access Token (Recommended)

Fine-grained tokens are more secure because they limit access to specific repositories.

1. Go to [GitHub Settings → Developer settings → Personal access tokens → Fine-grained tokens](https://github.com/settings/tokens?type=beta)
2. Click **"Generate new token"**
3. Configure the token:
- **Token name**: e.g., `review-roadmap`
- **Expiration**: Choose an appropriate duration
- **Repository access**: Select "Only select repositories" and choose the repos you want to analyze
- **Permissions**:
- **Pull requests**: Read-only (for basic usage) or Read and write (for `--post`)
- **Contents**: Read-only (required to fetch file contents for context expansion)
4. Click **"Generate token"** and copy the token value

#### Creating a Classic Personal Access Token

Classic tokens are simpler but grant broader access.

1. Go to [GitHub Settings → Developer settings → Personal access tokens → Tokens (classic)](https://github.com/settings/tokens)
2. Click **"Generate new token"** → **"Generate new token (classic)"**
3. Configure the token:
- **Note**: e.g., `review-roadmap`
- **Expiration**: Choose an appropriate duration
- **Scopes**:
- `repo` — Full access (works for both public and private repos)
- Or `public_repo` — Only for public repositories
4. Click **"Generate token"** and copy the token value

#### Important: Token Scopes vs. User Permissions

Even if you have collaborator access to a repository, your token must also have the correct **OAuth scopes** to perform write operations. A common mistake is using a token with read-only scopes while expecting write access to work based on your user permissions. The tool checks both your repository permissions and your token's scopes before attempting to post comments.

## Usage

Run the tool using the CLI:
Expand All @@ -69,7 +119,7 @@ review_roadmap {PR link in the form owner/repo/pr_number or just a URL to the PR

You can use both `-o` and `-p` together—the roadmap will be generated once and saved to both the file and the PR comment.

> **Note:** To use `--post`, your `GITHUB_TOKEN` must have write access to the repository. For fine-grained personal access tokens, ensure the **"Pull requests"** permission is set to **"Read and write"**. For classic tokens, the `repo` scope is required.
> **Note:** The `--post` flag requires a token with write access. See [GitHub Token Setup](#github-token-setup) for details.

### Examples

Expand Down
125 changes: 119 additions & 6 deletions review_roadmap/github/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@
import httpx

from review_roadmap.config import settings
from review_roadmap.models import PRContext, PRMetadata, FileDiff, PRComment
from review_roadmap.models import (
PRContext, PRMetadata, FileDiff, PRComment,
WriteAccessResult, WriteAccessStatus,
)


class GitHubClient:
Expand Down Expand Up @@ -220,17 +223,60 @@ def get_file_content(self, owner: str, repo: str, path: str, ref: str) -> str:
raw_resp.raise_for_status()
return raw_resp.text

def check_write_access(self, owner: str, repo: str) -> bool:
"""Check if the authenticated user has write access to the repository.
def _test_write_with_reaction(self, owner: str, repo: str, pr_number: int) -> bool:
"""Test write access by creating and immediately deleting a reaction.

Used to validate permissions before attempting to post a PR comment.
This is a non-destructive way to verify the token can write to the PR.
Uses the 'eyes' emoji (👀) as it's unobtrusive if cleanup fails.

Args:
owner: Repository owner.
repo: Repository name.
pr_number: PR number to test against.

Returns:
True if the user has push or admin access, False otherwise.
True if write access is confirmed, False otherwise.
"""
try:
# Create a reaction on the PR (PRs are issues in GitHub's API)
create_resp = self.client.post(
f"/repos/{owner}/{repo}/issues/{pr_number}/reactions",
json={"content": "eyes"}
)

if create_resp.status_code in (200, 201):
# Successfully created - now clean it up
reaction_id = create_resp.json().get("id")
if reaction_id:
self.client.delete(
f"/repos/{owner}/{repo}/issues/{pr_number}/reactions/{reaction_id}"
)
return True
elif create_resp.status_code == 403:
return False
else:
# Unexpected status - treat as uncertain
return False
except Exception:
return False

def check_write_access(
self, owner: str, repo: str, pr_number: Optional[int] = None
) -> WriteAccessResult:
"""Check if the authenticated token can write to the repository.

Validates:
1. The user has push/admin permissions on the repository
2. The token has the required OAuth scopes (for classic PATs/OAuth tokens)
3. For fine-grained PATs: performs a live test using reactions if pr_number provided

Args:
owner: Repository owner.
repo: Repository name.
pr_number: Optional PR number for live write test (used for fine-grained PATs).

Returns:
WriteAccessResult with status, confidence level, and explanation.

Raises:
httpx.HTTPStatusError: If the repository request fails.
Expand All @@ -239,8 +285,75 @@ def check_write_access(self, owner: str, repo: str) -> bool:
resp.raise_for_status()
repo_data = resp.json()

# Check user's role-based permissions
permissions = repo_data.get("permissions", {})
return permissions.get("push", False) or permissions.get("admin", False)
has_role_access = permissions.get("push", False) or permissions.get("admin", False)

if not has_role_access:
return WriteAccessResult(
status=WriteAccessStatus.DENIED,
is_fine_grained_pat=False,
message="Your user account does not have write access to this repository."
)

# Check token's OAuth scopes (only present for classic PATs and OAuth tokens)
oauth_scopes_header = resp.headers.get("X-OAuth-Scopes")

if oauth_scopes_header is not None:
# Classic PAT or OAuth token - can verify scopes definitively
scopes = [s.strip() for s in oauth_scopes_header.split(",") if s.strip()]
is_private = repo_data.get("private", False)

if is_private:
has_token_scope = "repo" in scopes
required_scope = "repo"
else:
has_token_scope = "repo" in scopes or "public_repo" in scopes
required_scope = "repo or public_repo"

if has_token_scope:
return WriteAccessResult(
status=WriteAccessStatus.GRANTED,
is_fine_grained_pat=False,
message="Classic token with correct scopes verified."
)
else:
return WriteAccessResult(
status=WriteAccessStatus.DENIED,
is_fine_grained_pat=False,
message=f"Token lacks required scope ({required_scope}). Current scopes: {', '.join(scopes) or 'none'}"
)

# No X-OAuth-Scopes header = fine-grained PAT
# Try a live write test if we have a PR number
if pr_number is not None:
if self._test_write_with_reaction(owner, repo, pr_number):
return WriteAccessResult(
status=WriteAccessStatus.GRANTED,
is_fine_grained_pat=True,
message="Fine-grained PAT write access verified via live test."
)
else:
return WriteAccessResult(
status=WriteAccessStatus.DENIED,
is_fine_grained_pat=True,
message=(
"Fine-grained PAT detected but write test failed. "
f"Ensure your token has 'Pull requests: Read and write' permission "
f"for {owner}/{repo}."
)
)

# No PR number provided - can't do live test
return WriteAccessResult(
status=WriteAccessStatus.UNCERTAIN,
is_fine_grained_pat=True,
message=(
"Fine-grained PAT detected. Cannot verify if this token is configured "
f"for {owner}/{repo}. If posting fails, ensure your token has "
"'Pull requests: Read and write' permission for this specific repository."
)
)

def post_pr_comment(self, owner: str, repo: str, pr_number: int, body: str) -> Dict[str, Any]:
"""Post a comment on a pull request.
Expand Down
23 changes: 18 additions & 5 deletions review_roadmap/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from review_roadmap.agent.graph import build_graph
from review_roadmap.config import settings
from review_roadmap.logging import configure_logging
from review_roadmap.models import WriteAccessStatus

app = typer.Typer(add_completion=False)
console = Console()
Expand All @@ -23,7 +24,7 @@ def format_pr_comment(roadmap_content: str) -> str:

header = f"""🗺️ **Auto-Generated Review Roadmap**

> This roadmap was automatically generated by [review-roadmap](https://github.com/jwm4/review-roadmap).
> This roadmap was automatically generated by [review-roadmap](https://github.com/ambient-code/review-roadmap).
> Model: `{provider}/{model}`

---
Expand Down Expand Up @@ -61,14 +62,20 @@ def generate(
if post:
console.print(f"[bold blue]Checking write access for {owner}/{repo}...[/bold blue]")
try:
has_access = gh_client.check_write_access(owner, repo)
if not has_access:
# Pass pr_number to enable live write test for fine-grained PATs
access_result = gh_client.check_write_access(owner, repo, pr_number)

if access_result.status == WriteAccessStatus.DENIED:
console.print(
"[red]Error: Your GitHub token does not have write access to this repository.[/red]\n"
f"[red]Error: {access_result.message}[/red]\n"
"[yellow]To use --post, your token needs 'Pull requests: Read and write' permission.[/yellow]"
)
raise typer.Exit(code=1)
console.print("[green]Write access confirmed.[/green]")
elif access_result.status == WriteAccessStatus.UNCERTAIN:
console.print(f"[yellow]Warning: {access_result.message}[/yellow]")
console.print("[yellow]Proceeding, but posting may fail...[/yellow]")
else:
console.print("[green]Write access confirmed.[/green]")
except typer.Exit:
raise
except Exception as e:
Expand Down Expand Up @@ -108,6 +115,12 @@ def generate(
console.print(f"[bold green]Roadmap posted to PR #{pr_number}[/bold green]")
except Exception as e:
console.print(f"[red]Error posting comment to PR: {e}[/red]")
if "403" in str(e):
console.print(
"\n[yellow]This usually means your token lacks write access to this repository.[/yellow]\n"
"[yellow]If using a fine-grained PAT, ensure it is configured for this specific repository[/yellow]\n"
f"[yellow]with 'Pull requests: Read and write' permission for {owner}/{repo}.[/yellow]"
)
raise typer.Exit(code=1)

# If neither output nor post, print to console
Expand Down
28 changes: 28 additions & 0 deletions review_roadmap/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,38 @@
fetched from GitHub's API, including PR metadata, file diffs, and comments.
"""

from enum import Enum
from typing import List, Optional
from pydantic import BaseModel, Field


class WriteAccessStatus(str, Enum):
"""Result of checking write access to a repository."""

GRANTED = "granted"
"""Write access confirmed with high confidence (classic PAT with correct scopes)."""

DENIED = "denied"
"""Write access definitely not available (missing permissions or scopes)."""

UNCERTAIN = "uncertain"
"""Cannot reliably determine access (fine-grained PAT - may or may not work)."""


class WriteAccessResult(BaseModel):
"""Result of a write access check.

Attributes:
status: The access status (granted, denied, or uncertain).
is_fine_grained_pat: True if the token appears to be a fine-grained PAT.
message: Human-readable explanation of the result.
"""

status: WriteAccessStatus
is_fine_grained_pat: bool = False
message: str = ""


class FileDiff(BaseModel):
"""Represents a single file change in a Pull Request.

Expand Down
Loading