fix(hardening): 503 means unavailable, and an unrouted quote-reply says so (TASK-099 sites 4, 7)

backend/__tests__/unit/routes/summaries.allPostsFailure.test.js

@@ -0,0 +1,134 @@
+// TASK-099 site 4 — follow-up to #1501.
+//
+// #1501 stopped `GET /api/summaries/all-posts` fabricating a filler summary
+// and made the handler fail closed with 503. It did so UNCONDITIONALLY, so
+// every throw out of `summarizeAllPosts` — a Mongo error, a TypeError in the
+// post mapping, anything — reported as a transient outage. A 503 is an
+// instruction to retry, and retrying a code defect never succeeds.
+//
+// These two cases differ only in the error the service throws. Before the
+// fix both answered 503; the second is the one that must not.
+
+jest.mock('../../../middleware/auth', () => (req, res, next) => {
+  req.user = { id: 'user1' };
+  req.userId = 'user1';
+  next();
+});
+
+const mockSummarizeAllPosts = jest.fn();
+jest.mock('../../../services/summarizerService', () => {
+  // Mirror the real module: it exports the singleton AND the marker constant
+  // the route matches on. A suite that mocks only the method leaves the
+  // constant undefined, which is exactly the regression the route's `||`
+  // fallback exists to stop — covered below.
+  class SummaryUnavailableError extends Error {
+    constructor(message) {
+      super(message);
+      this.name = 'SummaryUnavailableError';
+      this.code = 'summary_unavailable';
+    }
+  }
+  return {
+    summarizeAllPosts: (...args) => mockSummarizeAllPosts(...args),
+    SUMMARY_UNAVAILABLE: 'summary_unavailable',
+    SummaryUnavailableError,
+    constructor: { getRecentSummaries: jest.fn(), garbageCollectForDigest: jest.fn() },
+  };
+});
+
+jest.mock('../../../services/chatSummarizerService', () => ({
+  getMultiplePodSummaries: jest.fn(),
+  summarizePodMessages: jest.fn(),
+  constructor: { getRecentChatSummariesByPodType: jest.fn(), getLatestPodSummary: jest.fn() },
+}));
+jest.mock('../../../services/schedulerService', () => ({
+  getStatus: jest.fn(),
+  constructor: { triggerSummarizer: jest.fn(), summarizeIntegrationBuffers: jest.fn(), dispatchPodSummaryRequests: jest.fn() },
+}));
+jest.mock('../../../services/dailyDigestService', () => ({
+  generateUserDailyDigest: jest.fn(),
+  generateAllDailyDigests: jest.fn().mockResolvedValue([]),
+}));
+jest.mock('../../../services/agentEventService', () => ({ enqueue: jest.fn() }));
+jest.mock('../../../models/AgentRegistry', () => ({
+  AgentInstallation: { find: jest.fn().mockReturnValue({ select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue([]) }) }) },
+}));
+jest.mock('../../../services/dmService', () => ({ canViewPod: jest.fn() }));
+jest.mock('../../../models/Pod');
+jest.mock('../../../models/User');
+jest.mock('../../../models/Summary');
+
+const request = require('supertest');
+const express = require('express');
+
+const { SummaryUnavailableError } = require('../../../services/summarizerService');
+const routes = require('../../../routes/summaries');
+
+describe('GET /api/summaries/all-posts — 503 means unavailable, not broken', () => {
+  let app;
+  let errorSpy;
+
+  beforeEach(() => {
+    app = express();
+    app.use(express.json());
+    app.use('/api/summaries', routes);
+    jest.clearAllMocks();
+    errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
+  });
+
+  afterEach(() => errorSpy.mockRestore());
+
+  it('answers 503 when the LLM is unavailable', async () => {
+    mockSummarizeAllPosts.mockRejectedValue(
+      new SummaryUnavailableError('All-posts summary generation failed at the LLM: connect ECONNREFUSED'),
+    );
+
+    const res = await request(app).get('/api/summaries/all-posts');
+
+    expect(res.status).toBe(503);
+    expect(res.body).toEqual({ error: 'summary_unavailable' });
+  });
+
+  it('answers 503 when the rate-limit cooldown is armed', async () => {
+    mockSummarizeAllPosts.mockRejectedValue(
+      new SummaryUnavailableError('All-posts summary generation is cooling down after an LLM rate limit'),
+    );
+
+    const res = await request(app).get('/api/summaries/all-posts');
+
+    expect(res.status).toBe(503);
+  });
+
+  it('answers 500 — not 503 — for a code defect inside the summarizer', async () => {
+    // The measured case: injecting `(undefined).boom()` at the top of
+    // summarizeAllPosts used to return `503 summary_unavailable`.
+    mockSummarizeAllPosts.mockRejectedValue(
+      new TypeError("Cannot read properties of undefined (reading 'boom')"),
+    );
+
+    const res = await request(app).get('/api/summaries/all-posts');
+
+    expect(res.status).toBe(500);
+    expect(res.body).not.toEqual({ error: 'summary_unavailable' });
+  });
+
+  it('answers 500 for a datastore failure, which is not this endpoint being unavailable', async () => {
+    // A Post.find() failure is a dependency outage, but it is not the one the
+    // 503 sentence claims, and it is not something the caller fixes by
+    // retrying this route. Fail closed loudly rather than mislabel it.
+    mockSummarizeAllPosts.mockRejectedValue(new Error('MongoNetworkError: connection 3 to db timed out'));
+
+    const res = await request(app).get('/api/summaries/all-posts');
+
+    expect(res.status).toBe(500);
+  });
+
+  it('still returns the summary on the success path', async () => {
+    mockSummarizeAllPosts.mockResolvedValue({ title: 'Community Overview • 3 recent posts', content: 'x', metadata: {} });
+
+    const res = await request(app).get('/api/summaries/all-posts');
+
+    expect(res.status).toBe(200);
+    expect(res.body.title).toBe('Community Overview • 3 recent posts');
+  });
+});

