Skip to content

Conversation

@sheepdestroyer
Copy link
Owner

@sheepdestroyer sheepdestroyer commented Jan 30, 2026

User description

Updating PR Review Skill and documentation to enforced strict autonomous agent behaviors (The Loop, Freshness, Prohibitions).


PR Type

Enhancement, Documentation


Description

  • Enhanced PR review skill with stricter autonomous agent behavior enforcement

  • Added mandatory behavior rules for autonomy, freshness, completeness, and quality

  • Expanded next_step messages with detailed instructions for code review handling

  • Updated documentation paths and added new sections on freshness and rate limits

  • Clarified prohibitions against autonomous merging and branch deletion


Diagram Walkthrough

flowchart LR
  A["PR Review Skill"] -->|Enhanced next_step messages| B["Stricter Autonomy Rules"]
  A -->|Added mandatory behaviors| C["Freshness & Completeness"]
  D["PR Standards Doc"] -->|Expanded rules| E["Freshness & Rate Limits"]
  D -->|Clarified prohibitions| F["No Autonomous Merge/Delete"]
  G["SKILL.md Documentation"] -->|Updated paths| H["Consistent Path References"]
  G -->|Added behavior rules| I["Mandatory Enforcement"]
Loading

File Walkthrough

Relevant files
Enhancement
pr_skill.py
Enhanced next_step messages with stricter behavior enforcement

.agent/skills/pr_review/pr_skill.py

  • Enhanced trigger_review next_step message to enforce autonomous
    polling behavior
  • Expanded all status check next_step messages with detailed
    instructions for code review handling
  • Added requirements for pulling latest changes, addressing all reviews,
    running tests, and handling rate limits
  • Clarified prohibition against autonomous merging with instruction to
    ask for human review
+7/-7     
pr-standards.md
Added freshness and rate limit rules with path corrections

.agent/rules/pr-standards.md

  • Added clarification to exit condition for rate-limited scenarios
  • Corrected tool path reference from .agent/skills/pr_review/pr_skill.py
    to skills/pr_review/pr_skill.py
  • Added new section 11 on Freshness with mandatory pull and conflict
    resolution requirements
  • Added new section 12 on Rate Limits with awareness and backoff
    strategies
  • Renumbered Critical Prohibitions to section 13 and enhanced with
    explicit instruction against autonomous branch operations
+15/-5   
rules.md
Added compliance section enforcing PR standards                   

.agent/rules/rules.md

  • Added new section 1 on Compliance emphasizing enforcement of PR
    standards and prohibitions
  • Renumbered Context7 MCP section from 1 to 2
  • Established clear compliance framework referencing pr-standards.md
+8/-1     
Documentation
SKILL.md
Updated paths and added mandatory behavior rules documentation

.agent/skills/pr_review/SKILL.md

  • Updated documentation path reference from .agent/rules/pr-standards.md
    to ~/.gemini/rules/pr-standards.md
  • Added new section "Mandatory Behavior Rules" documenting six enforced
    behaviors
  • Updated all tool usage examples with new path format
    ~/.gemini/antigravity/skills/pr_review/pr_skill.py
  • Clarified tool output interpretation with emphasis on next_step
    instructions
+15/-7   
pr-review-cycle.md
Updated workflow paths and added freshness requirements   

.agent/workflows/pr-review-cycle.md

  • Updated all command paths to use
    ~/.gemini/antigravity/skills/pr_review/pr_skill.py format
  • Added Freshness and Completeness requirements in Step 4 (Analyze &
    Implement)
  • Updated compliance section path reference to
    ~/.gemini/rules/pr-standards.md
+5/-3     


Important

Enhances PR review skill with stricter autonomous agent behaviors and updates documentation for clarity and consistency.

  • Behavior:
    • Enhanced pr_skill.py to enforce stricter autonomous agent behaviors, including mandatory rules for autonomy, freshness, completeness, and quality.
    • Expanded next_step messages with detailed instructions for code review handling.
    • Clarified prohibitions against autonomous merging and branch deletion.
  • Documentation:
    • Updated pr-standards.md with new sections on freshness and rate limits, and clarified prohibitions.
    • Updated rules.md to include a compliance section enforcing PR standards.
    • Updated SKILL.md and pr-review-cycle.md with new path references and mandatory behavior rules.

This description was created by Ellipsis for 52fa107. You can customize this summary. It will automatically update as commits are pushed.


Summary by CodeRabbit

  • Documentation

    • Updated PR review standards with explicit compliance requirements and rate-limit handling.
    • Enhanced workflow guidance to include freshness verification and completeness checks before code review triggers.
    • Reorganized agent rules documentation for clarity on mandatory standards and prohibitions.
  • Chores

    • Improved internal configuration paths and tool references for better path resolution.
    • Added deploy.sh to version control ignore list.
    • Refined error handling and logging in PR review automation.

✏️ Tip: You can customize this high-level summary in your review settings.

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @sheepdestroyer, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@coderabbitai
Copy link

coderabbitai bot commented Jan 30, 2026

📝 Walkthrough

Walkthrough

This pull request updates agent rules, skills, and workflows to establish clearer deployment paths, enforce rate-limiting exit conditions, and introduce mandatory behavioral rules for PR review automation. Changes include restructured compliance documentation, enhanced tool path references, and improved instructional messaging across multiple configuration files.

Changes

Cohort / File(s) Summary
Agent Rules & Standards
.agent/rules/pr-standards.md, .agent/rules/rules.md, .agent/knowledge-base.md
Reorganized rules hierarchy with new Compliance section, added deployment location discovery paths, introduced Freshness and Rate Limits sections, expanded exit conditions to include rate-limiting, and emphasized critical prohibitions against merge/close/delete operations without human review.
PR Review Skill
.agent/skills/pr_review/SKILL.md, .agent/skills/pr_review/pr_skill.py
Updated file path references to user-local locations, introduced Mandatory Behavior Rules documentation section, added ACTION_INSTRUCTIONS and RATE_LIMIT_INSTRUCTION constants, refined logging/messaging, and improved subprocess error handling; enhanced JSON output structure with checked_at_utc field.
Workflows & Configuration
.agent/workflows/pr-review-cycle.md, .gitignore
Added dynamic PATH_TO_SKILLS placeholder for tool invocations, updated Compliance section references, incorporated explicit Freshness and Completeness guidance into workflow steps; excluded deploy.sh from version control.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Poem

🐰 Paths now clear, from local to global wide,
Rules of freshness, rate limits, and pride,
The loop runs fresh with each pull and push,
No merges without a human's brush!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'chore: update PR review skill and docs' accurately reflects the main changes across all modified files, which include updates to pr_skill.py, pr-standards.md, rules.md, SKILL.md, and pr-review-cycle.md.
Docstring Coverage ✅ Passed Docstring coverage is 84.62% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/update-pr-skill

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist
Copy link

Summary of Changes

Hello @sheepdestroyer, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refines the operational guidelines and associated tooling for autonomous agents involved in the PR review process. The primary goal is to enforce more robust and predictable agent behaviors by clarifying rules around review cycles, code freshness, rate limit management, and critical prohibitions. These updates ensure agents operate more autonomously and effectively within the defined review workflow, while also providing clearer instructions through tool outputs.

Highlights

  • Enhanced Agent Behavior Rules: The PR introduces stricter guidelines for autonomous agents, particularly around 'The Loop' rule, 'Freshness' (requiring agents to pull latest changes), and 'Rate Limits' awareness and handling.
  • Updated Tool Paths and Documentation: References to the pr_skill.py tool and pr-standards.md documentation have been updated across various files to reflect a new, more standardized path (~/.gemini/antigravity/skills/pr_review/pr_skill.py and ~/.gemini/rules/pr-standards.md).
  • Improved Agent Guidance in pr_skill.py: The pr_skill.py script now provides more explicit next_step instructions to agents, reinforcing mandatory behaviors like continuous polling, addressing all feedback, ensuring CI passes, and handling rate limits, as well as critical prohibitions against autonomous merging.
  • Centralized Compliance Definition: A new 'Compliance' section has been added to rules.md to explicitly state that the system enforces the standards defined in pr-standards.md, covering 'Push Before Trigger', 'The Loop', and 'Prohibitions'.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

This commit fixes the style issues introduced in 4d4a0f3 according to the output
from Autopep8, Black, isort, Ruff Formatter and Yapf.

Details: #8
@qodo-code-review
Copy link

qodo-code-review bot commented Jan 30, 2026

ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed everything up to 4d4a0f3 in 27 seconds. Click for details.
  • Reviewed 224 lines of code in 5 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

Workflow ID: wflow_Bxun4R9u8LnP9m5k

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@sheepdestroyer
Copy link
Owner Author

/gemini review

@sheepdestroyer
Copy link
Owner Author

@coderabbitai review

@sheepdestroyer
Copy link
Owner Author

@sourcery-ai review

@deepsource-io
Copy link
Contributor

deepsource-io bot commented Jan 30, 2026

Here's the code health analysis summary for commits da0274a..52fa107. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource Python LogoPython❌ Failure
❗ 12 occurences introduced
🎯 12 occurences resolved
View Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skipped PR review on 4df283c because no changed files had a supported extension. If you think this was in error, please contact us and we'll fix it right away.

@sheepdestroyer
Copy link
Owner Author

/review

@sheepdestroyer
Copy link
Owner Author

@ellipsis review this

@sourcery-ai
Copy link

sourcery-ai bot commented Jan 30, 2026

Sorry @sheepdestroyer, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@coderabbitai
Copy link

coderabbitai bot commented Jan 30, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@qodo-code-review
Copy link

qodo-code-review bot commented Jan 30, 2026

PR Reviewer Guide 🔍

(Review updated until commit 60c69b6)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 Security concerns

Sensitive information exposure:
The tool pulls GitHub tokens from environment variables / gh auth token and includes exception messages in some outputs. While many errors are masked via _mask_token, ensure all error paths (notably the gh auth token failure message and any subprocess exceptions) cannot include token strings in stderr/stdout JSON fields.

⚡ Recommended focus areas for review

Rate Limiting

The rules/docs introduce a “rate limits” and “exponential backoff” requirement, but the polling implementation uses fixed intervals and attempts. Validate expected behavior under GitHub API rate limits and bot rate-limit states, and consider aligning the implementation with the documented backoff strategy (or adjusting docs to match reality).

