feat: add parallel workflow for code and security reviews #19
Conversation
|
Droid finished @shashank-factory's task —— View job |
![factory-droid[bot]](https://avatars.githubusercontent.com/in/358017?s=60&v=4){kind=small}
| contents: write | ||
| pull-requests: write | ||
| issues: write | ||
| id-token: write |
There was a problem hiding this comment.
[P1] Run combine when either review ran (avoid skipping summary update)
combine currently runs only if both run_code_review and run_security_review are true, so if security is skipped by “run once” behavior (or code review is disabled), the tracking comment may never get a final combined/status summary update.
| // Check if security review already exists on this PR (run once behavior) | ||
| const hasExisting = await hasExistingSecurityReview(octokit, context); | ||
| if (hasExisting) { | ||
| console.log("Security review already exists on this PR, skipping"); |
There was a problem hiding this comment.
[P1] Set run_security_review=false when skipping existing security review
When automaticSecurityReview finds an existing security review, it returns skipped: true but never calls core.setOutput("run_security_review", "false"); downstream prepare/action.yml falls back to the input flag (true) and will still run the security job unnecessarily.
| try { | ||
| const githubToken = process.env.GITHUB_TOKEN!; | ||
| const reviewType = process.env.REVIEW_TYPE || "code"; | ||
| const commentId = parseInt(process.env.DROID_COMMENT_ID || "0"); |
There was a problem hiding this comment.
[P1] Validate DROID_COMMENT_ID to avoid updating comment 0
This parses DROID_COMMENT_ID but never asserts it’s non-zero; if it’s missing/miswired, downstream steps will try to operate with comment_id=0 and fail in harder-to-debug places (the combine entrypoint already guards this).
shashank-factory
force-pushed
the
ssharma/pr3-review-json-output
branch
from
394437b to
6d909b9
Compare
Implement parallel workflow architecture that runs code review and security review simultaneously, then combines results. New Composite Actions: - prepare/ - Initialize review, create tracking comment, detect modes - review/ - Standalone code review action - security/ - Standalone security review action - combine/ - Combine results and post inline comments New Features: - Parallel execution of code and security reviews - Combined summary with deduplication - Install security skills step in main action - PR branch checkout for full file access Workflow Changes: - .github/workflows/droid-review.yml now uses multi-job parallel workflow - @droid review security triggers both reviews - run_code_review and run_security_review output flags Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Restrict parallel review execution to automatic mode only (via automaticReview + automaticSecurityReview flags). For manual tagging, users must explicitly choose ONE of: - @droid review - Code review only - @droid security - Security review only @droid review security now parses as just @droid review. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
- Fix combine job to run when EITHER review ran (not both) - Set run_security_review=false when skipping existing security review - Validate DROID_COMMENT_ID is non-zero in generate-review-prompt Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
shashank-factory
force-pushed
the
ssharma/pr4-parallel-workflow
branch
from
4c7a37b to
22898eb
Compare
| // Set output for next steps | ||
| const githubOutput = process.env.GITHUB_OUTPUT; | ||
| if (githubOutput) { | ||
| appendFileSync(githubOutput, `github_token=${token}\n`); |
There was a problem hiding this comment.
why are we outputting GH token to this file?
| - name: Install Droid CLI | ||
| shell: bash | ||
| run: | | ||
| curl --retry 5 --retry-delay 2 --retry-all-errors -fsSL https://app.factory.ai/cli | sh | ||
| echo "$HOME/.local/bin" >> "$GITHUB_PATH" | ||
| "$HOME/.local/bin/droid" --version |
There was a problem hiding this comment.
Why do review/action.yml, security/action.yml and combine/action.yml all have droid cli installation step? can we combine them?
Consider caching the CLI or moving installation to the prepare step and passing it via artifact/cache.
varin-nair-factory
left a comment
varin-nair-factory
left a comment
There was a problem hiding this comment.
Mostly looks good, just see comments. Did you test this? Does it work?
Summary
Implement parallel workflow architecture that runs code review and security review simultaneously, then combines results.
New Composite Actions
prepare/- Initialize review, create tracking comment, detect modesreview/- Standalone code review actionsecurity/- Standalone security review actioncombine/- Combine results and post inline commentsNew Features
Workflow Changes
.github/workflows/droid-review.ymlnow uses multi-job parallel workflow@droid review securitytriggers both reviewsrun_code_reviewandrun_security_reviewoutput flagsNew Entrypoints
src/entrypoints/get-token.ts- OIDC token helpersrc/entrypoints/generate-review-prompt.ts- Prompt generationsrc/entrypoints/generate-combine-prompt.ts- Combine promptsrc/entrypoints/combine-reviews.ts- Combine logicThis is the final PR in the split from the security review feature branch.
PR Stack