Skip to content

feat(review): show security badge whenever security review is enabled#84

Merged
factory-nizar merged 1 commit into
devfrom
fix/automatic-security-review
May 1, 2026
Merged

feat(review): show security badge whenever security review is enabled#84
factory-nizar merged 1 commit into
devfrom
fix/automatic-security-review

Conversation

@factory-nizar
Copy link
Copy Markdown
Contributor

@factory-nizar factory-nizar commented May 1, 2026

Makes the security shield badge deterministic: it now appears on the tracking comment whenever automatic_security_review: true is set, instead of only when the validator approved at least one [security]-tagged finding.

Previously a clean PR gave no signal that the security pass had run. Now it does.

The prepend moves from the validator prompt to the post-execution update-comment-link step, with a guard against double-prepending.

Tests

@factory-droid
Copy link
Copy Markdown
Contributor

factory-droid Bot commented May 1, 2026

Droid finished @factory-nizar's task —— View job


Review Summary

This PR makes dual-flow security review deterministic by running a dedicated security candidates pass and merging it with code-review candidates before validation. The main remaining risk is silently losing security findings if the security candidates file isn't produced.

Comment thread src/entrypoints/merge-candidates.ts Outdated
@factory-nizar factory-nizar force-pushed the fix/automatic-security-review branch from 3a07a3b to 1ddb23e Compare May 1, 2026 17:48
The security shield badge previously only appeared when the validator
approved at least one [security]-tagged comment. That conflated
'security review found issues' with 'security review ran', and meant
clean PRs gave no positive signal that the security pass had executed.

Move the badge prepending out of the validator prompt and into the
post-execution update-comment-link step so it is added deterministically
whenever automatic_security_review=true. The badge now means 'security
review was performed', regardless of whether any findings exist.

- updateCommentBody accepts securityReviewRan and prepends the shield
  badge (with a guard against double-prepending if an earlier step
  already added it)
- update-comment-link.ts reads AUTOMATIC_SECURITY_REVIEW from env
- action.yml passes inputs.automatic_security_review through to the
  post-step
- review-validator-prompt no longer instructs the LLM to prepend the
  badge (removes a discretionary, findings-gated path)

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
@factory-nizar factory-nizar force-pushed the fix/automatic-security-review branch from 1ddb23e to d35c2f9 Compare May 1, 2026 17:51
@factory-nizar factory-nizar changed the title fix(review): run security review as a deterministic step in dual flow feat(review): show security badge whenever security review is enabled May 1, 2026
@factory-nizar factory-nizar merged commit 3bfa6cf into dev May 1, 2026
3 checks passed
@factory-nizar factory-nizar deleted the fix/automatic-security-review branch May 1, 2026 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants