From 671884638e74096b7acc4d81c06a52393a182625 Mon Sep 17 00:00:00 2001 From: nghetienhiep <13849419+nghetienhiep@users.noreply.github.com> Date: Thu, 16 Jul 2026 00:34:38 +0000 Subject: [PATCH] feat(mcp): extract buildFocusManifestValidation into @loopover/engine for offline loopover_validate_config Move the focus-manifest validation-result builder into @loopover/engine and leave a re-export shim at src/services/focus-manifest-validation.ts, so the local loopover_validate_config MCP tool computes the result in-process instead of proxying to POST /v1/validate/focus-manifest. The unknownTopLevelWarnings helper it depends on co-moves into the engine (single source of truth shared with the self-host config linter). Bump packages/loopover-mcp's @loopover/engine dependency to ^3.0.0 so it tracks the workspace engine, mirroring packages/loopover-miner. Remote server behavior is unchanged via the shim. Closes #6269 --- package-lock.json | 17 +- packages/loopover-engine/package.json | 4 + .../src/focus-manifest-validation.ts | 165 ++++++++++++++++++ packages/loopover-mcp/bin/loopover-mcp.js | 6 +- packages/loopover-mcp/package.json | 2 +- src/selfhost/config-lint.ts | 87 +-------- src/services/focus-manifest-validation.ts | 92 ++-------- 7 files changed, 194 insertions(+), 179 deletions(-) create mode 100644 packages/loopover-engine/src/focus-manifest-validation.ts diff --git a/package-lock.json b/package-lock.json index 54331fde45..967ac6d6e3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -20714,7 +20714,7 @@ "version": "3.0.0", "license": "AGPL-3.0-only", "dependencies": { - "@loopover/engine": "^1.0.0", + "@loopover/engine": "^3.0.0", "@modelcontextprotocol/sdk": "1.29.0", "zod": "^4.4.3" }, @@ -20725,21 +20725,6 @@ "node": ">=22.0.0" } }, - "packages/loopover-mcp/node_modules/@loopover/engine": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/@loopover/engine/-/engine-1.0.0.tgz", - "integrity": "sha512-5OULTIoyl3ttEVZ0Sa9D8y89CbyM7n1CZFqhhx3EHP9YpYC+65xJz4u892d6a3U1f7lra9Bf2DrWwJr/GFHOIA==", - "license": "AGPL-3.0-only", - "dependencies": { - "@anthropic-ai/claude-agent-sdk": "^0.3.205", - "tree-sitter-wasms": "^0.1.13", - "web-tree-sitter": "^0.20.8", - "yaml": "^2.9.0" - }, - "engines": { - "node": ">=22.0.0" - } - }, "packages/loopover-miner": { "name": "@loopover/miner", "version": "3.0.0", diff --git a/packages/loopover-engine/package.json b/packages/loopover-engine/package.json index 85364b186a..2b55ecf2fd 100644 --- a/packages/loopover-engine/package.json +++ b/packages/loopover-engine/package.json @@ -43,6 +43,10 @@ "types": "./dist/scoring/pending-pr-scenarios.d.ts", "default": "./dist/scoring/pending-pr-scenarios.js" }, + "./focus-manifest-validation": { + "types": "./dist/focus-manifest-validation.d.ts", + "default": "./dist/focus-manifest-validation.js" + }, "./signals/test-evidence": { "types": "./dist/signals/test-evidence.d.ts", "default": "./dist/signals/test-evidence.js" diff --git a/packages/loopover-engine/src/focus-manifest-validation.ts b/packages/loopover-engine/src/focus-manifest-validation.ts new file mode 100644 index 0000000000..073b352696 --- /dev/null +++ b/packages/loopover-engine/src/focus-manifest-validation.ts @@ -0,0 +1,165 @@ +import { parse as parseYaml } from "yaml"; +import { + MAX_FOCUS_MANIFEST_BYTES, + contentLaneConfigToJson, + featuresConfigToJson, + gateConfigToJson, + parseFocusManifestContent, + repoDocGenerationConfigToJson, + reviewConfigToJson, + reviewRecapConfigToJson, + maintainerRecapConfigToJson, + settingsOverrideToJson, + type FocusManifest, + type FocusManifestSource, +} from "./focus-manifest.js"; + +// The recognized top-level `.loopover.yml` fields. The single source of truth for both the unknown-field +// warning below and the self-host config linter's recognized-field report (src/selfhost/config-lint.ts) — +// keeping one list stops the two surfaces drifting when a new field lands (the class of miss #3002/#5281 fixed). +export const TOP_LEVEL_FIELDS = [ + "source", + "wantedPaths", + "preferredLabels", + "linkedIssuePolicy", + "testExpectations", + "issueDiscoveryPolicy", + "maintainerNotes", + "publicNotes", + "gate", + "settings", + "review", + "features", + "experimental", + "contentLane", + "repoDocGeneration", + "reviewRecap", + "maintainerRecap", +] as const; + +const TOP_LEVEL_FIELD_SET = new Set(TOP_LEVEL_FIELDS); + +// Fields retired from TOP_LEVEL_FIELDS that still warrant a migration-specific warning (rather than the +// generic "unknown field" message) pointing operators at their replacement mechanism. +const RETIRED_FIELD_MIGRATION_WARNINGS: Record = { + blockedPaths: "blockedPaths is retired; use settings.hardGuardrailGlobs for path holds.", +}; + +export function unknownTopLevelWarnings(text: string | null | undefined): string[] { + const raw = text ?? ""; + const trimmed = raw.trim(); + if (!trimmed || isOversize(raw)) return []; + const parsed = parseTopLevelObject(trimmed); + if (parsed === null) return []; + const keys = Object.keys(parsed).filter((key) => !TOP_LEVEL_FIELD_SET.has(key)); + // `hasOwnProperty.call`, NOT `key in`: a manifest field named like an Object.prototype member + // (`constructor`, `toString`, `hasOwnProperty`, ...) would otherwise test true for the inherited + // property and resolve to the prototype's function instead of a real retired-field warning string, + // corrupting the string[] result and suppressing the genuine unknown-field warning. + const isRetired = (key: string): boolean => Object.prototype.hasOwnProperty.call(RETIRED_FIELD_MIGRATION_WARNINGS, key); + const retiredWarnings = keys.filter(isRetired).map((key) => RETIRED_FIELD_MIGRATION_WARNINGS[key]!); + const unknown = keys.filter((key) => !isRetired(key)).map(formatFieldName); + return [ + ...retiredWarnings, + ...(unknown.length > 0 ? [`Manifest contains unknown top-level field${unknown.length === 1 ? "" : "s"}: ${unknown.join(", ")}.`] : []), + ]; +} + +function parseTopLevelObject(text: string): Record | null { + const looksLikeJson = text.startsWith("{") || text.startsWith("["); + if (looksLikeJson) { + try { + const parsed = JSON.parse(text); + return topLevelObjectOrNull(parsed); + } catch { + // YAML flow mappings can start with "{" or "[" while still being valid manifest syntax. + } + } + try { + return topLevelObjectOrNull(parseYaml(text)); + } catch { + return null; + } +} + +export function topLevelObjectOrNull(parsed: unknown): Record | null { + return parsed !== null && typeof parsed === "object" && !Array.isArray(parsed) + ? (parsed as Record) + : null; +} + +export function isOversize(text: string): boolean { + return text.length > MAX_FOCUS_MANIFEST_BYTES || new TextEncoder().encode(text).byteLength > MAX_FOCUS_MANIFEST_BYTES; +} + +function formatFieldName(name: string): string { + const trimmed = name.replace(/[^\w.-]/g, "_").slice(0, 80); + return trimmed || ""; +} + +export type FocusManifestValidationStatus = "ok" | "warn" | "error"; + +export type FocusManifestValidationResult = { + present: boolean; + warnings: string[]; + normalized: Record; + status: FocusManifestValidationStatus; +}; + +const PARSE_FAILURE_PATTERN = /not valid (JSON|YAML)|must be a mapping|exceeded \d+ bytes/i; + +export function buildFocusManifestValidation(input: { + content: string; + source?: FocusManifestSource | undefined; +}): FocusManifestValidationResult { + const manifest = parseFocusManifestContent(input.content, input.source ?? "repo_file"); + // Warn on unrecognized top-level fields (e.g. a typo'd `gates:` instead of `gate:`), matching the + // selfhost config-lint validator — parseFocusManifestContent reads only known fields, so a mistyped + // block is otherwise silently dropped with no warning (#5929). + const warnings = [...manifest.warnings, ...unknownTopLevelWarnings(input.content)]; + const normalized = focusManifestToNormalizedJson(manifest); + return { + present: manifest.present, + warnings, + normalized, + status: resolveValidationStatus(manifest, warnings), + }; +} + +function resolveValidationStatus(manifest: FocusManifest, warnings: string[]): FocusManifestValidationStatus { + if (warnings.some((warning) => PARSE_FAILURE_PATTERN.test(warning))) return "error"; + if (!manifest.present || warnings.length > 0) return "warn"; + return "ok"; +} + +function focusManifestToNormalizedJson(manifest: FocusManifest): Record { + const normalized: Record = { + present: manifest.present, + source: manifest.source, + }; + if (manifest.wantedPaths.length > 0) normalized.wantedPaths = manifest.wantedPaths; + if (manifest.preferredLabels.length > 0) normalized.preferredLabels = manifest.preferredLabels; + if (manifest.linkedIssuePolicy !== "optional") normalized.linkedIssuePolicy = manifest.linkedIssuePolicy; + if (manifest.testExpectations.length > 0) normalized.testExpectations = manifest.testExpectations; + if (manifest.issueDiscoveryPolicy !== "neutral") normalized.issueDiscoveryPolicy = manifest.issueDiscoveryPolicy; + if (manifest.publicNotes.length > 0) normalized.publicNotes = manifest.publicNotes; + + const gate = gateConfigToJson(manifest.gate); + if (gate !== null) normalized.gate = gate; + const settings = settingsOverrideToJson(manifest.settings); + if (settings !== null) normalized.settings = settings; + const review = reviewConfigToJson(manifest.review); + if (review !== null) normalized.review = review; + const features = featuresConfigToJson(manifest.features); + if (features !== null) normalized.features = features; + const contentLane = contentLaneConfigToJson(manifest.contentLane); + if (contentLane !== null) normalized.contentLane = contentLane; + const repoDocGeneration = repoDocGenerationConfigToJson(manifest.repoDocGeneration); + if (repoDocGeneration !== null) normalized.repoDocGeneration = repoDocGeneration; + const reviewRecap = reviewRecapConfigToJson(manifest.reviewRecap); + if (reviewRecap !== null) normalized.reviewRecap = reviewRecap; + const maintainerRecap = maintainerRecapConfigToJson(manifest.maintainerRecap); + if (maintainerRecap !== null) normalized.maintainerRecap = maintainerRecap; + + return normalized; +} diff --git a/packages/loopover-mcp/bin/loopover-mcp.js b/packages/loopover-mcp/bin/loopover-mcp.js index 93b3572300..cc9c217a49 100755 --- a/packages/loopover-mcp/bin/loopover-mcp.js +++ b/packages/loopover-mcp/bin/loopover-mcp.js @@ -6,6 +6,7 @@ import { delimiter, dirname, join } from "node:path"; import { McpServer, ResourceTemplate } from "@modelcontextprotocol/sdk/server/mcp.js"; import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; import { buildFeasibilityVerdict } from "@loopover/engine"; +import { buildFocusManifestValidation } from "@loopover/engine/focus-manifest-validation"; import { z } from "zod"; import { buildBranchAnalysisPayload, collectLocalDiff, collectLocalBranchMetadata, probeLocalScorer, referenceScorePreviewExample, resolveScorePreviewCommand, resolveWorkspaceCwd, sanitizeLocalScorerStatus, setupGuidanceForLocalScorer, isTestFile } from "../lib/local-branch.js"; import { formatTable } from "../lib/format-table.js"; @@ -819,7 +820,10 @@ registerStdioTool( description: stdioToolDescription("loopover_validate_config"), inputSchema: validateConfigShape, }, - async (input) => toolResult("LoopOver manifest validation.", await apiPost("/v1/validate/focus-manifest", input)), + // Computed in-process via @loopover/engine (#6269): the focus-manifest parser + validation result builder + // are deterministic and source-free, so the local server validates a .loopover.yml fully offline — no + // apiPost round-trip. The remote server keeps computing the identical result through its own import. + async (input) => toolResult("LoopOver manifest validation.", buildFocusManifestValidation(input)), ); registerStdioTool( diff --git a/packages/loopover-mcp/package.json b/packages/loopover-mcp/package.json index edf0c71f46..b7e2d9cd80 100644 --- a/packages/loopover-mcp/package.json +++ b/packages/loopover-mcp/package.json @@ -38,7 +38,7 @@ "build": "node --check bin/loopover-mcp.js && node --check lib/cli-error.js && node --check lib/local-branch.js && node --check lib/format-table.js && node --check scripts/gittensor-score-preview.mjs" }, "dependencies": { - "@loopover/engine": "^1.0.0", + "@loopover/engine": "^3.0.0", "@modelcontextprotocol/sdk": "1.29.0", "zod": "^4.4.3" }, diff --git a/src/selfhost/config-lint.ts b/src/selfhost/config-lint.ts index b1e78db458..5cb35ab7be 100644 --- a/src/selfhost/config-lint.ts +++ b/src/selfhost/config-lint.ts @@ -1,27 +1,12 @@ import { parse as parseYaml } from "yaml"; -import { MAX_FOCUS_MANIFEST_BYTES, parseFocusManifestContent } from "../signals/focus-manifest"; +import { parseFocusManifestContent } from "../signals/focus-manifest"; +import { + TOP_LEVEL_FIELDS, + isOversize, + topLevelObjectOrNull, + unknownTopLevelWarnings, +} from "../../packages/loopover-engine/src/focus-manifest-validation.js"; -const TOP_LEVEL_FIELDS = [ - "source", - "wantedPaths", - "preferredLabels", - "linkedIssuePolicy", - "testExpectations", - "issueDiscoveryPolicy", - "maintainerNotes", - "publicNotes", - "gate", - "settings", - "review", - "features", - "experimental", - "contentLane", - "repoDocGeneration", - "reviewRecap", - "maintainerRecap", -] as const; - -const TOP_LEVEL_FIELD_SET = new Set(TOP_LEVEL_FIELDS); const NO_RECOGNIZED_FOCUS_FIELDS_WARNING = "Manifest contained no recognized focus fields; falling back to deterministic signals."; @@ -63,32 +48,6 @@ function recognizedFieldsFor(text: string | null | undefined): string[] { ); } -// Fields retired from TOP_LEVEL_FIELDS that still warrant a migration-specific warning (rather than the -// generic "unknown field" message) pointing operators at their replacement mechanism. -const RETIRED_FIELD_MIGRATION_WARNINGS: Record = { - blockedPaths: "blockedPaths is retired; use settings.hardGuardrailGlobs for path holds.", -}; - -export function unknownTopLevelWarnings(text: string | null | undefined): string[] { - const raw = text ?? ""; - const trimmed = raw.trim(); - if (!trimmed || isOversize(raw)) return []; - const parsed = parseTopLevelObject(trimmed); - if (parsed === null) return []; - const keys = Object.keys(parsed).filter((key) => !TOP_LEVEL_FIELD_SET.has(key)); - // `hasOwnProperty.call`, NOT `key in`: a manifest field named like an Object.prototype member - // (`constructor`, `toString`, `hasOwnProperty`, ...) would otherwise test true for the inherited - // property and resolve to the prototype's function instead of a real retired-field warning string, - // corrupting the string[] result and suppressing the genuine unknown-field warning. - const isRetired = (key: string): boolean => Object.prototype.hasOwnProperty.call(RETIRED_FIELD_MIGRATION_WARNINGS, key); - const retiredWarnings = keys.filter(isRetired).map((key) => RETIRED_FIELD_MIGRATION_WARNINGS[key]!); - const unknown = keys.filter((key) => !isRetired(key)).map(formatFieldName); - return [ - ...retiredWarnings, - ...(unknown.length > 0 ? [`Manifest contains unknown top-level field${unknown.length === 1 ? "" : "s"}: ${unknown.join(", ")}.`] : []), - ]; -} - function parseCanonicalTopLevelObject(text: string | null | undefined): Record | null { const raw = text ?? ""; const trimmed = raw.trim(); @@ -101,38 +60,6 @@ function parseCanonicalTopLevelObject(text: string | null | undefined): Record | null { - const looksLikeJson = text.startsWith("{") || text.startsWith("["); - if (looksLikeJson) { - try { - const parsed = JSON.parse(text); - return topLevelObjectOrNull(parsed); - } catch { - // YAML flow mappings can start with "{" or "[" while still being valid manifest syntax. - } - } - try { - return topLevelObjectOrNull(parseYaml(text)); - } catch { - return null; - } -} - -function topLevelObjectOrNull(parsed: unknown): Record | null { - return parsed !== null && typeof parsed === "object" && !Array.isArray(parsed) - ? (parsed as Record) - : null; -} - -function isOversize(text: string): boolean { - return text.length > MAX_FOCUS_MANIFEST_BYTES || new TextEncoder().encode(text).byteLength > MAX_FOCUS_MANIFEST_BYTES; -} - -function formatFieldName(name: string): string { - const trimmed = name.replace(/[^\w.-]/g, "_").slice(0, 80); - return trimmed || ""; -} - function redactManifestWarning(warning: string): string { return warning .replace(/; ignoring "[^"]*"\./g, "; ignoring the supplied value.") diff --git a/src/services/focus-manifest-validation.ts b/src/services/focus-manifest-validation.ts index c1055585d0..3ec9b069ea 100644 --- a/src/services/focus-manifest-validation.ts +++ b/src/services/focus-manifest-validation.ts @@ -1,81 +1,11 @@ -import { - contentLaneConfigToJson, - featuresConfigToJson, - gateConfigToJson, - parseFocusManifestContent, - repoDocGenerationConfigToJson, - reviewConfigToJson, - reviewRecapConfigToJson, - maintainerRecapConfigToJson, - settingsOverrideToJson, - type FocusManifest, - type FocusManifestSource, -} from "../signals/focus-manifest"; -import { unknownTopLevelWarnings } from "../selfhost/config-lint"; - -export type FocusManifestValidationStatus = "ok" | "warn" | "error"; - -export type FocusManifestValidationResult = { - present: boolean; - warnings: string[]; - normalized: Record; - status: FocusManifestValidationStatus; -}; - -const PARSE_FAILURE_PATTERN = /not valid (JSON|YAML)|must be a mapping|exceeded \d+ bytes/i; - -export function buildFocusManifestValidation(input: { - content: string; - source?: FocusManifestSource | undefined; -}): FocusManifestValidationResult { - const manifest = parseFocusManifestContent(input.content, input.source ?? "repo_file"); - // Warn on unrecognized top-level fields (e.g. a typo'd `gates:` instead of `gate:`), matching the - // selfhost config-lint validator — parseFocusManifestContent reads only known fields, so a mistyped - // block is otherwise silently dropped with no warning (#5929). - const warnings = [...manifest.warnings, ...unknownTopLevelWarnings(input.content)]; - const normalized = focusManifestToNormalizedJson(manifest); - return { - present: manifest.present, - warnings, - normalized, - status: resolveValidationStatus(manifest, warnings), - }; -} - -function resolveValidationStatus(manifest: FocusManifest, warnings: string[]): FocusManifestValidationStatus { - if (warnings.some((warning) => PARSE_FAILURE_PATTERN.test(warning))) return "error"; - if (!manifest.present || warnings.length > 0) return "warn"; - return "ok"; -} - -function focusManifestToNormalizedJson(manifest: FocusManifest): Record { - const normalized: Record = { - present: manifest.present, - source: manifest.source, - }; - if (manifest.wantedPaths.length > 0) normalized.wantedPaths = manifest.wantedPaths; - if (manifest.preferredLabels.length > 0) normalized.preferredLabels = manifest.preferredLabels; - if (manifest.linkedIssuePolicy !== "optional") normalized.linkedIssuePolicy = manifest.linkedIssuePolicy; - if (manifest.testExpectations.length > 0) normalized.testExpectations = manifest.testExpectations; - if (manifest.issueDiscoveryPolicy !== "neutral") normalized.issueDiscoveryPolicy = manifest.issueDiscoveryPolicy; - if (manifest.publicNotes.length > 0) normalized.publicNotes = manifest.publicNotes; - - const gate = gateConfigToJson(manifest.gate); - if (gate !== null) normalized.gate = gate; - const settings = settingsOverrideToJson(manifest.settings); - if (settings !== null) normalized.settings = settings; - const review = reviewConfigToJson(manifest.review); - if (review !== null) normalized.review = review; - const features = featuresConfigToJson(manifest.features); - if (features !== null) normalized.features = features; - const contentLane = contentLaneConfigToJson(manifest.contentLane); - if (contentLane !== null) normalized.contentLane = contentLane; - const repoDocGeneration = repoDocGenerationConfigToJson(manifest.repoDocGeneration); - if (repoDocGeneration !== null) normalized.repoDocGeneration = repoDocGeneration; - const reviewRecap = reviewRecapConfigToJson(manifest.reviewRecap); - if (reviewRecap !== null) normalized.reviewRecap = reviewRecap; - const maintainerRecap = maintainerRecapConfigToJson(manifest.maintainerRecap); - if (maintainerRecap !== null) normalized.maintainerRecap = maintainerRecap; - - return normalized; -} +/** + * Focus-manifest validation shim (#6269). The result builder now lives engine-side in + * `packages/loopover-engine/src/focus-manifest-validation.ts` so the local `loopover_validate_config` MCP + * tool can compute it in-process (fully offline). This file re-exports the engine surface for the existing + * app callers (`src/api/routes.ts`, `src/mcp/server.ts`), which keep importing from here unchanged. + */ +export { + buildFocusManifestValidation, + type FocusManifestValidationResult, + type FocusManifestValidationStatus, +} from "../../packages/loopover-engine/src/focus-manifest-validation.js";