diff --git a/.github/workflows/news-realtime-monitor.md b/.github/workflows/news-realtime-monitor.md index f82acefa362..4414258e838 100644 --- a/.github/workflows/news-realtime-monitor.md +++ b/.github/workflows/news-realtime-monitor.md @@ -129,13 +129,25 @@ START_TIME=$(date +%s) |-------|---------|--------| | Setup | 0–3 | Date check, `get_sync_status()` warm-up | | Detect | 3–8 | Query MCP tools for today's activity | -| Generate | 8–35 | Run `generate-news-enhanced.ts` script (handles all 14 languages) | -| Validate | 35–38 | Run `validate-news-generation.sh` | -| Commit+PR | 38–43 | `git add && git commit`, then `safeoutputs___create_pull_request` | +| Generate | 8–30 | Run `generate-news-enhanced.ts` script (core languages by default; supports all 14 languages via `languages=all`) | +| Validate | 30–35 | Run `validate-news-generation.sh` | +| Commit+PR | 35–40 | `git add && git commit`, then `safeoutputs___create_pull_request` | -**Hard cutoffs** — check elapsed time before each phase: -- `>= 38 min` → Stop generating, commit what you have, create PR immediately -- `>= 43 min` → STOP ALL WORK, call safe output immediately +**Hard cutoffs** — check elapsed time before EVERY phase: +```bash +# Restore START_TIME if available so this snippet is safe to run standalone +if [ -f /tmp/gh-aw/agent/timing.env ]; then + . /tmp/gh-aw/agent/timing.env +fi +# Fallback: if START_TIME is still unset, initialize it to "now" to avoid huge elapsed times +: "${START_TIME:=$(date +%s)}" + +ELAPSED=$(( $(date +%s) - START_TIME )) +echo "⏱️ Elapsed: $((ELAPSED / 60))m $((ELAPSED % 60))s" +``` +- `>= 35 min` → Stop generating, commit what you have, create PR immediately +- `>= 40 min` → STOP ALL WORK, call safe output tool (`safeoutputs___noop` or `safeoutputs___create_pull_request`) IMMEDIATELY — do NOT run any more bash commands +- **CRITICAL**: If you have not called a safe output tool and time is running out, call `safeoutputs___noop` immediately. Failing to call a safe output tool causes a workflow failure. ## Step 1: Date Validation & MCP Health Check @@ -200,9 +212,11 @@ safeoutputs___noop({ "message": "No significant parliamentary events. Checked: v **🚨 ALWAYS use the TypeScript generation script — it handles MCP queries, HTML templating, all 14 languages, translation, and article quality internally.** ```bash -# Use the languages workflow dispatch parameter (defaults to "all") +# Use the languages workflow dispatch parameter +# For scheduled runs (empty input), default to core languages (en,sv) to stay within time budget. +# Manual dispatch can specify "all" for 14-language generation. LANGUAGES_INPUT="${{ github.event.inputs.languages }}" -[ -z "$LANGUAGES_INPUT" ] && LANGUAGES_INPUT="all" +[ -z "$LANGUAGES_INPUT" ] && LANGUAGES_INPUT="en,sv" case "$LANGUAGES_INPUT" in "nordic") LANG_ARG="en,sv,da,no,fi" ;; @@ -210,23 +224,41 @@ case "$LANGUAGES_INPUT" in "all") LANG_ARG="en,sv,da,no,fi,de,fr,es,nl,ar,he,ja,ko,zh" ;; *) LANG_ARG="$LANGUAGES_INPUT" ;; esac - -# Set up MCP connection and generate (source && npx must run on ONE line to preserve MCP_SERVER_URL) -source scripts/mcp-setup.sh && npx tsx scripts/generate-news-enhanced.ts \ - --types=breaking \ - --languages="$LANG_ARG" \ - --skip-existing -SCRIPT_EXIT=$? -echo "Script exit code: $SCRIPT_EXIT" - -# Check for newly generated files -TODAY="$(date +%Y-%m-%d)" -NEW_ARTICLES="$(git status --porcelain -- news/ | awk '{print $2}' | grep "${TODAY}-" || true)" -if [ -z "$NEW_ARTICLES" ]; then - echo "No new breaking news articles were created." +export LANG_ARG + +# Check elapsed time before starting generation +source /tmp/gh-aw/agent/timing.env 2>/dev/null || true +: "${START_TIME:=$(date +%s)}" +ELAPSED=$(( $(date +%s) - START_TIME )) +if [ "$ELAPSED" -ge 2100 ]; then + echo "⏱️ Time budget exceeded (${ELAPSED}s >= 35min) — skipping generation" + SCRIPT_EXIT=0 + NEW_ARTICLES="" else - echo "Newly generated articles:" - printf '%s\n' "$NEW_ARTICLES" + # Set up MCP connection and generate with 20-minute timeout + # LANG_ARG is exported so the subshell inherits it safely (no command-string interpolation) + timeout 1200 bash -lc 'source scripts/mcp-setup.sh && npx tsx scripts/generate-news-enhanced.ts --types=breaking --languages="$LANG_ARG" --skip-existing' + SCRIPT_EXIT=$? + TIMED_OUT=false + if [ "$SCRIPT_EXIT" -eq 124 ]; then + echo "⚠️ Script timed out after 20 minutes — proceeding with whatever was generated" + TIMED_OUT=true + fi + echo "Script exit code: $SCRIPT_EXIT" + + # Check for newly generated files + TODAY="$(date +%Y-%m-%d)" + NEW_ARTICLES="$(git status --porcelain -- news/ | awk '{print $2}' | grep "${TODAY}-" || true)" + if [ -z "$NEW_ARTICLES" ]; then + echo "No new breaking news articles were created." + else + echo "Newly generated articles:" + printf '%s\n' "$NEW_ARTICLES" + # Treat timeout as soft failure when content was produced + if [ "$TIMED_OUT" = true ]; then + SCRIPT_EXIT=0 + fi + fi fi ``` @@ -266,10 +298,23 @@ If untranslated content found, translate each `/dev/null || true +: "${START_TIME:=$(date +%s)}" +ELAPSED=$(( $(date +%s) - START_TIME )) +if [ "$ELAPSED" -ge 2100 ]; then + echo "⏱️ Time budget exceeded (${ELAPSED}s >= 35min) — skipping validation" + VALIDATION_EXIT=0 +else + timeout 300 bash scripts/validate-news-generation.sh + VALIDATION_EXIT=$? + if [ "$VALIDATION_EXIT" -eq 124 ]; then + echo "⚠️ Validation timed out after 5 minutes — proceeding anyway" + VALIDATION_EXIT=0 + fi + if [ "$VALIDATION_EXIT" -ne 0 ]; then + echo "Validation issues found — fix what you can, proceed if time allows" + fi fi ``` @@ -342,6 +387,10 @@ Fix any files flagged before committing. Articles with >3 English phrases in non | Tool not found | MCP server not initialized | Run `source scripts/mcp-setup.sh && echo "MCP_SERVER_URL=${MCP_SERVER_URL}"` — source and npx MUST be chained with `&&` on one line; expected output: `MCP_SERVER_URL=http://host.docker.internal:80/mcp/riksdag-regering` | | Empty results | No significant events detected in monitoring window | Skip generation with `safeoutputs___noop` | | Timeout | MCP server response exceeds `timeout-minutes` | Reduce query scope or increase timeout | +| Script timeout | Generation script exceeds 20-minute limit | Proceed with whatever was generated; the `timeout 1200` wrapper kills the script | | Stale data | `hoursSinceSync > 48` from `get_sync_status()` | Add disclaimer noting data staleness; proceed with cached data | +| Time running out | Elapsed >= 35 minutes | IMMEDIATELY call `safeoutputs___noop` or `safeoutputs___create_pull_request` — do NOT start new work | + +⚠️ **CRITICAL SAFETY NET**: Before EVERY bash block and EVERY tool call, mentally check: "Am I running out of time?" If more than 35 minutes have elapsed since workflow start, stop all work and call a safe output tool IMMEDIATELY. 🎯 **Now begin: Check date, warm up MCP with `get_sync_status()`, detect events, generate articles with the script, and call a safe output tool.** diff --git a/scripts/generate-news-enhanced/index.ts b/scripts/generate-news-enhanced/index.ts index 38d62c625c9..b8e8c36f234 100644 --- a/scripts/generate-news-enhanced/index.ts +++ b/scripts/generate-news-enhanced/index.ts @@ -150,7 +150,7 @@ export async function generateNews(): Promise { console.log(` 📰 Lead story: "${topTitle}"`); - await generateBreakingNews({ + const breakingResult = await generateBreakingNews({ languages, eventContext: topTitle, eventData: { @@ -160,8 +160,16 @@ export async function generateNews(): Promise { }, writeArticle: translatingWriteArticle, }); + if (!breakingResult.success) { + console.error('❌ Breaking news generation failed:', breakingResult.error ?? 'unknown error'); + stats.errors++; + } } catch (err: unknown) { - console.error('❌ Error generating breaking news:', (err as Error).message); + console.error( + '❌ Error generating breaking news:', + err instanceof Error ? err.message : String(err), + ); + stats.errors++; } break; } diff --git a/tests/generate-news-breaking-errors.test.ts b/tests/generate-news-breaking-errors.test.ts new file mode 100644 index 00000000000..ce177a52205 --- /dev/null +++ b/tests/generate-news-breaking-errors.test.ts @@ -0,0 +1,167 @@ +/** + * Tests that generateNews() correctly increments stats.errors when the + * 'breaking' article type fails — either via a returned {success: false} + * or a thrown exception. + * + * This covers the exit-code-critical behavior: runCli() uses stats.errors > 0 + * to decide exit code 1 vs 0, which the agent relies on to detect failures. + */ + +import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; +import type { GenerationStats, GenerationResult } from '../scripts/types/article.js'; + +// --------------------------------------------------------------------------- +// Hoisted mocks — vi.hoisted() runs before vi.mock() calls +// --------------------------------------------------------------------------- + +const { mockGenerateBreakingNews, mockStats, mockGetSharedClient } = vi.hoisted(() => { + const mockGenerateBreakingNews = vi.fn<(...args: unknown[]) => Promise>(); + const mockStats: GenerationStats = { + generated: 0, + errors: 0, + articles: [], + timestamp: new Date().toISOString(), + qualityScores: [] + }; + const mockGetSharedClient = vi.fn().mockResolvedValue({ + fetchVotingRecords: vi.fn().mockResolvedValue([{ doktyp: 'prop', titel: 'Test Prop' }]), + searchDocuments: vi.fn().mockResolvedValue([{ doktyp: 'prop', titel: 'Test Prop' }]) + }); + return { mockGenerateBreakingNews, mockStats, mockGetSharedClient }; +}); + +// Mock fs module at the module level (hoisted before imports) to prevent file writes +vi.mock('fs', async (importOriginal) => { + const original = await importOriginal(); + return { + ...original, + default: { + ...original, + writeFileSync: vi.fn(), + mkdirSync: vi.fn(), + existsSync: vi.fn().mockReturnValue(false), + readdirSync: vi.fn().mockReturnValue([]), + }, + writeFileSync: vi.fn(), + mkdirSync: vi.fn(), + existsSync: vi.fn().mockReturnValue(false), + readdirSync: vi.fn().mockReturnValue([]), + }; +}); + +// Mock the breaking-news module +vi.mock('../scripts/news-types/breaking-news.js', () => ({ + generateBreakingNews: mockGenerateBreakingNews +})); + +// Mock the config module — set articleTypes to ['breaking'] and expose stats +vi.mock('../scripts/generate-news-enhanced/config.js', async (importOriginal) => { + const original = await importOriginal(); + return { + ...original, + articleTypes: ['breaking'], + stats: mockStats, + getSharedClient: mockGetSharedClient, + // Provide stable values for other config exports + languages: ['en'], + allRequestedLanguages: ['en'], + batchSize: undefined, + skipExistingArg: false, + requireMcp: false, + }; +}); + +// Mock MCP client +vi.mock('../scripts/mcp-client.js', () => ({ + MCPClient: vi.fn().mockImplementation(() => ({ + fetchVotingRecords: vi.fn().mockResolvedValue([]), + searchDocuments: vi.fn().mockResolvedValue([]) + })), + getDefaultClient: vi.fn() +})); + +// --------------------------------------------------------------------------- +// Import after mocks +// --------------------------------------------------------------------------- + +interface GenerateNewsModule { + readonly generateNews: () => Promise; +} + +let moduleExports: GenerateNewsModule; + +beforeAll(async () => { + moduleExports = await import('../scripts/generate-news-enhanced/index.js') as unknown as GenerateNewsModule; +}); + +afterAll(() => { + vi.restoreAllMocks(); +}); + +describe('generateNews() — breaking news error tracking', () => { + beforeEach(() => { + // Reset stats before each test + mockStats.errors = 0; + mockStats.generated = 0; + mockStats.articles = []; + mockStats.qualityScores = []; + vi.clearAllMocks(); + + // Re-setup shared client mock after clearAllMocks + mockGetSharedClient.mockResolvedValue({ + fetchVotingRecords: vi.fn().mockResolvedValue([{ doktyp: 'prop', titel: 'Test Prop' }]), + searchDocuments: vi.fn().mockResolvedValue([{ doktyp: 'prop', titel: 'Test Prop' }]) + }); + }); + + it('should increment stats.errors when generateBreakingNews returns success=false', async () => { + mockGenerateBreakingNews.mockResolvedValueOnce({ + success: false, + error: 'MCP server unreachable' + }); + + const result = await moduleExports.generateNews(); + + expect(mockGenerateBreakingNews).toHaveBeenCalled(); + expect(result.errors).toBe(1); + }); + + it('should increment stats.errors when generateBreakingNews throws an exception', async () => { + // Make getSharedClient itself throw so the catch block fires + mockGetSharedClient.mockRejectedValueOnce(new Error('Connection timeout')); + + const result = await moduleExports.generateNews(); + + expect(result.errors).toBe(1); + }); + + it('should NOT increment stats.errors when generateBreakingNews returns success=true', async () => { + mockGenerateBreakingNews.mockResolvedValueOnce({ + success: true, + articles: [{ lang: 'en', html: '

