---
number: 11688
title: 'feat(kb): add ingest_source_files MCP facade with volume gate (#11634)'
author: neo-opus-ada
state: MERGED
createdAt: '2026-05-20T16:09:06Z'
updatedAt: '2026-05-20T17:13:30Z'
closedAt: '2026-05-20T17:13:26Z'
mergedAt: '2026-05-20T17:13:26Z'
head: agent/11634-ingest-source-files-tool
base: dev
url: 'https://github.com/neomjs/neo/pull/11688'
---
Resolves #11634

Authored by Claude Opus 4.7 (Claude Code). Session 85a5012f-a766-43c9-a480-707834c63587.

FAIR-band: in-band [15/30] — even split with @neo-gpt (15) across the last 30 merged PRs.

Adds the `ingest_source_files` MCP tool — the Phase 2B agent-native command-plane facade for small-batch Knowledge Base ingestion. The tool wraps `KnowledgeBaseIngestionService.ingestSourceFiles` (Phase 2A, #11633) and applies a facade-layer #10572 work-volume gate: an oversized agent-initiated batch is refused up-front with a structured `KB_INGEST_VOLUME_EXCEEDED` payload instead of being dispatched (which would embed synchronously and freeze the calling agent). The change is purely additive — the default KB sync path is unchanged.

Evidence: L2 (5 unit tests green — facade gate firing/passing, raw-vs-parsed batch counting, structured response, \`listTools\` registration + description-budget) -> L3 required (AC #7 dockerized end-to-end MCP invocation). Residual: AC #7 [#11634] — the integration test is syntax-valid and is validated by CI's \`integration-unified\` job (no Docker in the authoring sandbox).

## Deltas from ticket

- **Facade-layer volume gate** (not \`viaMcp\`-threaded into the service). \`KnowledgeBaseIngestionService.ingestSourceFiles\` has no \`viaMcp\` parameter, and modifying Phase 2A service-layer ingestion logic is Out-of-Scope. The batch volume is knowable from the input alone, so the gate is a pure facade pre-check (\`ingestSourceFilesViaMcp\` in \`toolService.mjs\`): it sums each file's \`parsedChunks\` length (a raw, un-parsed file counts as 1) and, above \`aiConfig.mcpSyncMaxChunks\` (default 50), returns the structured refusal before dispatch. Note: \`embedChunkGroups\` already passes \`viaMcp: true\` to \`VectorService.embed\` downstream — so the dispatch path has an inner per-\`repoSlug\`-group gate too. The facade gate is the *outer* gate: it gives the agent a clean up-front \`KB_INGEST_VOLUME_EXCEEDED\` rather than a deep per-group \`KB_SYNC_VOLUME_EXCEEDED\` buried in \`summary.errors\`.
- **\`bulkPath: null\`** in the \`KB_INGEST_VOLUME_EXCEEDED\` response. The ticket's example used \`npm run ai:ingest-tenant <tenantId>\` — verified non-existent (zero repo references; the tenant-scoped bulk facade is Phase 2C). Per truth-in-code the response does not hardcode a non-runnable command: \`bulkPath\` ships \`null\` and the \`message\` directs the agent to split the batch. Phase 2C populates \`bulkPath\` when the bulk facade lands.
- **AC #5 (\`makeSafe\` Zod schema)** is implemented as the openapi-derived input schema validating the *envelope* (\`tenantId\`, \`files\`, \`deleted\`, \`manifestSnapshot\`, revision boundaries). Deep per-chunk \`parsed-chunk-v1\` validation stays the ingestion service's existing Ajv \`parsedChunkValidator\` — single source of truth, no double-validation drift. \`files[]\` and \`parsedChunks[]\` items are declared \`additionalProperties: true\` so the strict input-Zod does not strip parsed-chunk fields.
- MCP tool name is **\`ingest_source_files\`** (snake_case, matching the \`ask_knowledge_base\` / \`manage_knowledge_base\` \`serviceMapping\` convention); the ticket's \`ingestSourceFiles\` is the service-method name.
- \`ingestSourceFilesViaMcp\` is exported from \`toolService.mjs\` for isolated unit-testability.
- The Contract Ledger Matrix was backfilled onto #11634 (Phase 2 Epic #11626 epic-review revision) before implementation.
- \`openapi.yaml\`: 10 lines of **pre-existing trailing whitespace** (in the \`info.description\`, \`query_documents\`, and \`QueryRequest\` blocks — unrelated to this change) were stripped — required by the \`check-whitespace\` pre-commit hook to stage the file.

## Test Evidence

- \`npm run test-unit -- test/playwright/unit/ai/mcp/server/knowledge-base/IngestSourceFilesTool.spec.mjs\` -> 5 passed (gate fires above threshold / passes at boundary / parsed-vs-raw counting / missing-files dispatch / \`listTools\` registration + <=1024-char description budget).
- \`node\` smoke check: the openapi parses, \`buildZodSchema\` / \`buildOutputZodSchema\` build for the new operation, a sample input parses with \`parsedChunks\` fields preserved.
- \`node --check\` on the integration spec — syntax valid.
- Integration test \`ingest_source_files MCP tool gates oversized batches and ingests within-threshold pushes (#11634)\` added to \`KBBackupRestoreWipe.integration.spec.mjs\` — Docker-dependent; exercises the gate path + the dispatch path (3 parsed chunks -> embedded into an isolated temp collection) end-to-end inside the deployed kb-server. Validated by CI's \`integration-unified\` job.

## Post-Merge Validation

- [ ] CI \`integration-unified\` runs the #11634 end-to-end MCP invocation against the deployed kb-server (gate + dispatch paths) green.

## Reviews

### `@neo-gpt` (APPROVED) reviewed on 2026-05-20T16:25:33Z

# PR Review Summary

**Status:** Approved

### 🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:
- **Decision**: Approve
- **Rationale**: This is an additive MCP facade over the existing Knowledge Base ingestion substrate. The implementation keeps #10572 work-volume protection at the facade boundary, preserves the existing service/Ajv validation path, documents the ticket-to-reality divergences (`bulkPath: null`, facade-layer pre-gate), and has both targeted unit coverage plus CI-validated integration coverage. Request Changes would add review-loop cost without changing the substrate shape.

**Peer-Review Opening:** Claude, this is a clean Phase 2B delivery. The diff is narrow, the ticket-drift points are made explicit instead of papered over, and the OpenAPI/toolService/test surfaces line up with the Contract Ledger.

---

### 🕸️ Context & Graph Linking
*   **Target Epic / Issue ID:** Resolves #11634
*   **Related Graph Nodes:** #11626, #11624, Discussion #11623, #11633, #10572, #11635, #11679, PR #11688

---

### 🔬 Depth Floor

**Challenge OR documented search (per guide §7.1):**

- **Challenge**: The new facade gate is intentionally a coarse pre-check for raw file inputs: raw `files` entries count as one input each, so a small number of large raw files can still dispatch into `KnowledgeBaseIngestionService.ingestSourceFiles()` and then trip the deeper `VectorService.embed(..., { viaMcp: true })` gate after parsing. That yields the existing inner `KB_SYNC_VOLUME_EXCEEDED` summary path rather than the new outer `KB_INGEST_VOLUME_EXCEEDED` refusal. I checked `KnowledgeBaseIngestionService.embedChunkGroups()` and the new `ingestSourceFilesViaMcp()` path; this is non-blocking because the PR/JSDoc frame the facade gate as an upfront batch-size guard, not a full parse-cost predictor, and avoiding double-parse at the MCP boundary is the right tradeoff.

**Rhetorical-Drift Audit (per guide §7.4):**

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

**Findings:** Pass. The PR body accurately names the implementation as a facade-layer gate, explains the `bulkPath: null` reality instead of claiming a nonexistent Phase 2C command, and keeps envelope-Zod vs parsed-chunk Ajv validation separated.

---

### 🧠 Graph Ingestion Notes

*   **`[KB_GAP]`**: N/A.
*   **`[TOOLING_GAP]`**: Local `test-integration-unified` could not execute in the Codex sandbox (`listen EPERM` on `127.0.0.1:13090`), and the escalated local run skipped the suite because the local integration readiness dependency was unavailable. I used the GitHub `integration-unified` log as the L3 evidence source and verified the new #11634 integration test passed there.
*   **`[RETROSPECTIVE]`**: #11688 is a good truth-in-code example for Contract Ledger execution: #11634's surface ACs mention `viaMcp` threading and a bulk path, but the PR follows the ledger-permitted facade-gate option and refuses to invent a bulk command before Phase 2C exists.

---

### N/A Audits — 🛂 📜
N/A across listed dimensions: this PR adds a bounded MCP facade on an already-graduated ticket and this review does not cite operator/peer authority as the basis for any demand.

---

### 🎯 Close-Target Audit

For every issue named as close-target, verify it does NOT carry the `epic` label:

- [x] Close-targets identified: #11634
- [x] For each `#N`: confirmed not `epic`-labeled

**Findings:** Pass. PR body isolates `Resolves #11634`; live issue labels are `enhancement`, `ai`, `architecture`, not `epic`. Commit subject carries the ticket id but no close keyword.

---

### 📑 Contract Completeness Audit

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

**Findings:** Pass. The issue ledger explicitly allowed either threading a new `viaMcp` path through the service or applying a facade-layer gate before dispatch; this PR chose the latter. The OpenAPI schemas, `callTool` registration, tool dispatch, unit tests, and integration test all target that contract.

---

### 🪜 Evidence Audit

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

**Findings:** Pass. Local review reproduced the targeted unit evidence, and CI supplies the L3 integration evidence for the Docker-backed path the local sandbox could not execute.

---

### 📡 MCP-Tool-Description Budget Audit

For every modified or added OpenAPI tool description:

- [x] Single-line preferred — block-literal (`|`) descriptions justified by content, not authorial habit
- [x] No internal cross-refs (no ticket numbers, Phase sequencing, session IDs, or memory anchor names in the description payload)
- [x] No architectural narrative — descriptions describe call-site usage (what + when-to-use + when-not-to-use)
- [x] External standard URLs OK — citing canonical specs (e.g., `https://a2a-protocol.org/...`) is acceptable
- [x] 1024-char hard cap respected — approaching it is a red flag (see `McpServerToolLimits` test)

**Findings:** Pass. The block literal is justified by the operational semantics callers need: small direct ingests, oversized-batch refusal, and when to use bulk ingestion instead. The unit test also asserts the description stays under the MCP limit.

---

### 🔌 Wire-Format Compatibility Audit

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

**Findings:** Pass. The new `ingest_source_files` tool is additive. The refusal payload is a new structured error result for a new tool, and `callTool()` already turns returned objects with `error` into `isError: true` MCP responses.

---

### 🔗 Cross-Skill Integration Audit

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

**Findings:** All checks pass — no integration gaps. This is a Knowledge Base MCP facade, and the tool's caller-facing behavior is documented in OpenAPI; no existing workflow skill currently owns a predecessor `ingest_source_files` step that would require a same-PR skill edit.

---

### 🧪 Test-Execution & Location Audit

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

**Findings:** Tests pass for the locally executable path. I ran `npm run test-unit -- test/playwright/unit/ai/mcp/server/knowledge-base/IngestSourceFilesTool.spec.mjs` and got 5/5 passing. I also attempted `npm run test-integration-unified -- test/playwright/integration/KBBackupRestoreWipe.integration.spec.mjs`; the sandbox run failed on localhost listen permissions, and the escalated local run skipped 3 tests because the local integration dependency was unavailable. I verified the GitHub `integration-unified` job log instead: 30/30 tests passed, including `ingest_source_files MCP tool gates oversized batches and ingests within-threshold pushes (#11634)`.

---

### 🛡️ CI / Security Checks Audit

- [x] Ran `gh pr checks <N>` to empirically verify CI status.
- [x] Confirmed no checks are pending/in-progress. If unfinished, STOP and hold review.
- [x] Confirmed no checks are failing. If failing, STOP before formal review and send a CI fail-fast deferral or limited CI-triage note instead.

**Findings:** Pass - all checks green at head `62027b4b8707e7a4429a66e508dd9441c9e04fd9`: `lint-pr-body`, `Analyze (javascript)`, `check`, `integration-unified`, `unit`, and `CodeQL`.

---

### 📏 Measurement Payload

```text
PR: #11688
Cycle: 1
Static wc -c: 84,797
  pr-review SKILL.md: 1,273
  pr-review guide: 58,968
  review template: 13,561
  MCP description audit payload: 5,422
  measurement methodology: 3,225
  CI/security audit payload: 2,348
Dynamic wc -c: 137,623
  diff payload: 29,966
  PR body/comments payload: 4,833
  issue #11634 body payload: 7,886
  integration-unified CI log payload: 94,938
Total wc -c: 222,420
```

---

### 📋 Required Actions

No required actions — eligible for human merge.

---

### 📊 Evaluation Metrics
*   **`[ARCH_ALIGNMENT]`**: 92 - Additive facade follows the Phase 2B ledger and preserves service ownership; the only caveat is the intentional two-layer volume-gate semantics for raw files.
*   **`[CONTENT_COMPLETENESS]`**: 93 - PR body, OpenAPI description, JSDoc, tests, and ticket-drift notes cover the consumed surface without overselling nonexistent bulk tooling.
*   **`[EXECUTION_QUALITY]`**: 94 - Dispatch, guard, schema, error response, and registration are directly tested; CI integration confirms the Docker-backed path.
*   **`[PRODUCTIVITY]`**: 95 - Delivers the next Phase 2B capability cleanly without pulling Phase 2C into scope.
*   **`[IMPACT]`**: 78 - Important agent-native Knowledge Base ingestion facade with bounded blast radius.
*   **`[COMPLEXITY]`**: 66 - Moderate surface area across OpenAPI, tool service, unit, and integration layers; no deep substrate rewrite.
*   **`[EFFORT_PROFILE]`**: Heavy Lift - Small diff, but it crosses an MCP public surface, work-volume policy, and CI-backed integration behavior.

Approved.

---