# Polling constants for review feedback
# Configurable via environment variables
try:
    POLL_INTERVAL_SECONDS = max(
        int(os.environ.get("PR_REVIEW_POLL_INTERVAL", "30")), 1)
except ValueError:
    POLL_INTERVAL_SECONDS = 30

try:
    POLL_MAX_ATTEMPTS = max(
        int(os.environ.get("PR_REVIEW_POLL_MAX_ATTEMPTS", "20")), 1)
except ValueError:
    POLL_MAX_ATTEMPTS = 20

# Default Validation Reviewer (The bot/user that must approve)
DEFAULT_VALIDATION_REVIEWER = os.environ.get(
    "PR_REVIEW_VALIDATION_REVIEWER", "gemini-code-assist[bot]"
)

# Common instructional strings for next_step
ACTION_INSTRUCTIONS = (
    "ANALYZE feedback -> FIX code -> SAFE_PUSH. DO NOT STOP. "
    "Pull and merge latest changes from the remote branch before starting to address code reviews, "
    "as bots may have since pushed formatting fixes to your previous changes. "
    "Be sure to address every comment and code review from all reviewers, ensure CI passes. "
    "Run and fix all available tests and Linting before pushing your next changes."
)

RATE_LIMIT_INSTRUCTION = " If main reviewer says it just became rate-limited, address remaining code reviews then stop there."


def print_json(data):
    """Helper to print JSON to stdout."""
    print(json.dumps(data, indent=2))


def print_error(message, code=1):
    """Helper to print error JSON to stdout and exit."""
    print_json({"status": "error", "message": message, "code": code})
    sys.exit(code)


class ReviewManager:
    def __init__(self):
        # Authenticate with GitHub
        self.token = os.environ.get(
            "GITHUB_TOKEN") or os.environ.get("GH_TOKEN")
        if not self.token:
            # Fallback to gh CLI for auth token if env var is missing
            try:
                res = subprocess.run(
                    ["gh", "auth", "token"],
                    capture_output=True,
                    text=True,
                    check=True,
                    timeout=GH_AUTH_TIMEOUT,
                )
                self.token = res.stdout.strip()
            except (
                subprocess.CalledProcessError,
                FileNotFoundError,
                subprocess.TimeoutExpired,
            ) as e:
                print_error(
                    f"No GITHUB_TOKEN found and 'gh' command failed: {e}")

        try:
            self.g = Github(auth=Auth.Token(self.token))
            self.repo = self._detect_repo()
            self._ensure_workspace()
        except (GithubException, OSError, ValueError) as e:
            # Mask token if present in error
            safe_msg = str(e)
            if self.token:
                safe_msg = safe_msg.replace(self.token, "********")
            print_error(f"Initialization failed: {safe_msg}")

    def _mask_token(self, text):
        """Redacts the GitHub token from the given text."""
        if not self.token or not text:
            return text
        return text.replace(self.token, "********")

    def _log(self, message):
        """Audit logging to stderr with timestamp."""
        timestamp = datetime.now(timezone.utc).isoformat()
        # Tag logs as [AUDIT] for compliance and easier filtering
        print(f"[{timestamp}] [AUDIT] {message}", file=sys.stderr)

    def _ensure_workspace(self):
        """Creates agent-workspace directory relative to repo root if possible."""
        try:
            # Try to find repo root
            root = subprocess.run(
                ["git", "rev-parse", "--show-toplevel"],
                capture_output=True,
                text=True,
                check=True,
            ).stdout.strip()
            if os.path.basename(root) == "agent-tools":
                self.workspace = os.path.join(root, "agent-workspace")
            else:
                self.workspace = os.path.join(
                    root, "agent-tools", "agent-workspace")
        except (subprocess.CalledProcessError, FileNotFoundError):
            # Fallback to current directory logic
            self.workspace = os.path.join(os.getcwd(), "agent-workspace")

        os.makedirs(self.workspace, exist_ok=True)

    def _detect_repo(self):
        """Auto-detects current repository from git remote (local check preferred)."""
        # 1. Try local git remote first (fast, no network)
        try:
            # Get origin URL
            res = subprocess.run(
                ["git", "config", "--get", "remote.origin.url"],
                capture_output=True,
                text=True,
                check=True,
                timeout=GIT_SHORT_TIMEOUT,
            )
            url = res.stdout.strip()

            # Extract owner/repo using regex
            # Matches: https://github.com/owner/repo.git, git@github.com:owner/repo.git, etc.
            match = re.search(
                r"github\.com[:/]([^/]+)/([^/]+?)(?:\.git)?$", url)
            if match:
                full_name = f"{match.group(1)}/{match.group(2)}"
                return self.g.get_repo(full_name)
        except (
            subprocess.CalledProcessError,
            subprocess.TimeoutExpired,
            FileNotFoundError,
        ):
            # Ignore local errors and fall back to gh
            self._log("Local git remote check failed, falling back to 'gh'...")
            pass

        # 2. Fallback to gh CLI (slower, network dependent)
        try:
            res = subprocess.run(
                ["gh", "repo", "view", "--json", "owner,name"],
                capture_output=True,
                text=True,
                check=True,
                timeout=GH_REPO_VIEW_TIMEOUT,
            )
            data = json.loads(res.stdout)
            full_name = f"{data['owner']['login']}/{data['name']}"
            return self.g.get_repo(full_name)
        except (
            subprocess.CalledProcessError,
            FileNotFoundError,
            json.JSONDecodeError,
            ValueError,
        ):
            raise RuntimeError(
                "Error checking repository context: Ensure 'gh' is installed and you are in a git repository."
            ) from None

    def _verify_clean_git(self):
        """
        Helper to check that the working directory is clean and we are on a valid branch.
        Returns: (is_valid, branch_name_or_error_msg)
        """
        try:
            # 1. Check for uncommitted changes
            status_proc = subprocess.run(
                ["git", "status", "--porcelain"],
                capture_output=True,
                text=True,
                check=True,
                timeout=GIT_SHORT_TIMEOUT,
            )
            if status_proc.stdout.strip():
                return (
                    False,
                    "Uncommitted changes detected. Please commit or stash them first.",
                )

            # 2. Get current branch
            branch_proc = subprocess.run(
                ["git", "rev-parse", "--abbrev-ref", "HEAD"],
                capture_output=True,
                text=True,
                check=True,
                timeout=GIT_SHORT_TIMEOUT,
            )
            branch = branch_proc.stdout.strip()
            if branch == "HEAD":
                return False, "Detached HEAD state detected. Please checkout a branch."

            return True, branch
        except (subprocess.CalledProcessError, FileNotFoundError) as e:
            return False, f"Git check failed: {self._mask_token(str(e))}"
        except subprocess.TimeoutExpired:
            return False, "Git check timed out."

    def _check_local_state(self):
        """
        Verifies:
        1. Clean git status (implemented in helper).
        2. Pushed to remote (upstream sync).
        """
        # 1. Check local cleanliness
        is_clean, branch_or_msg = self._verify_clean_git()
        if not is_clean:
            return False, branch_or_msg

        branch = branch_or_msg

        # 2. Check if pushed to upstream
        try:
            # Fetch latest state from remote for accurate comparison
            # Suppress stdout to avoid polluting structured output; inherit stderr so prompts/hangs remain visible
            subprocess.run(
                ["git", "fetch"],
                check=True,
                timeout=GIT_FETCH_TIMEOUT,
                stdout=subprocess.DEVNULL,
            )

            # Get current branch
            branch = subprocess.run(
                ["git", "rev-parse", "--abbrev-ref", "HEAD"],
                capture_output=True,
                text=True,
                check=True,
                timeout=GIT_SHORT_TIMEOUT,
            ).stdout.strip()

            # Check if upstream is configured
            upstream_proc = subprocess.run(
                ["git", "rev-parse", "--abbrev-ref", "@{u}"],
                capture_output=True,
                text=True,
                timeout=GIT_SHORT_TIMEOUT,
            )
            if upstream_proc.returncode != 0:
                return (
                    False,
                    f"No upstream configured for branch '{branch}'. Please 'git push -u origin {branch}' first.",
                )

            # Check for unpushed commits and upstream changes
            # git rev-list --left-right --count @{u}...HEAD
            # Output: "behind  ahead" (left=@{u}, right=HEAD)
            rev_list = subprocess.run(
                ["git", "rev-list", "--left-right", "--count", "@{u}...HEAD"],
                capture_output=True,
                text=True,
                timeout=GIT_SHORT_TIMEOUT,
            )
            if rev_list.returncode == 0:
                try:
                    parts = rev_list.stdout.split()
                    if len(parts) == 2:
                        behind, ahead = int(parts[0]), int(parts[1])
                        if ahead > 0:
                            return (
                                False,
                                f"Local branch '{branch}' has {ahead} unpushed commit(s). You MUST push before triggering a review.",
                            )
                        if behind > 0:
                            return (
                                False,
                                f"Local branch '{branch}' is behind upstream by {behind} commit(s). Please pull.",
                            )
                    else:
                        return (
                            False,
                            f"Unexpected git rev-list output: '{rev_list.stdout.strip()}'",
                        )
                except ValueError:
                    return (
                        False,
                        f"Failed to parse git rev-list output: '{rev_list.stdout.strip()}'",
                    )
            else:
                # Fallback/General error
                return False, f"Failed to check divergence: {rev_list.stderr.strip()}"
        except (subprocess.CalledProcessError, subprocess.TimeoutExpired) as e:
            return False, f"Git check failed: {self._mask_token(str(e))}"

        return True, "Code is clean and pushed."

    def safe_push(self):
        """Attempts to push changes safely, aborting if uncommitted changes exist. Ignores unpushed commits check."""
        self._log("Running safe_push verification...")

        # 1. Check local cleanliness using helper
        is_clean, branch_or_msg = self._verify_clean_git()
        if not is_clean:
            # Map helper error to JSON format
            return {
                "status": "error",
                "message": branch_or_msg,
                "next_step": "Ensure git is clean and valid.",
            }

        branch = branch_or_msg

        # Separately check upstream
        try:
            upstream_proc = subprocess.run(
                ["git", "rev-parse", "--abbrev-ref", "@{u}"],
                capture_output=True,
                text=True,
                timeout=GIT_SHORT_TIMEOUT,
            )
            if upstream_proc.returncode != 0:
                return {
                    "status": "error",
                    "message": f"No upstream configured for branch '{branch}'. Please 'git push -u origin {branch}' first.",
                    "next_step": "Configure upstream and retry safe_push.",
                }
        except subprocess.TimeoutExpired:
            return {"status": "error", "message": "Git upstream check timed out."}
        except (subprocess.CalledProcessError, FileNotFoundError):
            # This can happen if 'git rev-parse --abbrev-ref @{u}' fails for other reasons
            return {
                "status": "error",
                "message": f"Failed to determine upstream for branch '{branch}'. Please ensure it's configured.",
                "next_step": "Check git configuration and retry safe_push.",
            }

        # Attempt push
        try:
            subprocess.run(["git", "push"], check=True,
                           timeout=GIT_PUSH_TIMEOUT)
            return {
                "status": "success",
                "message": "Push successful.",
                "next_step": "Run 'trigger_review' to start the review cycle.",
            }
        except (
            subprocess.CalledProcessError,
            subprocess.TimeoutExpired,
            FileNotFoundError,
        ) as e:
            # Mask token if present in error
            safe_err = self._mask_token(str(e))
            return {
                "status": "error",
                "message": f"Push failed or timed out: {safe_err}. You may need to pull changes first or check your connection.",
                "next_step": "Pull changes, resolve conflicts, and retry safe_push.",
            }

    def _poll_for_main_reviewer(
        self,
        pr_number,
        since_iso,
        validation_reviewer,
        max_attempts=None,
        poll_interval=None,
    ):
        """
        Polls until the main reviewer has provided feedback since the given timestamp.
        Enforces the Loop Rule: never return until main reviewer responds or timeout.

        Returns the status data from check_status once main reviewer feedback is detected.
        """
        max_attempts = POLL_MAX_ATTEMPTS if max_attempts is None else max_attempts
        poll_interval = (
            POLL_INTERVAL_SECONDS if poll_interval is None else poll_interval
        )

        # Initialize status_data to handle edge case where max_attempts is 0 or loop is interrupted
        status_data = None

        for attempt in range(1, max_attempts + 1):
            try:
                self._log(f"Poll attempt {attempt}/{max_attempts}...")

                # Get current status
                status_data = self.check_status(
                    pr_number,
                    since_iso=since_iso,
                    return_data=True,
                    validation_reviewer=validation_reviewer,
                )

                # Check for any NEW feedback from main reviewer in items (filtered by since_iso)
                # IMPORTANT: Do NOT check main_reviewer_state here - that reflects ALL historical reviews
                # and would cause immediate exit if main reviewer ever commented before.
                # We only want to exit when the main reviewer has posted NEW feedback since trigger.
                main_reviewer_has_new_feedback = any(
                    item.get("user") == validation_reviewer
                    for item in status_data.get("items", [])
                )

                if main_reviewer_has_new_feedback:
                    main_reviewer_info = status_data.get("main_reviewer", {})
                    main_reviewer_state = main_reviewer_info.get(
                        "state", "PENDING")
                    self._log(
                        f"Main reviewer ({validation_reviewer}) has NEW feedback with state: {main_reviewer_state}"
                    )
                    return status_data

                # Not yet - wait and poll again
                if attempt < max_attempts:
                    self._log(
                        f"Main reviewer has not responded yet. Waiting {poll_interval}s before next poll..."
                    )
                    time.sleep(poll_interval)
            except KeyboardInterrupt:
                self._log("\nPolling interrupted by user.")
                # Return distinct status for interruption vs timeout
                if status_data is None:
                    status_data = {
                        "status": "interrupted",
                        "message": "Polling interrupted before first check.",
                    }
                status_data["polling_interrupted"] = True
                status_data["next_step"] = (
                    "INTERRUPTED: Polling cancelled by user. Resume with 'status' command."
                )
                return status_data

        # Timeout - return status with warning
        self._log(
            f"WARNING: Main reviewer did not respond within {max_attempts * poll_interval}s timeout."
        )

        # Handle case where no polls were made (e.g., max_attempts was 0)
        if status_data is None:
            status_data = {
                "status": "error",
                "message": "Polling failed - no status data available.",
            }

        status_data["polling_timeout"] = True
        status_data["next_step"] = (
            f"TIMEOUT: {validation_reviewer} did not respond. Poll again with 'status' or investigate bot issues."
        )
        return status_data
