Skip to content

Commit 22898eb

Browse files
fix: address P1 issues from code review
- 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>
1 parent 109bc81 commit 22898eb

File tree

3 files changed

+9
-3
lines changed

3 files changed

+9
-3
lines changed

.github/workflows/droid-review.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,11 +94,11 @@ jobs:
9494

9595
combine:
9696
needs: [prepare, code-review, security-review]
97-
# Only run combine when BOTH code review AND security review were executed
97+
# Run combine when EITHER code review OR security review was executed
9898
if: |
9999
always() &&
100-
needs.prepare.outputs.run_code_review == 'true' &&
101-
needs.prepare.outputs.run_security_review == 'true'
100+
(needs.prepare.outputs.run_code_review == 'true' ||
101+
needs.prepare.outputs.run_security_review == 'true')
102102
runs-on: ubuntu-latest
103103
permissions:
104104
contents: write

src/entrypoints/generate-review-prompt.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ async function run() {
2020
const reviewType = process.env.REVIEW_TYPE || "code";
2121
const commentId = parseInt(process.env.DROID_COMMENT_ID || "0");
2222

23+
if (!commentId) {
24+
throw new Error("DROID_COMMENT_ID is required and must be non-zero");
25+
}
26+
2327
const context = parseGitHubContext();
2428

2529
if (!isEntityContext(context)) {

src/tag/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,8 @@ export async function prepareTagExecution({
152152
const hasExisting = await hasExistingSecurityReview(octokit, context);
153153
if (hasExisting) {
154154
console.log("Security review already exists on this PR, skipping");
155+
core.setOutput("run_code_review", "false");
156+
core.setOutput("run_security_review", "false");
155157
return {
156158
skipped: true,
157159
reason: "security_review_exists",

0 commit comments

Comments
 (0)