fix: pre-compute review artifacts and deduplicate into shared module, remove security review from workflow

[varin-nair-factory]

No description provided.

[factory-droid]

.

[factory-droid]

.

[factory-droid]

Code review completed

Summary

This PR refactors review artifact computation by extracting shared functionality into a dedicated module (review-artifacts.ts) and removes the parallel security review job from the workflow, simplifying the overall review process.

Key Changes

• Added new src/github/data/review-artifacts.ts module that centralizes PR diff computation, comment fetching, and description storage
• Removed security review job from .github/workflows/droid-review.yml, making code review the sole review mechanism
• Updated review-validator.ts and review.ts to use the new shared computeReviewArtifacts function
• Added fallback mechanism for PR diff computation using gh pr diff when git merge-base fails
• Cleaned up workflow configuration to no longer reference security review results

Important Files Changed

• src/github/data/review-artifacts.ts - New module for computing and storing PR artifacts (diff, comments, description)
• src/tag/commands/review-validator.ts - Refactored to use shared artifact computation
• src/tag/commands/review.ts - Refactored to use shared artifact computation
• .github/workflows/droid-review.yml - Removed security review job and simplified workflow
• test/github/data/review-artifacts.test.ts - New comprehensive tests for the artifacts module

Code Review

Type	Count
🐛 Bugs	0
⚠️ Issues	0
💡 Suggestions	0

Security Review

Severity	Count
🚨 CRITICAL	0
🔴 HIGH	0
🟡 MEDIUM	0
🟢 LOW	0

Findings

No issues found during review.

[View workflow run](https://github.com/Factory-AI/droid-action/actions/runs/${{ github.run_id }})

[factory-droid]

[P2] Avoid shell-interpolating branch names in execSync

baseRef is interpolated directly into execSync("git ...") command strings, which are executed via a shell. Even if baseRef usually comes from GitHub metadata, treating it as untrusted input is safer; branch/ref names can contain shell metacharacters in some setups. Prefer execFileSync/spawnSync with an args array (no shell), or validate baseRef with git check-ref-format (and/or reject anything outside an allowlist like [A-Za-z0-9._/-]).

[factory-droid]

[P2] Existing comments fetching is capped at 100 without pagination

listComments / listReviewComments are called with per_page: 100 but without pagination, so PRs with >100 comments will silently drop older comments. That can break deduplication and cause Droid to re-suggest issues that were already discussed. Use octokit.paginate(...) (or manual paging) to fetch the full comment set.

[factory-droid]

[P1] Uploading ~/.factory/** as an artifact can leak sensitive data

This action uploads the entire ~/.factory/** directory whenever Droid runs. Factory sessions/logs frequently contain full prompts, tool outputs, and configuration that can include tokens or other sensitive data; uploading everything increases the risk of accidental secret disclosure and can also create very large artifacts. Consider gating this behind an explicit debug input and/or limiting the artifact to a small, known-safe allowlist (and excluding sessions by default).

[factory-droid]

This PR makes the review flow more robust by precomputing review artifacts and adding a validator-driven candidate generation path. Three issues were identified: (1) uploading all of ~/.factory/** as an artifact by default poses data leakage risk, (2) shell command injection risk from interpolating baseRef in git commands, and (3) missing pagination for comments causing incomplete data on busy PRs.