Path Assumptions

Workspace derivation assumes an agent-tools directory layout and may create paths like <repo>/agent-tools/agent-workspace for repos that don’t follow that structure. Confirm this doesn’t cause writes outside intended locations and that downstream code expects the resulting path.

def _ensure_workspace(self):
    """Creates agent-workspace directory relative to repo root if possible."""
    try:
        # Try to find repo root
        root = subprocess.run(
            ["git", "rev-parse", "--show-toplevel"],
            capture_output=True,
            text=True,
            check=True,
        ).stdout.strip()
        if os.path.basename(root) == "agent-tools":
            self.workspace = os.path.join(root, "agent-workspace")
        else:
            self.workspace = os.path.join(
                root, "agent-tools", "agent-workspace")
    except (subprocess.CalledProcessError, FileNotFoundError):
        # Fallback to current directory logic
        self.workspace = os.path.join(os.getcwd(), "agent-workspace")
Output Consistency

Several next_step messages are long, mixed-case, and include free-form instructions. If any downstream agent/parser relies on consistent phrasing, keywords, or length limits, verify compatibility. Also confirm print_error/JSON error output paths stay machine-readable in all failure modes.

# Common instructional strings for next_step
ACTION_INSTRUCTIONS = (
    "ANALYZE feedback -> FIX code -> SAFE_PUSH. DO NOT STOP. "
    "Pull and merge latest changes from the remote branch before starting to address code reviews, "
    "as bots may have since pushed formatting fixes to your previous changes. "
    "Be sure to address every comment and code review from all reviewers, ensure CI passes. "
    "Run and fix all available tests and Linting before pushing your next changes."
)

RATE_LIMIT_INSTRUCTION = " If main reviewer says it just became rate-limited, address remaining code reviews then stop there."


def print_json(data):
    """Helper to print JSON to stdout."""
    print(json.dumps(data, indent=2))


def print_error(message, code=1):
    """Helper to print error JSON to stdout and exit."""
    print_json({"status": "error", "message": message, "code": code})
    sys.exit(code)


