Merge pull request #192 from InsForge/feat/diagnose-advisor-oss-routing

feat(diagnose): advisor to project OSS backend

Author: CarmenDou

src/commands/diagnose/advisor.ts

@@ -1,20 +1,21 @@
 import type { Command } from 'commander';
 import { platformFetch } from '../../lib/api/platform.js';
+import { ossFetch } from '../../lib/api/oss.js';
 import { requireAuth } from '../../lib/credentials.js';
-import { handleError, getRootOpts, CLIError, ProjectNotLinkedError } from '../../lib/errors.js';
+import { CLIError, handleError, getRootOpts, ProjectNotLinkedError } from '../../lib/errors.js';
 import { getProjectConfig, FAKE_PROJECT_ID } from '../../lib/config.js';
 import { outputJson, outputTable } from '../../lib/output.js';
 import { reportCliUsage } from '../../lib/skills.js';
 import { trackDiagnose, shutdownAnalytics } from '../../lib/analytics.js';
 
-
 interface AdvisorScanSummary {
   scanId: string;
   status: string;
   scanType: string;
   scannedAt: string;
+  errorMessage?: string;
   summary: { total: number; critical: number; warning: number; info: number };
-  collectorErrors: { collector: string; error: string; timestamp: string }[];
+  collectorErrors?: { collector: string; error: string; timestamp: string }[];
 }
 
 interface AdvisorIssue {
@@ -26,22 +27,96 @@ interface AdvisorIssue {
   description: string;
   affectedObject: string;
   recommendation: string;
-  isResolved: boolean;
+  isResolved?: boolean;
 }
 
 interface AdvisorIssuesResponse {
   issues: AdvisorIssue[];
   total: number;
 }
 
-export async function fetchAdvisorSummary(
+/**
+ * True when an error from `ossFetch` is a route-level 404 — i.e. this backend
+ * is too old to expose the advisor endpoints at all. `ossFetch` throws a
+ * `CLIError` carrying the HTTP status for these. A backend that HAS the route
+ * but no scan yet returns 200 with a null/empty body, never a 404, so a 404
+ * here unambiguously means "route absent → fall back to cloud-backend".
+ */
+function isOssAdvisorRouteMissing(err: unknown): boolean {
+  return err instanceof CLIError && err.statusCode === 404;
+}
+
+async function fetchOssAdvisorLatest(): Promise<AdvisorScanSummary | null> {
+  const res = await ossFetch('/api/advisor/latest');
+  // Guard the parse: no-scan returns null and an empty body would make
+  // res.json() throw, matching the .catch pattern used across the codebase.
+  return (await res.json().catch(() => null)) as AdvisorScanSummary | null;
+}
+
+async function fetchOssAdvisorIssues(params: URLSearchParams): Promise<AdvisorIssuesResponse> {
+  const res = await ossFetch(`/api/advisor/issues?${params.toString()}`);
+  // The OSS backend returns { issues: [], total: 0 } for the no-scan state, but
+  // normalize defensively so a null/empty body can never crash the caller
+  // (res.json() throws on an empty body, so guard the parse itself too).
+  const data = (await res.json().catch(() => null)) as AdvisorIssuesResponse | null;
+  return data ?? { issues: [], total: 0 };
+}
+
+async function fetchPlatformAdvisorLatest(
   projectId: string,
   apiUrl?: string,
-): Promise<AdvisorScanSummary> {
-  const res = await platformFetch(`/projects/v1/${projectId}/advisor/latest`, {}, apiUrl);
+): Promise<AdvisorScanSummary | null> {
+  // 404 here means the legacy cloud-backend has no scan for this project — a
+  // normal "no scan yet" state, not a failure. Pass it through and return null.
+  const res = await platformFetch(
+    `/projects/v1/${projectId}/advisor/latest`,
+    { passThroughStatuses: [404] },
+    apiUrl,
+  );
+  if (res.status === 404) return null;
   return (await res.json()) as AdvisorScanSummary;
 }
 
+async function fetchPlatformAdvisorIssues(
+  projectId: string,
+  params: URLSearchParams,
+  apiUrl?: string,
+): Promise<AdvisorIssuesResponse> {
+  const res = await platformFetch(
+    `/projects/v1/${projectId}/advisor/latest/issues?${params.toString()}`,
+    { passThroughStatuses: [404] },
+    apiUrl,
+  );
+  if (res.status === 404) return { issues: [], total: 0 };
+  return (await res.json()) as AdvisorIssuesResponse;
+}
+
+/**
+ * Scan summary only, used by `diagnose` (no subcommand) to build the
+ * aggregate health report.
+ *
+ * The project's own OSS backend is authoritative — it holds the data and its
+ * running version is the only thing that decides whether the advisor route
+ * exists. So we always try the OSS advisor first and only fall back to the
+ * legacy cloud-backend when the OSS route is absent (backend older than the
+ * advisor feature). This avoids trusting possibly-stale Platform metadata.
+ * OSS `--api-key` mode has no cloud-backend to fall back to.
+ */
+export async function fetchAdvisorSummary(
+  projectId: string,
+  apiUrl?: string,
+): Promise<AdvisorScanSummary | null> {
+  if (projectId === FAKE_PROJECT_ID) {
+    return await fetchOssAdvisorLatest();
+  }
+  try {
+    return await fetchOssAdvisorLatest();
+  } catch (err) {
+    if (!isOssAdvisorRouteMissing(err)) throw err;
+    return await fetchPlatformAdvisorLatest(projectId, apiUrl);
+  }
+}
+
 export function registerDiagnoseAdvisorCommand(diagnoseCmd: Command): void {
   diagnoseCmd
     .command('advisor')
@@ -55,45 +130,47 @@ export function registerDiagnoseAdvisorCommand(diagnoseCmd: Command): void {
         await requireAuth(apiUrl);
         const config = getProjectConfig();
         if (!config) throw new ProjectNotLinkedError();
-        if (config.project_id === FAKE_PROJECT_ID) {
-          throw new CLIError(
-            'Advisor requires InsForge Platform login. Not available when linked via --api-key.',
-          );
-        }
         trackDiagnose('advisor', config);
 
         const projectId = config.project_id;
+        const ossMode = projectId === FAKE_PROJECT_ID;
 
-        // Fetch scan summary
-        const scanRes = await platformFetch(
-          `/projects/v1/${projectId}/advisor/latest`,
-          {},
-          apiUrl,
-        );
-        const scan = (await scanRes.json()) as AdvisorScanSummary;
-
-        // Fetch issues
         const issueParams = new URLSearchParams();
         if (opts.severity) issueParams.set('severity', opts.severity);
         if (opts.category) issueParams.set('category', opts.category);
         issueParams.set('limit', opts.limit);
 
-        const issuesRes = await platformFetch(
-          `/projects/v1/${projectId}/advisor/latest/issues?${issueParams.toString()}`,
-          {},
-          apiUrl,
-        );
-        const issuesData = (await issuesRes.json()) as AdvisorIssuesResponse;
+        let scan: AdvisorScanSummary | null;
+        let issuesData: AdvisorIssuesResponse;
+
+        // Try the project's own OSS advisor first (it holds the data and its
+        // running version is what decides whether the route exists). In
+        // Platform mode, fall back to the legacy cloud-backend only when the
+        // OSS route is absent (backend older than the advisor feature). OSS
+        // `--api-key` mode has no fallback — the oss.ts route-level-404 message
+        // guards backends too old to have the route.
+        try {
+          scan = await fetchOssAdvisorLatest();
+          issuesData = await fetchOssAdvisorIssues(issueParams);
+        } catch (err) {
+          if (ossMode || !isOssAdvisorRouteMissing(err)) throw err;
+          scan = await fetchPlatformAdvisorLatest(projectId, apiUrl);
+          issuesData = await fetchPlatformAdvisorIssues(projectId, issueParams, apiUrl);
+        }
 
         if (json) {
           outputJson({ scan, issues: issuesData.issues });
         } else {
-          // Scan summary line
-          const date = new Date(scan.scannedAt).toLocaleDateString();
-          const s = scan.summary;
-          console.log(
-            `Scan: ${date} (${scan.status}) — ${s.critical} critical, ${s.warning} warning, ${s.info} info\n`,
-          );
+          if (!scan) {
+            console.log('No scan yet.\n');
+          } else {
+            // Scan summary line
+            const date = new Date(scan.scannedAt).toLocaleDateString();
+            const s = scan.summary;
+            console.log(
+              `Scan: ${date} (${scan.status}) — ${s.critical} critical, ${s.warning} warning, ${s.info} info\n`,
+            );
+          }
 
           if (!issuesData.issues || issuesData.issues.length === 0) {
             console.log('No issues found.');

src/commands/diagnose/index.ts

@@ -35,9 +35,10 @@ async function collectDiagnosticData(
   const metricsPromise = ossMode
     ? Promise.reject(new Error('Platform login required (linked via --api-key)'))
     : fetchMetricsSummary(projectId, apiUrl);
-  const advisorPromise = ossMode
-    ? Promise.reject(new Error('Platform login required (linked via --api-key)'))
-    : fetchAdvisorSummary(projectId, apiUrl);
+  // Tries the project's own OSS advisor first, falling back to the
+  // cloud-backend Platform advisor for older project backends — works in
+  // both OSS and Platform link modes.
+  const advisorPromise = fetchAdvisorSummary(projectId, apiUrl);
 
   const [metricsResult, advisorResult, dbResult, logsResult] = await Promise.allSettled([
     metricsPromise,
@@ -75,6 +76,9 @@ async function collectDiagnosticData(
   }
 
   if (advisorResult.status === 'fulfilled') {
+    // A null summary is a healthy "no scan yet" state, not a failure — leave
+    // `advisor` null without recording an error so JSON consumers don't treat
+    // an unscanned project as a failed diagnostic run.
     advisor = advisorResult.value;
   } else {
     errors.push(advisorResult.reason?.message ?? 'Advisor unavailable');

src/lib/api/oss.ts

@@ -213,6 +213,15 @@ export async function ossFetch(
       message = 'The web scraper is not available on this backend.\nThe Apify web scraper is cloud-only. Self-hosted: this feature is not supported. Cloud: contact your InsForge admin to enable it.';
     }
 
+    // Safe to treat any 404 on /api/advisor/* as a route-level miss: the OSS
+    // advisor endpoints return 200 with a null/empty body when no scan exists
+    // (a resource-level "no data" state), so they never emit a 404 for a
+    // present-but-empty resource. A 404 here therefore means the route itself
+    // is absent (backend older than the advisor feature).
+    if (res.status === 404 && isRouteLevel404 && path.startsWith('/api/advisor')) {
+      message = 'Backend Advisor is not available on this backend.\nSelf-hosted: upgrade your InsForge instance. Cloud: update the project to a newer version.';
+    }
+
     throw new CLIError(message, 1, err.error, res.status);
   }