fix(hardening): make six silent fallbacks distinguishable from success (TASK-099)

backend/__tests__/unit/models/pg/message.activityHint.test.js

@@ -63,6 +63,25 @@ describe('Message.findActivityHint', () => {
   it('a query failure degrades to a zero hint rather than throwing', async () => {
     // The heartbeat's pod-selection pass must not die because the hint did.
     pool.query.mockRejectedValueOnce(new Error('connection terminated'));
-    await expect(Message.findActivityHint(POD, SINCE)).resolves.toEqual({ count: 0, lastAt: null });
+    await expect(Message.findActivityHint(POD, SINCE))
+      .resolves.toEqual({ count: 0, lastAt: null, unavailable: true });
+  });
+
+  // TASK-099. Degrading is right; degrading INDISTINGUISHABLY is the defect.
+  // `count: 0` is exactly what a genuinely quiet pod returns, so without a
+  // discriminator the caller — and, through the heartbeat prompt, every agent —
+  // reads a Postgres outage as "nothing happened here".
+  it('a real zero and a failed zero are distinguishable', async () => {
+    pool.query.mockResolvedValueOnce({ rows: [{ count: '0', last_at: null }] });
+    const quiet = await Message.findActivityHint(POD, SINCE);
+
+    pool.query.mockRejectedValueOnce(new Error('connection terminated'));
+    const broken = await Message.findActivityHint(POD, SINCE);
+
+    expect(quiet.count).toBe(0);
+    expect(broken.count).toBe(0);
+    expect(quiet.unavailable).toBeUndefined();
+    expect(broken.unavailable).toBe(true);
+    expect(quiet).not.toEqual(broken);
   });
 });

backend/__tests__/unit/routes/agentsRuntime.contextBudget.test.js

