From 83eae64fdf27ec616f590c0e2df11c83d131f062 Mon Sep 17 00:00:00 2001 From: damengrandom Date: Sat, 1 Aug 2026 17:15:44 +1000 Subject: [PATCH] fix(T-31): stop dropping failure alerts, and stop saving blank KPI summaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs found by a full-codebase scan, both confirmed against the live APIs. notifyError built an HTML message but never escaped the error text, and never checked whether Telegram accepted it. Telegram's HTML mode rejects any tag outside its whitelist with a 400, so an error carrying markup lost the alert entirely and left no trace — verified live: a Tavily gateway 502 (HTML body) and a tag from the curator retry path both returned 400 Bad Request: can't parse entities: Unsupported start tag "html" The error is now escaped, and a rejected alert is logged rather than discarded. The raw text is cut before escaping, never after: escaping a cut string is safe, but cutting an escaped one can split an entity ("&qu") and Telegram rejects that too. 600 raw chars stay inside the 4096 limit even at the sixfold worst case. The alert links to LangSmith for the full trace. github_summary was empty on every GitHub-only KPI day. The tool hardcodes summary: '' and GITHUB_PROMPT tells the agent to relay the output verbatim, so nothing ever filled it — confirmed end-to-end against the live GitHub API and LLM: 4 commits and 6 PRs stored with "". The summary is now built from the counts. Payloads that never carried the arrays stay blank, which keeps a broken payload distinguishable from a genuine zero-activity day. Closes #21 Closes #23 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F5CmYPdQcMeLzzJc3iDQ2u --- src/agent/kpi-record.test.ts | 21 +++++++++ src/agent/kpi-record.ts | 16 +++++-- src/agent/notify-error.test.ts | 83 ++++++++++++++++++++++++++++++++++ src/agent/utils.ts | 22 +++++++-- src/constants/index.ts | 1 + 5 files changed, 137 insertions(+), 6 deletions(-) create mode 100644 src/agent/notify-error.test.ts diff --git a/src/agent/kpi-record.test.ts b/src/agent/kpi-record.test.ts index 293e7b5..c98d3ec 100644 --- a/src/agent/kpi-record.test.ts +++ b/src/agent/kpi-record.test.ts @@ -4,6 +4,27 @@ import { toKpiRecord } from './kpi-record.ts' const NOW = '2026-07-31T00:00:00.000Z' +// The real tool hardcodes summary: '' and the agent is told not to touch it, so +// this is what every GitHub-only day actually looks like. +test('describes the activity when the agent supplied no summary', () => { + const record = toKpiRecord(JSON.stringify({ summary: '', commits: [1, 2, 3, 4], pullRequests: [1, 2, 3, 4, 5, 6] }), NOW) + + assert.equal(record.github_summary, '4 commits, 6 PRs on GitHub') +}) + +test('singularises a one-commit, one-PR day', () => { + const record = toKpiRecord(JSON.stringify({ commits: [1], pullRequests: [1] }), NOW) + + assert.equal(record.github_summary, '1 commit, 1 PR on GitHub') +}) + +// A genuine quiet day still reads as a day, not as a missing record. +test('describes a zero-activity day rather than leaving it blank', () => { + const record = toKpiRecord(JSON.stringify({ commits: [], pullRequests: [] }), NOW) + + assert.equal(record.github_summary, '0 commits, 0 PRs on GitHub') +}) + test('maps a complete GitHub payload', () => { const record = toKpiRecord(JSON.stringify({ summary: 'shipped the thing', commits: [1, 2, 3], pullRequests: [1] }), NOW) diff --git a/src/agent/kpi-record.ts b/src/agent/kpi-record.ts index b534be4..f192553 100644 --- a/src/agent/kpi-record.ts +++ b/src/agent/kpi-record.ts @@ -1,13 +1,23 @@ import { parseJson } from './utils.ts' import type { KpiRecord } from '../schemas/index.ts' +const plural = (n: number, word: string) => `${n} ${word}${n === 1 ? '' : 's'}` + +function describeActivity(commits: number, prs: number): string { + return `${plural(commits, 'commit')}, ${plural(prs, 'PR')} on GitHub` +} + export function toKpiRecord(githubOutput: string, now: string): KpiRecord { const data = parseJson<{ summary?: string; commits?: unknown[]; pullRequests?: unknown[] }>(githubOutput, {}) + const isDigest = Array.isArray(data.commits) || Array.isArray(data.pullRequests) + const commits_count = data.commits?.length ?? 0 + const prs_count = data.pullRequests?.length ?? 0 + const github_summary = data.summary?.trim() || (isDigest ? describeActivity(commits_count, prs_count) : '') return { - github_summary: data.summary ?? '', - commits_count: data.commits?.length ?? 0, - prs_count: data.pullRequests?.length ?? 0, + github_summary, + commits_count, + prs_count, activities: [], created_at: now, updated_at: now, diff --git a/src/agent/notify-error.test.ts b/src/agent/notify-error.test.ts new file mode 100644 index 0000000..31500fb --- /dev/null +++ b/src/agent/notify-error.test.ts @@ -0,0 +1,83 @@ +import { test, beforeEach, afterEach } from 'node:test' +import assert from 'node:assert/strict' +import { notifyError } from './utils.ts' + +const realFetch = globalThis.fetch +let sent: { text: string; parse_mode: string } | null = null +let status = 200 + +beforeEach(() => { + process.env.TELEGRAM_BOT_TOKEN = 'test-token' + process.env.TELEGRAM_CHAT_ID = '12345' + sent = null + status = 200 + globalThis.fetch = (async (_url: unknown, init: { body: string }) => { + sent = JSON.parse(init.body) + return new Response(JSON.stringify({ ok: status === 200 }), { status, headers: { 'content-type': 'application/json' } }) + }) as unknown as typeof fetch +}) + +afterEach(() => { + globalThis.fetch = realFetch +}) + +// Telegram's HTML mode rejects any tag outside its whitelist with a 400, and the +// alert is then never delivered. Real errors carry markup: gateway 502 bodies are +// HTML, and reasoning models emit . +test('escapes markup in the error so Telegram can parse the alert', async () => { + await notifyError('AI news search', new Error('Tavily API error 502: upstream connect error')) + + assert.ok(sent) + assert.equal(sent!.parse_mode, 'HTML') + assert.ok(!sent!.text.includes(''), 'raw would 400') + assert.ok(sent!.text.includes('<html>'), 'the error text should survive, escaped') + // The alert's own formatting must stay intact. + assert.ok(sent!.text.includes('Error:')) +}) + +test('escapes markup in the context too', async () => { + await notifyError('