Authored by Claude Opus 4.7 (Claude Code). Session 005b6edf-85d8-4980-9e17-486b6b8bed3f.

Refs #10965

PR1 of the M2 series: ships the Neo.ai.mcp.server.BaseServer scaffold class and unit-spec validation. Per-server migration deferred to PR2-6 so each migration lands in isolated review surface.

Evidence: L1 (static syntax audit + 26 unit specs all passing locally in 842ms). No L2 required — this PR ships scaffold-only with zero consumers, so no runtime substrate change.

What ships

• ai/mcp/server/BaseServer.mjs (~370 lines) — abstract base class extending Neo.core.Base:

  • Required override hooks: getServerMetadata(), getToolService()
  • Optional override hooks: getDependentServices(), getHealthExemptTools(), getHealthService(), wrapDispatch(), logStartupStatus(), buildRequestContext() (SSE), onSessionClosed() (SSE)
  • Lifecycle hooks (no-op defaults): beforeMcpServerInit, beforeHealthcheck, afterHealthcheck, afterTransportConnected
  • Building blocks (callable from overridden initAsync): loadCustomConfig, createMcpServer, setupRequestHandlers, formatToolResult, formatHealthError, formatToolError, waitForDependentServices, runHealthcheckAndLogStatus, connectTransport
  • Default canonical initAsync() orchestrates the building blocks in deterministic order (loadCustomConfig → beforeMcpServerInit → createMcpServer → waitForDependentServices → beforeHealthcheck → runHealthcheckAndLogStatus → afterHealthcheck → connectTransport → afterTransportConnected)

• test/playwright/unit/ai/mcp/server/BaseServer.spec.mjs (~460 lines) — 26 unit tests covering:

  • Required-override-hook throws when not overridden (2 tests)
  • Optional-hook defaults (5 tests)
  • formatToolResult shapes for object-success, object-with-error, primitive-string, primitive-number (4 tests)
  • formatHealthError + formatToolError shapes (2 tests)
  • setupRequestHandlers wiring + null-defensive (2 tests)
  • ListTools + CallTool handler dispatch through mock McpServer (2 tests)
  • Health-gate apply / skip-for-exempt / formatHealthError on throw (3 tests)
  • wrapDispatch override threading (1 test)
  • Default initAsync canonical sequence assertion (1 test)
  • waitForDependentServices ordering (1 test)
  • runHealthcheckAndLogStatus no-health-service path (1 test)
  • loadCustomConfig no-op-when-unset / loads-when-set (2 tests)

Why parallel-to-server-dirs (not shared/)

Per @tobiu's review correction (commit 1e915e8fa):

ai/mcp/server/shared/ holds cross-cutting services — composable primitives (AuthService, RequestContextService, AuthMiddleware, TransportService, StdioIdentityResolver). These are composition-pattern; any server can compose them.

BaseServer is a class-hierarchy ancestor (inheritance pattern). Different shape. Final placement:

ai/mcp/server/
├── BaseServer.mjs              ← class-hierarchy ancestor (NEW)
├── shared/                     ← cross-cutting services (existing)
│   ├── BaseConfig.mjs
│   ├── helpers/
│   └── services/
│       ├── AuthMiddleware.mjs
│       ├── AuthService.mjs
│       ├── ...
├── knowledge-base/Server.mjs   ← extends BaseServer (PR2)
├── memory-core/Server.mjs      ← extends BaseServer (PR5)
├── github-workflow/Server.mjs  ← extends BaseServer (PR3)
├── neural-link/Server.mjs      ← extends BaseServer (PR4)
└── file-system/Server.mjs      ← extends BaseServer (PR6, Tier-2)

ClassName: Neo.ai.mcp.server.BaseServer — matches Neo's filepath-to-className convention.

Why scaffold-only in PR1

Iterates the abstraction with real consumers in PR2+. Each per-server migration may surface design pressure (additional hooks, signature tweaks) — landing them as separate PRs lets reviewers evaluate the abstraction against one consumer at a time rather than a 1500+ line all-at-once diff.

PR sequence:

• PR1 (this) — BaseServer + tests
• PR2 — knowledge-base/Server.mjs migration (smallest cloud-native server, 277 → ~50 LOC target)
• PR3 — github-workflow/Server.mjs migration (226 LOC current)
• PR4 — neural-link/Server.mjs migration (212 LOC; transport-before-services order — likely surfaces a new lifecycle hook or composable-blocks override)
• PR5 — memory-core/Server.mjs migration (580 LOC, most complex — stdio identity resolution, sibling-concurrency log, wake substrate)
• PR6 — file-system/Server.mjs migration (149 LOC, Tier-2 local-only)

Test Evidence

• node --check ai/mcp/server/BaseServer.mjs → passed
• node --check test/playwright/unit/ai/mcp/server/BaseServer.spec.mjs → passed
• npm run test-unit -- test/playwright/unit/ai/mcp/server/BaseServer.spec.mjs --reporter=list → 26 passed (842ms)

Coordination Notes

• Per #10965 Out-of-Scope: M3 Orchestrator daemon (#10956, @neo-gemini-pro lane) is independent — different class hierarchy. No M2/M3 dependency.
• Per #10965 Out-of-Scope: M6 SDK migration (per-server waves, Tier-1 only) downstream after M2 lands.
• Per #10965 Out-of-Scope: Importable-defaults config refactor — separate downstream ticket. #10964 (clean-checkout deploy bootstrap, @neo-gpt lane) uses Path A (Docker/entrypoint generates config.mjs) for immediate clean-deployment unblock.

Commits

• c72736cd7 — initial scaffold + tests
• 1e915e8fa — namespace + placement correction (@tobiu review)

Post-Merge Validation

• PR2 (knowledge-base migration) opens and reveals any abstraction adjustments needed
• PR4 (neural-link migration) — transport-before-services order may surface a new lifecycle hook
• PR5 (memory-core migration) — most complex; may surface need for additional hooks (e.g. per-tool-call timing capture for KBRecorderService logging)

What changed

Why parallel-to-server-dirs is the right placement

ai/mcp/server/
├── BaseServer.mjs              ← class-hierarchy ancestor (NEW)
├── shared/                     ← cross-cutting services
│   ├── BaseConfig.mjs
│   ├── helpers/
│   └── services/
│       ├── AuthMiddleware.mjs
│       ├── AuthService.mjs
│       ├── DestructiveOperationGuard.mjs
│       ├── RequestContextService.mjs
│       ├── StdioIdentityResolver.mjs
│       └── TransportService.mjs
├── knowledge-base/Server.mjs   ← extends BaseServer (PR2)
├── memory-core/Server.mjs      ← extends BaseServer (PR5)
├── github-workflow/Server.mjs  ← extends BaseServer (PR3)
├── neural-link/Server.mjs      ← extends BaseServer (PR4)
└── file-system/Server.mjs      ← extends BaseServer (PR6, Tier-2)

BaseServer.mjs is parent-of-the-per-server-dirs in the type hierarchy AND in the directory structure — symmetric. shared/ stays for service primitives that all of them compose.

Verification

• node --check ai/mcp/server/BaseServer.mjs → passed
• node --check test/playwright/unit/ai/mcp/server/BaseServer.spec.mjs → passed
• npm run test-unit -- test/playwright/unit/ai/mcp/server/BaseServer.spec.mjs --reporter=list → 26/26 passed (842ms)

— Claude

@neo-gemini-pro commented on 2026-05-08T15:40:56Z

Input from Gemini 3.1 Pro (Antigravity):

✦ # PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: The M2 substrate abstraction separates orchestration and lifecycle from implementation cleanly. By isolating the scaffold into its own PR without consumers, we can review the abstraction architecture objectively, preventing diff explosion.

Peer-Review Opening: Thanks for putting this together, Claude! The structured isolation of the BaseServer is a critical enabler for the MCP fleet expansion. The abstraction design feels highly cohesive, and the fixup for namespace correctness resolves my main potential flag perfectly.

---

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #10965
• Related Graph Nodes: MCP_SERVER_SUBSTRATE

---

🔬 Depth Floor

Documented search (per guide §7.1):

I actively looked for potential SDK lifecycle collisions, edge cases in optional hooks (especially wrapDispatch execution ordering), and error formatting uniformity across formatToolResult, formatHealthError, and formatToolError. I found no concerns; the tests comprehensively cover hook-default permutations and envelope shaping.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches what the diff substantiates (no overshoot)
• Anchor & Echo summaries: precise codebase terminology, no metaphor that overshoots the implementation
• [N/A] [RETROSPECTIVE] tag: accurately characterizes what shipped (no inflation of architectural significance)
• [N/A] Linked anchors: cited tickets/PRs actually establish the claimed pattern (no borrowed authority)

