Merge branch 'pr-1556' into integration/validation-batch
This commit is contained in:
+10
-1
@@ -138,7 +138,7 @@ export function parseSummary(text: string, sessionId?: number): ParsedSummary |
|
||||
const next_steps = extractField(summaryContent, 'next_steps');
|
||||
const notes = extractField(summaryContent, 'notes'); // Optional
|
||||
|
||||
// NOTE FROM THEDOTMACK: 100% of the time we must SAVE the summary, even if fields are missing. 10/24/2025
|
||||
// NOTE FROM THEDOTMACK: 100% of the time we must SAVE the summary, even if fields are missing. 10/24/2025
|
||||
// NEVER DO THIS NONSENSE AGAIN.
|
||||
|
||||
// Validate required fields are present (notes is optional)
|
||||
@@ -154,6 +154,15 @@ export function parseSummary(text: string, sessionId?: number): ParsedSummary |
|
||||
// return null;
|
||||
// }
|
||||
|
||||
// Guard: if NO sub-tags matched at all, this is a false positive —
|
||||
// <summary> accidentally appeared inside an <observation> response with no structured content.
|
||||
// This is NOT the same as missing some fields (which we intentionally allow above).
|
||||
// Fix for #1360.
|
||||
if (!request && !investigated && !learned && !completed && !next_steps) {
|
||||
logger.warn('PARSER', 'Summary match has no sub-tags — skipping false positive', { sessionId });
|
||||
return null;
|
||||
}
|
||||
|
||||
return {
|
||||
request,
|
||||
investigated,
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
/**
|
||||
* Tests for parseSummary (fix for #1360)
|
||||
*
|
||||
* Validates that false-positive summary matches (no sub-tags) are rejected
|
||||
* while real summaries — even with some missing fields — are still saved.
|
||||
*/
|
||||
import { describe, it, expect } from 'bun:test';
|
||||
import { parseSummary } from '../../src/sdk/parser.js';
|
||||
|
||||
describe('parseSummary', () => {
|
||||
it('returns null when no <summary> tag present', () => {
|
||||
expect(parseSummary('<observation><title>foo</title></observation>')).toBeNull();
|
||||
});
|
||||
|
||||
it('returns null when <summary> has no sub-tags (false positive — fix for #1360)', () => {
|
||||
// This is the bug: observation response accidentally contains <summary>some text</summary>
|
||||
expect(parseSummary('<observation>done <summary>some content here</summary></observation>')).toBeNull();
|
||||
});
|
||||
|
||||
it('returns null for bare <summary> with only plain text, no sub-tags', () => {
|
||||
expect(parseSummary('<summary>This session was productive.</summary>')).toBeNull();
|
||||
});
|
||||
|
||||
it('returns summary when at least one sub-tag is present (respects maintainer note)', () => {
|
||||
const text = `<summary><request>Fix the bug</request></summary>`;
|
||||
const result = parseSummary(text);
|
||||
expect(result).not.toBeNull();
|
||||
expect(result?.request).toBe('Fix the bug');
|
||||
expect(result?.investigated).toBeNull();
|
||||
expect(result?.learned).toBeNull();
|
||||
});
|
||||
|
||||
it('returns full summary when all fields are present', () => {
|
||||
const text = `<summary>
|
||||
<request>Fix login bug</request>
|
||||
<investigated>Auth flow and JWT expiry</investigated>
|
||||
<learned>Token was expiring too soon</learned>
|
||||
<completed>Extended token TTL to 24h</completed>
|
||||
<next_steps>Monitor error rates</next_steps>
|
||||
</summary>`;
|
||||
const result = parseSummary(text);
|
||||
expect(result).not.toBeNull();
|
||||
expect(result?.request).toBe('Fix login bug');
|
||||
expect(result?.investigated).toBe('Auth flow and JWT expiry');
|
||||
expect(result?.learned).toBe('Token was expiring too soon');
|
||||
expect(result?.completed).toBe('Extended token TTL to 24h');
|
||||
expect(result?.next_steps).toBe('Monitor error rates');
|
||||
});
|
||||
|
||||
it('returns null when skip_summary tag is present', () => {
|
||||
expect(parseSummary('<skip_summary reason="no work done"/>')).toBeNull();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user