fix(github): /status reports credential liveness, not just presence

backend/__tests__/unit/routes/github.statusLiveness.test.js

@@ -0,0 +1,174 @@
+/**
+ * `GET /api/github/status` is the endpoint whose only job is to diagnose the
+ * GitHub credential. Until this change it answered `configured: true` off
+ * `!!process.env.GITHUB_PAT` — presence, not liveness — which is why it read
+ * `configured: true` for the whole 2026-08-04 outage while every proxied call
+ * returned 401.
+ *
+ * Two invariants are pinned here, and the second is the one most likely to be
+ * "cleaned up" later:
+ *
+ *  1. A dead credential is reported as `credentialLive: false`.
+ *  2. A dead credential is still an HTTP **200**. This route must NOT adopt
+ *     mapGitHubUpstreamError. If the diagnostic returned 502 on a rejected
+ *     credential, a caller could not tell "the credential is dead" from "the
+ *     diagnostic is broken" — the exact collapse #808 removed from the seven
+ *     proxying routes, reintroduced at the one endpoint that exists to
+ *     prevent it.
+ *
+ * Unreachable is a third answer, not a synonym for dead: reporting `false`
+ * when GitHub could not be reached would send an operator to rotate a working
+ * credential.
+ */
+
+jest.mock('../../../middleware/agentRuntimeAuth', () => (req, res, next) => {
+  req.agentUser = { _id: 'bot-1' };
+  next();
+});
+
+let mockCurrentUser = { _id: 'user-1', role: 'admin' };
+jest.mock('../../../middleware/auth', () => (req, res, next) => {
+  req.user = mockCurrentUser;
+  req.userId = mockCurrentUser._id;
+  next();
+});
+
+jest.mock('axios');
+
+const express = require('express');
+const request = require('supertest');
+const axios = require('axios');
+// eslint-disable-next-line import/no-unresolved, import/extensions
+const router = require('../../../routes/github');
+
+const app = express();
+app.use(express.json());
+app.use('/api/github', router);
+
+const upstream = (status, headers) => Object.assign(
+  new Error(`Request failed with status code ${status}`),
+  { response: { status, headers } },
+);
+
+describe('GET /status reports credential liveness, not just presence', () => {
+  const priorPat = process.env.GITHUB_PAT;
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+    process.env.GITHUB_PAT = 'ghp_test_token';
+    mockCurrentUser = { _id: 'user-1', role: 'admin' };
+  });
+
+  afterAll(() => {
+    if (priorPat === undefined) delete process.env.GITHUB_PAT;
+    else process.env.GITHUB_PAT = priorPat;
+  });
+
+  it('reports a working credential as live', async () => {
+    axios.get.mockResolvedValue({ data: { rate: { remaining: 4999 } } });
+
+    const res = await request(app).get('/api/github/status');
+
+    expect(res.status).toBe(200);
+    expect(res.body).toMatchObject({
+      mode: 'pat',
+      configured: true,
+      credentialLive: true,
+      credentialStatus: 'accepted',
+    });
+  });
+
+  it('probes /rate_limit, which GitHub does not charge against quota', async () => {
+    axios.get.mockResolvedValue({ data: {} });
+
+    await request(app).get('/api/github/status');
+
+    expect(axios.get).toHaveBeenCalledTimes(1);
+    expect(axios.get.mock.calls[0][0]).toBe('https://api.github.com/rate_limit');
+  });
+
+  it('reports a rejected credential as NOT live while still answering 200', async () => {
+    axios.get.mockRejectedValue(upstream(401));
+
+    const res = await request(app).get('/api/github/status');
+
+    // The whole finding: `configured: true` alone said everything was fine.
+    expect(res.body.configured).toBe(true);
+    expect(res.body.credentialLive).toBe(false);
+    expect(res.body.credentialStatus).toBe('rejected');
+    expect(res.body.upstreamStatus).toBe(401);
+    // Load-bearing: a diagnostic must not express its finding as its own
+    // failure status. 502 here would be indistinguishable from a broken
+    // diagnostic.
+    expect(res.status).toBe(200);
+  });
+
+  it('treats a bare 403 as credential rejection, same as 401', async () => {
+    // No rate-limit headers — this is GitHub refusing the credential, not
+    // throttling. It is the discriminating negative for the two tests below.
+    axios.get.mockRejectedValue(upstream(403));
+
+    const res = await request(app).get('/api/github/status');
+
+    expect(res.status).toBe(200);
+    expect(res.body.credentialLive).toBe(false);
+    expect(res.body.credentialStatus).toBe('rejected');
+    expect(res.body.upstreamStatus).toBe(403);
+  });
+
+  // GitHub overloads 403: refused credential AND exhausted quota. Reporting a
+  // throttled-but-valid PAT as dead sends an operator to rotate a working
+  // credential — the exact outcome `live: null` exists to prevent, arriving
+  // through the branch the first cut of this probe did not guard (msg 52320).
+  it('does not call a rate-limited PAT dead — primary quota exhausted', async () => {
+    axios.get.mockRejectedValue(upstream(403, { 'x-ratelimit-remaining': '0' }));
+
+    const res = await request(app).get('/api/github/status');
+
+    expect(res.status).toBe(200);
+    expect(res.body.credentialLive).toBeNull();
+    expect(res.body.credentialStatus).toBe('rate_limited');
+    expect(res.body.upstreamStatus).toBe(403);
+  });
+
+  it('does not call a rate-limited PAT dead — secondary limit (retry-after)', async () => {
+    // The likelier 403 on /rate_limit specifically: primary exhaustion does not
+    // throttle that endpoint, so a 403 there is more often abuse-detection.
+    axios.get.mockRejectedValue(upstream(403, { 'retry-after': '60' }));
+
+    const res = await request(app).get('/api/github/status');
+
+    expect(res.body.credentialLive).toBeNull();
+    expect(res.body.credentialStatus).toBe('rate_limited');
+  });
+
+  it('reports a 429 as rate-limited, never as dead', async () => {
+    axios.get.mockRejectedValue(upstream(429));
+
+    const res = await request(app).get('/api/github/status');
+
+    expect(res.body.credentialLive).toBeNull();
+    expect(res.body.credentialStatus).toBe('rate_limited');
+  });
+
+  it('reports unreachable as unknown, never as dead', async () => {
+    axios.get.mockRejectedValue(Object.assign(new Error('connect ETIMEDOUT'), {}));
+
+    const res = await request(app).get('/api/github/status');
+
+    expect(res.status).toBe(200);
+    // null, not false — `false` would send an operator to rotate a credential
+    // that may be perfectly good.
+    expect(res.body.credentialLive).toBeNull();
+    expect(res.body.credentialStatus).toBe('unreachable');
+  });
+
+  it('still refuses non-admins before probing anything', async () => {
+    mockCurrentUser = { _id: 'user-2', role: 'member' };
+
+    const res = await request(app).get('/api/github/status');
+
+    expect(res.status).toBe(403);
+    expect(axios.get).not.toHaveBeenCalled();
+  });
+});