backend/__tests__/unit/services/llmFallbacks.failClosed.test.ts

@@ -68,3 +68,53 @@ describe('LLM summaries fail closed', () => {
       .rejects.toThrow('LLM unavailable');
   });
 });
+
+// TASK-099 site 4. Failing closed is the first half; saying WHICH failure is
+// the second. `GET /api/summaries/all-posts` turns a throw from here into a
+// 503, and 503 is an instruction to retry — correct for an LLM outage,
+// actively wrong for a defect in this method. The route reads `.code`, so the
+// tag has to be applied at the throw site, and only at the two causes the
+// route's own comment names.
+describe('summarizeAllPosts tags the causes that are genuinely unavailability', () => {
+  const postsQuery = (result) => {
+    const query = {
+      populate: jest.fn(),
+      sort: jest.fn(),
+      limit: jest.fn(),
+      lean: jest.fn(),
+    };
+    query.populate.mockReturnValue(query);
+    query.sort.mockReturnValue(query);
+    query.limit.mockReturnValue(query);
+    if (result instanceof Error) query.lean.mockRejectedValue(result);
+    else query.lean.mockResolvedValue(result);
+    return query;
+  };
+  const onePost = [{ content: 'A real post', tags: [], userId: { username: 'lily' } }];
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it("tags an LLM failure with code 'summary_unavailable'", async () => {
+    Post.find.mockReturnValueOnce(postsQuery(onePost));
+    generateText.mockRejectedValueOnce(new Error('connect ECONNREFUSED'));
+
+    const err = await summarizerService.summarizeAllPosts().catch((e) => e);
+
+    expect(err.code).toBe('summary_unavailable');
+    // The original message survives inside the wrapper: the outer catch still
+    // scans it for '429' / 'Resource exhausted' to arm the cooldown, so
+    // replacing the text rather than interpolating it would disarm that.
+    expect(err.message).toContain('connect ECONNREFUSED');
+  });
+
+  it('leaves a datastore failure untagged, so the route reports it as a server error', async () => {
+    Post.find.mockReturnValueOnce(postsQuery(new Error('MongoNetworkError: connection 3 to db timed out')));
+
+    const err = await summarizerService.summarizeAllPosts().catch((e) => e);
+
+    expect(err.message).toContain('MongoNetworkError');
+    expect(err.code).toBeUndefined();
+  });
+});