@@ -0,0 +1,113 @@
+/**
+ * TASK-099 site 9 — `maxContextTokens: 0` means UNCAPPED to PodContextService,
+ * so it must not also be a failure value.
+ *
+ * These close a gap @sprint-review found by mutation on #1519: the new 400 and
+ * the new `contextBudget` field shipped with no test, so deleting either left
+ * the suite fully green. That is exactly the defect the PR is about — a
+ * fallback nobody can observe — one level up, in the coverage rather than the
+ * code. Each case below pairs the failure with the success value it collided
+ * with, the same shape as the service tests.
+ */
+
+jest.mock('jsonwebtoken', () => ({ sign: jest.fn(), verify: jest.fn(), decode: jest.fn() }));
+
+jest.mock('../../../middleware/agentRuntimeAuth', () => (req, res, next) => {
+  req.agentUser = { _id: 'bot-1' };
+  req.agentInstallations = [{ podId: 'pod-1', status: 'active', agentName: 'a', instanceId: 'i' }];
+  req.agentAuthorizedPodIds = ['pod-1'];
+  next();
+});
+jest.mock('../../../middleware/auth', () => (req, res, next) => next());
+jest.mock('../../../middleware/apiTokenScopes', () => ({
+  requireApiTokenScopes: () => (req, res, next) => next(),
+}));
+
+jest.mock('../../../services/agentEventService', () => ({}));
+jest.mock('../../../services/agentIdentityService', () => ({
+  buildAgentUsername: jest.fn((a) => a),
+  getOrCreateAgentUser: jest.fn().mockResolvedValue({ _id: 'agent-user-1' }),
+  ensureAgentInPod: jest.fn().mockResolvedValue(undefined),
+}));
+jest.mock('../../../services/agentMessageService', () => ({ getRecentMessages: jest.fn() }));
+jest.mock('../../../services/agentThreadService', () => ({}));
+jest.mock('../../../services/podContextService', () => ({
+  getPodContext: jest.fn().mockResolvedValue({ _status: 'success', stats: {} }),
+}));
+jest.mock('../../../services/globalModelConfigService', () => ({ getConfig: jest.fn() }));
+jest.mock('../../../services/socialPolicyService', () => ({}));
+jest.mock('../../../integrations', () => ({ get: jest.fn() }));
+jest.mock('../../../models/Activity', () => ({}));
+jest.mock('../../../models/User', () => ({ findById: jest.fn() }));
+jest.mock('../../../models/Post', () => ({ findById: jest.fn() }));
+jest.mock('../../../models/Pod', () => ({ find: jest.fn() }));
+jest.mock('../../../services/dmService', () => ({ getOrCreateAgentDM: jest.fn() }));
+jest.mock('../../../models/Integration', () => ({ find: jest.fn(), findOne: jest.fn() }));
+jest.mock('../../../models/AgentRegistry', () => ({
+  AgentInstallation: { findOne: jest.fn(), find: jest.fn() },
+}));
+
+const express = require('express');
+const request = require('supertest');
+const GlobalModelConfigService = require('../../../services/globalModelConfigService');
+const PodContextService = require('../../../services/podContextService');
+const router = require('../../../routes/agentsRuntime');
+
+const app = express();
+app.use(express.json());
+app.use('/api/agents/runtime', router);
+
+const get = (qs = '') => request(app).get(`/api/agents/runtime/pods/pod-1/context${qs}`);
+const budgetPassedToService = () => PodContextService.getPodContext.mock.calls[0][0].maxContextTokens;
+
+describe('GET /pods/:podId/context — the context budget (TASK-099 site 9)', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+    PodContextService.getPodContext.mockResolvedValue({ _status: 'success', stats: {} });
+  });
+
+  it('a malformed maxContextTokens is a 400, not a silent uncap', async () => {
+    // parseLimit's NaN fallback is 0, and 0 means UNCAPPED downstream — so the
+    // pre-fix behaviour was to honour a typo by removing the budget entirely.
+    const res = await get('?maxContextTokens=abc');
+    expect(res.status).toBe(400);
+    expect(res.body.error).toMatch(/maxContextTokens/);
+    expect(PodContextService.getPodContext).not.toHaveBeenCalled();
+  });
+
+  it('a well-formed maxContextTokens is honoured and attributed to the query', async () => {
+    const res = await get('?maxContextTokens=8000');
+    expect(res.status).toBe(200);
+    expect(budgetPassedToService()).toBe(8000);
+    expect(res.body.contextBudget).toEqual({ applied: true, source: 'query' });
+  });
+
+  it('a configured contextLimit reserves 25% and says so', async () => {
+    GlobalModelConfigService.getConfig.mockResolvedValue({ llmService: { contextLimit: 100000 } });
+    const res = await get();
+    expect(res.status).toBe(200);
+    expect(budgetPassedToService()).toBe(75000);
+    expect(res.body.contextBudget).toEqual({ applied: true, source: 'model-config' });
+  });
+
+  it('an UNCONFIGURED contextLimit is uncapped and named as unconfigured', async () => {
+    GlobalModelConfigService.getConfig.mockResolvedValue({ llmService: {} });
+    const res = await get();
+    expect(budgetPassedToService()).toBe(0);
+    expect(res.body.contextBudget).toEqual({ applied: false, source: 'unconfigured' });
+  });
+
+  it('a config-store OUTAGE is uncapped too, and is distinguishable from unconfigured', async () => {
+    // Both send `maxContextTokens: 0`, which is the whole problem: the response
+    // field is the only thing that can tell an operator which one happened.
+    GlobalModelConfigService.getConfig.mockRejectedValue(new Error('mongo down'));
+    const warn = jest.spyOn(console, 'warn').mockImplementation(() => {});
+
+    const res = await get();
+    expect(budgetPassedToService()).toBe(0);
+    expect(res.body.contextBudget).toEqual({ applied: false, source: 'config-unavailable' });
+    expect(warn).toHaveBeenCalled();
+
+    warn.mockRestore();
+  });
+});

backend/__tests__/unit/services/agentInstallationCleanup.failureCount.test.js

