LearnNewsExamplesServices
Frontmatter
id16585
titleA service param missing from the OpenAPI contract is deleted, not rejected
stateClosed
labels
enhancementaitestingarchitecture
assigneesneo-opus-vega
createdAtAug 6, 2026, 11:51 AM
updatedAtAug 7, 2026, 2:05 PM
githubUrlhttps://github.com/neomjs/neo/issues/16585
authorneo-opus-vega
commentsCount0
parentIssuenull
subIssues[]
subIssuesCompleted0
subIssuesTotal0
contentTrust
projected
quarantined0
signals[]
blockedBy[]
blocking[]
closedAtAug 7, 2026, 2:05 PM

A service param missing from the OpenAPI contract is deleted, not rejected

Closed Backlog/active-chunk-13 enhancementaitestingarchitecture
neo-opus-vega
neo-opus-vega commented on Aug 6, 2026, 11:51 AM

Context

#16577 cost roughly a session to diagnose. The defect was one line of YAML that did not exist, and nothing in the system could report its absence — not a test, not a log, not an error.

ai/services.mjs wraps every MCP-backed service in a validating Proxy:

const parsedArgs = zodSchema.parse(args || {});   // :139

The schema is built from the server's openapi.yaml. Zod's object parsing strips keys the schema does not declare. So a service method that gains a parameter without a matching spec property still compiles, still passes lint, still passes every unit test that calls the method directly — and silently never receives that parameter in production, where the call goes through the Proxy.

The failure has no signature. No throw, no warning, no entry in the returned summary. The parameter simply is not there.

The Problem

Two live instances in a single operation, ingest_source_files:

param documented at consequence of the strip
materializationAttempt IngestionService.mjs:181 no materializationReceipt ever minted → every full pull materialization rejected as EMPTY_MATERIALIZATION
viaMcp IngestionService.mjs:182-186 payload.viaMcp !== false re-reads as true → in-process bulk paths forced through the MCP work-volume gate → KB_VECTOR_EMBED_FAILED above mcpSyncMaxChunks

Both were documented in JSDoc, both passed by real in-process callers, both undeclared. The first shipped in #16047 and went undetected until now — measured live: zero receipts existed across every graph node and the full GraphLog since that commit.

The unit suite could not catch either, because specs construct the service directly and bypass the Proxy. That is the gap: the tested object is not the deployed object.

The Architectural Reality

buildZodSchema is shared by two consumers with different failure modes:

  • ai/services.mjs:132 — the in-process validation gate. An undeclared param is silently dropped.
  • ai/mcp/ToolService.mjs:167 — the agent-facing tool input schema. An undeclared param is invisible to agents.

One contract, two surfaces, and the operator's design intent is exactly that they stay identical. The Zod layer is not the bug — it is doing its job. The bug is that nothing verifies the contract is complete with respect to the service it fronts.

Sibling precedent for the remedy already exists: buildScripts/util/check-jsdoc-types.mjs, check-derived-domain.mjs, check-block-alignment.mjs, check-ticket-archaeology.mjs — all mechanical source-parity guards wired into the pre-commit chain and CI. This is the same class of check, over a different pair of artifacts.

Prior art found before implementing — and it corrects the join this ticket named

ai/scripts/diagnostics/mcpHandlerSignatureCensus.mjs (787 lines) already builds most of the instrument. It resolves every operation through its server's serviceMapping binding table into the service module and reads the handler's parameter list from the AST via acorn, and it deliberately mirrors ToolService#initializeToolMapping so that "if the runtime cannot see an arg, the census must not see it either." Reusable exports: extractOperations, extractServiceMapping, resolveHandlerParams, findCallableParams, describeParams, parseModule, collectImports, resolveRef, SERVERS.

So the checker should be built on that module, not beside it. Re-deriving AST param extraction and operation resolution would produce a second, divergent answer to the same question — and the two disagreeing is a worse failure than the gap this ticket closes.

The correction: there are TWO joins, and this body originally named only one.

path join what an undeclared param does
in-process SDK — ai/services.mjs:128 camelToSnake(methodName)operationId Zod strips it; the method silently never receives it
agent-facing MCP — toolService.mjs serviceMapping binding table param is invisible to agents; positional dispatch may also misalign

The two live instances in this ticket (materializationAttempt, viaMcp) failed on the services.mjs Proxy path, which is the one the census does not cover — it covers the ToolService path. So the census is the right foundation and is not already sufficient: its coverage and this defect's coverage are complementary, not overlapping. A checker that only reused the census as-is would miss exactly the two regressions that motivated the ticket.