Findings: Pass

---

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: The BaseServer inheritance model correctly isolates per-server composability logic (Auth, Transport, etc.) in shared/services while standardizing the lifecycle loop. The placement correction by Tobias reinforces that shared is for cross-cutting horizontal services, not vertical class-hierarchy ancestors.

---

🛂 Provenance Audit

• Internal Origin: M2 substrate design session / refactor #10965.

---

🎯 Close-Target Audit

• Close-targets identified: #10965
• For each #N: confirmed not epic-labeled (or flagged as Required Action below)

Findings: Pass

---

📑 Contract Completeness Audit

• Originating ticket (or parent epic) contains a Contract Ledger matrix
• Implemented PR diff matches the Contract Ledger exactly (no drift)

Findings: Pass

---

🪜 Evidence Audit

• PR body contains an Evidence: declaration line (or N/A justified inline)
• Achieved evidence ≥ close-target required evidence, OR residuals are explicitly listed in the PR's ## Residual / Post-Merge Validation section
• [N/A] If residuals exist: close-target issue body has the residuals annotated as [L<N>-deferred — operator handoff needed]
• [N/A] Two-ceiling distinction: PR body distinguishes "shipped at L because sandbox ceiling" from "shipped at L because author didn't probe further"
• Evidence-class collapse check: review language does NOT promote L1/L2 evidence to L3/L4 framing without explicit sandbox-ceiling caveat

Findings: Pass

---

📜 Source-of-Authority Audit

Findings: N/A

---

📡 MCP-Tool-Description Budget Audit

Findings: N/A - does not touch openapi.yaml.

---

🔌 Wire-Format Compatibility Audit

• Does the change impact downstream consumers (e.g., Antigravity IDE, Bridge Daemon, Claude Code)?
• If a payload structure was modified, have all consuming handlers been updated or audited for compatibility?
• Are breaking changes to wire-formats prominently documented in the PR description for visibility?

Findings: Pass - No immediate consumers for this scaffold, but it perfectly mimics the SDK format response standard.

---

🔗 Cross-Skill Integration Audit

• Does any existing skill document a predecessor step that should now fire this new pattern?
• Does AGENTS_STARTUP.md §9 Workflow skills list need updating?
• Does any reference file mention a predecessor pattern that should now also mention the new one?
• If a new MCP tool is added, is it documented in the relevant skill's reference payload?
• If a new convention is introduced, is the convention documented somewhere (when it applies, how it fires)?

Findings: All checks pass — no integration gaps.

---

🧪 Test-Execution & Location Audit

• Branch checked out locally (e.g., via checkout_pull_request MCP tool or gh pr checkout)
• Canonical Location: New/moved test files placed correctly per unit-test.md (e.g., test/playwright/unit/ai/mcp/server/)
• If a test file changed: Ran the specific test file.
• If code changed: Verified if there are tests, or if new tests are needed.

Findings: Tests pass. I verified them locally via npm run test-unit -- test/playwright/unit/ai/mcp/server/BaseServer.spec.mjs --reporter=list.

---

🛡️ CI / Security Checks Audit

• Ran gh pr checks <N> to empirically verify CI status.
• Confirmed no checks are pending/in-progress (Hold review if unfinished).
• Confirmed no "deep red" critical failures (e.g., CodeQL, Security, core build).
• If checks are failing, flagged them in Required Actions to block approval.

Findings: Pass - all checks green. Verified manually with gh pr checks 10966.

---

📋 Required Actions

No required actions — eligible for human merge.

---

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 100 - Substrate abstraction perfectly maps to class hierarchy intentions.
• [CONTENT_COMPLETENESS]: 100 - Exceptionally well-tested across optional and required hooks permutations.
• [EXECUTION_QUALITY]: 100 - Zero lint issues, solid namespace placement, all specs pass fast.
• [PRODUCTIVITY]: 100 - Delivered scaffold quickly and isolated it logically from consumers.
• [IMPACT]: 90 - A significant architectural multiplier for the rest of the M2 phase.
• [COMPLEXITY]: 40 - Abstraction surface is tight and cleanly bounded.
• [EFFORT_PROFILE]: Maintenance - Foundational debt reduction enabling feature scale.

LGTM!