diff --git a/ADR/phase4_plan.md b/ADR/phase4_plan.md index 7f54c84..88a1784 100644 --- a/ADR/phase4_plan.md +++ b/ADR/phase4_plan.md @@ -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" diff --git a/README.md b/README.md index 3670550..5fe1b23 100644 --- a/README.md +++ b/README.md @@ -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`) | @@ -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: @@ -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 diff --git a/review_roadmap/github/client.py b/review_roadmap/github/client.py index 0384cb0..651602d 100644 --- a/review_roadmap/github/client.py +++ b/review_roadmap/github/client.py @@ -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: @@ -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. @@ -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. diff --git a/review_roadmap/main.py b/review_roadmap/main.py index 37ed57e..bf66ab4 100644 --- a/review_roadmap/main.py +++ b/review_roadmap/main.py @@ -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() @@ -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}` --- @@ -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: @@ -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 diff --git a/review_roadmap/models.py b/review_roadmap/models.py index 1332302..8f5487f 100644 --- a/review_roadmap/models.py +++ b/review_roadmap/models.py @@ -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. diff --git a/tests/test_github_client.py b/tests/test_github_client.py index b56ceb4..cd76fee 100644 --- a/tests/test_github_client.py +++ b/tests/test_github_client.py @@ -3,6 +3,7 @@ from httpx import Response from review_roadmap.github.client import GitHubClient from review_roadmap.config import settings +from review_roadmap.models import WriteAccessStatus @respx.mock def test_get_pr_context_success(): @@ -73,15 +74,21 @@ def test_get_file_content_success(): @respx.mock -def test_check_write_access_with_push_permission(): - """Verifies that check_write_access returns True when user has push permission.""" +def test_check_write_access_fine_grained_pat_with_push(): + """Fine-grained PAT with push permission returns UNCERTAIN status. + + We can't verify fine-grained PAT access because GitHub returns user permissions, + not token-specific permissions for each repository. + """ owner = "owner" repo = "repo" url = f"https://api.github.com/repos/{owner}/{repo}" + # Fine-grained PATs don't return X-OAuth-Scopes header respx.get(url).mock(return_value=Response(200, json={ "id": 12345, "name": repo, + "private": False, "permissions": { "admin": False, "push": True, @@ -90,14 +97,16 @@ def test_check_write_access_with_push_permission(): })) client = GitHubClient(token="fake-token") - has_access = client.check_write_access(owner, repo) + result = client.check_write_access(owner, repo) - assert has_access is True + assert result.status == WriteAccessStatus.UNCERTAIN + assert result.is_fine_grained_pat is True + assert "Fine-grained PAT" in result.message @respx.mock -def test_check_write_access_with_admin_permission(): - """Verifies that check_write_access returns True when user has admin permission.""" +def test_check_write_access_fine_grained_pat_with_admin(): + """Fine-grained PAT with admin permission returns UNCERTAIN status.""" owner = "owner" repo = "repo" url = f"https://api.github.com/repos/{owner}/{repo}" @@ -105,6 +114,7 @@ def test_check_write_access_with_admin_permission(): respx.get(url).mock(return_value=Response(200, json={ "id": 12345, "name": repo, + "private": False, "permissions": { "admin": True, "push": False, @@ -113,14 +123,15 @@ def test_check_write_access_with_admin_permission(): })) client = GitHubClient(token="fake-token") - has_access = client.check_write_access(owner, repo) + result = client.check_write_access(owner, repo) - assert has_access is True + assert result.status == WriteAccessStatus.UNCERTAIN + assert result.is_fine_grained_pat is True @respx.mock -def test_check_write_access_no_permission(): - """Verifies that check_write_access returns False when user lacks write access.""" +def test_check_write_access_fine_grained_pat_no_permission(): + """Fine-grained PAT without push/admin returns DENIED status.""" owner = "owner" repo = "repo" url = f"https://api.github.com/repos/{owner}/{repo}" @@ -128,6 +139,7 @@ def test_check_write_access_no_permission(): respx.get(url).mock(return_value=Response(200, json={ "id": 12345, "name": repo, + "private": False, "permissions": { "admin": False, "push": False, @@ -136,14 +148,15 @@ def test_check_write_access_no_permission(): })) client = GitHubClient(token="fake-token") - has_access = client.check_write_access(owner, repo) + result = client.check_write_access(owner, repo) - assert has_access is False + assert result.status == WriteAccessStatus.DENIED + assert "does not have write access" in result.message @respx.mock def test_check_write_access_no_permissions_field(): - """Verifies that check_write_access returns False when permissions field is missing.""" + """Missing permissions field returns DENIED status.""" owner = "owner" repo = "repo" url = f"https://api.github.com/repos/{owner}/{repo}" @@ -155,9 +168,249 @@ def test_check_write_access_no_permissions_field(): })) client = GitHubClient(token="fake-token") - has_access = client.check_write_access(owner, repo) + result = client.check_write_access(owner, repo) + + assert result.status == WriteAccessStatus.DENIED + + +@respx.mock +def test_check_write_access_classic_pat_with_repo_scope(): + """Classic PAT with 'repo' scope returns GRANTED status.""" + owner = "owner" + repo = "repo" + url = f"https://api.github.com/repos/{owner}/{repo}" + + # Classic PATs include X-OAuth-Scopes header + respx.get(url).mock(return_value=Response( + 200, + json={ + "id": 12345, + "name": repo, + "private": False, + "permissions": {"admin": False, "push": True, "pull": True} + }, + headers={"X-OAuth-Scopes": "repo, read:user"} + )) + + client = GitHubClient(token="fake-token") + result = client.check_write_access(owner, repo) + + assert result.status == WriteAccessStatus.GRANTED + assert result.is_fine_grained_pat is False + + +@respx.mock +def test_check_write_access_classic_pat_with_public_repo_scope(): + """Classic PAT with 'public_repo' scope on public repo returns GRANTED.""" + owner = "owner" + repo = "repo" + url = f"https://api.github.com/repos/{owner}/{repo}" + + respx.get(url).mock(return_value=Response( + 200, + json={ + "id": 12345, + "name": repo, + "private": False, + "permissions": {"admin": False, "push": True, "pull": True} + }, + headers={"X-OAuth-Scopes": "public_repo, read:user"} + )) + + client = GitHubClient(token="fake-token") + result = client.check_write_access(owner, repo) + + assert result.status == WriteAccessStatus.GRANTED + + +@respx.mock +def test_check_write_access_classic_pat_without_write_scope(): + """Classic PAT without write scope returns DENIED, even with push permission. + + This is the key bug fix: the user may have push permission on the repo, + but the token lacks the OAuth scope to actually write. + """ + owner = "owner" + repo = "repo" + url = f"https://api.github.com/repos/{owner}/{repo}" + + respx.get(url).mock(return_value=Response( + 200, + json={ + "id": 12345, + "name": repo, + "private": False, + "permissions": {"admin": False, "push": True, "pull": True} # User has push access + }, + headers={"X-OAuth-Scopes": "read:user, read:org"} # But token lacks write scope + )) + + client = GitHubClient(token="fake-token") + result = client.check_write_access(owner, repo) + + assert result.status == WriteAccessStatus.DENIED + assert "lacks required scope" in result.message + + +@respx.mock +def test_check_write_access_classic_pat_private_repo_needs_repo_scope(): + """Private repo with only 'public_repo' scope returns DENIED.""" + owner = "owner" + repo = "repo" + url = f"https://api.github.com/repos/{owner}/{repo}" + + respx.get(url).mock(return_value=Response( + 200, + json={ + "id": 12345, + "name": repo, + "private": True, # Private repo + "permissions": {"admin": False, "push": True, "pull": True} + }, + headers={"X-OAuth-Scopes": "public_repo, read:user"} # Only has public_repo scope + )) + + client = GitHubClient(token="fake-token") + result = client.check_write_access(owner, repo) + + assert result.status == WriteAccessStatus.DENIED + + +@respx.mock +def test_check_write_access_classic_pat_private_repo_with_repo_scope(): + """Private repo with 'repo' scope returns GRANTED.""" + owner = "owner" + repo = "repo" + url = f"https://api.github.com/repos/{owner}/{repo}" + + respx.get(url).mock(return_value=Response( + 200, + json={ + "id": 12345, + "name": repo, + "private": True, + "permissions": {"admin": False, "push": True, "pull": True} + }, + headers={"X-OAuth-Scopes": "repo, read:user"} + )) + + client = GitHubClient(token="fake-token") + result = client.check_write_access(owner, repo) + + assert result.status == WriteAccessStatus.GRANTED + + +@respx.mock +def test_check_write_access_classic_pat_empty_scopes(): + """Classic PAT with empty scopes returns DENIED.""" + owner = "owner" + repo = "repo" + url = f"https://api.github.com/repos/{owner}/{repo}" + + respx.get(url).mock(return_value=Response( + 200, + json={ + "id": 12345, + "name": repo, + "private": False, + "permissions": {"admin": True, "push": True, "pull": True} + }, + headers={"X-OAuth-Scopes": ""} # Empty scopes + )) + + client = GitHubClient(token="fake-token") + result = client.check_write_access(owner, repo) + + assert result.status == WriteAccessStatus.DENIED + + +@respx.mock +def test_check_write_access_fine_grained_pat_live_test_success(): + """Fine-grained PAT with successful live write test returns GRANTED.""" + owner = "owner" + repo = "repo" + pr_number = 42 + + # Mock repo endpoint (no X-OAuth-Scopes = fine-grained PAT) + respx.get(f"https://api.github.com/repos/{owner}/{repo}").mock( + return_value=Response(200, json={ + "id": 12345, + "name": repo, + "private": False, + "permissions": {"admin": False, "push": True, "pull": True} + }) + ) + + # Mock successful reaction creation + respx.post(f"https://api.github.com/repos/{owner}/{repo}/issues/{pr_number}/reactions").mock( + return_value=Response(201, json={"id": 99999, "content": "eyes"}) + ) + + # Mock reaction deletion + respx.delete(f"https://api.github.com/repos/{owner}/{repo}/issues/{pr_number}/reactions/99999").mock( + return_value=Response(204) + ) + + client = GitHubClient(token="fake-token") + result = client.check_write_access(owner, repo, pr_number) + + assert result.status == WriteAccessStatus.GRANTED + assert result.is_fine_grained_pat is True + assert "verified via live test" in result.message + + +@respx.mock +def test_check_write_access_fine_grained_pat_live_test_denied(): + """Fine-grained PAT with failed live write test returns DENIED.""" + owner = "owner" + repo = "repo" + pr_number = 42 + + # Mock repo endpoint (no X-OAuth-Scopes = fine-grained PAT) + respx.get(f"https://api.github.com/repos/{owner}/{repo}").mock( + return_value=Response(200, json={ + "id": 12345, + "name": repo, + "private": False, + "permissions": {"admin": False, "push": True, "pull": True} + }) + ) + + # Mock 403 on reaction creation (token not configured for this repo) + respx.post(f"https://api.github.com/repos/{owner}/{repo}/issues/{pr_number}/reactions").mock( + return_value=Response(403, json={"message": "Resource not accessible by integration"}) + ) + + client = GitHubClient(token="fake-token") + result = client.check_write_access(owner, repo, pr_number) + + assert result.status == WriteAccessStatus.DENIED + assert result.is_fine_grained_pat is True + assert "write test failed" in result.message + + +@respx.mock +def test_check_write_access_fine_grained_pat_no_pr_number(): + """Fine-grained PAT without pr_number returns UNCERTAIN (can't do live test).""" + owner = "owner" + repo = "repo" + + # Mock repo endpoint (no X-OAuth-Scopes = fine-grained PAT) + respx.get(f"https://api.github.com/repos/{owner}/{repo}").mock( + return_value=Response(200, json={ + "id": 12345, + "name": repo, + "private": False, + "permissions": {"admin": False, "push": True, "pull": True} + }) + ) + + client = GitHubClient(token="fake-token") + # Don't pass pr_number - should return UNCERTAIN + result = client.check_write_access(owner, repo) - assert has_access is False + assert result.status == WriteAccessStatus.UNCERTAIN + assert result.is_fine_grained_pat is True @respx.mock diff --git a/tests/test_main.py b/tests/test_main.py index ef24d74..ad16d03 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -27,7 +27,7 @@ def test_format_pr_comment_structure(self): assert "🗺️ **Auto-Generated Review Roadmap**" in result # Check attribution link - assert "https://github.com/jwm4/review-roadmap" in result + assert "https://github.com/ambient-code/review-roadmap" in result # Check model info format (provider/model) assert "anthropic/claude-sonnet-4-20250514" in result @@ -198,7 +198,12 @@ def test_generate_posts_to_pr(self): with patch("review_roadmap.main.GitHubClient") as mock_gh: mock_client = MagicMock() mock_client.get_pr_context.return_value = mock_context - mock_client.check_write_access.return_value = True + from review_roadmap.models import WriteAccessResult, WriteAccessStatus + mock_client.check_write_access.return_value = WriteAccessResult( + status=WriteAccessStatus.GRANTED, + is_fine_grained_pat=False, + message="Classic token with correct scopes verified." + ) mock_gh.return_value = mock_client with patch("review_roadmap.main.build_graph") as mock_build: @@ -223,7 +228,12 @@ def test_generate_post_fails_without_write_access(self): with patch("review_roadmap.main.configure_logging"): with patch("review_roadmap.main.GitHubClient") as mock_gh: mock_client = MagicMock() - mock_client.check_write_access.return_value = False + from review_roadmap.models import WriteAccessResult, WriteAccessStatus + mock_client.check_write_access.return_value = WriteAccessResult( + status=WriteAccessStatus.DENIED, + is_fine_grained_pat=False, + message="Token lacks required scope." + ) mock_gh.return_value = mock_client from review_roadmap.main import app @@ -262,7 +272,12 @@ def test_generate_handles_post_error(self): with patch("review_roadmap.main.GitHubClient") as mock_gh: mock_client = MagicMock() mock_client.get_pr_context.return_value = mock_context - mock_client.check_write_access.return_value = True + from review_roadmap.models import WriteAccessResult, WriteAccessStatus + mock_client.check_write_access.return_value = WriteAccessResult( + status=WriteAccessStatus.GRANTED, + is_fine_grained_pat=False, + message="Classic token with correct scopes verified." + ) mock_client.post_pr_comment.side_effect = Exception("Post failed") mock_gh.return_value = mock_client