HarperFast / HarperFast/harper

MCP: remove lazy require() circular-dependency workarounds once TypeScript migration lands

Open
#879 0 comments 0 reactions 0 assignees View on GitHub
area:build area:mcp feature:mcp-v1
Dominant language
JavaScript
Stars
89
Forks
10
Avg merge
2d 6h
Merged PRs (30d)
200

Description

## Context

Follow-up from review of #856 (MCP v1 tools surface).

Several MCP modules reach Harper internals through **lazy `require()` inside try/catch** rather than top-level `import`, to dodge circular-dependency / boot-order init (RocksDB lock acquisition, schema preload, eager graph init) that breaks when the module is loaded from a unit test that hasn't booted Harper:

- `loadServerUtilities()` — `components/mcp/tools/operations.ts` (`require('../../../server/serverHelpers/serverUtilities')`)
- `loadResources()` — `components/mcp/tools/application.ts` (`require('../../../resources/Resources')`)
- `loadRequestTarget()` — `components/mcp/tools/application.ts` (`require('../../../resources/RequestTarget')`)
- `loadItcHandlers()` — `components/mcp/listChanged.ts` (`require('../../../server/itc/serverHandlers')`)

Each is paired with a test-seam override (`_set*ForTest`) and a graceful no-op-on-failure path.

## Problem

- These are workarounds, not intent — they obscure the real dependency graph and defeat static analysis.
- The graceful-degrade-to-no-op paths can mask genuine wiring failures (a real "Resources unavailable" looks the same as "not in a Harper process").

## Proposed direction

Once the broader TypeScript migration / module-graph PR lands (which should mitigate the circular-dependency and eager-init issues), revisit these sites and convert them to top-level `import`s where possible. Keep test seams only where genuinely needed for isolation. Confirm each removal doesn't reintroduce a boot-order or circular-import failure.

## Acceptance

- Lazy `require()` workarounds replaced with static imports where the dependency graph now allows.
- Test seams retained only where required; no silent no-op masking of real failures.

_Filed by Claude on behalf of @kriszyp during #856 review._

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.