---
id: 11107
title: Move toolService.mjs files back to MCP server directories (M6 retrospective architectural fix)
state: CLOSED
labels:
  - enhancement
  - ai
  - refactoring
  - architecture
  - model-experience
assignees:
  - neo-opus-ada
createdAt: '2026-05-10T13:14:54Z'
updatedAt: '2026-05-12T08:05:33Z'
githubUrl: 'https://github.com/neomjs/neo/issues/11107'
author: neo-opus-ada
commentsCount: 0
parentIssue: null
subIssues: []
subIssuesCompleted: 0
subIssuesTotal: 0
blockedBy: []
blocking: []
closedAt: '2026-05-12T08:05:33Z'
---
# Move toolService.mjs files back to MCP server directories (M6 retrospective architectural fix)

### Context

M6 epic [#10986](https://github.com/neomjs/neo/issues/10986) ("Migrate Tier-1 MCP services to flat SDK boundary", closed 2026-05-09) moved 4 `toolService.mjs` files from `ai/mcp/server/<name>/services/toolService.mjs` to `ai/services/<name>/toolService.mjs` as part of the SDK-flat migration. Operator @tobiu surfaced post-merge:

> "the goal was to move services outside mcp servers into the sdk. you guys rubber stamped it, and literally moved all one by one. for most services correct, except for the toolService files (one per server). these literally just map service methods to mcp tools. and these REALLY belong into the mcp servers."

Empirical anchor: github-workflow `toolService.mjs` is 48 lines of pure MCP-glue (imports `ToolService` MCP-base + 9 reusable service classes; defines `serviceMapping` MCP-tool-name → service-method; creates `Neo.create(ToolService, {openApiFilePath, serviceMapping})`; exports `callTool` / `listTools`). Same shape across all 4 (memory-core, knowledge-base, neural-link, github-workflow).

### The Problem

The 4 `toolService.mjs` files were moved as part of M6's mechanical migration without the substrate-quality test asking: **what makes a file SDK-suitable vs MCP-server-suitable?**

| Property | SDK-suitable | MCP-server-suitable |
|---|---|---|
| Cross-server reusable | yes | no |
| Independent of MCP framing | yes | no |
| Ships per-server `openapi.yaml` | no | yes |
| Wires MCP-tool-name → method | no | **yes** |
| Owns `ToolService` instantiation | no | yes |

`toolService.mjs` is a perfect-match **MCP-server-suitable** file — every line is MCP-server-specific glue. Yet it lives at `ai/services/<name>/`, an SDK-suitable namespace. This created two co-resident bugs in github-workflow's case (`ToolService` import + `openapi.yaml` path) because relative paths from the new location reach awkwardly into `ai/mcp/server/<name>/` to find their MCP-server-side dependencies. **PR #11103 / #11104 fix the immediate boot crash; the architectural fix is to put these files back where they belong.**

Same family as @tobiu's 4-item retrospective: rubber-stamp at M6 epic-level let the substrate-quality test slip; PR-level rubber-stamp let the resulting wrong-shape mechanical moves slip; result is wrong-shape architecture + multiple bugs hiding in awkward relative paths.

### The Fix

Move all 4 `toolService.mjs` files back to MCP server directories:

```diff
-ai/services/memory-core/toolService.mjs
+ai/mcp/server/memory-core/toolService.mjs

-ai/services/knowledge-base/toolService.mjs
+ai/mcp/server/knowledge-base/toolService.mjs

-ai/services/neural-link/toolService.mjs
+ai/mcp/server/neural-link/toolService.mjs

-ai/services/github-workflow/toolService.mjs
+ai/mcp/server/github-workflow/toolService.mjs
```

Per-file relative-path updates:
- `ToolService` import: `'../../mcp/ToolService.mjs'` → `'../ToolService.mjs'` (one less level)
- `openapi.yaml` path: `path.join(__dirname, '../../mcp/server/<name>/openapi.yaml')` → `path.join(__dirname, 'openapi.yaml')` (co-resident again)
- Service-class imports: `'./IssueService.mjs'` → `'../../../services/<name>/IssueService.mjs'` (now reaching INTO SDK for the reusable logic)

Naturally subsumes:
- PR #11104's import fix (file moves entirely, fix obsoletes itself)
- The openapi.yaml path bug (whether folded into PR #11106 or separately)

### Acceptance Criteria

- [ ] All 4 `toolService.mjs` files moved back to `ai/mcp/server/<name>/toolService.mjs`
- [ ] Per-file imports updated: `ToolService` (1 dot up), service-class imports (reach into SDK), `openapi.yaml` (co-resident path)
- [ ] Boot smoke test for each MCP server: `node ai/mcp/server/<name>/mcp-server.mjs` survives 3s probe
- [ ] **Listing/calling tools**: `listTools()` and `callTool()` exercise actually loads `openapi.yaml` and resolves service-method bindings (the test that would catch lazy-load-time failures)
- [ ] All existing test specs still pass (per-service tests in `test/playwright/unit/ai/services/` are unaffected because they import service classes directly, not `toolService.mjs`)
- [ ] PR body documents the architectural rationale + cites the substrate-quality test (cross-server reusable vs MCP-server-suitable)

### Out of Scope

- Other M6-migrated files (service classes themselves, ChromaManager, etc.) — those ARE SDK-suitable per the substrate-quality test and stay in `ai/services/`
- The CI gap that hid these bugs (separate ticket)
- ChromaManager dedup (separate ticket)
- M6 retrospective meta-lesson + epic-review skill enhancement (separate Discussion)

### Avoided Traps / Gold Standards Rejected

#### Decision Matrix

1. **Move 4 toolService.mjs back to MCP server dirs (Selected):** Architectural-correct shape per the substrate-quality test. Naturally fixes both broken paths in github-workflow case. Symmetric across all 4 servers.
2. **Refactor toolService.mjs into a generic MCP-glue primitive:** Rejected as scope-creep. Each toolService.mjs is per-server-specific (different `serviceMapping`); a generic primitive would obscure that specificity.
3. **Keep toolService.mjs in `ai/services/<name>/` and just fix the relative paths:** Rejected. Treats the symptom (broken paths) without addressing the root cause (wrong-shape placement). Per `feedback_challenge_prescribed_fixes`: when premise is wrong, fixing the prescription doesn't help.
4. **Move only github-workflow's toolService.mjs back (since it was the one with bugs):** Rejected. Asymmetric — doesn't address the root cause for the other 3 (their relative paths happen to work but the architectural shape is still wrong).

- **Trap:** Treating "everything moves to SDK" as the M6 success criterion. **Rejection:** The success criterion was substrate-correct architecture, not file-count moved. Mechanical migration without substrate-quality test = wrong-shape outcome.
- **Trap:** Letting PR #11104's surgical import-fix + (potential) #11106's openapi-fix substitute for architectural correctness. **Rejection:** Surgical fixes unblock the boot crash; they don't restore correct architecture. Both are needed: surgical for short-term, architectural for substrate-correctness.

### Related

- M6 epic #10986 (closed; the migration that created this gap)
- PR #10997 (sub-ticket execution that mechanically moved the toolService.mjs files)
- PR #11104 (#11103) — surgical fix for github-workflow ToolService import
- PR #11106 (#11105) — proposed home for the github-workflow openapi.yaml fix per @tobiu's piggyback directive
- @tobiu's M6 4-item retrospective (this turn 2026-05-10)
- AGENTS.md §3.5 V-B-A core value (would have caught the substrate-quality miss at M6 epic time)

Origin Session ID: c2912891-b459-4a03-b2af-154d5e264df1
Retrieval Hint: "M6 retrospective", "toolService MCP-glue misplacement", "SDK-suitable substrate test", "architectural fix vs surgical fix"

## Timeline

- 2026-05-10T13:14:56Z @neo-opus-ada assigned to @neo-opus-ada
- 2026-05-10T13:14:56Z @neo-opus-ada added the `enhancement` label
- 2026-05-10T13:14:56Z @neo-opus-ada added the `ai` label
- 2026-05-10T13:14:56Z @neo-opus-ada added the `refactoring` label
- 2026-05-10T13:14:57Z @neo-opus-ada added the `architecture` label
- 2026-05-10T13:14:57Z @neo-opus-ada added the `model-experience` label
- 2026-05-10T13:24:04Z @neo-opus-ada cross-referenced by PR #11106
- 2026-05-10T13:35:28Z @neo-opus-ada cross-referenced by #11110
- 2026-05-10T13:37:07Z @neo-opus-ada cross-referenced by #11111
- 2026-05-11T19:58:17Z @neo-gemini-pro cross-referenced by PR #11228
- 2026-05-11T21:42:23Z @neo-opus-ada cross-referenced by PR #11232
- 2026-05-11T23:13:08Z @neo-gpt cross-referenced by PR #11242
- 2026-05-11T23:22:46Z @neo-gpt cross-referenced by #11241
- 2026-05-11T23:30:04Z @neo-gpt cross-referenced by PR #11244
- 2026-05-12T02:23:49Z @neo-opus-ada referenced in commit `2818d33` - "refactor(mcp): move 4 toolService.mjs files back to MCP server directories (#11107)

Per #11107 retrospective architectural fix: 4 toolService.mjs files were moved
from MCP server dirs to SDK during M6 mechanical migration. These files are
pure MCP-server-suitable glue (ToolService instantiation + openapi.yaml-bound
serviceMapping + per-server callTool/listTools dispatch) — they don't satisfy
the SDK-suitable test (cross-server reusable, independent of MCP framing).

Operator-surfaced anchor from #11107 body:

> "the goal was to move services outside mcp servers into the sdk. you guys
>  rubber stamped it, and literally moved all one by one. for most services
>  correct, except for the toolService files (one per server). these literally
>  just map service methods to mcp tools. and these REALLY belong into the
>  mcp servers."

This PR moves all 4 back to flat layout (`ai/mcp/server/<name>/toolService.mjs`).

Files moved:
- ai/services/knowledge-base/toolService.mjs → ai/mcp/server/knowledge-base/toolService.mjs
- ai/services/memory-core/toolService.mjs → ai/mcp/server/memory-core/toolService.mjs
- ai/services/neural-link/toolService.mjs → ai/mcp/server/neural-link/toolService.mjs
- ai/services/github-workflow/toolService.mjs → ai/mcp/server/github-workflow/toolService.mjs

Per-file path adjustments:
- ToolService import: '../../mcp/ToolService.mjs' → '../ToolService.mjs' (one less level)
- openapi.yaml path: '../../mcp/server/<name>/openapi.yaml' → 'openapi.yaml' (co-resident)
- Service-class imports: './<Service>.mjs' → '../../../services/<name>/<Service>.mjs' (reach into SDK for reusable logic)
- (neural-link only) getCurrentTurnId import: '../../mcp/server/neural-link/Server.mjs' → './Server.mjs' (sibling)
- (github-workflow only) config import: '../../mcp/server/github-workflow/config.mjs' → './config.mjs' (sibling)

Server.mjs import per server:
- '../../../services/<name>/toolService.mjs' → './toolService.mjs' (sibling)

Test file path updates (5 files):
- test/playwright/unit/ai/services/knowledge-base/KBRecorderService.spec.mjs
- test/playwright/unit/ai/services/github-workflow/toolService.spec.mjs
- test/playwright/unit/ai/services/memory-core/WakeSubscriptionService.spec.mjs
- test/playwright/unit/ai/mcp/server/memory-core/McpServerToolLimits.spec.mjs
- test/playwright/mcp/github-workflow/ToolRegistration.spec.mjs

File-system server scope note: `ai/mcp/server/file-system/services/toolService.mjs`
remains at its services/ subfolder location (skipped from M6 mechanical migration,
hence skipped from this back-migration too). Normalization to flat layout for
file-system server is a separate scope concern (could be follow-up if substrate
coherence requires it).

13 files changed, 53 insertions/53 deletions (1:1 path rewrites; no logic changes).

Co-Authored-By: Claude Opus 4.7 <neo-opus-ada@neomjs.com>"
- 2026-05-12T02:23:49Z @neo-opus-ada referenced in commit `63a5f4e` - "fix(test): point manage_issue_projects test to new toolService location (#11107)

PR #11234 (merged after PR #11232 was authored) added a registration test
for `manage_issue_projects` referencing the pre-#11107 path
`ai/services/github-workflow/toolService.mjs`. With #11107's back-migration,
the file now lives at `ai/mcp/server/github-workflow/toolService.mjs`; the
test path is updated to match — preserving #11234's coverage at the new path.

Authored by Claude Opus 4.7 (Claude Code). Session
\`c2912891-b459-4a03-b2af-154d5e264df1\`.

Co-Authored-By: Claude Opus 4.7 <neo-opus-ada@neomjs.com>"
- 2026-05-12T08:05:33Z @tobiu referenced in commit `f8b76fa` - "refactor(mcp): move 4 toolService.mjs files back to MCP server dirs (#11107) (#11232)

* refactor(mcp): move 4 toolService.mjs files back to MCP server directories (#11107)

Per #11107 retrospective architectural fix: 4 toolService.mjs files were moved
from MCP server dirs to SDK during M6 mechanical migration. These files are
pure MCP-server-suitable glue (ToolService instantiation + openapi.yaml-bound
serviceMapping + per-server callTool/listTools dispatch) — they don't satisfy
the SDK-suitable test (cross-server reusable, independent of MCP framing).

Operator-surfaced anchor from #11107 body:

> "the goal was to move services outside mcp servers into the sdk. you guys
>  rubber stamped it, and literally moved all one by one. for most services
>  correct, except for the toolService files (one per server). these literally
>  just map service methods to mcp tools. and these REALLY belong into the
>  mcp servers."

This PR moves all 4 back to flat layout (`ai/mcp/server/<name>/toolService.mjs`).

Files moved:
- ai/services/knowledge-base/toolService.mjs → ai/mcp/server/knowledge-base/toolService.mjs
- ai/services/memory-core/toolService.mjs → ai/mcp/server/memory-core/toolService.mjs
- ai/services/neural-link/toolService.mjs → ai/mcp/server/neural-link/toolService.mjs
- ai/services/github-workflow/toolService.mjs → ai/mcp/server/github-workflow/toolService.mjs

Per-file path adjustments:
- ToolService import: '../../mcp/ToolService.mjs' → '../ToolService.mjs' (one less level)
- openapi.yaml path: '../../mcp/server/<name>/openapi.yaml' → 'openapi.yaml' (co-resident)
- Service-class imports: './<Service>.mjs' → '../../../services/<name>/<Service>.mjs' (reach into SDK for reusable logic)
- (neural-link only) getCurrentTurnId import: '../../mcp/server/neural-link/Server.mjs' → './Server.mjs' (sibling)
- (github-workflow only) config import: '../../mcp/server/github-workflow/config.mjs' → './config.mjs' (sibling)

Server.mjs import per server:
- '../../../services/<name>/toolService.mjs' → './toolService.mjs' (sibling)

Test file path updates (5 files):
- test/playwright/unit/ai/services/knowledge-base/KBRecorderService.spec.mjs
- test/playwright/unit/ai/services/github-workflow/toolService.spec.mjs
- test/playwright/unit/ai/services/memory-core/WakeSubscriptionService.spec.mjs
- test/playwright/unit/ai/mcp/server/memory-core/McpServerToolLimits.spec.mjs
- test/playwright/mcp/github-workflow/ToolRegistration.spec.mjs

File-system server scope note: `ai/mcp/server/file-system/services/toolService.mjs`
remains at its services/ subfolder location (skipped from M6 mechanical migration,
hence skipped from this back-migration too). Normalization to flat layout for
file-system server is a separate scope concern (could be follow-up if substrate
coherence requires it).

13 files changed, 53 insertions/53 deletions (1:1 path rewrites; no logic changes).

Co-Authored-By: Claude Opus 4.7 <neo-opus-ada@neomjs.com>

* fix(test): point manage_issue_projects test to new toolService location (#11107)

PR #11234 (merged after PR #11232 was authored) added a registration test
for `manage_issue_projects` referencing the pre-#11107 path
`ai/services/github-workflow/toolService.mjs`. With #11107's back-migration,
the file now lives at `ai/mcp/server/github-workflow/toolService.mjs`; the
test path is updated to match — preserving #11234's coverage at the new path.

Authored by Claude Opus 4.7 (Claude Code). Session
\`c2912891-b459-4a03-b2af-154d5e264df1\`.

Co-Authored-By: Claude Opus 4.7 <neo-opus-ada@neomjs.com>

---------

Co-authored-by: tobiu <tobiasuhlig78@gmail.com>"
- 2026-05-12T08:05:33Z @tobiu closed this issue
- 2026-05-12T22:33:14Z @neo-opus-ada cross-referenced by PR #11276
- 2026-05-13T05:28:17Z @tobiu referenced in commit `f7b2561` - "feat(github-workflow): atomic PR review create/update via dedicated MCP tool (#11273) (#11276)

* feat(github-workflow): atomic PR review create/update via dedicated MCP tool (#11273)

Resolves #11273

Closes the formal-state-gap pattern empirically anchored by PR #11234 +
PR #11271: agents posting substantive review prose via manage_issue_comment
then forgetting the second `gh pr review --approve|--request-changes` step,
breaking the cross-family review mandate gate per pull-request §6.1.

The new manage_pr_review MCP tool routes through the addPullRequestReview /
updatePullRequestReview GraphQL mutations — single atomic call posts the
review body AND flips GitHub's reviewDecision surface. Distinct from the
existing manage_pr_reviewers (which manages reviewer-request invitations)
and manage_issue_comment (generic issue/PR comments without formal state).

Surface (per #11273 AC1-AC3 + AC5 + AC6 + Discussion #11239 Option B):
- ai/services/github-workflow/queries/mutations.mjs:
  + ADD_PULL_REQUEST_REVIEW, UPDATE_PULL_REQUEST_REVIEW, GET_PULL_REQUEST_ID
- ai/services/github-workflow/PullRequestService.mjs:
  + async managePrReview({action, pr_number, state, body, review_id})
  + state APPROVED|REQUEST_CHANGES|COMMENT maps to GraphQL event enum
  + structured error envelopes (INVALID_ARGUMENTS / MISSING_ARGUMENTS /
    PR_NOT_FOUND / GRAPHQL_API_ERROR) following the manageIssueComment
    + managePrReviewers precedent
- ai/mcp/server/github-workflow/toolService.mjs:
  + manage_pr_review serviceMapping entry
- ai/mcp/server/github-workflow/openapi.yaml:
  + new /pulls/{pr_number}/review/manage POST operation with full schema
- test/playwright/unit/ai/services/github-workflow/PullRequestService.spec.mjs:
  + 11 new contract tests (4 happy paths + 6 validation + 1 GraphQL-error)
- test/playwright/unit/ai/mcp/server/github-workflow/ToolRegistration.spec.mjs:
  + relocated from legacy test/playwright/mcp/github-workflow/ to canonical
    path per feedback_mcp_test_location memory anchor (#11107 convention)
  + new manage_pr_review registration assertion
- .agents/skills/pr-review/references/pr-review-guide.md §2.7 + §7.7:
  + atomic-primitive preference; legacy two-step as fallback
- .agents/skills/pull-request/references/review-response-protocol.md §14:
  + reviewId vs commentId disambiguation for A2A hand-off

Substrate-author = @neo-gemini-pro (Discussion #11239 originator).
Filer = @neo-opus-ada per cross-family AC-CycleE rotation discipline.
Test evidence: 33/33 unit tests pass locally (3 ToolRegistration + 30
PullRequestService).

Co-Authored-By: Claude Opus 4.7 (1M context) <neo-opus-ada@neomjs.com>

* fix(github-workflow): address PR #11276 Cycle 1 RAs (#11273)

Addresses 3 Required Actions from @neo-gpt's PR #11276 Cycle 1 review
(commentId IC_kwDODSospM8AAAABCF9kWA, formal CHANGES_REQUESTED):

RA1 — commentId/reviewId contract drift:
  Reviewed against GitHub GraphQL schema: addPullRequestReview creates a
  PullRequestReview entity (PRR_*), NOT a separate IssueComment (IC_*).
  Original AC2/AC6 claims about commentId + get_conversation({comment_id})
  warm-cache propagation were substrate-incorrect; PullRequestReview and
  IssueComment are distinct types with distinct id spaces. Resolution:
  - review-response-protocol.md §14 revised to reviewId-only contract +
    explicit distinction-note vs IssueComment + correct fetch paths
  - #11273 ticket-body AC2/AC6 contract-revision documented via comment
    on the ticket (substrate-author chain preserved; AC text adjusts to
    implementation reality, not vice versa)

RA2 — OpenAPI description bloat per pr-review §5.3:
  Trimmed block-literal description with internal cross-refs (#11273,
  pull-request §6.1, review-response-protocol §14) to single-line
  call-contract per the MCP Tool Description Budget Audit (descriptions
  are runtime payload; should be usage-focused only).

RA3 — orphaned managePrReviewers JSDoc:
  managePrReview was inserted between managePrReviewers JSDoc (lines
  309-333) and its method (line 503). JSDoc no longer attached. Moved
  managePrReviewers JSDoc to its method location (now line 479 → 503);
  managePrReview JSDoc + method remain at line 310 → 344.

Test evidence: 33/33 unit tests still pass locally (no test changes
needed; tests already locked the reviewId-only contract correctly per
implementation reality; the broader AC2/AC6 claim was prose-only).

Co-Authored-By: Claude Opus 4.7 (1M context) <neo-opus-ada@neomjs.com>

* fix(mcp): quote openapi.yaml manage_pr_review description for YAML parser (#11273)

Plain-scalar description with backticks-then-colon-space (`` `create`: ``) tripped
YAML map-key-separator inference at line 637:122 — OpenApiValidatorCompliance failed
4/25 with YAMLException. Convert to double-quoted form and drop backticks around
create/update action verbs; semantic content unchanged.

Verified: 25/25 OpenApiValidatorCompliance + 35/35 PullRequestService green locally.

---------

Co-authored-by: tobiu <tobiasuhlig78@gmail.com>"

