Skip to content

feat(mcp): extract buildFocusManifestValidation into @loopover/engine for offline loopover_validate_config - #6305

Closed
nghetienhiep wants to merge 1 commit into
JSONbored:mainfrom
nghetienhiep:fix/issue-6269
Closed

feat(mcp): extract buildFocusManifestValidation into @loopover/engine for offline loopover_validate_config#6305
nghetienhiep wants to merge 1 commit into
JSONbored:mainfrom
nghetienhiep:fix/issue-6269

Conversation

@nghetienhiep

Copy link
Copy Markdown
Contributor

Summary

  • Extracts buildFocusManifestValidation (the focus-manifest validation-result builder) out of src/services/focus-manifest-validation.ts into @loopover/engine (packages/loopover-engine/src/focus-manifest-validation.ts), leaving a thin re-export shim at the original src/ path so the remote server (src/api/routes.ts, src/mcp/server.ts) keeps importing it unchanged.
  • Local loopover_validate_config now computes the result in-process via the engine instead of apiPost("/v1/validate/focus-manifest", …), so a user running the local MCP server can validate a .loopover.yml fully offline. Verified identical output to the remote route.

Closes #6269

Scope

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage — the extraction + the two src/ files it touches are at 100% patch coverage (statements/branches/functions/lines) under the existing focus-manifest-validation and selfhost-config-lint suites; no new uncovered lines/branches. (The unrelated sqlite3-backed and miner-discover suites fail only in this sandbox for lack of a local sqlite3 binary — identical failures on a clean checkout of main.)
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run engine-parity:drift-check, test:engine-parity, manifest:drift-check, docs:drift-check, command-reference:check, selfhost:env-reference:check
  • npm audit --audit-level=moderate (0 vulnerabilities)
  • New/changed behavior has tests — the existing test/unit/focus-manifest-validation.test.ts (13 cases) and test/unit/selfhost-config-lint.test.ts (20 cases) exercise every branch of the moved code against its new location and continue to pass.

