[MoralCode]

Note

This PR was ported from augurlabs/augur#3745, filed by @mn-ram because the contribution was deemed to still be useful. The below PR contents are lightly modified (to adjust issue and PR references mostly) and has been rebased to account for changes to CollectOSS since the fork (there were effectively no merge conflicts)

Description

Builds on augurlabs/augur#3783 which added the check_prs_enabled() infrastructure and applied it to retrieve_all_pr_data.

This PR applies the same check to collect_pull_request_review_comments, which hits the repos/{owner}/{repo}/pulls/comments endpoint. This endpoint also returns 404 when pull requests are disabled on a repo, and previously had no handling for that case — causing an unhandled exception in the secondary collection phase.

Changes:

• Call check_prs_enabled() early in collect_pull_request_review_comments and return gracefully if PRs are disabled
• Move GithubRandomKeyAuth and GithubDataAccess instantiation before the check
• Migrate URL construction to use GithubDataAccess.endpoint_url() following the pattern from retrieve_all_pr_data

Testing

• Manually add a repo with GitHub pull requests disabled and confirm collect_pull_request_review_comments logs an info message and returns without error

Signed commits

• Yes, I signed my commits.

[drkrillo]

hey, can you rebase into main when you have some time? I would like to review and test if that sounds useful

[MoralCode]

@drkrillo that would be fantastic! This is (hopefully) the resolution to one of our critical fix issues

[MoralCode]

@coderabbitai review

[coderabbitai]

Action performed

Review finished.

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.

[MoralCode]

#306 was the PR this builds on (which handled the fix for the PRs themselves) this applies it to comments, which i think is a valid second-order fix

[coderabbitai]

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

• Bug Fixes
  • Review-comment collection now skips repository lookup and comment retrieval when pull requests are disabled.

The review-comment task initializes GitHub access before checking whether pull requests are enabled. When they are disabled, the task logs a skip message and returns before repository lookup or comment retrieval.

Changes: Review-comment collection gate

Layer / File(s)	Summary
Initialize access and gate collection collectoss/tasks/github/pull_requests/tasks.py	The task initializes GitHub access before checking the pull-request setting. If pull requests are disabled, it logs a skip message and returns before repository lookup or collection. The task reuses the initialized clients.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Merge Risk: Minimal · up to e06e7

The change safely skips review-comment collection when pull requests are unavailable.

Security architecture risk: Low · up to e06e7

The change preserves the existing GitHub identity and request destination, but introduces authenticated activity for invocations that previously returned without collecting anything. No exploitable security regression was established; deployed credential and task-access boundaries remain incompletely verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• observed — Legacy credential loading can include the configured GitHub key and worker OAuth keys selected by platform, without repository filtering. Its Redis cache is instance-prefixed and shared across tasks. These credential-pool and cache scopes are unchanged; the PR broadens when this task touches them.

Trust Boundaries and Controls

• observed — Owner and repository remain derived from the task's repo_git argument. Both the new eligibility request and the existing initial comment request construct URLs under api.github.com. The ordinary scheduler chains review collection before comment collection using the same repository argument; this does not establish deployment-level authorization for direct task submission.

Resilience and Maintainability Implications

• inferred — An invocation without stored reviews can now incur authenticated-request retries and fail the secondary collection, whereas it previously returned before external access. Only the expected 404 becomes a successful skip. A later availability change can still fail comment retrieval, as before; the check is not an atomic guarantee.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title names the main change: PR review comment collection is skipped when pull requests are disabled. It matches the raw summary, which describes an early check_prs_enabled() return in collect_pul…
Description check		The description follows the repository template in part. It has a Description section that explains the change, the 404 problem, and the specific code changes. It has a Testing section with an uncheck…
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title names the main change: PR review comment collection is skipped when pull requests are disabled. It matches the raw summary, which describes an early check_prs_enabled() return in collect_pull_request_review_comments. The 'fix:' prefix follows conventional commit style. The title is 70 characters, which is acceptable.

Full details: Description check

Explanation

The description follows the repository template in part. It has a Description section that explains the change, the 404 problem, and the specific code changes. It has a Testing section with an unchecked manual test. It has a Signed commits section, checked. It does not include the Generative AI disclosure section, and it does not complete the 'This PR fixes #' line with an issue number. The Notes for Reviewers section is empty. The core content is present, so the check passes. The author should add the issue number, the Generative AI disclosure, and any reviewer notes.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Recent review info

Run configuration

• Source mode: Auto
• Repository: chaoss/CollectOSS
• Branch: mn-ram/fix/disabled-pr-review-comments
• File: .coderabbit.yaml
• Commit: e06e720
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 4ba9fd4a-e911-4d9a-bd43-f10057074216

Commits

Reviewing files that changed from the base of the PR and between d52912e and e06e720.

Files selected for processing: 1

• collectoss/tasks/github/pull_requests/tasks.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.