From 6ad5352b87a710c49b644777fb6af92bc817fcba Mon Sep 17 00:00:00 2001 From: Ruaan Deysel Date: Sat, 10 Oct 2026 16:33:29 +1000 Subject: [PATCH 1/6] chore(governance): remove redundant CONTRIBUTIONS.md in favor of GitHub standard CONTRIBUTING.md --- CONTRIBUTIONS.md | 126 ----------------------------------------------- 1 file changed, 126 deletions(-) delete mode 100644 CONTRIBUTIONS.md diff --git a/CONTRIBUTIONS.md b/CONTRIBUTIONS.md deleted file mode 100644 index 318d7a96..00000000 --- a/CONTRIBUTIONS.md +++ /dev/null @@ -1,126 +0,0 @@ -# Contributing to Vault - -Thank you for your interest in contributing to Vault. This project welcomes contributions from human developers and autonomous AI coding agents alike. - -Go backup and replication daemon with Svelte web UI for Unraid servers. - -To maintain code quality, protect maintainer capacity, and prevent maintainer burnout, all contributors must follow the standardized development and governance workflow outlined below. - ---- - -## Code of Conduct and Standards - -- Be respectful, constructive, and collaborative. -- Zero secrets policy: Never commit passwords, tokens, API keys, private certificates, or personal data. -- Anti-burnout principle: Perform full due diligence locally before opening pull requests or requesting review. - ---- - -## Development Workflow - -All contributions—whether submitted by humans or AI agents—must adhere to this step-by-step workflow: - -### 1. Open a GitHub Issue First -- Every bug fix, feature, or enhancement must start with an issue. -- Use the appropriate GitHub Issue Form: - - Bug Report (`01-bug-report.yml`) - - Enhancement Request (`02-enhancement-request.yml`) -- Blank or unstructured issues are disabled. Pull requests without a corresponding issue will be closed. - -### 2. CodeRabbit AI Plan Generation -- Once the issue is created, CodeRabbit AI is triggered to generate an implementation plan (either automatically via the `bug` or `enhancement` label, or by commenting `@coderabbitai plan` on the issue). -- CodeRabbit analyzes the codebase and posts a comprehensive Coding Plan containing: - - Codebase research and affected components - - Architectural design choices - - Phased task breakdowns - - Machine-readable agent prompts and verifiable acceptance criteria -- Contributors must review, refine, and confirm the plan before writing code. Do not start implementation without an approved plan. - -### 3. Create a Dedicated Branch -- Create a feature or bugfix branch off `main`: - - Features: `git checkout -b feat/-` - - Fixes: `git checkout -b fix/-` - - Documentation: `git checkout -b docs/-` -- Never commit directly to the default branch (`main`). - -### 4. Implementation and Local Validation -- Implement changes following project conventions and the approved plan. -- Update `CHANGELOG.md` under `## [Unreleased]` describing what changed and why. -- Run all required local validation checks (tests with coverage, linters, type checkers, and formatters). Ensure all checks pass with zero errors and zero warnings. - -### 5. Follow the Pull Request Template -- Open a Pull Request using `.github/PULL_REQUEST_TEMPLATE.md`. -- Complete all sections in the template: - - Summary of changes - - Linked issue keywords (`Fixes #`, `Closes #`, `Addresses #`) - - Verification steps and pasted terminal output - - Mandatory Pre-Submission Governance checklist (every box must be checked) - -### 6. Open PR as DRAFT First -- Pull requests MUST be opened in Draft mode (`gh pr create --draft` or choose "Create draft pull request" in GitHub). -- Draft PRs allow CI workflows to run in isolation without triggering premature reviewer notifications or consuming unnecessary review capacity. -- Verify that all CI checks pass in draft mode and confirm that everything is working as expected from your end. - -### 7. Mark Ready for Review -- Only after all local and draft CI checks have succeeded, convert the pull request to "Ready for Review". -- Pull requests marked ready for review with failing checks, incomplete templates, or unverified changes will be converted back to draft or closed. - -### 8. Automated Quality Gates and Merge Criteria -Before a pull request can be merged, it must satisfy all automated quality gates: -- CodeRabbit AI Review: All review comments and suggestions must be resolved or addressed. -- GitHub Copilot Code Review: Must pass with no blocking feedback. -- Codecov Coverage Reports: Must pass with zero errors, zero warnings, and no coverage regressions below the repository target. -- Branch Ruleset Checks: All required status checks defined in the default branch ruleset must pass. - ---- - -## Local Development and Verification - -### Prerequisites and Setup - -- Go 1.24 or later -- Node.js (v20+) and npm (for Svelte UI in `web/`) -- Pre-commit (`make pre-commit-install`) - -### Validation Commands - -```bash -# Go dependencies and local build -make deps -make build-local - -# Run Go unit and integration tests -make test - -# Run Go linter and security audit -make lint -make security-check - -# Build and test web frontend -cd web && npm ci && npm run build && npm run lint && npm test && cd .. - -# Run all pre-commit hooks -make pre-commit-run -``` - ---- - -## Commit Message Guidelines - -We follow Conventional Commits: - -- `feat(): ` — New feature -- `fix(): ` — Bug fix -- `docs(): ` — Documentation changes -- `test(): ` — Adding or updating tests -- `refactor(): ` — Code restructuring without functional change -- `chore(): ` — Tooling, dependencies, or repository maintenance - ---- - -## Pull Request Governance Summary - -- No spam or back-to-back PRs: Batch related changes into a single PR. -- AI-assisted PRs require human verification and local testing before submission. -- All template checkboxes must be confirmed. -- PRs must remain in Draft until verified. From 79a960e73641cfdf2ff187f9acf8744131ccd925 Mon Sep 17 00:00:00 2001 From: Ruaan Deysel Date: Sat, 10 Oct 2026 16:45:29 +1000 Subject: [PATCH 2/6] fix(web): prevent microtask starvation and page freeze on logs page (#454) - Share in-flight loadOlder Promise across concurrent callers (mount, scroll, loadAll, search) - Bind loadOlder to _contextSeq to prevent stale responses after category switch or reload - Terminate loadAll and search loops when no progress is made - Add synchronous onMount cleanup and store.dispose() method in Logs.svelte - Add Vitest store regression tests for concurrent loaders, stale category, and loop termination - Add Playwright browser regression test for responsiveness during delayed older page load - Update CHANGELOG.md under [Unreleased] Fixes #454 --- CHANGELOG.md | 4 + tests/logs.spec.ts | 78 +++++++ web/src/lib/unifiedlog.svelte.js | 214 ++++++++++-------- web/src/lib/unifiedlog.test.js | 363 ++++++++++++++++++++++++++++--- web/src/pages/Logs.svelte | 52 +++-- 5 files changed, 571 insertions(+), 140 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 479a2056..8322a4b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/), and this ## [Unreleased] +### Fixed + +- **Logs page unresponsiveness and freeze when loading older entries (#454):** Fixed microtask starvation caused by concurrent older-page requests during initial mount and background loading loops (`loadAll` and search). Active older-load requests now share the in-flight Promise across callers, older requests are bound to the store context sequence to discard stale responses, and background loading loops terminate when no progress is made. Also added synchronous `onMount` cleanup and a store `dispose()` method in `Logs.svelte` to abort background activity upon page navigation. Closes #454. + ## [v2026.10.00] - 2026-10-07 ### Added diff --git a/tests/logs.spec.ts b/tests/logs.spec.ts index 99728b4d..4499cfc8 100644 --- a/tests/logs.spec.ts +++ b/tests/logs.spec.ts @@ -72,4 +72,82 @@ test.describe('Activity Logs & Run Log Viewer', () => { // Logs are cleared await expect(page.getByText('No logs', { exact: true })).toBeVisible(); }); + + test('renders initial rows and remains responsive while older page is pending (#454)', async ({ page }) => { + // Generate 60 activity rows to exercise pagination (default batch is 30) + const rows = Array.from({ length: 60 }, (_, i) => { + const id = 60 - i; + return { + id, + level: 'info', + category: 'backup', + message: `Activity entry #${id}`, + details: null, + created_at: new Date(Date.UTC(2026, 7, 21, 15, 0, 0) + id * 1000).toISOString(), + }; + }); + + let releaseGate: () => void = () => {}; + const gateHeld = new Promise(resolve => { releaseGate = resolve; }); + let olderCalls = 0; + let gateReleased = false; + + // Route-specific override registered after shared mock + await page.route('**/api/v1/activity*', async route => { + if (route.request().method() !== 'GET') { + return route.fallback(); + } + const url = new URL(route.request().url()); + const beforeIdStr = url.searchParams.get('before_id'); + const beforeId = beforeIdStr ? parseInt(beforeIdStr, 10) : 0; + const limit = parseInt(url.searchParams.get('limit') || '30', 10); + const category = url.searchParams.get('category') || ''; + + if (beforeId > 0) { + olderCalls++; + if (!gateReleased) { + await gateHeld; + } + } + + let filtered = rows; + if (category) filtered = filtered.filter(r => r.category === category); + if (beforeId > 0) filtered = filtered.filter(r => r.id < beforeId); + filtered.sort((a, b) => new Date(b.created_at).getTime() - new Date(a.created_at).getTime() || b.id - a.id); + const pageRows = filtered.slice(0, limit); + + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify(pageRows), + }); + }); + + await page.goto('/#/logs'); + + // Initial rows render in the role="log" console + const logConsole = page.getByRole('log'); + await expect(logConsole.locator('[data-entry-id="60"]')).toBeVisible(); + + // "Loading logs..." spinner disappears + await expect(page.getByText('Loading logs...')).toHaveCount(0); + + // Search input remains interactive while older-page request is held + const searchInput = page.getByPlaceholder(/filter logs/i); + await expect(searchInput).toBeVisible(); + await searchInput.fill('Entry #60'); + await expect(searchInput).toHaveValue('Entry #60'); + await searchInput.fill(''); + + // Release the gate + gateReleased = true; + releaseGate(); + + // Older rows load and older entries become visible + await expect(logConsole.locator('[data-entry-id="1"]')).toBeVisible(); + + // Request count remains bounded + expect(olderCalls).toBeGreaterThanOrEqual(1); + expect(olderCalls).toBeLessThanOrEqual(5); + }); }); diff --git a/web/src/lib/unifiedlog.svelte.js b/web/src/lib/unifiedlog.svelte.js index 1dee4e98..f5049f13 100644 --- a/web/src/lib/unifiedlog.svelte.js +++ b/web/src/lib/unifiedlog.svelte.js @@ -40,6 +40,7 @@ export function createUnifiedLogStore() { let _error = $state('') let _hasMore = $state(false) let _loadingOlder = $state(false) + let _olderPromise = null let _oldestActivityId = $state(null) // cursor for "load older" let _category = $state('') let _levelFilter = $state('') @@ -63,6 +64,7 @@ export function createUnifiedLogStore() { _error = '' _hasMore = false _loadingOlder = false + _olderPromise = null _oldestActivityId = null _expanded = {} _seen = {} @@ -72,6 +74,13 @@ export function createUnifiedLogStore() { _contextSeq++ } + function dispose() { + _contextSeq++ + _loadSeq++ + _loadingOlder = false + _olderPromise = null + } + // Category is applied server-side via load(); the job and level filters are // client-side and independent — switching category must NOT clear them, so // filters compose (#328 round 2). @@ -114,19 +123,22 @@ export function createUnifiedLogStore() { // so the search covers the whole history (#328). await load() } - while (_search && _hasMore) { + const ctx = _contextSeq + while (_search && _hasMore && ctx === _contextSeq) { // Big pages + silent: a full-history search must not make ~333 // round-trips of BATCH_SIZE, and a transient error mid-search must // not surface as the console-wide error box (the scroll path // reports errors; the background search just retries) (#328). - await loadOlder({ limit: FULL_LOAD_LIMIT, silent: true }) + const outcome = await loadOlder({ limit: FULL_LOAD_LIMIT, silent: true }) + if (outcome === 'no-progress') break + if (outcome === 'superseded' && ctx !== _contextSeq) break } // The loop ended because _hasMore is false: the buffer now IS the // whole history. Pin it — pruning would drop the oldest entries // while _hasMore stays false, so the "— End of logs —" marker would // display over a truncated buffer and lie about the end (#328). // (Guard on _search: a query cleared mid-loop aborts the load.) - if (_search) _fullHistoryLoaded = true + if (_search && ctx === _contextSeq && !_hasMore) _fullHistoryLoaded = true } finally { _searching = false } @@ -242,6 +254,8 @@ export function createUnifiedLogStore() { _pruned = false _fullHistoryLoaded = false _contextSeq++ + _loadingOlder = false + _olderPromise = null // Expand terminal entries in parallel, then merge. Only the fresh // path fetches: the merge path must NOT refetch every terminal on @@ -399,96 +413,121 @@ export function createUnifiedLogStore() { } async function loadOlder({ smooth = false, limit = BATCH_SIZE, silent = false } = {}) { - if (_loadingOlder) return - if (!_hasMore && _pendingRunLogs.length === 0) return + if (_loadingOlder && _olderPromise) return _olderPromise + if (!_hasMore && _pendingRunLogs.length === 0) return 'no-progress' _loadingOlder = true - const startedAt = Date.now() - let loaded = false - try { - // Drain leftover run-log lines from a previously split terminal - // expansion first, so a long run's log is inserted across bounded - // steps instead of one unbounded dump (#328 r4 #5). A drain step - // inserts at most OLDER_BATCH_BUDGET lines and then returns; the - // activity fetch resumes on a later step once pending is empty. - if (_pendingRunLogs.length > 0) { - const take = _pendingRunLogs.slice(0, OLDER_BATCH_BUDGET) - _pendingRunLogs = _pendingRunLogs.slice(OLDER_BATCH_BUDGET) - _entries = prune(dedupeSummaries([..._entries, ...take]).sort((a, b) => tsMs(a.ts) - tsMs(b.ts))) - loaded = true - return - } + const ctx = _contextSeq + const promise = (async () => { + const startedAt = Date.now() + let loaded = false + try { + // Drain leftover run-log lines from a previously split terminal + // expansion first, so a long run's log is inserted across bounded + // steps instead of one unbounded dump (#328 r4 #5). A drain step + // inserts at most OLDER_BATCH_BUDGET lines and then returns; the + // activity fetch resumes on a later step once pending is empty. + if (_pendingRunLogs.length > 0) { + const take = _pendingRunLogs.slice(0, OLDER_BATCH_BUDGET) + _pendingRunLogs = _pendingRunLogs.slice(OLDER_BATCH_BUDGET) + _entries = prune(dedupeSummaries([..._entries, ...take]).sort((a, b) => tsMs(a.ts) - tsMs(b.ts))) + loaded = true + return 'advanced' + } - if (!_hasMore || !_oldestActivityId) return - const older = (await api.getActivity(limit, _category, _oldestActivityId)) || [] - if (older.length === 0) { _hasMore = false; return } - loaded = true + if (!_hasMore || !_oldestActivityId) return 'no-progress' + const prevOldestId = _oldestActivityId + const older = (await api.getActivity(limit, _category, prevOldestId)) || [] + if (ctx !== _contextSeq) return 'superseded' - const terminal = older.filter(e => { - const d = tryParse(e.details) - return d?.run_log === true && !_expanded[e.id] - }) - const runLogs = await fetchRunLogsForBatch(terminal) - - const perEntry = older.map(ae => buildUnified([ae], runLogs)) - // The step budget bounds RUN-LOG lines only; plain activity rows - // always materialize (same rationale as the fresh path). - const { fullCount, split } = planBatch( - perEntry.map(list => list.filter(e => e.type === 'runlog').length), - OLDER_BATCH_BUDGET, - ) - - // Consume fullCount entries in full, plus `split` lines of the next - // terminal entry (whose expansion exceeded the remaining budget). - let newUnified = perEntry.slice(0, fullCount).flat() - let consumed = fullCount - if (split > 0) { - const splitEntry = perEntry[fullCount] - newUnified = [...newUnified, ...splitEntry.slice(0, split)] - // Same as the fresh path: never defer a run's terminal summary. - const middle = splitEntry.slice(split, -1) - const tail = splitEntry.slice(-1) - if (tail.length > 0) newUnified = [...newUnified, ...tail] - if (middle.length > 0) { - _pendingRunLogs = [..._pendingRunLogs, ...middle] + if (older.length === 0) { + _hasMore = false + return 'no-progress' } - consumed++ - } - for (let i = consumed; i < perEntry.length; i++) { - const rows = perEntry[i].filter(e => e.type !== 'runlog') - if (rows.length > 0) newUnified = [...newUnified, ...rows] - const rl = perEntry[i].filter(e => e.type === 'runlog') - if (rl.length > 0) _pendingRunLogs = [..._pendingRunLogs, ...rl] - } - // Every terminal entry in this batch is accounted for: its run log is - // captured either in the buffer or in _pendingRunLogs, so a later - // fetch must not re-expand it. - for (const ae of older) { - const d = tryParse(ae.details) - if (d?.run_log === true) _expanded[ae.id] = true - } + const newOldestId = older[older.length - 1].id + if (!(newOldestId < prevOldestId)) { + _hasMore = false + return 'no-progress' + } - _oldestActivityId = older.length > 0 ? older[older.length - 1].id : null - _hasMore = (older.length === limit) || (_pendingRunLogs.length > 0) + const terminal = older.filter(e => { + const d = tryParse(e.details) + return d?.run_log === true && !_expanded[e.id] + }) + const runLogs = await fetchRunLogsForBatch(terminal) + if (ctx !== _contextSeq) return 'superseded' - const combined = [..._entries, ...newUnified] - _entries = prune(dedupeSummaries(combined).sort((a, b) => tsMs(a.ts) - tsMs(b.ts))) - } catch (e) { - // Background full-history loads (loadAll) must not surface a transient - // fetch error as the console-wide error box — they just stop and leave - // the scroll path (which reports errors) to retry (#328). - if (!silent) _error = e.message || 'Failed to load older logs' - } finally { - // Artificial minimum latency (see MIN_LOAD_OLDER_MS). Only hold the - // floor when a batch actually loaded — an empty page means the user is - // already at the oldest log, and lingering would show the spinner with - // nothing to show (#328 r3 #4). - if (smooth && loaded) { - const remaining = MIN_LOAD_OLDER_MS - (Date.now() - startedAt) - if (remaining > 0) await sleep(remaining) + loaded = true + + const perEntry = older.map(ae => buildUnified([ae], runLogs)) + // The step budget bounds RUN-LOG lines only; plain activity rows + // always materialize (same rationale as the fresh path). + const { fullCount, split } = planBatch( + perEntry.map(list => list.filter(e => e.type === 'runlog').length), + OLDER_BATCH_BUDGET, + ) + + // Consume fullCount entries in full, plus `split` lines of the next + // terminal entry (whose expansion exceeded the remaining budget). + let newUnified = perEntry.slice(0, fullCount).flat() + let consumed = fullCount + if (split > 0) { + const splitEntry = perEntry[fullCount] + newUnified = [...newUnified, ...splitEntry.slice(0, split)] + // Same as the fresh path: never defer a run's terminal summary. + const middle = splitEntry.slice(split, -1) + const tail = splitEntry.slice(-1) + if (tail.length > 0) newUnified = [...newUnified, ...tail] + if (middle.length > 0) { + _pendingRunLogs = [..._pendingRunLogs, ...middle] + } + consumed++ + } + for (let i = consumed; i < perEntry.length; i++) { + const rows = perEntry[i].filter(e => e.type !== 'runlog') + if (rows.length > 0) newUnified = [...newUnified, ...rows] + const rl = perEntry[i].filter(e => e.type === 'runlog') + if (rl.length > 0) _pendingRunLogs = [..._pendingRunLogs, ...rl] + } + + // Every terminal entry in this batch is accounted for: its run log is + // captured either in the buffer or in _pendingRunLogs, so a later + // fetch must not re-expand it. + for (const ae of older) { + const d = tryParse(ae.details) + if (d?.run_log === true) _expanded[ae.id] = true + } + + _oldestActivityId = newOldestId + _hasMore = (older.length === limit) || (_pendingRunLogs.length > 0) + + const combined = [..._entries, ...newUnified] + _entries = prune(dedupeSummaries(combined).sort((a, b) => tsMs(a.ts) - tsMs(b.ts))) + return 'advanced' + } catch (e) { + if (ctx !== _contextSeq) return 'superseded' + // Background full-history loads (loadAll) must not surface a transient + // fetch error as the console-wide error box — they just stop and leave + // the scroll path (which reports errors) to retry (#328). + if (!silent) _error = e.message || 'Failed to load older logs' + return 'no-progress' + } finally { + // Artificial minimum latency (see MIN_LOAD_OLDER_MS). Only hold the + // floor when a batch actually loaded — an empty page means the user is + // already at the oldest log, and lingering would show the spinner with + // nothing to show (#328 r3 #4). + if (smooth && loaded && ctx === _contextSeq) { + const remaining = MIN_LOAD_OLDER_MS - (Date.now() - startedAt) + if (remaining > 0) await sleep(remaining) + } + if (ctx === _contextSeq) { + _loadingOlder = false + _olderPromise = null + } } - _loadingOlder = false - } + })() + _olderPromise = promise + return promise } // loadAll() — background full-history load: keep pulling older pages until @@ -504,7 +543,9 @@ export function createUnifiedLogStore() { const ctx = _contextSeq try { while (ctx === _contextSeq && _hasMore && !_error) { - await loadOlder({ limit: FULL_LOAD_LIMIT, silent: true }) + const outcome = await loadOlder({ limit: FULL_LOAD_LIMIT, silent: true }) + if (outcome === 'no-progress') break + if (outcome === 'superseded' && ctx !== _contextSeq) break } } finally { // If the loop was aborted by a context change, the fresh load already @@ -716,5 +757,6 @@ export function createUnifiedLogStore() { get search() { return _search }, get searching() { return _searching }, reset, load, loadNewer, loadOlder, loadAll, setCategory, setLevelFilter, setJobFilter, setSearchFilter, setupWs, + dispose, } } diff --git a/web/src/lib/unifiedlog.test.js b/web/src/lib/unifiedlog.test.js index d1a0c581..54d4ee0f 100644 --- a/web/src/lib/unifiedlog.test.js +++ b/web/src/lib/unifiedlog.test.js @@ -1,28 +1,19 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' -// Regression tests for the unified log store (issue #328). -// The store previously had zero coverage; the scenarios below pin the -// invariants that the console depends on: +// Regression tests for the unified log store (issue #328, #454). +// The scenarios below pin the invariants that the console depends on: // 1. The FIRST PAINT materializes every activity row of the newest page, // even when the newest terminal's run-log expansion exceeds the step -// budget (the budget bounds RUN-LOG lines only). Previously the whole -// page beyond the split point was deferred, so a set of rows (health -// checks etc.) appeared only on the next merge — the visible "set -// replaced by another set" flash on every refresh. +// budget (the budget bounds RUN-LOG lines only). // 2. Streamed (WS) run-log lines for a live run are inserted in // CHRONOLOGICAL position, never appended at the bottom. // 3. The newest set stays at the bottom of the buffer across the refresh // sequence (load -> fillViewport -> loadAll -> poll merges). +// 4. Concurrent history loaders (loadAll, search) share the in-flight +// loadOlder promise, yield to the macrotask event loop, and terminate +// when no progress is made (#454). -// Server model used by all tests: -// - terminal run 60: activity id 400 (NEWEST, 15:58:04) + 200 run-log -// lines (15:55:00..15:56:40) — the expansion exceeds the 150-line step -// budget, exercising the split path exactly like a real long run -// - 5 health checks (ids 300..304, 15:57:59..15:58:03) -// - active run 7: plain "started" row (id 210, 15:54:00); its lines -// arrive only over WS (ts 15:54:10..30, chronologically mid-buffer) - -vi.mock('./api.js', () => { +const { defaultEntries, defaultRunLogs } = vi.hoisted(() => { const entries = [ { id: 400, level: 'info', category: 'backup', @@ -55,38 +46,52 @@ vi.mock('./api.js', () => { data: '', ts: new Date(Date.UTC(2026, 7, 21, 15, 55, 0) + i * 500).toISOString(), }) } - - const api = { - getActivity: vi.fn(async (limit = 30, _category = '', beforeId = 0) => { - let rows = entries - if (beforeId) rows = rows.filter(r => r.id < beforeId) - return rows.slice(0, limit) - }), - getRunLogs: vi.fn(async (runId) => ({ entries: runLogs[runId] || [] })), - } - return { api } + return { defaultEntries: entries, defaultRunLogs: runLogs } }) +vi.mock('./api.js', () => ({ + api: { + getActivity: vi.fn(), + getRunLogs: vi.fn(), + }, +})) + let wsHandler = null vi.mock('./ws.svelte.js', () => ({ onWsMessage: (fn) => { wsHandler = fn; return () => { wsHandler = null } }, })) import { createUnifiedLogStore } from './unifiedlog.svelte.js' +import { api } from './api.js' function bottomIds(store, n) { return store.entries.slice(-n).map(e => `${e.type}:${e.id}@${new Date(e.ts).toISOString().slice(11, 19)}`) } +function deferred() { + let resolve, reject + const promise = new Promise((res, rej) => { + resolve = res + reject = rej + }) + return { promise, resolve, reject } +} + describe('unified log store', () => { let store - beforeEach(() => { wsHandler = null; store = createUnifiedLogStore() }) + beforeEach(() => { + wsHandler = null + api.getActivity.mockImplementation(async (limit = 30, category = '', beforeId = 0) => { + let rows = defaultEntries + if (category) rows = rows.filter(r => r.category === category) + if (beforeId) rows = rows.filter(r => r.id < beforeId) + return rows.slice(0, limit) + }) + api.getRunLogs.mockImplementation(async (runId) => ({ entries: defaultRunLogs[runId] || [] })) + store = createUnifiedLogStore() + }) it('first paint includes every activity row even when a terminal expansion exceeds the budget', async () => { - // The newest page is [terminal run 60 (200 lines), 5 health checks, - // started row]. The terminal's expansion blows the 150-line step budget, - // but the health checks and the started row MUST be present immediately - // after load() — no loadOlder/merge may be required to surface them. await store.load() const ids = store.entries.map(e => e.id) @@ -105,7 +110,6 @@ describe('unified log store', () => { await store.load() store.setupWs() - // Baseline: the bottom of the buffer is the newest health-check set const baseline = bottomIds(store, 3) expect(baseline.every(s => s.includes('15:58:0'))).toBe(true) @@ -114,11 +118,8 @@ describe('unified log store', () => { wsHandler({ type: 'run_log', entry: { id: 70002, run_id: 7, level: 'info', message: 'docker: stopping container', data: '', ts: '2026-08-21T15:54:20Z' } }) wsHandler({ type: 'run_log', entry: { id: 70003, run_id: 7, level: 'info', message: 'docker: backing up', data: '', ts: '2026-08-21T15:54:30Z' } }) - // The bottom set must NOT change: a streamed line is not the newest log. expect(bottomIds(store, 3)).toEqual(baseline) - // The streamed lines sit chronologically between the "started" row and - // the terminal run 60's lines. const rl = store.entries.filter(e => e.type === 'runlog' && e.runId === 7) expect(rl).toHaveLength(3) const first = store.entries.indexOf(rl[0]) @@ -141,4 +142,296 @@ describe('unified log store', () => { const stable = bottomIds(store, 3) expect(stable).toEqual(afterLoad) }) + + it('yields to macrotask queue when loadOlder is pending during loadAll', async () => { + const plain30 = Array.from({ length: 30 }, (_, i) => ({ + id: 1000 - i, + level: 'info', + category: 'backup', + message: `msg ${1000 - i}`, + details: null, + created_at: new Date(Date.UTC(2026, 7, 21, 15, 0, 0) + (1000 - i) * 1000).toISOString(), + })) + + const hold = deferred() + let olderCalls = 0 + + api.getActivity.mockImplementation(async (limit = 30, _category = '', beforeId = 0) => { + if (!beforeId) return plain30.slice(0, limit) + olderCalls++ + return hold.promise + }) + + try { + await store.load() + expect(store.hasMore).toBe(true) + + let sentinelFired = false + const timer = setTimeout(() => { + sentinelFired = true + }, 5) + + let loadOlderCalls = 0 + const origLoadOlder = store.loadOlder + store.loadOlder = (...args) => { + loadOlderCalls++ + if (loadOlderCalls > 50) { + store.setError('busy loop detected') + clearTimeout(timer) + } + return origLoadOlder.apply(store, args) + } + + const olderPromise = store.loadOlder() + const allPromise = store.loadAll() + + await new Promise((resolve) => setTimeout(resolve, 20)) + + if (!sentinelFired) { + store.setError('stop') + hold.resolve([]) + } + expect(sentinelFired).toBe(true) + expect(loadOlderCalls).toBeLessThanOrEqual(2) + expect(olderCalls).toBe(1) + + hold.resolve([]) + await olderPromise + await allPromise + expect(store.loadingOlder).toBe(false) + } finally { + hold.resolve([]) + } + }) + + it('yields to macrotask queue when loadOlder is pending during setSearchFilter', async () => { + const plain30 = Array.from({ length: 30 }, (_, i) => ({ + id: 1000 - i, + level: 'info', + category: 'backup', + message: `msg ${1000 - i}`, + details: null, + created_at: new Date(Date.UTC(2026, 7, 21, 15, 0, 0) + (1000 - i) * 1000).toISOString(), + })) + + const hold = deferred() + let olderCalls = 0 + + api.getActivity.mockImplementation(async (limit = 30, _category = '', beforeId = 0) => { + if (!beforeId) return plain30.slice(0, limit) + olderCalls++ + return hold.promise + }) + + try { + await store.load() + expect(store.hasMore).toBe(true) + + let sentinelFired = false + const timer = setTimeout(() => { + sentinelFired = true + }, 5) + + let loadOlderCalls = 0 + const origLoadOlder = store.loadOlder + store.loadOlder = (...args) => { + loadOlderCalls++ + if (loadOlderCalls > 50) { + store.setError('busy loop detected') + store.setSearchFilter('') + clearTimeout(timer) + } + return origLoadOlder.apply(store, args) + } + + const olderPromise = store.loadOlder() + store.setSearchFilter('needle') + + await new Promise((resolve) => setTimeout(resolve, 20)) + + if (!sentinelFired) { + store.setError('stop') + store.setSearchFilter('') + hold.resolve([]) + } + expect(sentinelFired).toBe(true) + expect(loadOlderCalls).toBeLessThanOrEqual(2) + expect(olderCalls).toBe(1) + + hold.resolve([]) + await olderPromise + await vi.waitFor(() => expect(store.searching).toBe(false)) + expect(store.loadingOlder).toBe(false) + } finally { + hold.resolve([]) + } + }) + + it('discards older page for stale category and does not block new category loadAll', async () => { + const backupRows = Array.from({ length: 30 }, (_, i) => ({ + id: 2000 - i, + level: 'info', + category: 'backup', + message: `backup ${2000 - i}`, + details: null, + created_at: new Date(Date.UTC(2026, 7, 21, 15, 0, 0) + (2000 - i) * 1000).toISOString(), + })) + const restoreRows = Array.from({ length: 30 }, (_, i) => ({ + id: 1000 - i, + level: 'info', + category: 'restore', + message: `restore ${1000 - i}`, + details: null, + created_at: new Date(Date.UTC(2026, 7, 21, 15, 0, 0) + (1000 - i) * 1000).toISOString(), + })) + + const holdA = deferred() + + api.getActivity.mockImplementation(async (limit = 30, category = '', beforeId = 0) => { + if (category === 'backup') { + if (!beforeId) return backupRows.slice(0, limit) + return holdA.promise + } + if (category === 'restore') { + if (!beforeId) return restoreRows.slice(0, limit) + return [] + } + return [] + }) + + try { + await store.setCategory('backup') + expect(store.hasMore).toBe(true) + + const olderAPromise = store.loadOlder() + + await store.setCategory('restore') + + const olderBackupRows = Array.from({ length: 30 }, (_, i) => ({ + id: 1970 - i, + level: 'info', + category: 'backup', + message: `backup ${1970 - i}`, + details: null, + created_at: new Date(Date.UTC(2026, 7, 21, 14, 0, 0) + (1970 - i) * 1000).toISOString(), + })) + holdA.resolve(olderBackupRows) + await olderAPromise + + expect(store.category).toBe('restore') + expect(store.entries.every(e => e.category === 'restore')).toBe(true) + expect(store.entries.some(e => e.category === 'backup')).toBe(false) + expect(store.hasMore).toBe(false) + expect(store.loadingOlder).toBe(false) + } finally { + holdA.resolve([]) + } + }) + + it('terminates loadAll after bounded calls when cursor does not advance', async () => { + const fixedRows = Array.from({ length: 30 }, (_, i) => ({ + id: 500 - i, + level: 'info', + category: 'backup', + message: `msg ${500 - i}`, + details: null, + created_at: new Date(Date.UTC(2026, 7, 21, 15, 0, 0) + (500 - i) * 1000).toISOString(), + })) + + let calls = 0 + api.getActivity.mockImplementation(async (limit = 30, _category = '', _beforeId = 0) => { + calls++ + return fixedRows.slice(0, limit) + }) + + await store.load() + expect(store.hasMore).toBe(true) + const initialCalls = calls + + await store.loadAll() + expect(calls - initialCalls).toBe(1) + expect(store.hasMore).toBe(false) + expect(store.loadingOlder).toBe(false) + }) + + it('terminates loadAll after one silent failure and preserves existing rows', async () => { + const plain30 = Array.from({ length: 30 }, (_, i) => ({ + id: 1000 - i, + level: 'info', + category: 'backup', + message: `msg ${1000 - i}`, + details: null, + created_at: new Date(Date.UTC(2026, 7, 21, 15, 0, 0) + (1000 - i) * 1000).toISOString(), + })) + + let calls = 0 + api.getActivity.mockImplementation(async (limit = 30, _category = '', beforeId = 0) => { + calls++ + if (beforeId) { + throw new Error('network down') + } + return plain30.slice(0, limit) + }) + + await store.load() + expect(store.hasMore).toBe(true) + const rowCountBefore = store.entries.length + expect(rowCountBefore).toBe(30) + + await store.loadAll() + expect(calls).toBe(2) + expect(store.entries.length).toBe(rowCountBefore) + expect(store.error).toBe('') + expect(store.hasMore).toBe(true) + expect(store.loadingOlder).toBe(false) + }) + + it('dispose increments sequences and stops background loading without clearing entries', async () => { + const plain30 = Array.from({ length: 30 }, (_, i) => ({ + id: 1000 - i, + level: 'info', + category: 'backup', + message: `msg ${1000 - i}`, + details: null, + created_at: new Date(Date.UTC(2026, 7, 21, 15, 0, 0) + (1000 - i) * 1000).toISOString(), + })) + + const hold = deferred() + let calls = 0 + api.getActivity.mockImplementation(async (limit = 30, _category = '', beforeId = 0) => { + calls++ + if (!beforeId) return plain30.slice(0, limit) + return hold.promise + }) + + await store.load() + const count = store.entries.length + expect(count).toBe(30) + expect(store.hasMore).toBe(true) + + // Start loadAll which triggers an older-page load that is held + const allPromise = store.loadAll() + expect(calls).toBe(2) + + // Dispose while older load is pending + store.dispose() + + // Release held older rows + const olderRows = Array.from({ length: 30 }, (_, i) => ({ + id: 970 - i, + level: 'info', + category: 'backup', + message: `msg ${970 - i}`, + details: null, + created_at: new Date(Date.UTC(2026, 7, 21, 14, 0, 0) + (970 - i) * 1000).toISOString(), + })) + hold.resolve(olderRows) + await allPromise + + // Stale older rows must not be added to store entries + expect(store.entries.length).toBe(count) + // No subsequent calls should have run + expect(calls).toBe(2) + expect(store.loadingOlder).toBe(false) + }) }) diff --git a/web/src/pages/Logs.svelte b/web/src/pages/Logs.svelte index f1e61ee0..526bc11b 100644 --- a/web/src/pages/Logs.svelte +++ b/web/src/pages/Logs.svelte @@ -63,25 +63,39 @@ { value: 'info', label: 'Info' }, ] - onMount(async () => { - await store.load() - await fillViewport() - // Seed the scroll flags from the real geometry. Without this, a history - // short enough to fit the viewport (no scrollbar at all) leaves atOldest - // false, so the "jump to oldest" overlay hovers over a console that is - // already showing its oldest line. - handleScroll() - // Background full-history load: the console spans ALL logs (uniform with - // the search view), so scrolling reaches the true end and the - // "— End of logs —" marker shows there (#328). - store.loadAll() - const unsub = store.setupWs() - // Poll safety net: in poll mode the timer is the primary path (10s); in - // live/WS mode it runs as a slower catch-up (30s) so a missed WS event - // (e.g. a terminal summary lost across a reconnect) is still surfaced - // instead of waiting for a reload (#328 r9 #5). - const pollTimer = setInterval(() => store.loadNewer(), liveMode === 'poll' ? 10000 : 30000) - return () => { unsub(); if (pollTimer) clearInterval(pollTimer) } + onMount(() => { + let destroyed = false + let unsub = null + let pollTimer = null + + ;(async () => { + await store.load() + if (destroyed) return + await fillViewport() + if (destroyed) return + // Seed the scroll flags from the real geometry. Without this, a history + // short enough to fit the viewport (no scrollbar at all) leaves atOldest + // false, so the "jump to oldest" overlay hovers over a console that is + // already showing its oldest line. + handleScroll() + // Background full-history load: the console spans ALL logs (uniform with + // the search view), so scrolling reaches the true end and the + // "— End of logs —" marker shows there (#328). + store.loadAll() + unsub = store.setupWs() + // Poll safety net: in poll mode the timer is the primary path (10s); in + // live/WS mode it runs as a slower catch-up (30s) so a missed WS event + // (e.g. a terminal summary lost across a reconnect) is still surfaced + // instead of waiting for a reload (#328 r9 #5). + pollTimer = setInterval(() => store.loadNewer(), liveMode === 'poll' ? 10000 : 30000) + })() + + return () => { + destroyed = true + if (unsub) unsub() + if (pollTimer) clearInterval(pollTimer) + store.dispose() + } }) // Track new entries arriving while follow is off. The marker snapshots the From b62b6d9650d73772c00b7b6b1481ccd27e91427f Mon Sep 17 00:00:00 2001 From: Ruaan Deysel Date: Sat, 10 Oct 2026 16:51:54 +1000 Subject: [PATCH 3/6] test(web): expand coverage for unified log store older-load branches --- web/src/lib/unifiedlog.test.js | 104 +++++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/web/src/lib/unifiedlog.test.js b/web/src/lib/unifiedlog.test.js index 54d4ee0f..3e57d470 100644 --- a/web/src/lib/unifiedlog.test.js +++ b/web/src/lib/unifiedlog.test.js @@ -434,4 +434,108 @@ describe('unified log store', () => { expect(calls).toBe(2) expect(store.loadingOlder).toBe(false) }) + + it('loadOlder expands terminal run logs with split budgeting and smooth latency', async () => { + const plain30 = Array.from({ length: 30 }, (_, i) => ({ + id: 1000 - i, + level: 'info', + category: 'backup', + message: `msg ${1000 - i}`, + details: null, + created_at: new Date(Date.UTC(2026, 7, 21, 15, 0, 0) + (1000 - i) * 1000).toISOString(), + })) + + const older30 = [ + { + id: 970, + level: 'info', + category: 'backup', + message: 'Backup completed: old-job', + details: JSON.stringify({ run_id: 88, run_log: true, job_id: 2, job_name: 'old-job' }), + created_at: '2026-08-21T14:58:00Z', + }, + ...Array.from({ length: 29 }, (_, i) => ({ + id: 969 - i, + level: 'info', + category: 'backup', + message: `msg ${969 - i}`, + details: null, + created_at: new Date(Date.UTC(2026, 7, 21, 14, 0, 0) + (969 - i) * 1000).toISOString(), + })), + ] + + const runLogs88 = Array.from({ length: 200 }, (_, i) => ({ + id: 88000 + i, + run_id: 88, + level: 'info', + message: `run 88 line ${i}`, + data: '', + ts: new Date(Date.UTC(2026, 7, 21, 14, 50, 0) + i * 500).toISOString(), + })) + + api.getActivity.mockImplementation(async (limit = 30, _category = '', beforeId = 0) => { + if (!beforeId) return plain30.slice(0, limit) + return older30.slice(0, limit) + }) + api.getRunLogs.mockImplementation(async (runId) => { + if (Number(runId) === 88) return { entries: runLogs88 } + return { entries: [] } + }) + + await store.load() + expect(store.hasMore).toBe(true) + + // Call loadOlder with smooth: true + const outcome = await store.loadOlder({ smooth: true }) + expect(outcome).toBe('advanced') + expect(store.hasMore).toBe(true) + + // Next loadOlder drains pending run-log lines + const drainOutcome = await store.loadOlder() + expect(drainOutcome).toBe('advanced') + }) + + it('loadOlder surfaces error when not silent', async () => { + const plain30 = Array.from({ length: 30 }, (_, i) => ({ + id: 1000 - i, + level: 'info', + category: 'backup', + message: `msg ${1000 - i}`, + details: null, + created_at: new Date(Date.UTC(2026, 7, 21, 15, 0, 0) + (1000 - i) * 1000).toISOString(), + })) + + api.getActivity.mockImplementation(async (limit = 30, _category = '', beforeId = 0) => { + if (!beforeId) return plain30.slice(0, limit) + throw new Error('fetch older failed') + }) + + await store.load() + const outcome = await store.loadOlder({ silent: false }) + expect(outcome).toBe('no-progress') + expect(store.error).toBe('fetch older failed') + }) + + it('setSearchFilter pins full history when search completes all older pages', async () => { + const plain30 = Array.from({ length: 30 }, (_, i) => ({ + id: 1000 - i, + level: 'info', + category: 'backup', + message: `msg ${1000 - i}`, + details: null, + created_at: new Date(Date.UTC(2026, 7, 21, 15, 0, 0) + (1000 - i) * 1000).toISOString(), + })) + + api.getActivity.mockImplementation(async (limit = 30, _category = '', beforeId = 0) => { + if (!beforeId) return plain30.slice(0, limit) + return [] + }) + + await store.load() + expect(store.hasMore).toBe(true) + + store.setSearchFilter('needle') + await vi.waitFor(() => expect(store.searching).toBe(false)) + expect(store.hasMore).toBe(false) + }) }) From d9924b3baf63177347c11ccb730d8e5d0ac0a04b Mon Sep 17 00:00:00 2001 From: Ruaan Deysel Date: Sat, 10 Oct 2026 17:01:06 +1000 Subject: [PATCH 4/6] fix(web): bind in-flight older loads to context and guard store disposal --- CHANGELOG.md | 2 +- web/src/lib/unifiedlog.svelte.js | 52 +++++++++++++++++++++------ web/src/lib/unifiedlog.test.js | 62 ++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8322a4b3..58de0afc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/), and this ### Fixed -- **Logs page unresponsiveness and freeze when loading older entries (#454):** Fixed microtask starvation caused by concurrent older-page requests during initial mount and background loading loops (`loadAll` and search). Active older-load requests now share the in-flight Promise across callers, older requests are bound to the store context sequence to discard stale responses, and background loading loops terminate when no progress is made. Also added synchronous `onMount` cleanup and a store `dispose()` method in `Logs.svelte` to abort background activity upon page navigation. Closes #454. +- **Logs page unresponsiveness and freeze when loading older entries (#454):** Fixed microtask starvation caused by concurrent older-page requests during initial mount and background loading loops (`loadAll` and search). Active older-load requests now share the in-flight Promise across callers, older requests are bound to the store context sequence to discard stale responses, and background loading loops terminate when no progress is made. Also added a store `dispose()` method, called from the synchronous `onMount` cleanup in `Logs.svelte`, to abort background activity on page navigation. Closes #454. ## [v2026.10.00] - 2026-10-07 diff --git a/web/src/lib/unifiedlog.svelte.js b/web/src/lib/unifiedlog.svelte.js index f5049f13..d0115359 100644 --- a/web/src/lib/unifiedlog.svelte.js +++ b/web/src/lib/unifiedlog.svelte.js @@ -41,6 +41,8 @@ export function createUnifiedLogStore() { let _hasMore = $state(false) let _loadingOlder = $state(false) let _olderPromise = null + let _olderCtx = -1 + let _olderSilent = true let _oldestActivityId = $state(null) // cursor for "load older" let _category = $state('') let _levelFilter = $state('') @@ -56,6 +58,7 @@ export function createUnifiedLogStore() { let _contextSeq = 0 // bumped on every fresh load/reset: aborts an in-flight loadAll (#328) let _pendingRunLogs = [] // leftover run-log lines from a split terminal expansion (#328 r4 #5) let _fullHistoryLoaded = false // the buffer is (or is intended to become) the whole history (#328) + let _disposed = false function reset() { _entries = [] @@ -65,6 +68,8 @@ export function createUnifiedLogStore() { _hasMore = false _loadingOlder = false _olderPromise = null + _olderCtx = -1 + _olderSilent = true _oldestActivityId = null _expanded = {} _seen = {} @@ -75,18 +80,24 @@ export function createUnifiedLogStore() { } function dispose() { + _disposed = true _contextSeq++ _loadSeq++ + _loading = false _loadingOlder = false _olderPromise = null + _olderCtx = -1 + _olderSilent = true } // Category is applied server-side via load(); the job and level filters are // client-side and independent — switching category must NOT clear them, so // filters compose (#328 round 2). async function setCategory(v) { + if (_disposed) return _category = v await load() + if (_disposed) return // If the active job filter no longer appears in the (category-filtered) // buffer, fall back to All so the Job dropdown doesn't render blank when // the category has no entries for that job (#328 r3 #1). (Level and @@ -101,6 +112,7 @@ export function createUnifiedLogStore() { function setLevelFilter(v) { _levelFilter = v } function setJobFilter(v) { _jobFilter = v } function setSearchFilter(v) { + if (_disposed) return _search = v if (!v) { _searching = false @@ -117,14 +129,16 @@ export function createUnifiedLogStore() { ;(async () => { try { if (!_hasMore && _pruned) { + if (_disposed) return // The cursor is exhausted but the buffer was truncated (the scroll // window dropped its oldest entries): a search over this window // would miss the dropped older logs. Reload from the newest page // so the search covers the whole history (#328). await load() } + if (_disposed) return const ctx = _contextSeq - while (_search && _hasMore && ctx === _contextSeq) { + while (!_disposed && _search && _hasMore && ctx === _contextSeq) { // Big pages + silent: a full-history search must not make ~333 // round-trips of BATCH_SIZE, and a transient error mid-search must // not surface as the console-wide error box (the scroll path @@ -138,7 +152,7 @@ export function createUnifiedLogStore() { // while _hasMore stays false, so the "— End of logs —" marker would // display over a truncated buffer and lie about the end (#328). // (Guard on _search: a query cleared mid-loop aborts the load.) - if (_search && ctx === _contextSeq && !_hasMore) _fullHistoryLoaded = true + if (!_disposed && _search && ctx === _contextSeq && !_hasMore) _fullHistoryLoaded = true } finally { _searching = false } @@ -211,6 +225,7 @@ export function createUnifiedLogStore() { // load() — full reset: fetch the newest page and REPLACE the buffer. // Used on mount, category switch, and purge. async function load() { + if (_disposed) return return _loadNewest(true) } @@ -219,6 +234,7 @@ export function createUnifiedLogStore() { // cursor-keyed lazy-load (the "older" side is loadOlder, keyed on // _oldestActivityId). Used by the poll timer and scroll-to-bottom refresh. async function loadNewer() { + if (_disposed) return return _loadNewest(false) } @@ -227,6 +243,7 @@ export function createUnifiedLogStore() { // batch is MERGED into the loaded scrollback so poll cycles don't discard // "Load older" history. async function _loadNewest(fresh) { + if (_disposed) return const seq = ++_loadSeq _loading = true _error = '' @@ -256,6 +273,8 @@ export function createUnifiedLogStore() { _contextSeq++ _loadingOlder = false _olderPromise = null + _olderCtx = -1 + _olderSilent = true // Expand terminal entries in parallel, then merge. Only the fresh // path fetches: the merge path must NOT refetch every terminal on @@ -412,15 +431,22 @@ export function createUnifiedLogStore() { } } + // When an older-page fetch is already in flight in the current context, concurrent + // callers join the shared promise. If any joined caller is non-silent (e.g. user + // scrolling), any network failure is surfaced to _error. async function loadOlder({ smooth = false, limit = BATCH_SIZE, silent = false } = {}) { - if (_loadingOlder && _olderPromise) return _olderPromise + if (_disposed) return 'no-progress' + if (!silent) _olderSilent = false + if (_loadingOlder && _olderPromise && _olderCtx === _contextSeq) return _olderPromise if (!_hasMore && _pendingRunLogs.length === 0) return 'no-progress' _loadingOlder = true + _olderSilent = silent const ctx = _contextSeq const promise = (async () => { const startedAt = Date.now() let loaded = false try { + if (_disposed) return 'no-progress' // Drain leftover run-log lines from a previously split terminal // expansion first, so a long run's log is inserted across bounded // steps instead of one unbounded dump (#328 r4 #5). A drain step @@ -437,7 +463,7 @@ export function createUnifiedLogStore() { if (!_hasMore || !_oldestActivityId) return 'no-progress' const prevOldestId = _oldestActivityId const older = (await api.getActivity(limit, _category, prevOldestId)) || [] - if (ctx !== _contextSeq) return 'superseded' + if (_disposed || ctx !== _contextSeq) return 'superseded' if (older.length === 0) { _hasMore = false @@ -455,7 +481,7 @@ export function createUnifiedLogStore() { return d?.run_log === true && !_expanded[e.id] }) const runLogs = await fetchRunLogsForBatch(terminal) - if (ctx !== _contextSeq) return 'superseded' + if (_disposed || ctx !== _contextSeq) return 'superseded' loaded = true @@ -505,28 +531,31 @@ export function createUnifiedLogStore() { _entries = prune(dedupeSummaries(combined).sort((a, b) => tsMs(a.ts) - tsMs(b.ts))) return 'advanced' } catch (e) { - if (ctx !== _contextSeq) return 'superseded' + if (_disposed || ctx !== _contextSeq) return 'superseded' // Background full-history loads (loadAll) must not surface a transient // fetch error as the console-wide error box — they just stop and leave // the scroll path (which reports errors) to retry (#328). - if (!silent) _error = e.message || 'Failed to load older logs' + if (!_olderSilent) _error = e.message || 'Failed to load older logs' return 'no-progress' } finally { // Artificial minimum latency (see MIN_LOAD_OLDER_MS). Only hold the // floor when a batch actually loaded — an empty page means the user is // already at the oldest log, and lingering would show the spinner with // nothing to show (#328 r3 #4). - if (smooth && loaded && ctx === _contextSeq) { + if (smooth && loaded && !_disposed && ctx === _contextSeq) { const remaining = MIN_LOAD_OLDER_MS - (Date.now() - startedAt) if (remaining > 0) await sleep(remaining) } - if (ctx === _contextSeq) { + if (!_disposed && ctx === _contextSeq) { _loadingOlder = false _olderPromise = null + _olderCtx = -1 + _olderSilent = true } } })() _olderPromise = promise + _olderCtx = ctx return promise } @@ -538,11 +567,12 @@ export function createUnifiedLogStore() { // Big pages (FULL_LOAD_LIMIT) keep the round-trip count low. Stale loops // abort via _contextSeq (fresh load / category switch / reset). async function loadAll() { + if (_disposed) return if (_fullHistoryLoaded && !_hasMore) return // already complete _fullHistoryLoaded = true const ctx = _contextSeq try { - while (ctx === _contextSeq && _hasMore && !_error) { + while (!_disposed && ctx === _contextSeq && _hasMore && !_error) { const outcome = await loadOlder({ limit: FULL_LOAD_LIMIT, silent: true }) if (outcome === 'no-progress') break if (outcome === 'superseded' && ctx !== _contextSeq) break @@ -551,7 +581,7 @@ export function createUnifiedLogStore() { // If the loop was aborted by a context change, the fresh load already // reset _fullHistoryLoaded; leave it otherwise so the pinned buffer // keeps the marker honest. - if (ctx === _contextSeq) _fullHistoryLoaded = !_hasMore ? true : _fullHistoryLoaded + if (!_disposed && ctx === _contextSeq) _fullHistoryLoaded = !_hasMore ? true : _fullHistoryLoaded } } diff --git a/web/src/lib/unifiedlog.test.js b/web/src/lib/unifiedlog.test.js index 3e57d470..2d6a7cdb 100644 --- a/web/src/lib/unifiedlog.test.js +++ b/web/src/lib/unifiedlog.test.js @@ -538,4 +538,66 @@ describe('unified log store', () => { await vi.waitFor(() => expect(store.searching).toBe(false)) expect(store.hasMore).toBe(false) }) + + it('dispose prevents post-dispose calls and clears loading states', async () => { + let resolveActivity + api.getActivity.mockImplementation(() => new Promise((resolve) => { + resolveActivity = resolve + })) + + const pendingLoad = store.load() + expect(store.loading).toBe(true) + + store.dispose() + expect(store.loading).toBe(false) + expect(store.loadingOlder).toBe(false) + + // Late resolve should not mutate store or resurrect loading state + resolveActivity([]) + await pendingLoad + expect(store.loading).toBe(false) + expect(store.entries).toEqual([]) + + // Subsequent calls are no-ops + api.getActivity.mockClear() + expect(await store.loadOlder()).toBe('no-progress') + await store.loadAll() + await store.setCategory('backup') + store.setSearchFilter('test') + expect(api.getActivity).not.toHaveBeenCalled() + }) + + it('surfaces error when non-silent caller joins in-flight silent loadOlder', async () => { + const plain30 = Array.from({ length: 30 }, (_, i) => ({ + id: 1000 - i, + level: 'info', + category: 'backup', + message: `msg ${1000 - i}`, + details: null, + created_at: new Date(Date.UTC(2026, 7, 21, 15, 0, 0) + (1000 - i) * 1000).toISOString(), + })) + + let rejectOlder + api.getActivity.mockImplementation(async (limit = 30, _category = '', beforeId = 0) => { + if (!beforeId) return plain30.slice(0, limit) + return new Promise((_, reject) => { + rejectOlder = reject + }) + }) + + await store.load() + expect(store.hasMore).toBe(true) + + // First caller: silent background load + const silentPromise = store.loadOlder({ silent: true }) + // Second caller: user scrolling (non-silent) joins the in-flight load + const userPromise = store.loadOlder({ silent: false }) + + rejectOlder(new Error('network error')) + const [res1, res2] = await Promise.all([silentPromise, userPromise]) + + expect(res1).toBe('no-progress') + expect(res2).toBe('no-progress') + expect(store.error).toBe('network error') + }) }) From f826cad7e75a5d4cd172288d905eddbde720df06 Mon Sep 17 00:00:00 2001 From: Ruaan Deysel Date: Sat, 10 Oct 2026 17:02:54 +1000 Subject: [PATCH 5/6] fix(web): scope non-silent flag override to joined loadOlder caller --- web/src/lib/unifiedlog.svelte.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/web/src/lib/unifiedlog.svelte.js b/web/src/lib/unifiedlog.svelte.js index d0115359..d04b36dd 100644 --- a/web/src/lib/unifiedlog.svelte.js +++ b/web/src/lib/unifiedlog.svelte.js @@ -436,8 +436,10 @@ export function createUnifiedLogStore() { // scrolling), any network failure is surfaced to _error. async function loadOlder({ smooth = false, limit = BATCH_SIZE, silent = false } = {}) { if (_disposed) return 'no-progress' - if (!silent) _olderSilent = false - if (_loadingOlder && _olderPromise && _olderCtx === _contextSeq) return _olderPromise + if (_loadingOlder && _olderPromise && _olderCtx === _contextSeq) { + if (!silent) _olderSilent = false + return _olderPromise + } if (!_hasMore && _pendingRunLogs.length === 0) return 'no-progress' _loadingOlder = true _olderSilent = silent From f7763203fe4e6658b6781db3a75d14740d3df6ba Mon Sep 17 00:00:00 2001 From: Ruaan Deysel Date: Sat, 10 Oct 2026 17:13:59 +1000 Subject: [PATCH 6/6] fix(web): differentiate failed older loads and retry transient search errors --- web/src/lib/unifiedlog.svelte.js | 12 ++++++++++-- web/src/lib/unifiedlog.test.js | 33 +++++++++++++++++++++++++++++--- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/web/src/lib/unifiedlog.svelte.js b/web/src/lib/unifiedlog.svelte.js index d04b36dd..f14898b1 100644 --- a/web/src/lib/unifiedlog.svelte.js +++ b/web/src/lib/unifiedlog.svelte.js @@ -28,6 +28,7 @@ const OLDER_BATCH_BUDGET = 150 // max unified entries added per loadOlder step const MAX_ENTRIES = 50000 const WS_DEDUP_CAP = 200 // seen-IDs cap for WS dedup const MIN_LOAD_OLDER_MS = 700 // floor for user-initiated loadOlder so the spinner reads as real work (#328 round 2) +const SEARCH_RETRY_DELAY_MS = 250 // delay multiplier when retrying transient errors during background search function sleep(ms) { return new Promise((resolve) => setTimeout(resolve, ms)) @@ -137,6 +138,7 @@ export function createUnifiedLogStore() { await load() } if (_disposed) return + let failures = 0 const ctx = _contextSeq while (!_disposed && _search && _hasMore && ctx === _contextSeq) { // Big pages + silent: a full-history search must not make ~333 @@ -144,6 +146,12 @@ export function createUnifiedLogStore() { // not surface as the console-wide error box (the scroll path // reports errors; the background search just retries) (#328). const outcome = await loadOlder({ limit: FULL_LOAD_LIMIT, silent: true }) + if (outcome === 'failed') { + if (++failures > 3) break + await sleep(SEARCH_RETRY_DELAY_MS * failures) + continue + } + failures = 0 if (outcome === 'no-progress') break if (outcome === 'superseded' && ctx !== _contextSeq) break } @@ -538,7 +546,7 @@ export function createUnifiedLogStore() { // fetch error as the console-wide error box — they just stop and leave // the scroll path (which reports errors) to retry (#328). if (!_olderSilent) _error = e.message || 'Failed to load older logs' - return 'no-progress' + return 'failed' } finally { // Artificial minimum latency (see MIN_LOAD_OLDER_MS). Only hold the // floor when a batch actually loaded — an empty page means the user is @@ -576,7 +584,7 @@ export function createUnifiedLogStore() { try { while (!_disposed && ctx === _contextSeq && _hasMore && !_error) { const outcome = await loadOlder({ limit: FULL_LOAD_LIMIT, silent: true }) - if (outcome === 'no-progress') break + if (outcome === 'no-progress' || outcome === 'failed') break if (outcome === 'superseded' && ctx !== _contextSeq) break } } finally { diff --git a/web/src/lib/unifiedlog.test.js b/web/src/lib/unifiedlog.test.js index 2d6a7cdb..b9eb4196 100644 --- a/web/src/lib/unifiedlog.test.js +++ b/web/src/lib/unifiedlog.test.js @@ -512,7 +512,7 @@ describe('unified log store', () => { await store.load() const outcome = await store.loadOlder({ silent: false }) - expect(outcome).toBe('no-progress') + expect(outcome).toBe('failed') expect(store.error).toBe('fetch older failed') }) @@ -596,8 +596,35 @@ describe('unified log store', () => { rejectOlder(new Error('network error')) const [res1, res2] = await Promise.all([silentPromise, userPromise]) - expect(res1).toBe('no-progress') - expect(res2).toBe('no-progress') + expect(res1).toBe('failed') + expect(res2).toBe('failed') expect(store.error).toBe('network error') }) + + it('setSearchFilter retries transient failures during older loads before stopping', async () => { + const plain30 = Array.from({ length: 30 }, (_, i) => ({ + id: 1000 - i, + level: 'info', + category: 'backup', + message: `msg ${1000 - i}`, + details: null, + created_at: new Date(Date.UTC(2026, 7, 21, 15, 0, 0) + (1000 - i) * 1000).toISOString(), + })) + + let attempts = 0 + api.getActivity.mockImplementation(async (limit = 30, _category = '', beforeId = 0) => { + if (!beforeId) return plain30.slice(0, limit) + attempts++ + if (attempts === 1) throw new Error('transient error') + return [] + }) + + await store.load() + expect(store.hasMore).toBe(true) + + store.setSearchFilter('needle') + await vi.waitFor(() => expect(store.searching).toBe(false), { timeout: 3000 }) + expect(attempts).toBe(2) + expect(store.hasMore).toBe(false) + }) })