If any required check was skipped, explain why:

  • ui:lint / ui:typecheck / ui:build are unaffected — this PR touches no apps/** UI code (no UI surface).

Safety

  • No secrets, wallet details, hotkeys, coldkeys, PATs, private keys, trust scores, private rankings, or maintainer evidence are exposed. The extracted code is pure, deterministic, and source-free; existing tests assert the normalized output never echoes private maintainerNotes.
  • Public GitHub text stays sanitized and low-noise.
  • No auth, cookie, CORS, GitHub App, Cloudflare, or session changes.
  • MCP behavior is updated and tested — the local loopover_validate_config handler now returns the byte-identical result the remote POST /v1/validate/focus-manifest route produces.
  • No UI data-source changes.
  • No visible UI changes (no UI Evidence needed).
  • No docs/changelog edits required.

Notes

  • unknownTopLevelWarnings co-moves with the builder (required, not scope-creep). buildFocusManifestValidation depends on unknownTopLevelWarnings, which lived in src/selfhost/config-lint.ts; the engine cannot import from src/, so that helper (plus its TOP_LEVEL_FIELDS allowlist and the isOversize/topLevelObjectOrNull primitives) moves into the engine module as the single source of truth. config-lint.ts now imports them back — no duplicate implementation, and its recognized-field report keeps sharing the same allowlist so the two surfaces cannot drift (the class of miss feat(config): add .gittensory.yml surface to enable/scope repo-doc generation #3002/Add "experimental" to config-lint.ts's TOP_LEVEL_FIELDS allowlist #5281 fixed).
  • @loopover/engine dependency bump in packages/loopover-mcp. The local MCP package pinned @loopover/engine@^1.0.0 (a published copy that predates this function), so it could not consume the extracted symbol. Bumped to ^3.0.0 to track the workspace engine — mirroring packages/loopover-miner, which already declares ^3.0.0 and consumes workspace-only 3.x engine exports the same way. package-lock.json is regenerated accordingly (the stale nested @loopover/engine@1.0.0 is dropped in favor of the workspace link).
  • No behavior change to the remote server: it still imports buildFocusManifestValidation from src/services/focus-manifest-validation.ts, now resolving through the shim to the 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 JSONbored#6269
@nghetienhiep
nghetienhiep requested a review from JSONbored as a code owner July 16, 2026 00:35
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb loopover-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 16, 2026
@loopover-orb

loopover-orb Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - fixes required

Review updated: 2026-07-16 00:48:44 UTC

7 files · 1 AI reviewer · no blockers · CI failing · blocked

🛑 Suggested Action - Fix Blockers

Review summary
This PR relocates the existing `buildFocusManifestValidation` logic and its `unknownTopLevelWarnings`/`TOP_LEVEL_FIELDS` helpers verbatim into `packages/loopover-engine/src/focus-manifest-validation.ts`, leaving `src/services/focus-manifest-validation.ts` and `src/selfhost/config-lint.ts` as thin re-export/import shims, and switches `loopover-mcp.js`'s `loopover_validate_config` tool from an `apiPost` round-trip to calling the engine function in-process. The extracted code is a faithful copy (comments and logic unchanged), and the package-lock/package.json updates correctly bump `@​loopover/engine` to `^3.0.0` and drop a stale nested `node_modules` copy pinned at `1.0.0` that lacked the new `./focus-manifest-validation` export subpath — that fix is necessary and easy to miss. The one real behavioral change in this diff (removing the network round-trip in favor of local computation) ships with no visible test coverage anywhere in the diff, which is notable given the PR's coverage claims.

Nits — 6 non-blocking
  • The core behavioral change — `loopover-mcp.js`'s `loopover_validate_config` switching from `apiPost` to `buildFocusManifestValidation(input)` — has zero test-path evidence in this diff; the PR description only claims manual verification of identical output, not an automated regression test for the new offline code path.
  • `src/services/focus-manifest-validation.ts` and `src/selfhost/config-lint.ts` import the moved code via a relative path into `../../packages/loopover-engine/src/...`, while `loopover-mcp.js` imports the same function through the published `@​loopover/engine/focus-manifest-validation` package export — worth a comment or confirmation that this dual import strategy (src-relative vs. package-export) is intentional and won't drift if the engine package's internal file layout changes.
  • `formatFieldName`, `parseTopLevelObject`, and `topLevelObjectOrNull` are duplicated/removed from `config-lint.ts` in favor of the engine copy — confirm no other module in `src/` still expects these to be locally defined in `config-lint.ts` (grep turned up none in the diff, but worth double-checking call sites outside the visible hunks).
  • Add or point to a test that exercises the new in-process `buildFocusManifestValidation(input)` call inside `loopover-mcp.js` (packages/loopover-mcp/bin/loopover-mcp.js) rather than relying solely on manual verification.
  • Consider having `src/services/focus-manifest-validation.ts` and `src/selfhost/config-lint.ts` import from the same `@​loopover/engine/focus-manifest-validation` package export that `loopover-mcp.js` uses, for a single consistent import surface across the monorepo.
  • Code changes lack test evidence — Add focused regression tests or explain why existing coverage is sufficient.

CI checks failing

  • validate
  • validate-code

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #6269
Related work ⚠️ 3 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 112 registered-repo PR(s), 64 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor nghetienhiep; Gittensor profile; 112 PR(s), 0 issue(s).
Improvement ℹ️ None detected risk: low · value: none · LLM: moderate
Linked issue satisfaction

Addressed
The PR extracts buildFocusManifestValidation and its helper functions into packages/loopover-engine/src/focus-manifest-validation.ts, leaves a re-export shim at the original src/services path for existing app callers, adds the exports map entry in the engine's package.json, and updates the local loopover_validate_config handler in loopover-mcp.js to call buildFocusManifestValidation directly inste

Review context
Contributor next steps
  • Start here: Review top overlaps.
  • Then work through the remaining 2 steps in the Signals table above.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb

loopover-orb Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (CI is failing (validate, validate-code)). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

@loopover-orb loopover-orb Bot closed this Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(mcp): extract buildFocusManifestValidation into @loopover/engine so loopover_validate_config can work offline

1 participant