---
number: 10071
title: 'fix(mcp): preserve open-bag input payloads through Zod validation (#10070)'
author: tobiu
state: MERGED
createdAt: '2026-04-18T20:50:29Z'
updatedAt: '2026-04-18T21:03:23Z'
closedAt: '2026-04-18T21:03:18Z'
mergedAt: '2026-04-18T21:03:18Z'
head: agent/10070-input-open-bag
base: dev
url: 'https://github.com/neomjs/neo/pull/10071'
---
## Summary

Six Neural Link write tools have been silently no-op'ing since #10043. The MCP call returned `{success: true}` while the actual payload was stripped during Zod validation and never reached the worker. User tobiu surfaced it when `set_instance_properties` didn't change a button's DOM text despite a "success" response.

Concrete reproduction (pre-fix):
```js
buildZodSchema(doc, setInstancePropertiesOp).parse({id: 'neo-button-1', properties: {text: 'X'}})
// → {id: 'neo-button-1', properties: {}}    payload silently stripped
```

## Root Cause

Before #10043, `buildZodSchema` used a hardcoded switch where `type: object` fell through to `z.any()` (preserves payload as-is). #10043's recursion refactor emits strict `z.object({})` for `type: object` declarations that have no child `properties` — Zod's default strict parse then drops every unknown key.

Six neural-link tools declare `type: object` with no `properties` intentionally — the field is an open bag where the caller decides the shape:

| Tool | Broken field | Effect (pre-fix) |
|---|---|---|
| `set_instance_properties` | `.properties` | Silent no-op on config writes |
| `find_instances` | `.selector` | Selector empty — matches all or none |
| `query_component` | `.selector` | Same |
| `query_vdom` | `.selector` | Same |
| `modify_state_provider` | `.data` | Silent no-op on state updates |
| `manage_neo_config` | `.config` | Silent no-op on global config changes |

## Fix

In [ai/mcp/validation/OpenApiValidator.mjs](ai/mcp/validation/OpenApiValidator.mjs) `buildZodSchemaFromNode`, when walking `type: object` that has **no declared `properties` AND no declared `additionalProperties`**, emit `z.object({}).passthrough()` instead of strict `z.object({})`. Intent signal: the absence of a declared shape means "caller decides the keys". Strict parsing only makes sense when the author explicitly listed expected fields.

Branches:
- **Input** + has declared `properties` → **strict** (unknown top-level keys = caller error)
- **Input** + no declared `properties` + no `additionalProperties` → **open-bag passthrough** (this fix)
- **Input** + explicit `additionalProperties: <schema>` → respects declaration (existing behavior)
- **Output** side → unchanged, still lenient via #10067's earlier `{lenient}` path

This extends the schema-intent categorization the codebase has been building in #10064 / #9837 / #10067:

| Side | Has declared properties? | Correct strategy |
|---|---|---|
| Input (this PR) | **No** | **Open** — the omission signals "caller decides the shape" |
| Input | Yes | Strict — unknown keys are caller errors |
| Output (closed contract) | Yes, stable | Strict OK |
| Output (runtime introspection) | Varies / none | Open via `.passthrough()` (#10067) |

## Verification

### Unit spec
Extended [test/playwright/unit/ai/mcp/validation/OpenApiValidatorCompliance.spec.mjs](test/playwright/unit/ai/mcp/validation/OpenApiValidatorCompliance.spec.mjs) with:
- Direct payload-preservation assertion for `set_instance_properties.properties` (the exact regression)
- Per-server `findSilentlyStrippingOpenBags` walker: asserts no nested input object emits `{type: object, no properties, additionalProperties: false}` — the exact JSON-Schema shape that triggers Zod's payload strip

```
$ npx playwright test -c test/playwright/playwright.config.unit.mjs test/playwright/unit/ai/mcp/validation/
  23 passed (934ms)
```

### Whitebox E2E
The existing [test/playwright/e2e/ButtonBaseNL.spec.mjs](test/playwright/e2e/ButtonBaseNL.spec.mjs) was passing 1/4 before this PR (only the non-mutating "Layout vs DOM Physics" test survived). After the fix:

```
$ npx playwright test test/playwright/e2e/ButtonBaseNL.spec.mjs -c test/playwright/playwright.config.e2e.mjs
  4 passed (8.0s)
```

All three mutation tests (direct component mutation, user-driven UI → config → button, agent-driven NL → config → button) now round-trip correctly.

## Avoided Traps

- **Not** reverting #10043. Its recursion refactor was correct for declared object shapes; the bug was only in the undeclared-properties edge case.
- **Not** blanket-loosening all input validation. Inputs with declared `properties` stay strict — unknown fields there remain legitimate caller errors (regression-guarded by the existing #9837 input-strictness test).
- **Not** updating the six handlers to re-extract raw request bodies. The validator is the right layer; per-handler fixes would scatter the same workaround six times.
- **Not** bundling an explicit YAML audit (add `additionalProperties: true` to the 6 affected schemas). Worth doing eventually for clarity, but the validator change handles it robustly today. Flagged as optional Step 3 in #10070's body.

## Direct Motivation

Session `7258d884-6c5d-40d5-b180-d58ae9a9d729` bisected the regression in ~15 minutes by applying the memory-first reflex formalized in #10068/#10069:
1. User reported set_instance_properties no-op
2. Memory query showed prior successful uses in 2026-02-25 + 2026-03-24 → real regression confirmed
3. ButtonBaseNL.spec.mjs dropped 1/4 → framework-wide
4. Read+write bisection: both `text` and `_text` unchanged → NL routing, not reactivity
5. Traced to Zod `z.object({})` strict empty-object behavior → fix scoped

The bisection also surfaced a durable lesson saved as feedback memory: **`{success: true}` from an MCP write is not proof of effect**. Always read-back-after-write. The hollow success response in this bug looks identical to real success.

## Origin Session ID

`7258d884-6c5d-40d5-b180-d58ae9a9d729`

Resolves #10070

## Comments

### `@tobiu` commented on 2026-04-18T20:53:03Z

**Input from Claude Opus 4.7 (Claude Code):**

> ❋ # PR Review Summary
> 
> **Status:** Comment (Self-Review)
> 
> **Self-Review Opening:** Self-review of #10070. The tightest, highest-impact PR of this session's arc — restores actual user-facing functionality (six silently-broken Neural Link write tools) rather than tolerating drift. The fix itself is one conditional branch. What makes this PR worth scrutinizing is the diagnosis path: I fell for the exact false-positive pattern earlier in this session, then had to live-bisect my way out. Honest accounting below.
> 
> **Tech Debt Radar**: deliberately **not** invoked. Single-branch fix to `buildZodSchemaFromNode`, no architectural shift. Scope confined to a specific schema-validation edge case.
> 
> ---
> 
> ### 📊 Evaluation Metrics
> *   **`[ARCH_ALIGNMENT]`**: 90 — Clean extension of the schema-intent categorization the codebase has been evolving through #10064 / #9837 / #10067. The "undeclared properties = open-bag" rule lands as a third category in a coherent four-cell table (input × output × declared-shape × undeclared). `isRoot` exclusion in the spec walker is the right nuance — top-level input strictness is intentional, only nested open-bags were the trap.
> *   **`[CONTENT_COMPLETENESS]`**: 92 — Full root-cause chain with concrete repro, before/after test numbers, category table expansion. Inline code comment references #10070 so the next agent reading `buildZodSchemaFromNode` gets the context. **Deduction**: my prior `feedback_mcp_output_category_split.md` from earlier today is now incomplete — it framed two *output* categories but didn't consider inputs. Should be updated post-merge to cover the full four-cell table.
> *   **`[EXECUTION_QUALITY]`**: 93 — Two-layer verification: unit spec asserts the validator-level invariant (23/23), whitebox e2e asserts the end-to-end user-facing behavior (4/4 up from 1/4). Clean diffs. Honest deduction: **e2e coverage for the other 5 affected tools doesn't exist**. I validated `set_instance_properties` end-to-end via ButtonBaseNL.spec.mjs; `find_instances`, `query_component`, `query_vdom`, `modify_state_provider`, `manage_neo_config` rely on the unit-level guard alone. Parametrized e2e probe per tool would close this gap.
> *   **`[PRODUCTIVITY]`**: 100 — Full workflow with tobiu's corrections from prior PRs baked in: assigned first, dedup swept, read-back verification applied (the diagnostic step that cracked it).
> *   **`[IMPACT]`**: 95 — Six production-critical NL write tools restored. Higher impact than my earlier PRs in this arc because it restores core functionality rather than tolerating symptoms.
> *   **`[COMPLEXITY]`**: 40 — Small code delta, moderate cognitive work (distinguishing four validation branches + understanding Zod's strict-object semantics + proving the fix doesn't leak into input strictness).
> *   **`[EFFORT_PROFILE]`**: Quick Win — high impact for ~20 validator lines + ~50 test lines.
> 
> ---
> 
> ### 🕸️ Context & Graph Linking
> *   **Target Epic / Issue ID:** Resolves #10070
> *   **Related Graph Nodes:** Sibling to #10064 (array-missing-items), #9837 (output-schema-drift, my prior PR #10067). All three are post-#10043 discoveries — same validator refactor exposed strictness inversions in three distinct code paths.
> *   **Origin Session ID:** `7258d884-6c5d-40d5-b180-d58ae9a9d729`
> 
> ---
> 
> ### 🧠 Graph Ingestion Notes
> 
> *   **`[KB_GAP]`**: My `feedback_mcp_output_category_split.md` (saved earlier today after #9837) is now stale — it named two *output* categories only and missed the input symmetry this PR exposed. The full taxonomy has four cells. Worth an update so future agents see the complete picture when reasoning about schema strictness.
> *   **`[TOOLING_GAP]`**: Five of the six affected tools (all except `set_instance_properties`) have no end-to-end coverage proving they work post-fix. Unit-level payload-preservation guard catches the class of bug, but per-tool e2e smoke tests would be stronger. Worth a targeted follow-up — especially for `find_instances`/`query_component`/`query_vdom` which are read-path tools used heavily by agent introspection workflows.
> *   **`[RETROSPECTIVE]`**: Three durable lessons from this PR's diagnosis:
>     1. **False-positive verification trap**: Earlier in this same session I reported `set_instance_properties` working based on `{success: true}` alone. A read-after-write would have caught the regression ~2 hours earlier. Codified in `feedback_verify_effect_not_just_success.md`. Carrying this forward as a hard verification gate for all MCP writes.
>     2. **Ambitious refactors seed multi-session bugs**: #10043 was a single refactor that produced #9837 (output side) and now #10070 (input side) as post-merge discoveries. Future large recursive refactors in validation code need explicit payload round-trip tests at merge time, not just emitted-schema-shape tests. Worth considering as a commit-gate rule for this specific file.
>     3. **Memory-first reflex paid off**: Querying `query_raw_memories` at minute one returned prior-successful uses from 2026-02-25 + 2026-03-24 in under 30 seconds, confirming real regression vs misconfiguration. Treated the symptom as diagnosable-not-noise from the start. Exactly the pattern #10068/#10069 formalized — this diagnosis is the first evidence the reflex works.
> 
> ---
> 
> ### 📋 Required Actions
> 
> - [ ] Human QA: merge + refresh devindex, visually confirm `set_instance_properties({text: ...})` on a button produces the expected DOM update.
> - [ ] (Optional follow-up) Update `feedback_mcp_output_category_split.md` to reflect the four-cell input × output × declared/undeclared taxonomy from this PR's body.
> - [ ] (Optional follow-up) Add parametrized e2e smoke coverage for the remaining 5 affected tools — `[TOOLING_GAP]` #2 above.
> - [ ] (Optional follow-up, tracked in #10070 body) Step 3: make the six YAML declarations explicit with `additionalProperties: true`.
> 
> No block-level code changes. The fix is tight and verified at both layers.
> 
> *Handoff: awaiting human merge per `pull-request-workflow.md` §5 (Human-in-the-Loop). I will not self-merge.*
> 

---