Breaking

', slug: 'test', filename: 'test.html' }] + }); + + const result = await moduleExports.generateNews(); + + expect(mockGenerateBreakingNews).toHaveBeenCalled(); + expect(result.errors).toBe(0); + }); + + it('should log the error message from failed generation result', async () => { + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + mockGenerateBreakingNews.mockResolvedValueOnce({ + success: false, + error: 'Breaking: no significant events today' + }); + + await moduleExports.generateNews(); + + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining('Breaking news generation failed'), + expect.stringContaining('no significant events today') + ); + + consoleSpy.mockRestore(); + }); +}); diff --git a/tests/news-types/breaking-news.test.ts b/tests/news-types/breaking-news.test.ts index dcbebbc4137..597a4469f5a 100644 --- a/tests/news-types/breaking-news.test.ts +++ b/tests/news-types/breaking-news.test.ts @@ -276,4 +276,56 @@ describe('Breaking News Article Generation', () => { expect(svArticle!.html).toContain('Senaste nytt'); }); }); + + describe('Error Handling', () => { + it('should return success=false with error message when event data is null', async () => { + const result = await breakingNewsModule.generateBreakingNews({ + languages: ['en'], + eventData: null + }); + + expect(result.success).toBe(false); + expect(result.error).toBeDefined(); + }); + + it('should return success=false with error message when event data is undefined', async () => { + const result = await breakingNewsModule.generateBreakingNews({ + languages: ['en'] + }); + + expect(result.success).toBe(false); + expect(result.error).toBeDefined(); + expect(result.error).toContain('requires event context'); + }); + + it('should include mcpCalls even on early failure', async () => { + const result = await breakingNewsModule.generateBreakingNews({ + languages: ['en'] + }); + + expect(result.mcpCalls).toBeDefined(); + expect(Array.isArray(result.mcpCalls)).toBe(true); + }); + + it('should handle MCP tool failures gracefully without throwing', async () => { + mockClientInstance.fetchVotingRecords.mockRejectedValueOnce(new Error('MCP timeout')); + mockClientInstance.fetchVotingGroup.mockRejectedValueOnce(new Error('MCP timeout')); + mockClientInstance.searchSpeeches.mockRejectedValueOnce(new Error('MCP timeout')); + mockClientInstance.fetchMPs.mockRejectedValueOnce(new Error('MCP timeout')); + + const eventData: BreakingEventData = { + slug: 'test', + topic: 'test' + }; + + const result = await breakingNewsModule.generateBreakingNews({ + languages: ['en'], + eventData + }); + + // Should still succeed with empty data rather than throwing + expect(result.success).toBe(true); + expect(result.mcpCalls).toBeDefined(); + }); + }); });