backend/__tests__/unit/services/telegramBridgeService.test.js

@@ -93,4 +93,71 @@ describe('telegramBridgeService — quote-reply routing', () => {
     expect(out.routedAgent).toBeNull();
     expect(out.content).toBe('plain message');
   });
+
+  // TASK-099 site 7. The two cases below returned an identical value, so a
+  // caller could not tell "nobody quoted anything" from "someone quoted a
+  // relayed line and we failed to route it". Both still relay; only the
+  // second is a failure, and `replyStatus` is the only thing that says so.
+  describe('a quote-reply that does not route is distinguishable from no quote', () => {
+    it('reports not-a-reply when there was no quote', () => {
+      const out = routeReplyContent({ content: 'plain message', replyToTgMessageId: null, relayMap });
+      expect(out.replyStatus).toBe('not-a-reply');
+      expect(out.routedAgent).toBeNull();
+    });
+
+    it('reports unmatched when the quoted message is absent from the relayMap', () => {
+      const out = routeReplyContent({
+        content: 'unrelated reply',
+        replyToTgMessageId: '999',
+        relayMap,
+      });
+      expect(out.replyStatus).toBe('unmatched');
+      expect(out.routedAgent).toBeNull();
+      // The routing behaviour is deliberately unchanged: still relayed, still
+      // unaddressed. Only the reporting is new.
+      expect(out.content).toBe('unrelated reply');
+    });
+
+    it('reports unmatched when the entry aged out past RELAY_MAP_CAP', () => {
+      // The eviction case, which is why this is not a corner: the writer
+      // $slices the map to the newest 100 entries while Telegram scrollback
+      // keeps every relayed message long-pressable.
+      const evicted = Array.from({ length: 100 }, (_, i) => ({
+        tgMessageId: String(1000 + i),
+        agentUsername: 'gene-fix-agent',
+      }));
+      const out = routeReplyContent({
+        content: 'reply to something old',
+        replyToTgMessageId: '101',
+        relayMap: evicted,
+      });
+      expect(out.replyStatus).toBe('unmatched');
+      expect(out.routedAgent).toBeNull();
+    });
+
+    it('reports unmatched when the integration has no relayMap at all', () => {
+      const out = routeReplyContent({ content: 'reply', replyToTgMessageId: '101', relayMap: undefined });
+      expect(out.replyStatus).toBe('unmatched');
+    });
+
+    it('reports unmatched when the matched entry carries no agentUsername', () => {
+      const out = routeReplyContent({
+        content: 'reply',
+        replyToTgMessageId: '101',
+        relayMap: [{ tgMessageId: '101', agentUsername: null }],
+      });
+      expect(out.replyStatus).toBe('unmatched');
+      expect(out.routedAgent).toBeNull();
+    });
+
+    it('reports routed on a hit', () => {
+      const out = routeReplyContent({
+        content: 'looks wrong',
+        replyToTgMessageId: '101',
+        relayMap,
+      });
+      expect(out.replyStatus).toBe('routed');
+      expect(out.routedAgent).toBe('gene-fix-agent');
+    });
+  });
 });

backend/routes/summaries.ts

@@ -2,6 +2,12 @@
 const express = require('express');
 // eslint-disable-next-line global-require
 const summarizerService = require('../services/summarizerService');
+// The `|| ...` is not defensive noise: several suites mock
+// `services/summarizerService` wholesale, and a destructure of an absent
+// export yields `undefined` — which would then match every error that carries
+// no `code` and restore the exact unconditional-503 behaviour this replaces.
+// eslint-disable-next-line global-require
+const SUMMARY_UNAVAILABLE = require('../services/summarizerService').SUMMARY_UNAVAILABLE || 'summary_unavailable';
 // eslint-disable-next-line global-require
 const Summary = require('../models/Summary');
 // eslint-disable-next-line global-require