class ReviewManager:
    def __init__(self):
        # Authenticate with GitHub
        self.token = os.environ.get(
            "GITHUB_TOKEN") or os.environ.get("GH_TOKEN")
        if not self.token:
            # Fallback to gh CLI for auth token if env var is missing
            try:
                res = subprocess.run(
                    ["gh", "auth", "token"],
                    capture_output=True,
                    text=True,
                    check=True,
                    timeout=GH_AUTH_TIMEOUT,
                )
                self.token = res.stdout.strip()
            except (
                subprocess.CalledProcessError,
                FileNotFoundError,
                subprocess.TimeoutExpired,
            ) as e:
                print_error(
                    f"No GITHUB_TOKEN found and 'gh' command failed: {e}")

        try:
            self.g = Github(auth=Auth.Token(self.token))
            self.repo = self._detect_repo()
            self._ensure_workspace()
        except (GithubException, OSError, ValueError) as e:
            # Mask token if present in error
            safe_msg = str(e)
            if self.token:
                safe_msg = safe_msg.replace(self.token, "********")
            print_error(f"Initialization failed: {safe_msg}")

    def _mask_token(self, text):
        """Redacts the GitHub token from the given text."""
        if not self.token or not text:
            return text
        return text.replace(self.token, "********")

    def _log(self, message):
        """Audit logging to stderr with timestamp."""
        timestamp = datetime.now(timezone.utc).isoformat()
        # Tag logs as [AUDIT] for compliance and easier filtering
        print(f"[{timestamp}] [AUDIT] {message}", file=sys.stderr)

    def _ensure_workspace(self):
        """Creates agent-workspace directory relative to repo root if possible."""
        try:
            # Try to find repo root
            root = subprocess.run(
                ["git", "rev-parse", "--show-toplevel"],
                capture_output=True,
                text=True,
                check=True,
            ).stdout.strip()
            if os.path.basename(root) == "agent-tools":
                self.workspace = os.path.join(root, "agent-workspace")
            else:
                self.workspace = os.path.join(
                    root, "agent-tools", "agent-workspace")
        except (subprocess.CalledProcessError, FileNotFoundError):
            # Fallback to current directory logic
            self.workspace = os.path.join(os.getcwd(), "agent-workspace")

        os.makedirs(self.workspace, exist_ok=True)

    def _detect_repo(self):
        """Auto-detects current repository from git remote (local check preferred)."""
        # 1. Try local git remote first (fast, no network)
        try:
            # Get origin URL
            res = subprocess.run(
                ["git", "config", "--get", "remote.origin.url"],
                capture_output=True,
                text=True,
                check=True,
                timeout=GIT_SHORT_TIMEOUT,
            )
            url = res.stdout.strip()

            # Extract owner/repo using regex
            # Matches: https://github.com/owner/repo.git, git@github.com:owner/repo.git, etc.
            match = re.search(
                r"github\.com[:/]([^/]+)/([^/]+?)(?:\.git)?$", url)
            if match:
                full_name = f"{match.group(1)}/{match.group(2)}"
                return self.g.get_repo(full_name)
        except (
            subprocess.CalledProcessError,
            subprocess.TimeoutExpired,
            FileNotFoundError,
        ):
            # Ignore local errors and fall back to gh
            self._log("Local git remote check failed, falling back to 'gh'...")
            pass

        # 2. Fallback to gh CLI (slower, network dependent)
        try:
            res = subprocess.run(
                ["gh", "repo", "view", "--json", "owner,name"],
                capture_output=True,
                text=True,
                check=True,
                timeout=GH_REPO_VIEW_TIMEOUT,
            )
            data = json.loads(res.stdout)
            full_name = f"{data['owner']['login']}/{data['name']}"
            return self.g.get_repo(full_name)
        except (
            subprocess.CalledProcessError,
            FileNotFoundError,
            json.JSONDecodeError,
            ValueError,
        ):
            raise RuntimeError(
                "Error checking repository context: Ensure 'gh' is installed and you are in a git repository."
            ) from None

    def _verify_clean_git(self):
        """
        Helper to check that the working directory is clean and we are on a valid branch.
        Returns: (is_valid, branch_name_or_error_msg)
        """
        try:
            # 1. Check for uncommitted changes
            status_proc = subprocess.run(
                ["git", "status", "--porcelain"],
                capture_output=True,
                text=True,
                check=True,
                timeout=GIT_SHORT_TIMEOUT,
            )
            if status_proc.stdout.strip():
                return (
                    False,
                    "Uncommitted changes detected. Please commit or stash them first.",
                )

            # 2. Get current branch
            branch_proc = subprocess.run(
                ["git", "rev-parse", "--abbrev-ref", "HEAD"],
                capture_output=True,
                text=True,
                check=True,
                timeout=GIT_SHORT_TIMEOUT,
            )
            branch = branch_proc.stdout.strip()
            if branch == "HEAD":
                return False, "Detached HEAD state detected. Please checkout a branch."

            return True, branch
        except (subprocess.CalledProcessError, FileNotFoundError) as e:
            return False, f"Git check failed: {self._mask_token(str(e))}"
        except subprocess.TimeoutExpired:
            return False, "Git check timed out."

    def _check_local_state(self):
        """
        Verifies:
        1. Clean git status (implemented in helper).
        2. Pushed to remote (upstream sync).
        """
        # 1. Check local cleanliness
        is_clean, branch_or_msg = self._verify_clean_git()
        if not is_clean:
            return False, branch_or_msg

        branch = branch_or_msg

        # 2. Check if pushed to upstream
        try:
            # Fetch latest state from remote for accurate comparison
            # Suppress stdout to avoid polluting structured output; inherit stderr so prompts/hangs remain visible
            subprocess.run(
                ["git", "fetch"],
                check=True,
                timeout=GIT_FETCH_TIMEOUT,
                stdout=subprocess.DEVNULL,
            )

            # Get current branch
            branch = subprocess.run(
                ["git", "rev-parse", "--abbrev-ref", "HEAD"],
                capture_output=True,
                text=True,
                check=True,
                timeout=GIT_SHORT_TIMEOUT,
            ).stdout.strip()

            # Check if upstream is configured
            upstream_proc = subprocess.run(
                ["git", "rev-parse", "--abbrev-ref", "@{u}"],
                capture_output=True,
                text=True,
                timeout=GIT_SHORT_TIMEOUT,
            )
            if upstream_proc.returncode != 0:
                return (
                    False,
                    f"No upstream configured for branch '{branch}'. Please 'git push -u origin {branch}' first.",
                )

            # Check for unpushed commits and upstream changes
            # git rev-list --left-right --count @{u}...HEAD
            # Output: "behind  ahead" (left=@{u}, right=HEAD)
            rev_list = subprocess.run(
                ["git", "rev-list", "--left-right", "--count", "@{u}...HEAD"],
                capture_output=True,
                text=True,
                timeout=GIT_SHORT_TIMEOUT,
            )
            if rev_list.returncode == 0:
                try:
                    parts = rev_list.stdout.split()
                    if len(parts) == 2:
                        behind, ahead = int(parts[0]), int(parts[1])
                        if ahead > 0:
                            return (
                                False,
                                f"Local branch '{branch}' has {ahead} unpushed commit(s). You MUST push before triggering a review.",
                            )
                        if behind > 0:
                            return (
                                False,
                                f"Local branch '{branch}' is behind upstream by {behind} commit(s). Please pull.",
                            )
                    else:
                        return (
                            False,
                            f"Unexpected git rev-list output: '{rev_list.stdout.strip()}'",
                        )
                except ValueError:
                    return (
                        False,
                        f"Failed to parse git rev-list output: '{rev_list.stdout.strip()}'",
                    )
            else:
                # Fallback/General error
                return False, f"Failed to check divergence: {rev_list.stderr.strip()}"
        except (subprocess.CalledProcessError, subprocess.TimeoutExpired) as e:
            return False, f"Git check failed: {self._mask_token(str(e))}"

        return True, "Code is clean and pushed."

    def safe_push(self):
        """Attempts to push changes safely, aborting if uncommitted changes exist. Ignores unpushed commits check."""
        self._log("Running safe_push verification...")

        # 1. Check local cleanliness using helper
        is_clean, branch_or_msg = self._verify_clean_git()
        if not is_clean:
            # Map helper error to JSON format
            return {
                "status": "error",
                "message": branch_or_msg,
                "next_step": "Ensure git is clean and valid.",
            }

        branch = branch_or_msg

        # Separately check upstream
        try:
            upstream_proc = subprocess.run(
                ["git", "rev-parse", "--abbrev-ref", "@{u}"],
                capture_output=True,
                text=True,
                timeout=GIT_SHORT_TIMEOUT,
            )
            if upstream_proc.returncode != 0:
                return {
                    "status": "error",
                    "message": f"No upstream configured for branch '{branch}'. Please 'git push -u origin {branch}' first.",
                    "next_step": "Configure upstream and retry safe_push.",
                }
        except subprocess.TimeoutExpired:
            return {"status": "error", "message": "Git upstream check timed out."}
        except (subprocess.CalledProcessError, FileNotFoundError):
            # This can happen if 'git rev-parse --abbrev-ref @{u}' fails for other reasons
            return {
                "status": "error",
                "message": f"Failed to determine upstream for branch '{branch}'. Please ensure it's configured.",
                "next_step": "Check git configuration and retry safe_push.",
            }

        # Attempt push
        try:
            subprocess.run(["git", "push"], check=True,
                           timeout=GIT_PUSH_TIMEOUT)
            return {
                "status": "success",
                "message": "Push successful.",
                "next_step": "Run 'trigger_review' to start the review cycle.",
            }
        except (
            subprocess.CalledProcessError,
            subprocess.TimeoutExpired,
            FileNotFoundError,
        ) as e:
            # Mask token if present in error
            safe_err = self._mask_token(str(e))
            return {
                "status": "error",
                "message": f"Push failed or timed out: {safe_err}. You may need to pull changes first or check your connection.",
                "next_step": "Pull changes, resolve conflicts, and retry safe_push.",
            }

    def _poll_for_main_reviewer(
        self,
        pr_number,
        since_iso,
        validation_reviewer,
        max_attempts=None,
        poll_interval=None,
    ):
        """
        Polls until the main reviewer has provided feedback since the given timestamp.
        Enforces the Loop Rule: never return until main reviewer responds or timeout.

        Returns the status data from check_status once main reviewer feedback is detected.
        """
        max_attempts = POLL_MAX_ATTEMPTS if max_attempts is None else max_attempts
        poll_interval = (
            POLL_INTERVAL_SECONDS if poll_interval is None else poll_interval
        )

        # Initialize status_data to handle edge case where max_attempts is 0 or loop is interrupted
        status_data = None

        for attempt in range(1, max_attempts + 1):
            try:
                self._log(f"Poll attempt {attempt}/{max_attempts}...")

                # Get current status
                status_data = self.check_status(
                    pr_number,
                    since_iso=since_iso,
                    return_data=True,
                    validation_reviewer=validation_reviewer,
                )

                # Check for any NEW feedback from main reviewer in items (filtered by since_iso)
                # IMPORTANT: Do NOT check main_reviewer_state here - that reflects ALL historical reviews
                # and would cause immediate exit if main reviewer ever commented before.
                # We only want to exit when the main reviewer has posted NEW feedback since trigger.
                main_reviewer_has_new_feedback = any(
                    item.get("user") == validation_reviewer
                    for item in status_data.get("items", [])
                )

                if main_reviewer_has_new_feedback:
                    main_reviewer_info = status_data.get("main_reviewer", {})
                    main_reviewer_state = main_reviewer_info.get(
                        "state", "PENDING")
                    self._log(
                        f"Main reviewer ({validation_reviewer}) has NEW feedback with state: {main_reviewer_state}"
                    )
                    return status_data

                # Not yet - wait and poll again
                if attempt < max_attempts:
                    self._log(
                        f"Main reviewer has not responded yet. Waiting {poll_interval}s before next poll..."
                    )
                    time.sleep(poll_interval)
            except KeyboardInterrupt:
                self._log("\nPolling interrupted by user.")
                # Return distinct status for interruption vs timeout
                if status_data is None:
                    status_data = {
                        "status": "interrupted",
                        "message": "Polling interrupted before first check.",
                    }
                status_data["polling_interrupted"] = True
                status_data["next_step"] = (
                    "INTERRUPTED: Polling cancelled by user. Resume with 'status' command."
                )
                return status_data

        # Timeout - return status with warning
        self._log(
            f"WARNING: Main reviewer did not respond within {max_attempts * poll_interval}s timeout."
        )

        # Handle case where no polls were made (e.g., max_attempts was 0)
        if status_data is None:
            status_data = {
                "status": "error",
                "message": "Polling failed - no status data available.",
            }

        status_data["polling_timeout"] = True
        status_data["next_step"] = (
            f"TIMEOUT: {validation_reviewer} did not respond. Poll again with 'status' or investigate bot issues."
        )
        return status_data

    def trigger_review(
        self,
        pr_number,
        wait_seconds=180,
        validation_reviewer=DEFAULT_VALIDATION_REVIEWER,
    ):
        """
        1. Checks local state (Hard Constraint).
        2. Post comments to trigger bots.
        3. Polls for main reviewer feedback.
        """
        # Step 1: Enforce Push
        is_safe, msg = self._check_local_state()
        if not is_safe:
            print_error(
                f"FAILED: {msg}\nTip: Use the 'safe_push' tool or run 'git push' manually."
            )

        self._log(f"State verified: {msg}")

        # Capture start time for status check
        start_time = datetime.now(timezone.utc)

        # Step 2: Trigger Bots
        triggered_bots = []
        try:
            pr = self.repo.get_pull(pr_number)
            self._log(f"Triggering reviews on PR #{pr_number} ({pr.title})...")

            for cmd in REVIEW_COMMANDS:
                pr.create_issue_comment(cmd)
                self._log(f"  Posted: {cmd}")
                triggered_bots.append(cmd)

            self._log("All review bots triggered successfully.")

            # Step 3: Poll for Main Reviewer Response (Enforce Loop Rule)
            if wait_seconds > 0:
                self._log("-" * 40)
                self._log(
                    f"Auto-waiting {wait_seconds} seconds for initial bot responses..."
                )
                try:
                    time.sleep(wait_seconds)
                except KeyboardInterrupt:
                    self._log(
                        "\nWait interrupted. Checking status immediately...")

                self._log("-" * 40)
                self._log(
                    "Polling for main reviewer feedback (enforcing Loop Rule)...")

                # Poll until main reviewer responds or timeout
                status_data = self._poll_for_main_reviewer(
                    pr_number=pr_number,
                    since_iso=start_time.isoformat(),
                    validation_reviewer=validation_reviewer,
                )
            else:
                status_data = {
                    "status": "skipped",
                    "message": "Initial status check skipped due to wait_seconds=0.",
                }

            message = "Triggered reviews."
            if wait_seconds > 0 and not status_data.get("polling_interrupted"):
                message = "Triggered reviews and polled for main reviewer feedback."
            elif status_data.get("polling_interrupted"):
                message = "Triggered reviews; polling was interrupted."
            elif wait_seconds <= 0:
                message = "Triggered reviews; polling was skipped."

            return {
                "status": "success",
                "message": message,
                "triggered_bots": triggered_bots,
                "initial_status": status_data,
                "next_step": status_data.get(
                    "next_step",
                    "Run 'status' to check for feedback. Be autonomous, don't stop the cycle. you must regularly poll for the main-reviewer's review until it eventually gets posted, then continue.",
                ),
            }

        except GithubException as e:
            print_error(f"GitHub API Error: {self._mask_token(str(e))}")

    def check_status(
        self,
        pr_number,
        since_iso=None,
        return_data=False,
        validation_reviewer="gemini-code-assist[bot]",
    ):
        """
        Stateless check of PR feedback using PyGithub.
        Returns and/or prints JSON summary of status.
        """

        def get_aware_utc_datetime(dt_obj):
            """Converts a naive datetime from PyGithub into a timezone-aware one."""
            if dt_obj is None:
                return None
            if dt_obj.tzinfo is None:
                return dt_obj.replace(tzinfo=timezone.utc)
            return dt_obj.astimezone(timezone.utc)

        try:
            pr = self.repo.get_pull(pr_number)

            since_dt = datetime(1970, 1, 1, tzinfo=timezone.utc)
            if since_iso:
                try:
                    if since_iso.endswith("Z"):
                        since_iso = since_iso[:-1] + "+00:00"
                    since_dt = datetime.fromisoformat(since_iso)
                    if since_dt.tzinfo is None:
                        since_dt = since_dt.replace(tzinfo=timezone.utc)
                except ValueError:
                    # Log warning but continue
                    print(
                        f"[{datetime.now(timezone.utc).isoformat()}] [AUDIT] Warning: Invalid timestamp {since_iso}, ignoring.",
                        file=sys.stderr,
                    )

            # Fetch comments (paginated by PyGithub automatically)
            new_feedback = []

            # 1. Issue Comments (General)
            issue = self.repo.get_issue(pr_number)
            for comment in issue.get_comments(since=since_dt):
                new_feedback.append(
                    {
                        "type": "issue_comment",
                        "user": comment.user.login,
                        "body": comment.body,
                        "url": comment.html_url,
                        "updated_at": (
                            get_aware_utc_datetime(
                                comment.updated_at).isoformat()
                            if comment.updated_at
                            else None
                        ),
                        "created_at": get_aware_utc_datetime(
                            comment.created_at
                        ).isoformat(),
                    }
                )

            # 2. Review Comments (Inline)
            # Fetch all review comments to ensure we catch edits (since param might only check creation time)
            for comment in pr.get_review_comments():
                # Use updated_at to catch edits
                comment_dt = get_aware_utc_datetime(comment.updated_at)
                if comment_dt and comment_dt >= since_dt:
                    new_feedback.append(
                        {
                            "type": "inline_comment",
                            "user": comment.user.login,
                            "body": comment.body,
                            "path": comment.path,
                            "line": comment.line,
                            "created_at": (
                                get_aware_utc_datetime(
                                    comment.created_at).isoformat()
                                if comment.created_at
                                else None
                            ),
                            "updated_at": comment_dt.isoformat(),
                            "url": comment.html_url,
                        }
                    )

            # 3. Reviews (Approvals/changes requested) - Filter locally
            # Materialize list for multiple iteration
            reviews = list(pr.get_reviews())
            for review in reviews:
                if (
                    review.submitted_at
                ):  # Ensure submitted_at is not None before processing
                    review_dt = get_aware_utc_datetime(review.submitted_at)
                    if review_dt and review_dt >= since_dt:
                        new_feedback.append(
                            {
                                "type": "review_summary",
                                "user": review.user.login,
                                "state": review.state,
                                "body": review.body,
                                "created_at": review_dt.isoformat(),
                            }
                        )

            # Determine next_step based on findings AND validation_reviewer
            next_step = "Wait for reviews."
            has_changes_requested = any(
                item.get("state") == "CHANGES_REQUESTED"
                for item in new_feedback
                if item.get("type") == "review_summary"
            )

            # Check Main Reviewer Status
            # Track latest state AND most recent approval separately
            # (fixes bug where APPROVED -> COMMENTED leaves approval_dt as None)
            main_reviewer_state = "PENDING"
            main_reviewer_last_approval_dt = None

            # Single pass: find latest state AND most recent approval timestamp
            found_latest_state = False
            for review in reversed(reviews):
                if review.user.login == validation_reviewer:
                    if not found_latest_state:
                        main_reviewer_state = review.state
                        found_latest_state = True

                    if (
                        main_reviewer_last_approval_dt is None
                        and review.state == "APPROVED"
                    ):
                        main_reviewer_last_approval_dt = get_aware_utc_datetime(
                            review.submitted_at
                        )

                    # Exit early if both are found
                    if (
                        found_latest_state
                        and main_reviewer_last_approval_dt is not None
                    ):
                        break

            # Check for comments from main_reviewer AFTER approval
            # Note: Only check if approval exists, not if current state is APPROVED
            # (fixes bug where APPROVED -> COMMENTED was not detected)
            has_new_main_reviewer_comments = False
            if main_reviewer_last_approval_dt:
                for item in new_feedback:
                    # Check for issue_comment, inline_comment, OR review comments (state=COMMENTED)
                    is_main_reviewer = item.get("user") == validation_reviewer
                    is_comment = item.get("type") in [
                        "issue_comment", "inline_comment"]
                    is_review_comment = (
                        item.get("type") == "review_summary"
                        and item.get("state") == "COMMENTED"
                    )

                    if is_main_reviewer and (is_comment or is_review_comment):
                        # Use helper for consistent parsing
                        # Only use created_at to avoid treating edits (updated_at) of old comments as new feedback
                        created_at_val = item.get("created_at")
                        if created_at_val:
                            try:
                                # created_at is always an ISO string from our processing
                                # Handle Z suffix for Python < 3.11 compatibility
                                dt_val = datetime.fromisoformat(
                                    created_at_val.replace("Z", "+00:00")
                                )
                                comment_dt = get_aware_utc_datetime(dt_val)

                                # Use >= to catch comments made at the exact same second
                                if comment_dt >= main_reviewer_last_approval_dt:
                                    has_new_main_reviewer_comments = True
                                    break
                            except (ValueError, TypeError) as e:
                                self._log(
                                    f"Warning: Could not parse date '{created_at_val}'. Error: {e}. Skipping item."
                                )
                                continue

            if has_changes_requested:
                next_step = f"CRITICAL: Changes requested by reviewer. {ACTION_INSTRUCTIONS}{RATE_LIMIT_INSTRUCTION}"
            elif has_new_main_reviewer_comments:
                next_step = f"New comments from {validation_reviewer} after approval. {ACTION_INSTRUCTIONS}{RATE_LIMIT_INSTRUCTION}"
            elif main_reviewer_state == "APPROVED":
                # Check if there's any OTHER feedback besides the main reviewer's approval
                other_feedback = [
                    item
                    for item in new_feedback
                    if not (
                        item.get("user") == validation_reviewer
                        and item.get("type") == "review_summary"
                        and item.get("state") == "APPROVED"
                    )
                ]
                if other_feedback:
                    next_step = f"New feedback received. {ACTION_INSTRUCTIONS}"
                else:
                    next_step = "Validation Complete (STOP LOOP - DO NOT MERGE AUTONOMOUSLY). Notify User. Never merge or delete a branch on your own, if you believe the main reviewer said that the PR is ready, just stop and ask for Human review"
            elif new_feedback:
                next_step = f"New feedback received. {ACTION_INSTRUCTIONS}"
            else:
                next_step = f"Waiting for approval from {validation_reviewer} (Current: {main_reviewer_state}). Poll again. Be autonomous, don't stop the cycle. you must regularly poll for the main-reviewer's review until it eventually gets posted, then continue"

            output = {
                "status": "success",
                "pr_number": pr_number,
                "checked_at_utc": datetime.now(timezone.utc).isoformat(),
                "new_item_count": len(new_feedback),
                "items": new_feedback,
                "main_reviewer": {
                    "user": validation_reviewer,
                    "state": main_reviewer_state,
                },
                "next_step": next_step,
            }

            if return_data:
                return output
            else:
                print_json(output)
                return output

        except GithubException as e:
            if return_data:
                raise
            print_error(f"GitHub API Error: {self._mask_token(str(e))}")


