feat(security-agent): warn about disabled Dependabot alerts - #4904
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Executive SummaryAll four previously reported findings are fixed in Overview
Issue Details (click to expand)SUGGESTION
Resolved since the previous review
Files Reviewed (4 files)
Fix these issues in Kilo Cloud Previous Review Summary (commit fcbf765)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit fcbf765)Status: 4 Issues Found | Recommendation: Address before merge Executive SummaryThe new module-level Dependabot availability cache in Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (9 files)
Verified as correct (no finding)
Reviewed by claude-opus-5 · Input: 38 · Output: 10.5K · Cached: 1.1M Review guidance: REVIEW.md from base branch |
| status, | ||
| expiresAtMs: Date.now() + DEPENDABOT_AVAILABILITY_CACHE_TTL_MS, | ||
| }); | ||
| sweepDependabotAvailabilityCache(Date.now()); |
There was a problem hiding this comment.
SUGGESTION: Per-write sweep makes a cold check O(repositories x cache size)
sweepDependabotAvailabilityCache iterates the entire Map (up to DEPENDABOT_AVAILABILITY_CACHE_MAX_ENTRIES = 10,000 entries). Calling it here means one full scan per repository that resolves to enabled/disabled, on top of the scan already performed at the start of checkDependabotAlertsAvailability (line 207). For an installation with several hundred repositories on a cold cache this is hundreds of full-map scans per request, and the client now refetches getRepositories on the default 1-minute staleTime.
The request-start sweep already handles TTL eviction, so this call only needs to enforce the size cap; an unconditional while (size > MAX) trim (or sweeping once after Promise.all resolves) gets the same bound without the repeated O(n) expiry scan.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| expect(mockListAlertsForRepo).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('isolates malformed repository names instead of failing the whole check', async () => { |
There was a problem hiding this comment.
SUGGESTION: This test does not cover the new try/catch it appears to target
With fullName: undefined, dependabotAvailabilityCacheKey returns null, so checkRepositoryDependabotAlertsAvailability returns 'unknown' at the guard (dependabot-api.ts:282) and never reaches fetchRepositoryDependabotAlertsAvailability. Nothing throws, so the new catch block that maps an unexpected rejection to 'unknown' (dependabot-api.ts:249) stays uncovered.
Adding a case where the per-repository check rejects unexpectedly (for example mockListAlertsForRepo.mockImplementationOnce(() => { throw new Error('boom'); }) with a well-formed name, or an Octokit constructor that throws) would exercise the isolation path this commit adds.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
2 participants
Summary
Warn users when selected repositories cannot supply Security Agent findings because Dependabot alerts are disabled.
Why this change is needed
Security Agent relies on GitHub Dependabot alerts. Repositories without alerts enabled can currently be selected without any indication that findings cannot be imported, which can give users a false sense of coverage.
How this is addressed
Verification
Reviewer Notes
Human Reviewer Flags
Code Reviewer Agent
Code Reviewer Notes
enabled,disabled, orunknownDependabot status.