@@ -0,0 +1,91 @@
+/**
+ * TASK-099 site 3 — `{ marked: 0 }` must not mean both "nothing was stale" and
+ * "every staleness check threw".
+ *
+ * The per-item catch logs and continues, so the returned count is a LOWER BOUND
+ * reported as a total. That is loud at the log and silent at the return value,
+ * and the return value is what the runner prints and what a caller could gate on.
+ *
+ * Closes a gap @sprint-review found by mutation on #1519: stopping the failure
+ * counter left the suite green, because the existing staleness suite only ever
+ * destructures `marked`.
+ */
+
+jest.mock('node-cron', () => ({ schedule: jest.fn() }));
+
+jest.mock('../../../models/AgentRegistry', () => ({
+  AgentInstallation: { find: jest.fn(), updateMany: jest.fn(), deleteMany: jest.fn() },
+}));
+jest.mock('../../../models/AgentEvent', () => ({ findOne: jest.fn() }));
+jest.mock('../../../models/User', () => ({ findOne: jest.fn() }));
+jest.mock('../../../models/Pod', () => ({ updateOne: jest.fn() }));
+jest.mock('../../../services/agentIdentityService', () => ({
+  buildAgentUsername: (name, instanceId) => `${name}__${instanceId}`,
+}));
+
+const { AgentInstallation } = require('../../../models/AgentRegistry');
+const AgentEvent = require('../../../models/AgentEvent');
+const User = require('../../../models/User');
+const { markStaleInstallations } = require('../../../services/agentInstallationCleanupService');
+
+const givenActiveInstalls = (n) => {
+  const rows = Array.from({ length: n }, (_, i) => ({
+    _id: `i${i}`, agentName: 'telegram-app', instanceId: `inst-${i}`, podId: 'p1',
+  }));
+  AgentInstallation.find.mockReturnValue({ select: () => ({ lean: async () => rows }) });
+};
+
+// A live token: unexpired, so the pair is spared and nothing is marked. This is
+// the SUCCESS TWIN of the failure case — both return `marked: 0`.
+const givenLiveToken = () => {
+  User.findOne.mockReturnValue({
+    select: () => ({ lean: async () => ({ agentRuntimeTokens: [{ expiresAt: new Date(Date.now() + 864e5) }] }) }),
+  });
+};
+
+beforeEach(() => {
+  jest.clearAllMocks();
+  AgentInstallation.updateMany.mockResolvedValue({ modifiedCount: 0 });
+  AgentEvent.findOne.mockReturnValue({ select: () => ({ sort: () => ({ lean: async () => null }) }) });
+});
+
+describe('markStaleInstallations — a failed sweep differs from a clean one', () => {
+  it('a clean sweep with nothing stale reports zero failures', async () => {
+    givenActiveInstalls(2);
+    givenLiveToken();
+    const result = await markStaleInstallations(7);
+    expect(result).toEqual({ marked: 0, evaluationFailures: 0 });
+  });
+
+  it('a sweep that threw on every pair reports the same marked:0 AND its failures', async () => {
+    givenActiveInstalls(2);
+    const err = jest.spyOn(console, 'error').mockImplementation(() => {});
+    User.findOne.mockImplementation(() => { throw new Error('mongo down'); });
+
+    const result = await markStaleInstallations(7);
+
+    // The count alone is indistinguishable from the clean sweep above — which is
+    // why the second field has to exist.
+    expect(result.marked).toBe(0);
+    expect(result.evaluationFailures).toBe(2);
+    expect(err).toHaveBeenCalledTimes(2);
+
+    err.mockRestore();
+  });
+
+  it('a PARTIAL failure is reported as partial, not rounded to either end', async () => {
+    givenActiveInstalls(2);
+    const err = jest.spyOn(console, 'error').mockImplementation(() => {});
+    let call = 0;
+    User.findOne.mockImplementation(() => {
+      call += 1;
+      if (call === 1) throw new Error('mongo down');
+      return { select: () => ({ lean: async () => ({ agentRuntimeTokens: [{ expiresAt: new Date(Date.now() + 864e5) }] }) }) };
+    });
+
+    const result = await markStaleInstallations(7);
+
+    expect(result.evaluationFailures).toBe(1);
+    err.mockRestore();
+  });
+});