def main():
    parser = argparse.ArgumentParser(description="PR Skill Agent Tool")
    subparsers = parser.add_subparsers(dest="command", required=True)

    # Trigger Review
    p_trigger = subparsers.add_parser(
        "trigger_review", help="Trigger reviews safely")
    p_trigger.add_argument("pr_number", type=int)
    p_trigger.add_argument(
        "--wait",
        type=int,
        default=180,
        help="Seconds to wait for initial feedback (default: 180)",
    )
    p_trigger.add_argument(
        "--validation-reviewer",
        default=DEFAULT_VALIDATION_REVIEWER,
        help="Username of the main reviewer that must approve",
    )

    # Status
    p_status = subparsers.add_parser("status", help="Check review status")
    p_status.add_argument("pr_number", type=int)
    p_status.add_argument("--since", help="ISO 8601 timestamp")
    p_status.add_argument(
        "--validation-reviewer",
        default=DEFAULT_VALIDATION_REVIEWER,
        help="Username of the main reviewer that must approve",
    )

    # Safe Push
    subparsers.add_parser("safe_push", help="Push changes safely")

    args = parser.parse_args()

    try:
        mgr = ReviewManager()

        if args.command == "trigger_review":
            result = mgr.trigger_review(
                args.pr_number,
                wait_seconds=args.wait,
                validation_reviewer=args.validation_reviewer,
            )
            print_json(result)
        elif args.command == "status":
            mgr.check_status(
                args.pr_number, args.since, validation_reviewer=args.validation_reviewer
            )
        elif args.command == "safe_push":
            result = mgr.safe_push()
            print_json(result)
            if result["status"] != "success":
                sys.exit(1)
    except Exception as e:
        # Catch-all for unhandled exceptions to prevent raw tracebacks in JSON output
        # Log full traceback to stderr for debugging
        sys.stderr.write(f"CRITICAL ERROR: {str(e)}\n")
        import traceback

        traceback.print_exc(file=sys.stderr)

        # Output clean JSON error
        print(
            json.dumps(
                {
                    "status": "error",
                    "message": "An internal error occurred. See stderr for details.",
                    "error_type": type(e).__name__,
                }
            )
        )
        sys.exit(1)

