From d5e1d9d21401a0243f0f8977002a301c8bf22500 Mon Sep 17 00:00:00 2001 From: Or Dvir Date: Sat, 11 Jul 2026 13:06:24 +0300 Subject: [PATCH] fix(doctor): validate the active workspace against the backend + workspace-404 recovery hint (AIT-51) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - doctor: the Active workspace check trusted config.json alone — after a DB re-seed it reported OK while every workspace-scoped command 404'd. Reuse the auth probe's /workspaces response to flag a persisted activeWorkspaceId missing from the backend as stale, with a 'hookmyapp workspace use' hint. Fail-open on network flakes, matching the auth probe. - api: 404 + WORKSPACE_NOT_FOUND now appends the same recovery hint so users don't need doctor to find the fix. --- src/api/__tests__/client.test.ts | 10 ++++++ src/api/client.ts | 5 +++ src/commands/__tests__/doctor.test.ts | 50 +++++++++++++++++++++++++++ src/commands/doctor.ts | 28 ++++++++++++--- 4 files changed, 89 insertions(+), 4 deletions(-) diff --git a/src/api/__tests__/client.test.ts b/src/api/__tests__/client.test.ts index 1653449..b0df189 100644 --- a/src/api/__tests__/client.test.ts +++ b/src/api/__tests__/client.test.ts @@ -68,6 +68,16 @@ describe('mapApiError — Wave 0 RED', () => { expect(err.message).toBe('M'); }); + it('404 + WORKSPACE_NOT_FOUND → ApiError hinting workspace use (AIT-51)', async () => { + const { mapApiError } = await import('../client.js'); + const err = (await ( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + mapApiError as any + )(mkRes(404, { code: 'WORKSPACE_NOT_FOUND', message: 'Workspace not found' }))) as ApiError; + expect(err).toBeInstanceOf(ApiError); + expect(err.message).toMatch(/workspace use/); + }); + it('410 + BILLING_PORTAL_RETIRED → ApiError pointing at billing manage', async () => { const { mapApiError } = await import('../client.js'); const err = (await ( diff --git a/src/api/client.ts b/src/api/client.ts index dc2d1f8..0f46684 100644 --- a/src/api/client.ts +++ b/src/api/client.ts @@ -140,6 +140,11 @@ export async function mapApiError(res: Response): Promise { // "already spent code"). ApiError.exitCode defaults to 1; we override // to 5 on the instance so the CLI's exit-code contract stays honest // ("API rejected the bootstrap code" is a distinct failure class). + // AIT-51: a stale activeWorkspaceId (DB re-seed, deleted workspace) makes + // every workspace-scoped command 404 — point at the recovery command. + if (res.status === 404 && code === 'WORKSPACE_NOT_FOUND') { + return new ApiError(`${msg}. Run: hookmyapp workspace use `, 404); + } if (res.status === 404 && code === 'BOOTSTRAP_NOT_FOUND') { const err = new ApiError( 'Code invalid or already used. Ask the dashboard user to click Copy again.', diff --git a/src/commands/__tests__/doctor.test.ts b/src/commands/__tests__/doctor.test.ts index d581118..edce84a 100644 --- a/src/commands/__tests__/doctor.test.ts +++ b/src/commands/__tests__/doctor.test.ts @@ -1,8 +1,10 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; vi.mock('../../auth/store.js', () => ({ readCredentials: vi.fn(async () => null) })); vi.mock('../../api/client.js', () => ({ apiClient: vi.fn() })); +vi.mock('../workspace.js', () => ({ readWorkspaceConfig: vi.fn(() => ({})) })); import { readCredentials } from '../../auth/store.js'; import { apiClient } from '../../api/client.js'; +import { readWorkspaceConfig } from '../workspace.js'; import { AuthError, NetworkError } from '../../output/error.js'; import { collectDoctorReport } from '../doctor.js'; @@ -68,3 +70,51 @@ describe('doctor — auth probe uses the real authenticated request path', () => expect(report.checks.find((c) => c.id === 'auth')!.detail).toBe('credentials present'); }); }); + +describe('doctor — active workspace is validated against the backend (AIT-51)', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.stubGlobal('fetch', vi.fn(async () => ({ ok: true, status: 200 }))); + vi.mocked(readCredentials).mockResolvedValue({ + accessToken: 't', + refreshToken: 'rt', + expiresAt: Math.floor(Date.now() / 1000) + 3600, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any); + vi.mocked(readWorkspaceConfig).mockReturnValue({ + activeWorkspaceId: 'ws_stale123', + activeWorkspaceSlug: 'My Workspace', + }); + }); + + it('flags a persisted workspace missing from the backend list as stale', async () => { + vi.mocked(apiClient).mockResolvedValue([{ id: 'ws_other456' }]); + + const report = await collectDoctorReport({ checkTools: false }); + + const ws = report.checks.find((c) => c.id === 'workspace')!; + expect(ws.ok).toBe(false); + expect(ws.detail).toContain('workspace use'); + expect(report.ok).toBe(true); // informational, never a hard gate + }); + + it('passes when the persisted workspace exists on the backend', async () => { + vi.mocked(apiClient).mockResolvedValue([{ id: 'ws_stale123' }]); + + const report = await collectDoctorReport({ checkTools: false }); + + const ws = report.checks.find((c) => c.id === 'workspace')!; + expect(ws.ok).toBe(true); + expect(ws.detail).toBe('My Workspace'); + }); + + it('keeps the cache-based verdict when the workspaces fetch fails', async () => { + vi.mocked(apiClient).mockRejectedValue(new NetworkError()); + + const report = await collectDoctorReport({ checkTools: false }); + + const ws = report.checks.find((c) => c.id === 'workspace')!; + expect(ws.ok).toBe(true); + expect(ws.detail).toBe('My Workspace'); + }); +}); diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 65e88b0..79dc989 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -60,9 +60,15 @@ export async function collectDoctorReport( // stored accessToken false-FAILs on tokens that are merely expired but // refreshable (2026-07-08 audit). let authDetail = loggedIn ? 'credentials present' : 'not logged in — run: hookmyapp login'; + // Kept from the auth probe so the workspace check below can validate the + // persisted activeWorkspaceId against the backend instead of trusting the + // cache (AIT-51: after a DB re-seed doctor said OK while every scoped + // command 404'd). + let workspaces: Array<{ id?: string }> | null = null; if (loggedIn && opts.checkNetwork !== false) { try { - await apiClient('/workspaces'); + const res = await apiClient('/workspaces'); + if (Array.isArray(res)) workspaces = res; authDetail = 'credentials valid for this env'; } catch (err) { if (err instanceof AuthError || err instanceof PermissionError) { @@ -75,9 +81,23 @@ export async function collectDoctorReport( // Informational: not-logged-in is reported, not a hard prereq failure. checks.push({ id: 'auth', label: 'Logged in', ok: loggedIn, hard: false, detail: authDetail }); - let activeWs: string | undefined; - try { activeWs = readWorkspaceConfig().activeWorkspaceSlug ?? undefined; } catch { /* ignore */ } - checks.push({ id: 'workspace', label: 'Active workspace', ok: true, hard: false, detail: activeWs ?? '(none — auto-resolves on first call)' }); + let wsId: string | undefined; + let wsSlug: string | undefined; + try { + const cfg = readWorkspaceConfig(); + wsId = cfg.activeWorkspaceId ?? undefined; + wsSlug = cfg.activeWorkspaceSlug ?? undefined; + } catch { /* ignore */ } + let wsOk = true; + let wsDetail = wsSlug ?? '(none — auto-resolves on first call)'; + // Only a definitive backend list can fail this check — on a network flake + // (workspaces === null) the cache-based verdict stands, matching the auth + // probe's fail-open posture above. + if (wsId && workspaces && !workspaces.some((w) => w?.id === wsId)) { + wsOk = false; + wsDetail = `"${wsSlug ?? wsId}" not found on this env (stale selection) — run: hookmyapp workspace use `; + } + checks.push({ id: 'workspace', label: 'Active workspace', ok: wsOk, hard: false, detail: wsDetail }); const envChannel = process.env.HOOKMYAPP_CHANNEL_ID; checks.push({ id: 'default-channel', label: 'Default channel (HOOKMYAPP_CHANNEL_ID)', ok: true, hard: false, detail: envChannel || '(none — pass --channel or set HOOKMYAPP_CHANNEL_ID)' });