Skip to content

Commit 8ffd7a2

Browse files
Merge dev into main (#92)
* feat(review): show security badge whenever security review is enabled (#84) 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> * fix(review-depth): use kimi-k2.6 for shallow preset (#91) The shallow depth preset pointed at "kimi-k2-0711", which is a stale identifier the public Droid CLI no longer recognizes (verified on a GitLab pipeline: `Invalid model: kimi-k2-0711`). The deep preset ("gpt-5.2") works fine; only shallow was broken. Anyone passing review_depth=shallow today would silently hit this error. Per `droid exec --model <invalid>` the current valid Kimi K2 model IDs are `kimi-k2.6` (newest) and `kimi-k2.5`. Switch the preset to `kimi-k2.6` which preserves the original design intent: a fast, cheap Kimi K2 model for quick shallow reviews. Explicit `review_model` override still wins via `resolveReviewConfig`. Updated: - src/utils/review-depth.ts: SHALLOW_DEFAULTS.model - action.yml + review/action.yml: input descriptions - README.md: presets table + reference in review_model docs All 377 tests pass. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --------- Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
1 parent 8ea31f3 commit 8ffd7a2

8 files changed

Lines changed: 77 additions & 9 deletions

File tree

README.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -230,10 +230,10 @@ jobs:
230230

231231
The `review_depth` input controls which model and reasoning effort are used for code reviews. Two presets are available:
232232

233-
| Depth | Model | Reasoning Effort | Best For |
234-
| ----------- | -------------- | ---------------- | ------------------------------------------------------- |
235-
| **deep** | `gpt-5.2` | `high` | Thorough reviews catching subtle bugs and design issues |
236-
| **shallow** | `kimi-k2-0711` | default | Fast, cost-effective reviews for straightforward PRs |
233+
| Depth | Model | Reasoning Effort | Best For |
234+
| ----------- | ----------- | ---------------- | ------------------------------------------------------- |
235+
| **deep** | `gpt-5.2` | `high` | Thorough reviews catching subtle bugs and design issues |
236+
| **shallow** | `kimi-k2.6` | default | Fast, cost-effective reviews for straightforward PRs |
237237

238238
**Examples:**
239239

@@ -262,7 +262,7 @@ The `review_depth` input controls which model and reasoning effort are used for
262262

263263
> **Tip:** Setting `review_model` or `reasoning_effort` explicitly always takes priority over the depth preset. You can mix and match -- for example, use `review_depth: shallow` but override just `reasoning_effort: high` to get the shallow model with higher reasoning.
264264

265-
The default models (`gpt-5.2` for `deep`, `kimi-k2-0711` for `shallow`) are managed by Factory and may change over time. To pin a specific model regardless of the depth preset, set `review_model` to any model ID supported by `droid exec --model`. A few common choices:
265+
The default models (`gpt-5.2` for `deep`, `kimi-k2.6` for `shallow`) are managed by Factory and may change over time. To pin a specific model regardless of the depth preset, set `review_model` to any model ID supported by `droid exec --model`. A few common choices:
266266

267267
- `claude-opus-4-7`
268268
- `claude-sonnet-4-6`

action.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ inputs:
9292
required: false
9393
default: "7"
9494
review_depth:
95-
description: "Review depth preset: 'shallow' (fast, uses kimi-k2-0711) or 'deep' (thorough, uses gpt-5.2 with high reasoning). Defaults to deep. Setting review_model or reasoning_effort explicitly overrides the preset values."
95+
description: "Review depth preset: 'shallow' (fast, uses kimi-k2.6) or 'deep' (thorough, uses gpt-5.2 with high reasoning). Defaults to deep. Setting review_model or reasoning_effort explicitly overrides the preset values."
9696
required: false
9797
default: "deep"
9898
review_model:
@@ -352,6 +352,7 @@ runs:
352352
USE_STICKY_COMMENT: ${{ inputs.use_sticky_comment }}
353353
TRACK_PROGRESS: ${{ inputs.track_progress }}
354354
AUTOMATIC_REVIEW: ${{ inputs.automatic_review }}
355+
AUTOMATIC_SECURITY_REVIEW: ${{ inputs.automatic_security_review }}
355356

356357
- name: Collect .factory debug files
357358
if: always() && steps.prepare.outputs.contains_trigger == 'true'

review/action.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ inputs:
1313
description: "ID of the tracking comment to update"
1414
required: true
1515
review_depth:
16-
description: "Review depth preset: 'shallow' (fast, uses kimi-k2-0711) or 'deep' (thorough, uses gpt-5.2 with high reasoning). Defaults to deep. Setting review_model or reasoning_effort explicitly overrides the preset values."
16+
description: "Review depth preset: 'shallow' (fast, uses kimi-k2.6) or 'deep' (thorough, uses gpt-5.2 with high reasoning). Defaults to deep. Setting review_model or reasoning_effort explicitly overrides the preset values."
1717
required: false
1818
default: "deep"
1919
review_model:

src/create-prompt/templates/review-validator-prompt.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,6 @@ After writing \`${reviewValidatedPath}\`, post comments ONLY for \`status === "a
126126
* Do **NOT** post comments individually — batch them all into one \`submit_review\` call.
127127
* Do **NOT** include a \`body\` parameter in \`submit_review\`.
128128
* Use \`github_comment___update_droid_comment\` to update the tracking comment with the review summary.
129-
* If any approved comments contain \`[security]\` in their body, prepend a security badge to the tracking comment: \`![Security Review](https://img.shields.io/badge/security%20review-ran-blue)\`. This indicates that security analysis was performed as part of the review.
130129
* Do **NOT** post the summary as a separate comment or as the body of \`submit_review\`.
131130
* Do not approve or request changes.
132131
`;

src/entrypoints/update-comment-link.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ async function run() {
159159
branchName: undefined,
160160
triggerUsername,
161161
errorDetails,
162+
securityReviewRan: process.env.AUTOMATIC_SECURITY_REVIEW === "true",
162163
};
163164

164165
const updatedBody = updateCommentBody(commentInput);

src/github/operations/comment-logic.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,12 @@ export type CommentUpdateInput = {
1616
branchName?: string;
1717
triggerUsername?: string;
1818
errorDetails?: string;
19+
securityReviewRan?: boolean;
1920
};
2021

22+
export const SECURITY_REVIEW_BADGE =
23+
"![Security Review](https://img.shields.io/badge/security%20review-ran-blue)";
24+
2125
export function ensureProperlyEncodedUrl(url: string): string | null {
2226
try {
2327
// First, try to parse the URL to see if it's already properly encoded
@@ -77,6 +81,7 @@ export function updateCommentBody(input: CommentUpdateInput): string {
7781
branchName,
7882
triggerUsername,
7983
errorDetails,
84+
securityReviewRan,
8085
} = input;
8186

8287
// Extract content from the original comment body
@@ -209,6 +214,10 @@ export function updateCommentBody(input: CommentUpdateInput): string {
209214
// Remove any existing duration info at the bottom
210215
bodyContent = bodyContent.replace(/\n*---\n*Duration: [0-9]+m? [0-9]+s/g, "");
211216

217+
if (securityReviewRan && !bodyContent.includes("security%20review-ran")) {
218+
bodyContent = `${SECURITY_REVIEW_BADGE}\n\n${bodyContent}`.trim();
219+
}
220+
212221
// Add the cleaned body content
213222
newBody += bodyContent;
214223

src/utils/review-depth.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ export enum ReviewDepth {
44
}
55

66
const SHALLOW_DEFAULTS = {
7-
model: "kimi-k2-0711",
7+
model: "kimi-k2.6",
88
reasoningEffort: undefined as string | undefined,
99
};
1010

test/comment-logic.test.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,4 +441,62 @@ describe("updateCommentBody", () => {
441441
expect(result).not.toContain("tree/droid/issue-123");
442442
});
443443
});
444+
445+
describe("security review badge", () => {
446+
const SHIELD_URL_FRAGMENT = "security%20review-ran";
447+
448+
it("prepends the security badge when securityReviewRan is true", () => {
449+
const input: CommentUpdateInput = {
450+
...baseInput,
451+
currentBody: "Droid is reviewing code and running a security check…",
452+
executionDetails: { duration_ms: 60000 },
453+
securityReviewRan: true,
454+
};
455+
456+
const result = updateCommentBody(input);
457+
expect(result).toContain(SHIELD_URL_FRAGMENT);
458+
expect(result).toContain(
459+
"![Security Review](https://img.shields.io/badge/security%20review-ran-blue)",
460+
);
461+
});
462+
463+
it("does not add the badge when securityReviewRan is false", () => {
464+
const input: CommentUpdateInput = {
465+
...baseInput,
466+
currentBody: "Droid is working…",
467+
executionDetails: { duration_ms: 60000 },
468+
securityReviewRan: false,
469+
};
470+
471+
const result = updateCommentBody(input);
472+
expect(result).not.toContain(SHIELD_URL_FRAGMENT);
473+
});
474+
475+
it("does not add the badge when securityReviewRan is undefined", () => {
476+
const input: CommentUpdateInput = {
477+
...baseInput,
478+
currentBody: "Droid is working…",
479+
executionDetails: { duration_ms: 60000 },
480+
};
481+
482+
const result = updateCommentBody(input);
483+
expect(result).not.toContain(SHIELD_URL_FRAGMENT);
484+
});
485+
486+
it("does not double-prepend when the badge is already present", () => {
487+
const input: CommentUpdateInput = {
488+
...baseInput,
489+
currentBody:
490+
"Droid is reviewing code and running a security check…\n\n" +
491+
"![Security Review](https://img.shields.io/badge/security%20review-ran-blue)\n\n" +
492+
"Validator approved 2 findings.",
493+
executionDetails: { duration_ms: 60000 },
494+
securityReviewRan: true,
495+
};
496+
497+
const result = updateCommentBody(input);
498+
const occurrences = result.split(SHIELD_URL_FRAGMENT).length - 1;
499+
expect(occurrences).toBe(1);
500+
});
501+
});
444502
});

0 commit comments

Comments
 (0)