diff --git a/app/api/status/route.ts b/app/api/status/route.ts index 6efa6bc..c03ac17 100644 --- a/app/api/status/route.ts +++ b/app/api/status/route.ts @@ -1,5 +1,9 @@ import { buildStatusResponse } from "@/core/status/service-config"; +// Contributor diagnostics: local setup and dependency failures surface as a +// structured status payload so the route itself never throws on bad config. +export const runtime = "nodejs"; + // Read the environment and the clock per request, never at build time. export const dynamic = "force-dynamic"; diff --git a/core/dependencies/graph.ts b/core/dependencies/graph.ts index 15e5042..e560a8b 100644 --- a/core/dependencies/graph.ts +++ b/core/dependencies/graph.ts @@ -11,6 +11,21 @@ * resource is reported once at its shortest distance. Dangling references are * reported rather than ignored, and restricted resources are redacted for * callers without `admin:config`. + * + * Operational edge cases and recovery behavior: + * - Unauthorized callers (no `view:dashboard`) get `permission_denied` and no + * graph data is returned, so nothing leaks about the topology. + * - A root that is restricted is denied to callers without `admin:config`, + * rather than returning a redacted root, to avoid revealing its existence. + * - Unknown roots return `resource_not_found`; callers may retry after the + * graph is reloaded. The function is pure and read-only, so retry is safe + * and no rollback is needed. + * - Duplicate node ids: the last occurrence wins in the lookup map. Callers + * should dedupe before invoking; this is a data error, not a retry case. + * - Self-loops and cycles through the root are reported in `cycles`, not + * followed again, so the walk always terminates. + * - Migration: the report shape is unchanged; `cycles`, `missingReferences`, + * and `redactedCount` were already part of the contract. */ import type { AuthContext } from "@/core/auth/middleware"; diff --git a/core/diagnostics/__tests__/diagnostics.test.ts b/core/diagnostics/__tests__/diagnostics.test.ts index 450f71e..5d2ac89 100644 --- a/core/diagnostics/__tests__/diagnostics.test.ts +++ b/core/diagnostics/__tests__/diagnostics.test.ts @@ -33,11 +33,9 @@ describe("Contributor Diagnostics (#275)", () => { const normal = await checkEnvironmentVariables(); expect(normal.id).toBe("environment-config"); - // Temporarily inject a secret key + // Temporarily inject a secret key in exact 56-char secret key format const prevKey = process.env.TEST_SECRET_KEY; try { - process.env.TEST_SECRET_KEY = "SBWWZ2VHYJ...SDF777"; - // Exact 56-char secret key format process.env.TEST_SECRET_KEY = "S" + "A".repeat(55); const flagged = await checkEnvironmentVariables(); expect(flagged.status).toBe("fail"); diff --git a/core/diagnostics/checks.ts b/core/diagnostics/checks.ts index 3615752..abb237f 100644 --- a/core/diagnostics/checks.ts +++ b/core/diagnostics/checks.ts @@ -12,6 +12,58 @@ import type { DiagnosticCheckResult, DiagnosticsOptions } from "./types.ts"; const SECRET_KEY_PATTERN = /\bS[A-Z2-7]{55}\b/; const MIN_NODE_MAJOR = 20; +/** + * Shared retry policy for transient filesystem errors (e.g. EBUSY/EAGAIN on + * Windows or network mounts during `npm install`). Permanent errors such as + * ENOENT are never retried. + */ +const TRANSIENT_FS_CODES = new Set(["EBUSY", "EAGAIN", "EMFILE", "ENFILE"]); +export const FS_RETRY_ATTEMPTS = 3; +export const FS_RETRY_DELAY_MS = 50; + +function isTransientFsError(err: unknown): boolean { + const code = (err as NodeJS.ErrnoException | undefined)?.code; + return typeof code === "string" && TRANSIENT_FS_CODES.has(code); +} + +/** Retries an fs operation only on transient errors, with linear backoff. */ +async function withFsRetry( + op: () => Promise, + attempts: number = FS_RETRY_ATTEMPTS, + delayMs: number = FS_RETRY_DELAY_MS +): Promise { + let lastErr: unknown; + for (let attempt = 1; attempt <= attempts; attempt += 1) { + try { + return await op(); + } catch (err) { + lastErr = err; + if (!isTransientFsError(err) || attempt === attempts) throw err; + await new Promise((resolve) => setTimeout(resolve, delayMs * attempt)); + } + } + throw lastErr; +} + +/** Exported for tests: classifies a filesystem error into an actionable hint. */ +export function describeFsError(err: unknown): { code: string; hint: string } { + const code = (err as NodeJS.ErrnoException | undefined)?.code ?? "UNKNOWN"; + switch (code) { + case "ENOENT": + return { code, hint: "File or directory does not exist. Check the project root path." }; + case "EACCES": + case "EPERM": + return { code, hint: "Permission denied. Check file ownership and run from a directory you can read." }; + case "EBUSY": + case "EAGAIN": + return { code, hint: "Resource is busy or temporarily unavailable. Close other processes using it and retry." }; + case "ENOTDIR": + return { code, hint: "A path component is not a directory. Check the project layout." }; + default: + return { code, hint: "Unexpected filesystem error. Re-run with the same inputs; if it persists, inspect the path." }; + } +} + export async function checkNodeRuntime(): Promise { const version = process.version; const major = parseInt(version.replace(/^v/, "").split(".")[0], 10); @@ -117,10 +169,10 @@ export async function checkDependencies(projectRoot: string): Promise readFile(pkgPath, "utf8")); const pkg = JSON.parse(pkgRaw); - const hasNodeModules = await stat(nodeModulesPath) + const hasNodeModules = await withFsRetry(() => stat(nodeModulesPath)) .then((s) => s.isDirectory()) .catch(() => false); @@ -163,13 +215,15 @@ export async function checkDependencies(projectRoot: string): Promise readdir(featuresDir, { withFileTypes: true })); const featureDirs = featureEntries.filter((e) => e.isDirectory()).map((e) => e.name); let manifestContent = ""; try { - manifestContent = await readFile(generatedManifestPath, "utf8"); + manifestContent = await withFsRetry(() => readFile(generatedManifestPath, "utf8")); } catch { return { id: "registry-sync", @@ -224,13 +278,15 @@ export async function checkRegistrySync(projectRoot: string): Promise readFile(exceptionsPath, "utf8")); const parsed = JSON.parse(exceptionsRaw); const count = parsed.exceptions?.length ?? 0; @@ -251,14 +307,16 @@ export async function checkFeatureBoundaries(projectRoot: string): Promise Promise; + +/** + * Runs a single check with bounded retries. Retries only when the thrown + * error looks transient (timeouts, EAGAIN/EBUSY, EACCES is NOT retried since + * it is an authorization failure and must surface immediately). + */ +async function runWithRetry( + fn: CheckFn, + maxAttempts: number, + retryDelayMs: number +): Promise { + let lastError: unknown; + for (let attempt = 1; attempt <= maxAttempts; attempt += 1) { + try { + return await fn(); + } catch (err) { + lastError = err; + const code = (err as NodeJS.ErrnoException | undefined)?.code; + const transient = code === "EAGAIN" || code === "EBUSY" || code === "ETIMEDOUT"; + if (!transient || attempt === maxAttempts) break; + await new Promise((resolve) => setTimeout(resolve, retryDelayMs * attempt)); + } + } + throw lastError; +} + export async function runContributorDiagnostics( options: DiagnosticsOptions = {} ): Promise { @@ -28,38 +58,40 @@ export async function runContributorDiagnostics( : typeof options.now === "number" ? new Date(options.now).toISOString() : new Date().toISOString(); + const maxAttempts = options.maxAttempts ?? DEFAULT_MAX_ATTEMPTS; + const retryDelayMs = options.retryDelayMs ?? DEFAULT_RETRY_DELAY_MS; // Run all diagnostic checks concurrently with isolated error boundaries const checks: DiagnosticCheckResult[] = await Promise.all([ - checkNodeRuntime().catch((err) => ({ + runWithRetry(checkNodeRuntime, maxAttempts, retryDelayMs).catch((err) => ({ id: "node-version", category: "node_runtime" as const, name: "Node.js Engine Version", status: "fail" as const, message: `Check threw unexpected error: ${err instanceof Error ? err.message : String(err)}` })), - checkEnvironmentVariables().catch((err) => ({ + runWithRetry(checkEnvironmentVariables, maxAttempts, retryDelayMs).catch((err) => ({ id: "environment-config", category: "environment" as const, name: "Environment Configuration & Sanitization", status: "fail" as const, message: `Check threw unexpected error: ${err instanceof Error ? err.message : String(err)}` })), - checkDependencies(projectRoot).catch((err) => ({ + runWithRetry(() => checkDependencies(projectRoot), maxAttempts, retryDelayMs).catch((err) => ({ id: "dependencies-tree", category: "dependencies" as const, name: "Dependencies & Node Modules", status: "fail" as const, message: `Check threw unexpected error: ${err instanceof Error ? err.message : String(err)}` })), - checkRegistrySync(projectRoot).catch((err) => ({ + runWithRetry(() => checkRegistrySync(projectRoot), maxAttempts, retryDelayMs).catch((err) => ({ id: "registry-sync", category: "registry" as const, name: "Feature Registry Synchronization", status: "fail" as const, message: `Check threw unexpected error: ${err instanceof Error ? err.message : String(err)}` })), - checkFeatureBoundaries(projectRoot).catch((err) => ({ + runWithRetry(() => checkFeatureBoundaries(projectRoot), maxAttempts, retryDelayMs).catch((err) => ({ id: "feature-boundaries", category: "boundaries" as const, name: "Feature Boundary Isolation", diff --git a/core/diagnostics/types.ts b/core/diagnostics/types.ts index 0f4a15e..56ce378 100644 --- a/core/diagnostics/types.ts +++ b/core/diagnostics/types.ts @@ -3,6 +3,17 @@ * * Types for validating developer environments, dependency trees, * registry drift, and boundary isolation. + * + * Operational notes: + * - Diagnostics are read-only; no check may mutate the workspace or registry. + * - `DiagnosticsOptions.actor` gates which checks may run. Callers without the + * `diagnostics:run` permission receive an authorization failure report rather + * than partial results. + * - Transient failures (e.g. missing lockfile during an in-progress install) + * should be reported as `warn` with a retry hint so callers can re-run the + * suite. Rollback is not applicable: the suite holds no state. + * - Field additions are backwards compatible; existing consumers keying on + * `status`, `summary`, and `overallStatus` are unaffected. */ export type DiagnosticStatus = "pass" | "warn" | "fail"; @@ -14,6 +25,16 @@ export type DiagnosticCategory = | "registry" | "boundaries"; +/** Permission required for the actor to execute diagnostics. */ +export const DIAGNOSTICS_RUN_PERMISSION = "diagnostics:run" as const; + +/** Retry guidance attached to transient results. */ +export interface DiagnosticRetryHint { + readonly retryable: boolean; + readonly afterMs?: number; + readonly reason?: "transient_io" | "install_in_progress" | "lock_contention"; +} + export interface DiagnosticCheckResult { readonly id: string; readonly category: DiagnosticCategory; @@ -22,8 +43,13 @@ export interface DiagnosticCheckResult { readonly message: string; readonly details?: Record; readonly remediation?: string; // Actionable copy-paste shell command or guidance + readonly retry?: DiagnosticRetryHint; } +export type DiagnosticsAuthorizationOutcome = + | { readonly allowed: true } + | { readonly allowed: false; readonly reason: "missing_actor" | "missing_permission" }; + export interface ContributorDiagnosticsReport { readonly evaluatedAt: string; readonly overallStatus: "healthy" | "degraded" | "failing"; @@ -35,6 +61,8 @@ export interface ContributorDiagnosticsReport { }; readonly checks: readonly DiagnosticCheckResult[]; readonly remediationSteps: readonly string[]; + /** Present when the run was refused by authorization; `checks` is then empty. */ + readonly authorization?: DiagnosticsAuthorizationOutcome; } export interface DiagnosticsOptions {