fix: $global scanner config correctly inherited by per-repo entries#76
Closed
Valyrian-Code wants to merge 4 commits into
Closed
fix: $global scanner config correctly inherited by per-repo entries#76Valyrian-Code wants to merge 4 commits into
Valyrian-Code wants to merge 4 commits into
Conversation
semgrep, trufflehog, claude, and piAgent blocks were merged from DEFAULT_CONFIG + per-repo overrides only — globalConfig was extracted but never spread into the scanner merges. trigger and comment handled this correctly; scanner blocks did not. Adds two tests covering global inheritance and per-repo override precedence for semgrep.
|
|
Author
|
Hey @julio-rocketchat |
scanBatchWithPrompt used max_tokens: 1024 but never checked
stop_reason on the response. When the API hit the token limit,
extractFindings found no tool_use block and returned { findings: [] }
— identical to a clean empty result. No error was logged, errorCount
stayed at zero, and findings were silently dropped.
Added stop_reason check before extractFindings: a max_tokens response
now returns { error: true } and logs an error, so the batch is counted
as failed and the caller knows findings may be incomplete.
…emoveOnException KNOWN_GLOBAL_KEYS was missing semgrep, trufflehog, claude, and piAgent, so using \$global.semgrep (functional since the config-inheritance fix) caused validate-config to report an unknown key error despite the config being correct. Also wires their content validators inside validateGlobal. validateLabels was also missing removeOnException from its checked key list even though the worker reads and applies it for exception label cleanup.
…unconditional
Two independent fixes:
1. Semgrep adapter only captured start.line and left startLine/endLine
unset, collapsing every multi-line finding to a single annotation
line. Added end?: {line} to SemgrepRawResult and populate startLine
and endLine in toFinding, falling back to startLine when end is absent.
2. The issue_comment confirmation comment always said "Re-running scan..."
even when no scan was queued (check run was success/neutral/missing).
The suffix is now conditional — only appended when enqueueScan actually
resolves successfully.
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
$globalscanner configs (semgrep,trufflehog,claude,piAgent) were silently ignored.If a repo did not define its own scanner block, the code fell back to
DEFAULT_CONFIGinstead of inheriting from$global, despite the documented merge behavior and examples using$global.semgrep.extraArgs.triggerandcommentalready supported global inheritance correctly. Scanner blocks did not. The issue has existed since the TypeScript migration (23f1b32).Root Cause
globalConfigwas extracted insrc/config.tsbut never included in scanner config merges.Changes
Updated
src/config.tsto includeglobalConfigas the middle merge layer for:semgreptrufflehogclaudepiAgentAdded tests for:
$globalscanner inheritanceTest Plan
npm test— 618 tests passnpm run lint— no warningsnpm run build && npm run validate-config— config validates successfully