Skip to content

Commit dca02a7

Browse files
fullsend-ai-coder[bot]ben-alkov
authored andcommitted
fix: address review feedback on PR #2303
- Raise security triage threshold from 30 to 50 files to align with step 2's per-file diff boundary, resolving ambiguity in the 30-49 file range where per-file diffs were not available [edge-case] - Add clarifying note to roster table documenting that security-triage and challenger use non-standard dispatch and are excluded from step 4's parallel loop [logic-error, design-direction] - Clarify step 4 heading to explicitly scope dispatch to dimension sub-agents only [logic-error] - Remove parenthetical from security-triage sub-agent title to match naming convention of other sub-agents [naming-convention] Addresses review feedback on #2303
1 parent 7a8a541 commit dca02a7

2 files changed

Lines changed: 25 additions & 13 deletions

File tree

internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,20 @@ and `description`.
4545
| `style-conventions` | sonnet | parallel | Naming, error handling idioms, API shape, code organization |
4646
| `docs-currency` | sonnet | parallel | Documentation staleness (follows docs-review skill inline) |
4747
| `cross-repo-contracts` | sonnet | parallel | API contract breakage affecting other repos (conditional) |
48-
| `security-triage` | haiku | pre-pass | Classifies files by security relevance for large PRs (≥ 30 files) |
48+
| `security-triage` | haiku | pre-pass | Classifies files by security relevance for large PRs (≥ 50 files) |
4949
| `challenger` | opus | sequential | Adversarial challenge of findings, false-positive removal, deduplication |
5050

5151
The Model column reflects each sub-agent's current frontmatter. Any
5252
value accepted by the Agent tool's `model` parameter is valid in
5353
sub-agent frontmatter.
5454

55+
**Non-standard dispatch types:** `security-triage` (pre-pass) and
56+
`challenger` (sequential) are not dimension sub-agents and are NOT
57+
dispatched in step 4's parallel loop. `security-triage` runs as a
58+
preprocessing classifier in step 3c-1; `challenger` runs as a
59+
post-processing adversarial pass in step 6d. Both produce different
60+
output formats from the standard findings array.
61+
5562
## Findings vs inline comments
5663

5764
Findings are the canonical review output. Each finding records a
@@ -290,17 +297,20 @@ complex PR that triggers all conditions legitimately needs all 6.
290297

291298
#### 3c-1. Security-critical file triage (large PRs)
292299

293-
When `FILE_COUNT` (from step 2) is **30**, run a lightweight triage
300+
When `FILE_COUNT` (from step 2) is **50**, run a lightweight triage
294301
pass to identify security-critical files before preparing context
295-
packages. For PRs under 30 files, skip this step — all files receive
302+
packages. For PRs under 50 files, skip this step — all files receive
296303
uniform attention.
297304

298-
**Why:** On large PRs, security-critical files compete with
299-
boilerplate for the review agent's context window and reasoning
300-
budget. A triage pass ensures files touching auth, permissions,
301-
token handling, trust boundaries, and similar concerns receive
302-
dedicated review context rather than being diluted across dozens of
303-
routine changes. See #2096 for the motivating incident.
305+
**Why:** On large PRs (≥ 50 files, where step 2 already produces
306+
per-file diffs), security-critical files compete with boilerplate for
307+
the review agent's context window and reasoning budget. A triage pass
308+
ensures files touching auth, permissions, token handling, trust
309+
boundaries, and similar concerns receive dedicated review context
310+
rather than being diluted across dozens of routine changes. The
311+
threshold aligns with step 2's per-file diff boundary so that
312+
per-file diff summaries are always available for the triage prompt.
313+
See #2096 for the motivating incident.
304314

305315
**Procedure:**
306316

@@ -383,7 +393,7 @@ For each selected sub-agent, assemble a context package containing:
383393
**Security-prioritized context (large PRs with triage results):**
384394

385395
When step 3c-1 produced a security triage classification (i.e., the PR
386-
has ≥ 30 files and the triage pass succeeded), modify the context
396+
has ≥ 50 files and the triage pass succeeded), modify the context
387397
packages for the `security` and `correctness` sub-agents as follows:
388398

389399
1. **Security sub-agent:** Provide the full per-file diffs for all
@@ -415,13 +425,15 @@ packages for the `security` and `correctness` sub-agents as follows:
415425
Security-critical files: <list with reasons>
416426
```
417427

418-
If step 3c-1 was skipped (PR under 30 files) or the triage sub-agent
428+
If step 3c-1 was skipped (PR under 50 files) or the triage sub-agent
419429
failed (fallback to uniform attention), prepare all context packages
420430
using the standard format described above — no prioritization.
421431

422432
### 4. Dispatch sub-agents
423433

424-
For each selected sub-agent:
434+
For each selected **dimension** sub-agent (from step 3c — excludes
435+
`security-triage` which runs in step 3c-1, and `challenger` which
436+
runs in step 6d):
425437

426438
1. Read the sub-agent definition from `sub-agents/{name}.md`
427439
2. Extract the `model` from frontmatter

internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/security-triage.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ description: Lightweight classifier that identifies security-critical files in l
44
model: haiku
55
---
66

7-
# Security Triage (File Classifier)
7+
# Security Triage
88

99
You are a security triage classifier. Your job is to scan a PR's
1010
changed file list and diff summary to identify **security-critical

0 commit comments

Comments
 (0)