From acb43d982e6779ca7c4033d94701948346e3f773 Mon Sep 17 00:00:00 2001
From: Sucukdeluxe <259325684+Sucukdeluxe@users.noreply.github.com>
Date: Thu, 13 Aug 2026 22:28:26 +0200
Subject: [PATCH] fix: unify diagnostics history access

Resolve diagnostic history through the injected loadHistory dependency whenever it is available, retaining config history only for legacy collectors without a dedicated reader.

Build server_health error and batch summaries from one validated snapshot so migrated installations cannot combine current batches with stale errors or parse the external history twice.

Add focused regressions for migrated history consistency and fail-closed handling of invalid dedicated history results.
---
 lib/diagnostics-collectors.js        | 32 +++++++++++-----
 tests/diagnostics-collectors.test.js | 56 ++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+), 9 deletions(-)

diff --git a/lib/diagnostics-collectors.js b/lib/diagnostics-collectors.js
index bce9e38..01d5bea 100644
--- a/lib/diagnostics-collectors.js
+++ b/lib/diagnostics-collectors.js
@@ -19,6 +19,15 @@ function createCollectors(deps) {
     try { return support.collectSecretValues(loadConfig()); } catch { return []; }
   }
 
+  function _currentHistory() {
+    if (typeof loadHistory === 'function') {
+      const history = loadHistory();
+      return Array.isArray(history) ? history : [];
+    }
+    const cfg = loadConfig();
+    return Array.isArray(cfg && cfg.history) ? cfg.history : [];
+  }
+
   function _deepRedact(value, secrets) {
     return support.valueScrub(value, secrets || _secrets());
   }
@@ -162,15 +171,18 @@ function createCollectors(deps) {
     return { errors, byCategory };
   }
 
-  function listErrors(args) {
+  function _listErrors(args, history) {
     const a = args || {};
-    const cfg = loadConfig();
-    const { errors, byCategory } = _historyErrors(cfg.history, a);
+    const { errors, byCategory } = _historyErrors(history, a);
     const limit = Math.min(Math.max(Number(a.limit) || 100, 1), 1000);
     const window = Number.isFinite(a.sinceMs) ? `since ${new Date(a.sinceMs).toISOString()}` : 'all history';
     return { window, total: errors.length, byCategory, errors: errors.slice(-limit) };
   }
 
+  function listErrors(args) {
+    return _listErrors(args, _currentHistory());
+  }
+
   function getQueueState(args) {
     const a = args || {};
     const cfg = loadConfig();
@@ -200,11 +212,8 @@ function createCollectors(deps) {
     return result;
   }
 
-  function getHistory(args) {
+  function _getHistory(args, history) {
     const a = args || {};
-    const history = typeof loadHistory === 'function'
-      ? (loadHistory() || [])
-      : (Array.isArray(loadConfig().history) ? loadConfig().history : []);
     const limit = Math.min(Math.max(Number(a.limit) || 20, 1), 200);
     const perHoster = stats.summarizePerHoster(history);
     const recent = [...history].slice(-limit).reverse();
@@ -227,6 +236,10 @@ function createCollectors(deps) {
     return { totalBatches: history.length, returned: batches.length, perHoster, batches };
   }
 
+  function getHistory(args) {
+    return _getHistory(args, _currentHistory());
+  }
+
   function getRotationState() {
     const cfg = loadConfig();
     return { rotationCursors: _deepRedact(cfg.rotationCursors || {}) };
@@ -246,9 +259,10 @@ function createCollectors(deps) {
     const a = args || {};
     const errorLimit = Math.min(Math.max(Number(a.errorLimit) || 20, 1), 200);
     const errArgs = Number.isFinite(a.errorSinceMs) ? { sinceMs: a.errorSinceMs, limit: errorLimit } : { limit: errorLimit };
-    const errors = listErrors(errArgs);
+    const currentHistory = _currentHistory();
+    const errors = _listErrors(errArgs, currentHistory);
     const queue = getQueueState({ includeJobs: false });
-    const history = getHistory({ limit: 5 });
+    const history = _getHistory({ limit: 5 }, currentHistory);
     const warnings = [];
     if (queue.source === 'persisted' && queue.stale) warnings.push('queue state is from the persisted snapshot (may lag live state; UploadManager not introspected in this build).');
     if (errors.total > 0) warnings.push(`${errors.total} non-success result(s) in the error window.`);
diff --git a/tests/diagnostics-collectors.test.js b/tests/diagnostics-collectors.test.js
index b146f67..e115e4b 100644
--- a/tests/diagnostics-collectors.test.js
+++ b/tests/diagnostics-collectors.test.js
@@ -86,6 +86,62 @@ test('getHistory falls back to loadConfig().history when loadHistory is absent (
   assert.equal(c.getHistory({ limit: 10 }).totalBatches, 1, 'legacy path reads load().history when loadHistory not injected');
 });
 
+test('getHistory, listErrors and serverHealth share loadHistory after migration', () => {
+  let historyRevision = 0;
+  const c = createCollectors({
+    loadConfig: () => ({ hosters: {}, globalSettings: {}, history: [] }),
+    loadHistory: () => {
+      historyRevision++;
+      const timestamp = `2026-01-${String(historyRevision).padStart(2, '0')}T00:00:00.000Z`;
+      return [{ timestamp, files: [{ name: `failed-${historyRevision}.mkv`, results: [{ hoster: 'voe.sx', status: 'error', error: 'Not video file format' }] }] }];
+    },
+    getAllLogPaths: () => ({ logDir: os.tmpdir() }),
+    support, stats,
+    appInfo: () => ({}), systemInfo: () => ({}), agentInfo: () => ({})
+  });
+  const health = c.serverHealth({ errorLimit: 5 });
+  const history = c.getHistory({ limit: 5 });
+  const errors = c.listErrors({ limit: 5 });
+  assert.deepEqual({
+    historyBatches: history.totalBatches,
+    listedErrors: errors.total,
+    healthBatches: health.recentBatches.length,
+    healthErrors: health.errors.total
+  }, {
+    historyBatches: 1,
+    listedErrors: 1,
+    healthBatches: 1,
+    healthErrors: 1
+  });
+  assert.equal(health.recentBatches[0].timestamp, health.errors.errors[0].ts, 'serverHealth must summarize one history snapshot');
+});
+
+test('history commands fail closed instead of reading stale config history', () => {
+  const c = createCollectors({
+    loadConfig: () => ({
+      hosters: {},
+      globalSettings: {},
+      history: [{ timestamp: '2025-12-31T00:00:00.000Z', files: [{ name: 'stale.mkv', results: [{ hoster: 'voe.sx', status: 'error', error: 'stale error' }] }] }]
+    }),
+    loadHistory: () => null,
+    getAllLogPaths: () => ({ logDir: os.tmpdir() }),
+    support, stats,
+    appInfo: () => ({}), systemInfo: () => ({}), agentInfo: () => ({})
+  });
+  const health = c.serverHealth({ errorLimit: 5 });
+  assert.deepEqual({
+    historyBatches: c.getHistory({ limit: 5 }).totalBatches,
+    listedErrors: c.listErrors({ limit: 5 }).total,
+    healthBatches: health.recentBatches.length,
+    healthErrors: health.errors.total
+  }, {
+    historyBatches: 0,
+    listedErrors: 0,
+    healthBatches: 0,
+    healthErrors: 0
+  });
+});
+
 test('readLog redacts a planted token and a Bearer line; doodstream is NOT readable; unknown name rejected', () => {
   const { collectors, dir, paths } = makeFixture();
   const dbg = collectors.readLog({ name: 'debug', tailKb: 64 });