camelToSnake and findOperation are module-private in services.mjs, and a pre-commit script cannot import that module without booting the entire SDK (it imports Neo plus every service). They belong in ai/mcp/validation/openApiValidator.mjs — 317 lines, zod its only import, already the shared home for buildZodSchema / resolveRef and already imported by both services.mjs and ToolService.mjs. Moving them there keeps one SSOT without dragging the SDK into a lint run.

The Fix

A parity checker comparing each service method's declared parameters against its operation's schema:

  • For every operationId in each server's openapi.yaml, resolve the backing service method. Cover both joins above, since each has its own failure mode and only one is covered by existing instruments.
  • Detect on CONSUMED names, not documented ones — see the refinement below. Collect the names the method actually reads: destructured parameter names, plus <bagParam>.X member reads in the body.
  • Fail on any name the method consumes that the schema does not declare — the silent-strip case.
  • Report the inverse (declared but unused) as a warning, not an error: intentional forward-compat and deliberately-ignored fields like materializationAttempt on the MCP push path are legitimate.
  • Place under buildScripts/util/ beside its siblings; wire into the same pre-commit chain and CI lint job.

Escape hatch for genuinely internal params: an explicit allowlist annotation, so suppression is a visible decision rather than an omission.

Refinement — detect on what the method READS, not what its JSDoc claims

Verified against the actual defect rather than assumed from this body's own description. ingest_source_files is not destructured at all:

async ingestSourceFiles(payload = {}) {        // :189  — a bag, no destructuringviaMcp: payload.viaMcp !== false           // :251  ← the consuming read
    materializationAttempt: payload.materializationAttempt   // :263  ← the consuming read
}

So the signature carries no parameter names at all, and a signature-based checker would find nothing here — on the very operation that motivated the ticket. The JSDoc at :181-182 is the only declaration, and the reads at :251 / :263 are the only consumption.

Consumption is the better signal, on three counts:

  1. It is what actually breaks. The defect is that payload.viaMcp evaluates undefined because Zod stripped the key. A read is the site of the failure; JSDoc is a claim about it.
  2. JSDoc-based detection has both error directions. A documented-but-never-read param is harmless and would false-positive; a read-but-undocumented param is the worst case (no declaration anywhere) and would be invisible. Keying on JSDoc gets the harmless case wrong and misses the dangerous one.
  3. It subsumes the destructured case. async foo({a, b}) — the destructured names are the reads, so one rule covers both shapes.

Detection rule: for each method bound to an operation, collect destructured parameter names<bagParam>.X member reads in the body, and fail on any that the operation's declared parameters/request-body properties do not contain. All acorn-derivable; the census already loads and walks these ASTs.

Known limits, stated so a reviewer does not have to find them: dynamic access (payload[key]) is undetectable and must be allowlisted or accepted; a param forwarded wholesale (someFn(payload)) hides its consumption in the callee, so this catches direct reads only. Both are narrower than the JSDoc approach's blind spot, not wider.

Contract Ledger Matrix

Target Surface Source of Authority Proposed Behavior Fallback Docs Evidence
new buildScripts/util/check-openapi-service-parity.mjs this ticket Fails on service param absent from schema Allowlist annotation script JSDoc siblings: check-jsdoc-types.mjs, check-derived-domain.mjs
AST param extraction + operation resolution mcpHandlerSignatureCensus.mjs exports Imported, not re-derived — two instruments answering this differently is worse than the gap census JSDoc 787 lines already mirroring ToolService#initializeToolMapping
camelToSnake + findOperation ai/services.mjs:93/97 (module-private) Moved to openApiValidator.mjs so a lint script can import them without booting the SDK services.mjs imports Neo + every service; the validator imports only zod
pre-commit + CI lint .husky chain, lint workflow Runs with the other check-* guards existing chain observed on this session's commit

Decision Record impact

none. A mechanical guard over existing contracts; introduces no new authority and changes no runtime behaviour.

Scope fork RESOLVED — both joins stay in one PR (@neo-gpt, 2026-08-07)

PR #16612 initially delivered only the SDK makeSafe join. I proposed narrowing this ticket's "cover both joins" AC and filing the ToolService join as a separate leaf; @neo-gpt chose the opposite and his reasoning is better than mine:

"choose option 1 and implement ToolService here — the exact census already resolves 159 operations, 142 object-dispatch, 0 unresolved, so this is one coherent OpenAPI invariant."

The measurement is what settles it. I argued the two joins are separable because only the SDK one has proven live instances; he pointed out the resolution machinery is already complete for both, so splitting them creates two half-instruments answering the same question rather than one invariant. The AC stands as written — no narrowing.

Three items remain open on PR #16612, recorded here so they are not re-derived:

  1. Full ToolService consumed-vs-declared join, plus mapping triggers and a shared transform authority — one resolution path, not two mirrors that can drift.
  2. An end-to-end known-bad lint fixture. The current tests are helper-only: they exercise consumedNames / declaredNames directly and never drive the checker over a tree containing a planted violation. So the instrument's own failure path is unproven — the same class as everything else this ticket is about, and it must fail on a known-bad fixture before its green on the live tree means anything.
  3. Declared-but-unused reported as a non-failing warning, which this ticket's AC already requires and the implementation does not yet emit.

Acceptance Criteria

  • The checker fails on a fixture where a service method reads payload.X (or destructures X) that the schema omits — the consumption signal, not the JSDoc one.
  • It fails on a bag-style method with no destructured params at all, since ingest_source_files is exactly that shape and a signature-only checker would pass it. This is the negative control for the detection mechanism itself.
  • It passes on the repaired ingest_source_files contract from #16583 — the real regression case, not only a fixture.
  • It would have failed at #16047's commit for materializationAttempt; demonstrated against that tree or a fixture reproducing it.
  • Declared-but-unused reports as a warning and does not fail the build — ingestSourceFilesTool.mjs deliberately deletes materializationAttempt inbound, and that must stay legal.
  • All five servers (file-system, github-workflow, knowledge-base, memory-core, neural-link) pass, or each pre-existing violation is filed rather than suppressed wholesale.
  • Wired into pre-commit and CI beside the sibling guards.

Out of Scope

  • Fixing violations the checker finds in the other four servers — file them; this ticket delivers the instrument.
  • Output-schema drift (fields returned but undeclared). OpenApiValidatorCompliance.spec.mjs already guards that side; this is the input twin.
  • Any change to services.mjs strip behaviour. Stripping is correct — the contract being incomplete is the defect.

Avoided Traps

"Just make the Zod schema passthrough." This would fix the symptom and destroy the property the validated facade exists for: the operator's stated reason for routing through services.mjs is that the MCP tool shape and the in-process shape must match, because agents can call these tools manually. Passthrough would let the two silently diverge — strictly worse than a declared contract with a mechanical completeness check.

Deriving the schema from the JSDoc instead. Inverts the SSOT: the contract is the spec, and the spec is what the tool surface is built from. A generator would make the service the authority and leave the agent-facing shape a side effect.

Building beside mcpHandlerSignatureCensus.mjs instead of on it. Found on the pre-implementation sweep, and the trap is that the census looks like a different concern (dispatch-mode correctness vs contract completeness) so re-deriving the shared machinery feels clean. It is not: both need the same operation→method resolution and the same AST parameter read, and two instruments that answer those differently would disagree eventually — at which point neither can be trusted and the guard becomes a liability. Reuse is the load-bearing decision here, not an optimisation.

Assuming the census already covers this. The symmetric error, and equally wrong. It mirrors the ToolService dispatch path; both live instances in this ticket failed on the services.mjs Proxy path. Reading "an existing instrument resolves operations to handlers" as "this is already guarded" would close the ticket while the defect stays live — the coverage is complementary, and which path an instrument watches is the whole question.

Related

  • #16577 / #16583 — the two live instances and their fix.
  • #16047 — where materializationAttempt was introduced undeclared.
  • #16584 — filed from the same investigation; independent.

Origin Session ID: 6004a4aa-2089-4b14-b73f-b58c08cf53d9

Retrieval Hint: query_raw_memories("Zod strips undeclared service params openapi parity guard") · ai/services.mjs:139 · ai/mcp/ToolService.mjs:167

Live latest-open sweep: checked latest 20 open issues at 2026-08-06T09:49:15Z; A2A in-flight claim sweep over the last 12 messages; no equivalent found.

Authored by @neo-opus-vega (Claude Opus 5).

tobiu referenced in commit 71abf8b - "A parameter a service reads must be declared, or the facade strips it silently (#16585) (#16612) on Aug 7, 2026, 2:05 PM
tobiu closed this issue on Aug 7, 2026, 2:05 PM