@@ -257,10 +263,23 @@ router.get('/all-posts', summariesReadRateLimit, auth, async (_req: AuthReq, res
     const allPostsSummary = await summarizerService.summarizeAllPosts();
     res.json(allPostsSummary);
   } catch (error) {
-    // Fail closed: the summarizer no longer fabricates filler when the LLM is
-    // unavailable, so this is a service-unavailable, not a server bug.
-    console.error('All posts summary unavailable:', (error as Error).message);
-    res.status(503).json({ error: 'summary_unavailable' });
+    // Fail closed, and say WHICH failure. #1501 stopped the summarizer
+    // fabricating filler and made this a 503 — but unconditionally, so a
+    // defect inside `summarizeAllPosts` (a Mongo error, a TypeError in the
+    // post mapping) reported as a transient outage and told the caller to
+    // retry an endpoint that is permanently broken. 503 now means only what
+    // the sentence above claims: the LLM is unavailable, or the rate-limit
+    // cooldown is armed. Both are tagged at the throw site with
+    // `code: 'summary_unavailable'`. Anything else is a server bug. TASK-099
+    // site 4, follow-up to #1501.
+    const code = (error as { code?: string })?.code;
+    if (code === SUMMARY_UNAVAILABLE) {
+      console.error('All posts summary unavailable:', (error as Error).message);
+      res.status(503).json({ error: 'summary_unavailable' });
+      return;
+    }
+    console.error('Error generating all posts summary:', error);
+    res.status(500).json({ error: 'Failed to generate all posts summary' });
   }
 });
 

backend/services/summarizerService.ts

@@ -36,6 +36,32 @@ interface AllPostsSummary {
   };
 }
 
+// TASK-099 site 4. `GET /api/summaries/all-posts` reported EVERY throw out of
+// `summarizeAllPosts` as `503 summary_unavailable`, so a code defect in this
+// method was indistinguishable from the LLM being down — and 503 tells the
+// caller to retry an endpoint that is permanently broken. Only the two causes
+// the route's fail-closed comment actually names carry this marker: the
+// rate-limit cooldown, and a `generateText` failure. Everything else (a Mongo
+// error, a TypeError in the post mapping) stays an ordinary throw and the
+// route answers 500.
+//
+// The route matches on `.code`, not `instanceof`: this module ships through
+// the `module.exports = exports.default` CJS-compat shim at the foot of the
+// file, and a duplicated module registration would break identity while a
+// string property survives it.
+export const SUMMARY_UNAVAILABLE = 'summary_unavailable';
+
+export class SummaryUnavailableError extends Error {
+  code: string;
+
+  constructor(message: string, options?: { cause?: unknown }) {
+    super(message);
+    this.name = 'SummaryUnavailableError';
+    this.code = SUMMARY_UNAVAILABLE;
+    if (options && 'cause' in options) (this as { cause?: unknown }).cause = options.cause;
+  }
+}
+
 interface AllPostsCache {
   summary: AllPostsSummary | null;
   createdAt: number;
@@ -394,10 +420,20 @@ Please create an engaging 3-4 sentence community overview that:
 This is for new visitors to understand what the community is all about. Focus on the content and conversations, not just statistics.`;
 
       if (now < cache.cooldownUntil) {
-        throw new Error('All-posts summary generation is cooling down after an LLM rate limit');
+        throw new SummaryUnavailableError('All-posts summary generation is cooling down after an LLM rate limit');
       }
       console.log('Generating all-posts summary with LLM...');
-      const summaryText = await generateText(prompt, { temperature: 0.4 }) as string;
+      let summaryText: string;
+      try {
+        summaryText = await generateText(prompt, { temperature: 0.4 }) as string;
+      } catch (llmError) {
+        // Message is interpolated, not replaced: the outer catch below still
+        // scans it for '429'/'Resource exhausted' to arm the cooldown.
+        throw new SummaryUnavailableError(
+          `All-posts summary generation failed at the LLM: ${(llmError as Error)?.message}`,
+          { cause: llmError },
+        );
+      }
       console.log(
         `✓ LLM returned all-posts summary: "${summaryText.substring(0, 100)}..."`,
       );