@qodo-code-review
Copy link

qodo-code-review bot commented Jan 30, 2026

ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Use structured data for agent instructions

Instead of embedding complex, natural-language instructions in the next_step
string, the tool should return structured data about the PR's state. This allows
the agent's core logic to make more reliable decisions.

Examples:

.agent/skills/pr_review/pr_skill.py [525]
                next_step = "CRITICAL: Changes requested by reviewer. ANALYZE feedback -> FIX code -> SAFE_PUSH. DO NOT STOP. Pull and merge latest changes from the remote branch before starting addressing code reviews, as bots may since have pushed formatting fixes to your previous changes. Be sure to address every comments and code reviews from all reviewers, ensure CI passes. Be sure to run and fix all available tests and Linting before pushing your next changes. If main reviewer says it just became rate-limited, address remaining code reviews then stop there."
.agent/skills/pr_review/pr_skill.py [539]
                     next_step = "Validation Complete (STOP LOOP - DO NOT MERGE AUTONOMOUSLY). Notify User. Never merge or delete an branch on your own, if you believe the main reviewer said that the PR is ready, just stop and ask for Human review"

Solution Walkthrough:

Before:

def check_status(pr_number, ...):
    # ... logic to determine PR status ...

    if has_changes_requested:
        next_step = "CRITICAL: Changes requested... Pull and merge latest... Be sure to address every comment..."
    elif main_reviewer_state == "APPROVED":
        next_step = "Validation Complete (STOP LOOP - DO NOT MERGE AUTONOMOUSLY). Notify User. Never merge..."
    else:
        next_step = "Waiting for approval... Poll again. Be autonomous, don't stop the cycle..."

    return {
        "status": "success",
        "items": [...],
        "next_step": next_step
    }

After:

def check_status(pr_number, ...):
    # ... logic to determine PR status ...
    # Determine state flags
    review_status = "CHANGES_REQUESTED" # or "APPROVED", "PENDING"
    is_rate_limited = False # check for rate limit comments
    has_other_feedback = True

    # The agent's logic, not the tool, will construct the next action
    # based on these structured flags.
    return {
        "status": "success",
        "items": [...],
        "state": {
            "review_status": review_status,
            "is_rate_limited": is_rate_limited,
            "has_other_feedback": has_other_feedback,
            "main_reviewer_state": main_reviewer_state
        }
    }
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical design flaw where complex behavioral logic is embedded in a string, making the system brittle; it proposes a much more robust, structured data approach that improves reliability and maintainability.

High
General
Extract repeated messages into constants

Extract the repeated next_step instruction strings into named constants to
reduce duplication and improve maintainability.

.agent/skills/pr_review/pr_skill.py [525]

-next_step = "CRITICAL: Changes requested by reviewer. ANALYZE feedback -> FIX code -> SAFE_PUSH. DO NOT STOP. Pull and merge latest changes from the remote branch before starting addressing code reviews, as bots may since have pushed formatting fixes to your previous changes. Be sure to address every comments and code reviews from all reviewers, ensure CI passes. Be sure to run and fix all available tests and Linting before pushing your next changes. If main reviewer says it just became rate-limited, address remaining code reviews then stop there."
+next_step = (
+    "CRITICAL: Changes requested by reviewer. ANALYZE feedback -> FIX code -> SAFE_PUSH. DO NOT STOP. "
+    f"{FRESHNESS_MSG} {COMPLETENESS_MSG} {RATE_LIMIT_MSG}"
+)
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: This is a good refactoring suggestion that improves maintainability by reducing the significant string duplication introduced in the PR.

Low
Trim trailing space in default next_step

Remove the trailing space from the default next_step string for consistency.

.agent/skills/pr_review/pr_skill.py [385]

-"next_step": status_data.get("next_step", "Run 'status' to check for feedback. Be autonomous, don't stop the cycle. you must regularly poll for the main-reviewer's review until it eventually gets posted, then continue ")
+"next_step": status_data.get(
+    "next_step",
+    "Run 'status' to check for feedback. Be autonomous, don't stop the cycle. you must regularly poll for the main-reviewer's review until it eventually gets posted, then continue"
+)
  • Apply / Chat
Suggestion importance[1-10]: 1

__

Why: The suggestion correctly identifies and fixes a trailing space, which is a minor cosmetic improvement with very low impact.

Low
  • Update

@SourceryAI
Copy link

Sorry @sheepdestroyer, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request enhances the PR review skill and its associated documentation to enforce stricter behaviors for autonomous agents. The changes introduce new rules for freshness, rate limits, and clarify prohibitions. The skill's Python code is updated to provide more detailed next_step instructions.

My review focuses on improving the maintainability of the Python code and correcting minor grammatical issues in the documentation. The main suggestion is to refactor the heavily duplicated instruction strings in pr_skill.py into constants to improve readability and ease of maintenance. I've also pointed out several typos and grammatical errors in the markdown files.

* **No Idling**: Agents must actively monitor PR status. Do NOT exit/notify the user just to wait for a bot or a long process.
* **Polling Strategy**: Use GitHub MCP tools (`mcp_github_pull_request_read`) for reliable, non-blocking status polling. Wait ~3 minutes after triggering before first check, then poll every 2 minutes.
* **Autonomous Action**: Agents are AUTHORIZED and REQUIRED to `git commit` and `git push` fixes autonomously if tests pass. Do not ask the user to push for you.
* **Freshness**: Pull and merge latest changes from the remote branch before starting addressing code reviews, as bots may since have pushed formatting fixes to your previous changes.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The phrasing "may since have pushed" is slightly awkward. For better clarity, consider rephrasing to "may have since pushed".

Suggested change
* **Freshness**: Pull and merge latest changes from the remote branch before starting addressing code reviews, as bots may since have pushed formatting fixes to your previous changes.
* **Freshness**: Pull and merge latest changes from the remote branch before starting addressing code reviews, as bots may have since pushed formatting fixes to your previous changes.

**ABSOLUTE RULE**: Agents must **NEVER** merge a PR, close a PR, or delete a PR's branch.
* **Reason**: Merging, closing, and branch deletion are privileged actions reserved for human maintainers or specific CI/CD pipelines.
* **Action**: If you believe a PR is ready to be merged or should be closed, you must notify the user and await instructions. **Do NOT** execute commands to merge, close, or delete the branch.
* **Action**: If you believe a PR is ready to be merged or should be closed, you must notify the user and await instructions. **Do NOT** execute commands to merge, close, or delete the branch. Never merge or delete an branch on your own, if you believe the main reviewer said that the PR is ready, just stop and ask for Human review.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There is a typo in this line. "an branch" should be "a branch".

Suggested change
* **Action**: If you believe a PR is ready to be merged or should be closed, you must notify the user and await instructions. **Do NOT** execute commands to merge, close, or delete the branch. Never merge or delete an branch on your own, if you believe the main reviewer said that the PR is ready, just stop and ask for Human review.
* **Action**: If you believe a PR is ready to be merged or should be closed, you must notify the user and await instructions. **Do NOT** execute commands to merge, close, or delete the branch. Never merge or delete a branch on your own, if you believe the main reviewer said that the PR is ready, just stop and ask for Human review.


### Mandatory Behavior Rules (Enforced by Tool Output)
1. **Autonomy**: "Be autonomous, don't stop the cycle. you must regularly poll for the main-reviewer's review until it eventually gets posted, then continue"
2. **Freshness**: "Pull and merge latest changes from the remote branch before starting addressing code reviews, as bots may since have pushed formatting fixes to your previous changes"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The phrasing "may since have pushed" is a bit awkward. For better readability, I suggest changing it to "may have since pushed".

Suggested change
2. **Freshness**: "Pull and merge latest changes from the remote branch before starting addressing code reviews, as bots may since have pushed formatting fixes to your previous changes"
2. **Freshness**: "Pull and merge latest changes from the remote branch before starting addressing code reviews, as bots may have since pushed formatting fixes to your previous changes"

### Mandatory Behavior Rules (Enforced by Tool Output)
1. **Autonomy**: "Be autonomous, don't stop the cycle. you must regularly poll for the main-reviewer's review until it eventually gets posted, then continue"
2. **Freshness**: "Pull and merge latest changes from the remote branch before starting addressing code reviews, as bots may since have pushed formatting fixes to your previous changes"
3. **Completeness**: "Be sure to address every comments and code reviews from all reviewers, ensure CI passes"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There's a grammatical error here. "every comments" should be either "every comment" or "all comments". I'd suggest "all comments" for consistency.

Suggested change
3. **Completeness**: "Be sure to address every comments and code reviews from all reviewers, ensure CI passes"
3. **Completeness**: "Be sure to address all comments and code reviews from all reviewers, ensure CI passes"

3. **Completeness**: "Be sure to address every comments and code reviews from all reviewers, ensure CI passes"
4. **Quality**: "Be sure to run and fix all available tests and Linting before pushing your next changes"
5. **Rate Limits**: "If main reviewer says it just became rate-limited, address remaining code reviews then stop there"
6. **Prohibitions**: "Never merge or delete an branch on your own, if you believe the main reviewer said that the PR is ready, just stop and ask for Human review"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There is a typo in this line. "an branch" should be "a branch".

Suggested change
6. **Prohibitions**: "Never merge or delete an branch on your own, if you believe the main reviewer said that the PR is ready, just stop and ask for Human review"
6. **Prohibitions**: "Never merge or delete a branch on your own, if you believe the main reviewer said that the PR is ready, just stop and ask for Human review"