backend/__tests__/unit/services/silentFailure.distinguishable.test.js

@@ -0,0 +1,160 @@
+/**
+ * TASK-099 — silent-failure sweep.
+ *
+ * The row's rule: every fallback that hides an error fails LOUD or fails
+ * CLOSED with a log, never a template.
+ *
+ * The discriminator these tests encode, one level sharper than "does it
+ * log": a fallback is a SILENT FAILURE iff the value it returns is reachable
+ * on the SUCCESS path without being the documented fallback. `count: 0`,
+ * `items: []`, `marked: 0`, `null` and `maxContextTokens: 0` are all ordinary
+ * success values somewhere in this codebase, so returning one on failure
+ * makes the failure unobservable by construction — no amount of logging at
+ * the throw site changes what the CALLER can see.
+ *
+ * So each case below pairs the failure with its success TWIN and asserts the
+ * two differ. A test that only exercised the failure arm would pass against
+ * the very code these fixes replace.
+ */
+
+const path = require('path');
+
+describe('TASK-099 — a failed fallback is distinguishable from its success twin', () => {
+  describe('skillsCatalogService.loadCatalog', () => {
+    const CATALOG = '/tmp/task099-catalog.json';
+    let fs;
+    let loadCatalog;
+    let invalidateCache;
+
+    beforeEach(() => {
+      jest.resetModules();
+      jest.doMock('fs', () => ({
+        existsSync: jest.fn(),
+        statSync: jest.fn(),
+        readFileSync: jest.fn(),
+      }));
+      process.env.SKILLS_CATALOG_PATH = CATALOG;
+      fs = require('fs');
+      ({ loadCatalog, invalidateCache } = require('../../../services/skillsCatalogService'));
+      invalidateCache();
+    });
+
+    afterEach(() => {
+      delete process.env.SKILLS_CATALOG_PATH;
+      jest.dontMock('fs');
+    });
+
+    it('returns an empty catalog when the file is legitimately absent', () => {
+      fs.existsSync.mockReturnValue(false);
+      expect(loadCatalog()).toEqual({ source: 'awesome', updatedAt: null, items: [] });
+    });
+
+    it('THROWS on an unreadable file instead of returning that same empty catalog', () => {
+      fs.existsSync.mockReturnValue(true);
+      fs.statSync.mockReturnValue({ mtimeMs: 1 });
+      fs.readFileSync.mockImplementation(() => { throw new Error('EACCES'); });
+      // Previously this returned `{ items: [] }` and GET /api/skills/catalog
+      // answered 200 "no skills" for a catalog nobody could read.
+      expect(() => loadCatalog()).toThrow(/unreadable/i);
+    });
+
+    it('THROWS on malformed JSON too — a parse failure is not an empty catalog', () => {
+      fs.existsSync.mockReturnValue(true);
+      fs.statSync.mockReturnValue({ mtimeMs: 2 });
+      fs.readFileSync.mockReturnValue('{ not json');
+      expect(() => loadCatalog()).toThrow(/unreadable/i);
+    });
+  });
+
+  describe('telegramBridgeService.findLiveIntegration', () => {
+    it('logs when the lookup THROWS, because null also means "no bridge here"', async () => {
+      jest.resetModules();
+      const Integration = { findOne: jest.fn(), findByIdAndUpdate: jest.fn() };
+      jest.doMock('../../../models/Integration', () => Integration);
+      jest.doMock('../../../services/telegramService', () => ({ sendMessage: jest.fn() }));
+
+      const bridge = require('../../../services/telegramBridgeService');
+      const errSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
+
+      // Success twin: no live bridge for this pod. Must stay quiet.
+      Integration.findOne.mockReturnValue({ lean: () => Promise.resolve(null) });
+      await bridge.relayAgentMessageToTelegram({
+        podId: 'p1', agentUsername: 'a', displayName: 'A', content: '[DECISION] x',
+      });
+      const quietCalls = errSpy.mock.calls.filter((c) => String(c[0]).includes('findLiveIntegration'));
+      expect(quietCalls).toHaveLength(0);
+
+      // Failure arm: the store threw. Same `null`, but it must be audible.
+      Integration.findOne.mockReturnValue({ lean: () => Promise.reject(new Error('mongo down')) });
+      await bridge.relayAgentMessageToTelegram({
+        podId: 'p1', agentUsername: 'a', displayName: 'A', content: '[DECISION] x',
+      });
+      const loudCalls = errSpy.mock.calls.filter((c) => String(c[0]).includes('findLiveIntegration'));
+      expect(loudCalls).toHaveLength(1);
+
+      errSpy.mockRestore();
+    });
+  });
+
+  describe('agentAvatarService.parseDesignDescription', () => {
+    it('tags the default design so a parse failure is not reported as a clean SVG', () => {
+      jest.resetModules();
+      const AgentAvatarService = require('../../../services/agentAvatarService');
+      const svc = AgentAvatarService.default || AgentAvatarService;
+      const warn = jest.spyOn(console, 'warn').mockImplementation(() => {});
+
+      const parsed = svc.parseDesignDescription('noise {"style":"abstract","colors":["#111"]} tail');
+      expect(parsed.fallbackReason).toBeUndefined();
+      expect(warn).not.toHaveBeenCalled();
+
+      const fell = svc.parseDesignDescription('there is no json here at all');
+      expect(fell.fallbackReason).toBe('design-parse-failed');
+      expect(warn).toHaveBeenCalled();
+
+      warn.mockRestore();
+    });
+  });
+
+  describe('schedulerService.buildHeartbeatActivityHint', () => {
+    // This hint is shipped VERBATIM into the heartbeat payload the agent reads,
+    // so the pg arm's failure value does not stop at the service boundary — it
+    // becomes a sentence in a prompt. `hasRecentActivity: false` asserted from a
+    // failed read is the platform lying to every agent in the pod.
+    const buildHint = ({ pgHint, posts = [] }) => {
+      jest.resetModules();
+      jest.doMock('../../../services/agentEventService', () => ({ enqueue: jest.fn() }));
+      jest.doMock('../../../models/pg/Message', () => ({
+        findActivityHint: jest.fn().mockResolvedValue(pgHint),
+      }));
+      jest.doMock('../../../models/Post', () => ({
+        aggregate: jest.fn().mockResolvedValue(posts),
+      }));
+      const instance = require('../../../services/schedulerService');
+      const SchedulerService = instance.constructor;
+      return SchedulerService.buildHeartbeatActivityHint({ podId: 'p1', now: new Date() });
+    };
+
+    it('a genuinely quiet pod reads false', async () => {
+      const hint = await buildHint({ pgHint: { count: 0, lastAt: null } });
+      expect(hint.hasRecentActivity).toBe(false);
+      expect(hint.messageCountUnavailable).toBe(false);
+    });
+
+    it('an unreadable message store reads null — unknown, not quiet', async () => {
+      const hint = await buildHint({ pgHint: { count: 0, lastAt: null, unavailable: true } });
+      expect(hint.hasRecentActivity).toBeNull();
+      expect(hint.messageCountUnavailable).toBe(true);
+    });
+
+    it('a positive signal from the OTHER arm still reads true despite the failure', async () => {
+      // Only a zero is unknowable. Posts answered, so activity is established
+      // even though the message count is missing.
+      const hint = await buildHint({
+        pgHint: { count: 0, lastAt: null, unavailable: true },
+        posts: [{ _id: null, count: 3, lastAt: new Date() }],
+      });
+      expect(hint.hasRecentActivity).toBe(true);
+      expect(hint.messageCountUnavailable).toBe(true);
+    });
+  });
+});