backend/__tests__/unit/routes/github.upstreamErrorRoutes.test.js

@@ -0,0 +1,116 @@
+/**
+ * AX #9 maps GitHub's credential rejection to a non-retryable 502. The
+ * mapper unit tests prove the taxonomy; these mount the real router so every
+ * route that proxies GitHub is pinned to that taxonomy.
+ *
+ * Keep this explicit table when adding a GitHub proxy route. The request
+ * shapes are intentionally visible here: deriving cases from `router.stack`
+ * would hide the route-specific validation each request must pass before it
+ * reaches the upstream service. `/status` is excluded because it only reads
+ * local configuration and signs locally; it must keep its honest local 500.
+ */
+
+jest.mock('../../../middleware/agentRuntimeAuth', () => (req, res, next) => {
+  req.agentUser = { _id: 'bot-1' };
+  next();
+});
+
+jest.mock('../../../middleware/auth', () => (req, res, next) => {
+  req.user = { _id: 'user-1', role: 'member' };
+  req.userId = 'user-1';
+  next();
+});
+
+jest.mock('../../../services/githubAppService', () => ({
+  isPatConfigured: jest.fn(),
+  isConfigured: jest.fn(),
+  getInstallationToken: jest.fn(),
+  listOpenIssues: jest.fn(),
+  createIssue: jest.fn(),
+  addIssueComment: jest.fn(),
+  closeIssue: jest.fn(),
+  getPullDiff: jest.fn(),
+  createPullReview: jest.fn(),
+}));
+
+const express = require('express');
+const request = require('supertest');
+// The backend source is TypeScript, while the legacy ESLint resolver only
+// discovers JavaScript module extensions.
+// eslint-disable-next-line import/no-unresolved, import/extensions
+const GitHubAppService = require('../../../services/githubAppService');
+// eslint-disable-next-line import/no-unresolved, import/extensions
+const router = require('../../../routes/github');
+
+const app = express();
+app.use(express.json());
+app.use('/api/github', router);
+
+const credentialRejected = {
+  message: 'Request failed with status code 401',
+  response: { status: 401 },
+};
+const CREDENTIAL_REJECTION_TITLE = '$name maps an upstream 401 to non-retryable credential guidance';
+
+// Each row is a distinct route-level call site of mapGitHubUpstreamError.
+// Adding another GitHub-proxy route means adding a row here with the smallest
+// valid request that reaches its service boundary.
+const PROXYING_ROUTE_CASES = [
+  {
+    name: 'POST /token',
+    service: 'getInstallationToken',
+    send: (client) => client.post('/api/github/token'),
+  },
+  {
+    name: 'GET /issues',
+    service: 'listOpenIssues',
+    send: (client) => client.get('/api/github/issues'),
+  },
+  {
+    name: 'POST /issues',
+    service: 'createIssue',
+    send: (client) => client.post('/api/github/issues').send({ title: 'Test issue' }),
+  },
+  {
+    name: 'POST /issues/:number/comment',
+    service: 'addIssueComment',
+    send: (client) => client.post('/api/github/issues/1/comment').send({ body: 'Test comment' }),
+  },
+  {
+    name: 'POST /issues/:number/close',
+    service: 'closeIssue',
+    send: (client) => client.post('/api/github/issues/1/close'),
+  },
+  {
+    name: 'GET /pulls/:number/diff',
+    service: 'getPullDiff',
+    send: (client) => client.get('/api/github/pulls/1/diff'),
+  },
+  {
+    name: 'POST /pulls/:number/review',
+    service: 'createPullReview',
+    send: (client) => client.post('/api/github/pulls/1/review').send({ event: 'APPROVE' }),
+  },
+];
+
+describe('GitHub proxy routes preserve upstream credential guidance (AX #9)', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+    GitHubAppService.isPatConfigured.mockReturnValue(false);
+    GitHubAppService.isConfigured.mockReturnValue(true);
+  });
+
+  test.each(PROXYING_ROUTE_CASES)(CREDENTIAL_REJECTION_TITLE, async ({ service, send }) => {
+    GitHubAppService[service].mockRejectedValue(credentialRejected);
+
+    const res = await send(request(app));
+
+    expect(GitHubAppService[service]).toHaveBeenCalledTimes(1);
+    expect(res.status).toBe(502);
+    expect(res.body).toEqual(expect.objectContaining({
+      code: 'github_credential_rejected',
+      upstreamStatus: 401,
+      retryable: false,
+    }));
+  });
+});