poll_interval=None,
):
"""
Polls until the main reviewer has provided feedback since the given timestamp.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This string has a trailing space, which is likely unintentional. It's best to remove it for code cleanliness.

Suggested change
Polls until the main reviewer has provided feedback since the given timestamp.
"next_step": status_data.get("next_step", "Run 'status' to check for feedback. Be autonomous, don't stop the cycle. you must regularly poll for the main-reviewer's review until it eventually gets posted, then continue")

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the PR review skill and related documentation to enforce stricter autonomous agent behaviors. The changes introduce new rules for freshness, rate limits, and prohibitions in the documentation. The Python skill script is updated to reflect these new rules in its next_step messages and has been auto-formatted for better readability. My review focuses on improving the maintainability of the new instructional strings and correcting some minor typos in the documentation and code. All original comments were kept as they did not contradict any provided rules.

**ABSOLUTE RULE**: Agents must **NEVER** merge a PR, close a PR, or delete a PR's branch.
* **Reason**: Merging, closing, and branch deletion are privileged actions reserved for human maintainers or specific CI/CD pipelines.
* **Action**: If you believe a PR is ready to be merged or should be closed, you must notify the user and await instructions. **Do NOT** execute commands to merge, close, or delete the branch.
* **Action**: If you believe a PR is ready to be merged or should be closed, you must notify the user and await instructions. **Do NOT** execute commands to merge, close, or delete the branch. Never merge or delete an branch on your own, if you believe the main reviewer said that the PR is ready, just stop and ask for Human review.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There's a small grammatical error here. 'an branch' should be 'a branch'.

Suggested change
* **Action**: If you believe a PR is ready to be merged or should be closed, you must notify the user and await instructions. **Do NOT** execute commands to merge, close, or delete the branch. Never merge or delete an branch on your own, if you believe the main reviewer said that the PR is ready, just stop and ask for Human review.
* **Action**: If you believe a PR is ready to be merged or should be closed, you must notify the user and await instructions. **Do NOT** execute commands to merge, close, or delete the branch. Never merge or delete a branch on your own, if you believe the main reviewer said that the PR is ready, just stop and ask for Human review.

3. **Completeness**: "Be sure to address every comments and code reviews from all reviewers, ensure CI passes"
4. **Quality**: "Be sure to run and fix all available tests and Linting before pushing your next changes"
5. **Rate Limits**: "If main reviewer says it just became rate-limited, address remaining code reviews then stop there"
6. **Prohibitions**: "Never merge or delete an branch on your own, if you believe the main reviewer said that the PR is ready, just stop and ask for Human review"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There's a small grammatical error here. 'an branch' should be 'a branch'.

Suggested change
6. **Prohibitions**: "Never merge or delete an branch on your own, if you believe the main reviewer said that the PR is ready, just stop and ask for Human review"
6. **Prohibitions**: "Never merge or delete a branch on your own, if you believe the main reviewer said that the PR is ready, just stop and ask for Human review"

"next_step": status_data.get("next_step", "Run 'status' to check for feedback.")
"next_step": status_data.get(
"next_step",
"Run 'status' to check for feedback. Be autonomous, don't stop the cycle. you must regularly poll for the main-reviewer's review until it eventually gets posted, then continue ",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The default next_step message appears to have a typo. It ends with a space and a comma ( ,) inside the string, which is likely unintentional. This should probably be a period or removed entirely for clarity.

Suggested change
"Run 'status' to check for feedback. Be autonomous, don't stop the cycle. you must regularly poll for the main-reviewer's review until it eventually gets posted, then continue ",
"Run 'status' to check for feedback. Be autonomous, don't stop the cycle. you must regularly poll for the main-reviewer's review until it eventually gets posted, then continue.",

Comment on lines 732 to 754
if has_changes_requested:
next_step = "CRITICAL: Changes requested by reviewer. ANALYZE feedback -> FIX code -> SAFE_PUSH. DO NOT STOP."
next_step = "CRITICAL: Changes requested by reviewer. ANALYZE feedback -> FIX code -> SAFE_PUSH. DO NOT STOP. Pull and merge latest changes from the remote branch before starting addressing code reviews, as bots may since have pushed formatting fixes to your previous changes. Be sure to address every comments and code reviews from all reviewers, ensure CI passes. Be sure to run and fix all available tests and Linting before pushing your next changes. If main reviewer says it just became rate-limited, address remaining code reviews then stop there."
elif has_new_main_reviewer_comments:
next_step = f"New comments from {validation_reviewer} after approval. ANALYZE feedback -> FIX code -> SAFE_PUSH."
next_step = f"New comments from {validation_reviewer} after approval. ANALYZE feedback -> FIX code -> SAFE_PUSH. Pull and merge latest changes from the remote branch before starting addressing code reviews, as bots may since have pushed formatting fixes to your previous changes. Be sure to address every comments and code reviews from all reviewers, ensure CI passes. Be sure to run and fix all available tests and Linting before pushing your next changes. If main reviewer says it just became rate-limited, address remaining code reviews then stop there."
elif main_reviewer_state == "APPROVED":
# Check if there's any OTHER feedback besides the main reviewer's approval
other_feedback = [
item for item in new_feedback
if not (item.get("user") == validation_reviewer
and item.get("type") == "review_summary"
and item.get("state") == "APPROVED")
]
if other_feedback:
next_step = "New feedback received. ANALYZE items -> FIX issues -> SAFE_PUSH. DO NOT STOP."
else:
next_step = "Validation Complete (STOP LOOP - DO NOT MERGE AUTONOMOUSLY). Notify User."
# Check if there's any OTHER feedback besides the main reviewer's approval
other_feedback = [
item
for item in new_feedback
if not (
item.get("user") == validation_reviewer
and item.get("type") == "review_summary"
and item.get("state") == "APPROVED"
)
]
if other_feedback:
next_step = "New feedback received. ANALYZE items -> FIX issues -> SAFE_PUSH. DO NOT STOP. Pull and merge latest changes from the remote branch before starting addressing code reviews, as bots may since have pushed formatting fixes to your previous changes. Be sure to address every comments and code reviews from all reviewers, ensure CI passes. Be sure to run and fix all available tests and Linting before pushing your next changes."
else:
next_step = "Validation Complete (STOP LOOP - DO NOT MERGE AUTONOMOUSLY). Notify User. Never merge or delete an branch on your own, if you believe the main reviewer said that the PR is ready, just stop and ask for Human review"
elif new_feedback:
next_step = "New feedback received. ANALYZE items -> FIX issues -> SAFE_PUSH. DO NOT STOP."
next_step = "New feedback received. ANALYZE items -> FIX issues -> SAFE_PUSH. DO NOT STOP. Pull and merge latest changes from the remote branch before starting addressing code reviews, as bots may since have pushed formatting fixes to your previous changes. Be sure to address every comments and code reviews from all reviewers, ensure CI passes. Be sure to run and fix all available tests and Linting before pushing your next changes."
else:
next_step = f"Waiting for approval from {validation_reviewer} (Current: {main_reviewer_state}). Poll again."
next_step = f"Waiting for approval from {validation_reviewer} (Current: {main_reviewer_state}). Poll again. Be autonomous, don't stop the cycle. you must regularly poll for the main-reviewer's review until it eventually gets posted, then continue"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The long instructional strings are duplicated across several next_step assignments (e.g., lines 733, 735, 748, 752). This duplication makes the code harder to read and maintain. If these instructions need to be updated, the change has to be made in multiple places, which is error-prone.

Consider extracting the common instructional text into a constant and building the final next_step strings dynamically. For example:

ACTION_INSTRUCTIONS = (" Pull and merge latest changes..." 
                       " ensure CI passes..." 
                       " run and fix all tests...")

# ...

if has_changes_requested:
    next_step = f"CRITICAL: Changes requested...{ACTION_INSTRUCTIONS}"
elif new_feedback:
    next_step = f"New feedback received...{ACTION_INSTRUCTIONS}"
# ...

if other_feedback:
next_step = "New feedback received. ANALYZE items -> FIX issues -> SAFE_PUSH. DO NOT STOP. Pull and merge latest changes from the remote branch before starting addressing code reviews, as bots may since have pushed formatting fixes to your previous changes. Be sure to address every comments and code reviews from all reviewers, ensure CI passes. Be sure to run and fix all available tests and Linting before pushing your next changes."
else:
next_step = "Validation Complete (STOP LOOP - DO NOT MERGE AUTONOMOUSLY). Notify User. Never merge or delete an branch on your own, if you believe the main reviewer said that the PR is ready, just stop and ask for Human review"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There's a small grammatical error in this instructional string. 'an branch' should be 'a branch'.

Suggested change
next_step = "Validation Complete (STOP LOOP - DO NOT MERGE AUTONOMOUSLY). Notify User. Never merge or delete an branch on your own, if you believe the main reviewer said that the PR is ready, just stop and ask for Human review"
next_step = "Validation Complete (STOP LOOP - DO NOT MERGE AUTONOMOUSLY). Notify User. Never merge or delete a branch on your own, if you believe the main reviewer said that the PR is ready, just stop and ask for Human review"

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed bb67257 in 24 seconds. Click for details.
  • Reviewed 67 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

Workflow ID: wflow_0FDxWrfi4TF8FKGH

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@sheepdestroyer
Copy link
Owner Author

/gemini review

@sheepdestroyer
Copy link
Owner Author

@coderabbitai review

@qodo-code-review
Copy link

ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan

Persistent review updated to latest commit c9dcad1

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed c9dcad1 in 26 seconds. Click for details.
  • Reviewed 73 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

Workflow ID: wflow_rb4fotV9MmrER5NQ

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@SourceryAI
Copy link

Sorry @sheepdestroyer, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request effectively enhances the PR review skill and its documentation to enforce stricter autonomous agent behaviors. The changes are well-organized and consistently applied across both the Python script and the markdown files. The refactoring in pr_skill.py improves readability, and the new next_step messages clearly define the expected agent actions regarding freshness, rate limits, and operational prohibitions. The documentation updates accurately reflect these new standards. I have identified a couple of minor issues in the documentation files and have provided suggestions for correction. Overall, this is a strong improvement for the agent's review cycle management.

**ABSOLUTE RULE**: Agents must **NEVER** merge a PR, close a PR, or delete a PR's branch.
* **Reason**: Merging, closing, and branch deletion are privileged actions reserved for human maintainers or specific CI/CD pipelines.
* **Action**: If you believe a PR is ready to be merged or should be closed, you must notify the user and await instructions. **Do NOT** execute commands to merge, close, or delete the branch.
* **Action**: If you believe a PR is ready to be merged or should be closed, you must notify the user and await instructions. **Do NOT** execute commands to merge, close, or delete the branch. Never merge or delete an branch on your own, if you believe the main reviewer said that the PR is ready, just stop and ask for Human review.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There is a minor typo in the action description. "an branch" should be corrected to "a branch" for grammatical accuracy.

Suggested change
* **Action**: If you believe a PR is ready to be merged or should be closed, you must notify the user and await instructions. **Do NOT** execute commands to merge, close, or delete the branch. Never merge or delete an branch on your own, if you believe the main reviewer said that the PR is ready, just stop and ask for Human review.
* **Action**: If you believe a PR is ready to be merged or should be closed, you must notify the user and await instructions. **Do NOT** execute commands to merge, close, or delete the branch. Never merge or delete a branch on your own, if you believe the main reviewer said that the PR is ready, just stop and ask for Human review.

3. **Completeness**: "Be sure to address every comments and code reviews from all reviewers, ensure CI passes"
4. **Quality**: "Be sure to run and fix all available tests and Linting before pushing your next changes"
5. **Rate Limits**: "If main reviewer says it just became rate-limited, address remaining code reviews then stop there"
6. **Prohibitions**: \"Never merge or delete a branch on your own, if you believe the main reviewer said that the PR is ready, just stop and ask for Human review\"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The backslashes (\) used to escape the double quotes are unnecessary in this Markdown context and can be removed to improve readability.

Suggested change
6. **Prohibitions**: \"Never merge or delete a branch on your own, if you believe the main reviewer said that the PR is ready, just stop and ask for Human review\"
6. **Prohibitions**: "Never merge or delete a branch on your own, if you believe the main reviewer said that the PR is ready, just stop and ask for Human review"

@sheepdestroyer
Copy link
Owner Author

/gemini review

@sheepdestroyer
Copy link
Owner Author

@coderabbitai review

@gemini-code-assist
Copy link

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@sheepdestroyer
Copy link
Owner Author

@sourcery-ai review

@sheepdestroyer
Copy link
Owner Author

/review

@sheepdestroyer
Copy link
Owner Author

@ellipsis review this

@sourcery-ai
Copy link

sourcery-ai bot commented Jan 30, 2026

Sorry @sheepdestroyer, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 Fix all issues with AI agents
In @.agent/rules/pr-standards.md:
- Line 37: Update pr-standards.md to clarify that the referenced path
"skills/pr_review/pr_skill.py" is the local-repo path for development, and add a
brief note that system-wide installations use
"~/.gemini/antigravity/skills/pr_review/pr_skill.py" and agent-local copies use
".agent/skills/pr_review/pr_skill.py"; follow the dual-path wording pattern used
in knowledge-base.md (or reference it) so readers know when to use each variant
and where to find the alternate path.

In @.agent/rules/rules.md:
- Around line 3-8: The Compliance section contains an incomplete sentence
starting with "enforces the **Standards & Rules**..."—update that sentence to
include a clear subject (e.g., "This document enforces the **Standards & Rules**
defined in `~/.gemini/rules/pr-standards.md`.") so the sentence is grammatical
and explicit; adjust the paragraph under the "## 1. Compliance" heading (the
line that currently begins "enforces the **Standards & Rules**...") and keep the
rest of the bullets (`pr_skill.py` references and prohibitions) unchanged.

In @.agent/skills/pr_review/pr_skill.py:
- Line 765: The string assigned to next_step in the PR review skill contains a
grammar mistake: the second sentence starts with a lowercase "you"; update the
literal in the assignment (the next_step variable in
.agent/skills/pr_review/pr_skill.py) to capitalize "You" so the sentence reads
"...Poll again. Be autonomous, don't stop the cycle. You must regularly poll..."
ensuring only the capitalization change to the existing message.
- Around line 54-63: Fix grammar in the instruction constants: update
ACTION_INSTRUCTIONS to say "address every comment" (not "comments") and change
"as bots may since have pushed" to "as bots may have since pushed", and tidy
other awkward phrasing such as removing the duplicated "Be sure to" or rewording
"before starting addressing code reviews" to "before starting to address code
reviews"; also adjust RATE_LIMIT_INSTRUCTION spacing/phrasing if needed to read
naturally. Target the ACTION_INSTRUCTIONS and RATE_LIMIT_INSTRUCTION constants
in .agent/skills/pr_review/pr_skill.py when making these edits.
- Around line 340-341: Remove the duplicated comment "# Separately check
upstream" that appears twice in a row in .agent/skills/pr_review/pr_skill.py
(locate the consecutive duplicate comments near the existing check for upstream
logic inside the surrounding function or method where that comment is used) so
only a single instance of the comment remains; ensure no other nearby comments
or logic are altered.

In @.agent/skills/pr_review/SKILL.md:
- Around line 31-38: The "Prohibitions" line in SKILL.md contains escaped double
quotes (`\"Never merge or delete a branch on your own, if you believe the main
reviewer said that the PR is ready, just stop and ask for Human review\"`) which
are unnecessary in Markdown; edit the "Prohibitions" rule to remove the
backslashes so the text reads with normal quotes (or no quotes) around the
sentence, ensuring the symbol "Prohibitions" and its quoted rule content are
updated accordingly and the Markdown renders correctly.
🧹 Nitpick comments (2)
.agent/skills/pr_review/SKILL.md (1)

48-49: Consider using a placeholder for path consistency.

The hardcoded path ~/.gemini/antigravity/skills/pr_review/pr_skill.py differs from the {PATH_TO_SKILLS} placeholder approach used in pr-review-cycle.md. Consider aligning the documentation to use a consistent path resolution strategy across all files.

.agent/skills/pr_review/pr_skill.py (1)

761-761: Consider consistent grammar in next_step messages.

This message has the same grammar pattern as the constants. For consistency, consider using the ACTION_INSTRUCTIONS constant or applying similar grammar fixes.

Also note: The message ending lacks proper punctuation after "Human review".


## 5. Tool Usage
* **Primary Tool**: Use `.agent/skills/pr_review/pr_skill.py` for triggering, status checks, and safe pushing.
* **Primary Tool**: Use `skills/pr_review/pr_skill.py` for triggering, status checks, and safe pushing.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for different path references to pr_skill.py across documentation
rg -n "pr_skill.py" --type=md

Repository: sheepdestroyer/agent-tools

Length of output: 2666


Clarify path references across documentation to distinguish local vs. system-wide deployments.

The path in pr-standards.md (skills/pr_review/pr_skill.py) differs from SKILL.md (~/.gemini/antigravity/skills/pr_review/pr_skill.py), and agent-tools.md uses yet another format (.agent/skills/pr_review/pr_skill.py). These represent different deployment contexts (local repository vs. system-wide installation), but this distinction isn't clearly documented. Add a note in pr-standards.md clarifying whether this path is for local development or reference the dual-path pattern from knowledge-base.md.

🤖 Prompt for AI Agents
In @.agent/rules/pr-standards.md at line 37, Update pr-standards.md to clarify
that the referenced path "skills/pr_review/pr_skill.py" is the local-repo path
for development, and add a brief note that system-wide installations use
"~/.gemini/antigravity/skills/pr_review/pr_skill.py" and agent-local copies use
".agent/skills/pr_review/pr_skill.py"; follow the dual-path wording pattern used
in knowledge-base.md (or reference it) so readers know when to use each variant
and where to find the alternate path.

else:
next_step = f"Waiting for approval from {validation_reviewer} (Current: {main_reviewer_state}). Poll again."
next_step = f"Waiting for approval from {validation_reviewer} (Current: {main_reviewer_state}). Poll again. Be autonomous, don't stop the cycle. you must regularly poll for the main-reviewer's review until it eventually gets posted, then continue"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Grammar: "you" should be capitalized at sentence start.

The message "Be autonomous, don't stop the cycle. you must regularly poll..." should capitalize "You" at the start of the second sentence.

📝 Proposed fix
-                next_step = f"Waiting for approval from {validation_reviewer} (Current: {main_reviewer_state}). Poll again. Be autonomous, don't stop the cycle. you must regularly poll for the main-reviewer's review until it eventually gets posted, then continue"
+                next_step = f"Waiting for approval from {validation_reviewer} (Current: {main_reviewer_state}). Poll again. Be autonomous, don't stop the cycle. You must regularly poll for the main-reviewer's review until it eventually gets posted, then continue."
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
next_step = f"Waiting for approval from {validation_reviewer} (Current: {main_reviewer_state}). Poll again. Be autonomous, don't stop the cycle. you must regularly poll for the main-reviewer's review until it eventually gets posted, then continue"
next_step = f"Waiting for approval from {validation_reviewer} (Current: {main_reviewer_state}). Poll again. Be autonomous, don't stop the cycle. You must regularly poll for the main-reviewer's review until it eventually gets posted, then continue."
🤖 Prompt for AI Agents
In @.agent/skills/pr_review/pr_skill.py at line 765, The string assigned to
next_step in the PR review skill contains a grammar mistake: the second sentence
starts with a lowercase "you"; update the literal in the assignment (the
next_step variable in .agent/skills/pr_review/pr_skill.py) to capitalize "You"
so the sentence reads "...Poll again. Be autonomous, don't stop the cycle. You
must regularly poll..." ensuring only the capitalization change to the existing
message.

@coderabbitai
Copy link

coderabbitai bot commented Jan 30, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed be41ea7 in 28 seconds. Click for details.
  • Reviewed 23 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

Workflow ID: wflow_i1yPgy4K4QWfyk6l

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@qodo-code-review
Copy link

ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan

Persistent review updated to latest commit be41ea7

@SourceryAI
Copy link

Sorry @sheepdestroyer, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed 60c69b6 in 10 seconds. Click for details.
  • Reviewed 41 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

Workflow ID: wflow_rx4pw0Spa2OnloNv

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@sheepdestroyer
Copy link
Owner Author

/gemini review

@sheepdestroyer
Copy link
Owner Author

@coderabbitai review

@gemini-code-assist
Copy link

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@sheepdestroyer
Copy link
Owner Author

@sourcery-ai review

@sheepdestroyer
Copy link
Owner Author

/review

@sheepdestroyer
Copy link
Owner Author

@ellipsis review this

@sourcery-ai
Copy link

sourcery-ai bot commented Jan 30, 2026

Sorry @sheepdestroyer, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@coderabbitai
Copy link

coderabbitai bot commented Jan 30, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@qodo-code-review
Copy link

ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan

Persistent review updated to latest commit 60c69b6

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed 52fa107 in 10 seconds. Click for details.
  • Reviewed 26 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

Workflow ID: wflow_fqXXn6nKf7hlVEuu

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants