fix(bootstrap): resolve 9 bootstrap defects with 50+ regression tests #20
There are no files selected for viewing
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -59,6 +59,17 @@ func NewFindingWithEvidence( | |||||||||||||||||||||||||||||||||||||
| metadata map[string]any, | ||||||||||||||||||||||||||||||||||||||
| ) Finding { | ||||||||||||||||||||||||||||||||||||||
| confidenceScore, confidenceLabel := ParseConfidence(confidence) | ||||||||||||||||||||||||||||||||||||||
| convertedEvidence := ConvertEvidence(evidence) | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // Gate 3: Findings without verifiable evidence (non-empty FilePath) start as | ||||||||||||||||||||||||||||||||||||||
| // "skipped" rather than "pending" to prevent hallucinated findings from being | ||||||||||||||||||||||||||||||||||||||
| // auto-linked into the knowledge graph. | ||||||||||||||||||||||||||||||||||||||
| verificationStatus := VerificationStatusPending | ||||||||||||||||||||||||||||||||||||||
| tempFinding := Finding{Evidence: convertedEvidence} | ||||||||||||||||||||||||||||||||||||||
| if !tempFinding.HasEvidence() { | ||||||||||||||||||||||||||||||||||||||
| verificationStatus = VerificationStatusSkipped | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+62
to
+71
There was a problem hiding this comment. Gate 3 bypass: empty-FilePath evidence slips through as Pending The gate check at line 67 uses evidence[i] = Evidence(e) // empty FilePath preserved as-isThis creates a divergence with
This allows hallucinated findings with empty file paths to be auto-linked into the knowledge graph, defeating the Gate 3 enforcement intended to prevent exactly this. Fix — align the gate with
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: internal/agents/core/parsers.go
Line: 62-69
Comment:
**Gate 3 bypass: empty-FilePath evidence slips through as Pending**
The gate check at line 67 uses `len(convertedEvidence) == 0` as the sole criterion for flagging as `Skipped`. However, `ConvertEvidence()` (line 26–34) performs a direct struct copy without filtering:
```go
evidence[i] = Evidence(e) // empty FilePath preserved as-is
```
This creates a divergence with `HasEvidence()` in types.go (line 65–71), which explicitly requires `FilePath != ""`. An LLM response with `[{"file_path": "", "snippet": "..."}]` will:
- Pass the gate: `len(convertedEvidence) == 1` → `VerificationStatus = Pending`
- Fail `HasEvidence()`: empty `FilePath` → returns `false`
This allows hallucinated findings with empty file paths to be auto-linked into the knowledge graph, defeating the Gate 3 enforcement intended to prevent exactly this.
Fix — align the gate with `HasEvidence()` by checking that evidence entries have non-empty FilePath:
```suggestion
convertedEvidence := ConvertEvidence(evidence)
// Gate 3: Findings without verifiable evidence (non-empty FilePath) start as
// "skipped" rather than "pending" to prevent hallucinated findings from being
// auto-linked into the knowledge graph.
verificationStatus := VerificationStatusPending
tempFinding := Finding{Evidence: convertedEvidence}
if !tempFinding.HasEvidence() {
verificationStatus = VerificationStatusSkipped
}
```
How can I resolve this? If you propose a fix, please make it concise.
Owner
Author
There was a problem hiding this comment. Fixed in 8255ebe — gate now uses HasEvidence() which checks for non-empty FilePath. |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| return Finding{ | ||||||||||||||||||||||||||||||||||||||
| Type: findingType, | ||||||||||||||||||||||||||||||||||||||
| Title: title, | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -67,8 +78,8 @@ func NewFindingWithEvidence( | |||||||||||||||||||||||||||||||||||||
| Tradeoffs: tradeoffs, | ||||||||||||||||||||||||||||||||||||||
| ConfidenceScore: confidenceScore, | ||||||||||||||||||||||||||||||||||||||
| Confidence: confidenceLabel, | ||||||||||||||||||||||||||||||||||||||
| Evidence: convertedEvidence, | ||||||||||||||||||||||||||||||||||||||
| VerificationStatus: verificationStatus, | ||||||||||||||||||||||||||||||||||||||
| SourceAgent: sourceAgent, | ||||||||||||||||||||||||||||||||||||||
| Metadata: metadata, | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add
timeout-minutesto prevent CI hangs on stuck testsThe
bootstrap-integrationjob runs 50+ tests with filesystem and SQLite operations but has no job-leveltimeout-minutesguard. If any test deadlocks or blocks indefinitely, the job will consume runner time until GitHub's 6-hour timeout, blocking other CI runs and wasting minutes.The
-failfastflag in the test command (line 58) handles fast failure within the binary, but cannot interrupt a process that has already hung.Prompt To Fix With AI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 8255ebe — added timeout-minutes: 10.