Skip to content

fix(verification): reuse validators for shared output schema ids - #1527

Merged
N0zoM1z0 merged 4 commits into
morluto:mainfrom
peter-research:fix/duplicate-output-schema-validators
Oct 10, 2026
Merged

N0zoM1z0 merged 4 commits into
morluto:mainfrom
peter-research:fix/duplicate-output-schema-validators

Conversation

@peter-research

@peter-research peter-research commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

The Ghidra, Hopper and Electron verification lanes stopped during catalog registration: distinct tools with identical output schemas intentionally share a content-derived $id, and compiling that ID again in one Ajv instance throws. Reuse the registered output validator for a shared ID while retaining maps keyed by tool name. Existing input-schema checks and each verifier's Ajv options remain unchanged.

Closes #1518.

Validation

  • Normal integration with main 9964af6b, preserving both contributor commits, plus the discovered Electron follow-up. Final reviewed head 3582a3f52cc8955f7214f218b3c2ac12a94d2d41; all maintenance commit identities use verified GitHub no-reply email. Three verification scripts differ from that main baseline.
  • Both actual native boundary functions exercised with the real SDK stdio catalog: 139 tools and 118 distinct output IDs. Schemas sharing an ID were checked for complete equality. Main fails before any tool call; both fixed functions register the complete catalog and reach their first actual MCP call, correctly returning target_unavailable with no app open. No provider results are simulated. These native helper/runtime sources remain unchanged by the Electron follow-up.
  • The exact Electron registration statement selected from its parsed source was executed against that SDK-captured catalog. Main reproduces the same duplicate-ID exception; the final formatted follow-up registers all 139 tools, retains named lookup, reuses the same validator for shared IDs and validates the actual JavaScript result. This is a producer control for registration, not a full Electron application run.
  • Actual public CLI and SDK stdio MCP analysis of a source-owned 44-byte JavaScript target passes: equal Evidence IDs, complete text/structured parity, Ajv validation of the real result, unchanged input SHA-256 and subsequent ping.
  • Fresh compile, full serial typecheck, lint/module boundaries, syntax and changed-file formatting pass with the pinned toolchain and resource guard. Final Electron source was checked again after formatting; runtime sources did not change.

Verification scope

Independent coverage verifies the catalog failure, native verification startup past that boundary, the Electron registration producer and an actual JavaScript CLI/MCP workflow. Full Ghidra/Hopper binary-analysis lanes, the full Electron application fixture and native macOS/Windows provider behavior were not run locally. The seven-fixture Ghidra result in #1518 is attributed to the issue reporter, rather than claimed on this reviewed head. Fresh current-head CI must pass before merge.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

@N0zoM1z0 N0zoM1z0 changed the title fix: reuse validators for duplicate output schema ids fix(verification): reuse validators for shared output schema ids Oct 10, 2026
@N0zoM1z0

Copy link
Copy Markdown
Collaborator

Independent review and actual reproduction are complete on 3d03294c21fe831834a0a2b263a3923dc4272e34, a normal integration of main 9964af6b preserving the contributor history. Main fails in both real boundary functions before their first tool call. This head registers all 139 actual SDK-advertised tools/118 distinct output schemas and reaches the first actual MCP call, returning the expected target_unavailable for an unopened app. A real 44-byte JavaScript public CLI/MCP workflow also passes, including complete Evidence parity, Ajv validation, unchanged input and ping.

Fresh compile/typecheck/lint/module/syntax/format checks pass. The PR description now records the independent evidence and explicit provider scope; no full local Ghidra/Hopper analysis is claimed. Waiting for new CI on this exact head; no reviewers requested.

@N0zoM1z0

Copy link
Copy Markdown
Collaborator

The consumer audit found the same registration failure in scripts/verify/electron/tool-contracts.mjs. Its exact parsed-source registration statement also fails on the actual SDK-captured catalog. Follow-up 3582a3f52cc8955f7214f218b3c2ac12a94d2d41 applies the same shared-ID lookup there, preserves input-schema compilation/Ajv options and passes the final formatted-source control: 139 named validators, shared-ID validator identity and validation of the actual JavaScript result. The two previously tested native helper/runtime sources are unchanged.

The PR now covers all three discovered consumers. The body distinguishes actual SDK/native-startup evidence from the Electron registration producer control; no full Electron or native-engine workflow is claimed. Waiting for fresh CI on this final head.

@N0zoM1z0
N0zoM1z0 merged commit c73cf0a into morluto:main Oct 10, 2026
19 checks passed
@N0zoM1z0

Copy link
Copy Markdown
Collaborator

Merged as c73cf0ab7695049e5b8267f18182cb87200c78b2 after all 19 checks passed on final reviewed head 3582a3f52cc8955f7214f218b3c2ac12a94d2d41, with no outstanding change requests or unresolved review threads. Both contributor authors and patch IDs are preserved; maintenance identities use verified GitHub no-reply email. All three changed script blobs match the reviewed head.

Merged main was freshly rebuilt after the intervening main updates. The actual 44-byte public CLI/SDK stdio MCP workflow passes again with equal Evidence IDs, full text/structured parity, actual-result Ajv validation, unchanged input and ping. The current 139-tool/118-output-ID catalog passes both actual native boundary startup controls and the exact Electron registration producer control. Main's original duplicate-ID failures remain retained as diagnosis. Full Ghidra/Hopper binary-analysis and the complete Electron app fixture remain outside this local verification scope.

The completed owned worktree/build was removed normally after this verification. Source branches, inputs/results and failed receipts remain recoverable; shared dependencies and other workspaces were preserved.

@N0zoM1z0

Copy link
Copy Markdown
Collaborator

Scoped independent verification after the merge:

  • The actual Ghidra and Hopper verifier registration functions were each run with the live 139-tool SDK catalog. Each made 118 actual Ajv compilations and reused 21 existing schema IDs. The sentinel stopped before a provider query, so this proves registration behavior, not real Hopper execution.
  • Separate public real Linux Ghidra CLI/MCP collection workflows passed (all 15 facets plus a nonempty parameter/parameter-use function), including exact page content/Evidence IDs, stable run identity, close/expiry, input identity and database cleanup; detailed coverage is recorded on feat(native): inspect selected facets of retained function dossiers #1247.
  • The complete local verify:ghidra lane did not pass: attempts hit the existing 2304 MiB total family-RSS guard. Investigation found the standalone native-value verifier discarded NODE_OPTIONS; bug(verification): native value adapter checks discard Node heap budgets #1548 records that separate concrete boundary defect. After forwarding it, an actual adapter child received the configured heap budget, but the full lane still exceeded the total-process budget. No full-lane or real Hopper success is claimed.

The third Electron registration loop was inspected but was not independently executed in this scoped probe.

@CassieFox23

CassieFox23 commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

TL;DR: This is a post-merge check. Before the fix, all three Ghidra checks stopped within 30 seconds. With it, all three full lanes pass here, and a tool whose output breaks its schema is still rejected.

Results

I tested e707642c before the merge. Both boundary files on main bb742110 are byte for byte the files I tested. The last commit adds the same reuse to the Electron check, and I did not run that one.

N0zoM1z0's full verify:ghidra run hit the memory guard (#1548). Here all three full lanes passed with no extra Node options.

Check main 46c9cba03 #1527 e707642c2
npm run verify:ghidra:cross-format Fail, exit 1, 30 s (about 28 s in #1518): schema with key or id "urn:rea:tool-output:sha256:8d99d0ac…" already exists at real-ghidra-boundaries.mjs:43 Pass, exit 0, 370 s (373 s in #1518, on 983b070d4 with the same change). All 7 fixtures, cleanup: complete
npm run verify:ghidra Fail, exit 1, 28 s, same error as the row above Pass, exit 0, 343 s. 4 fixtures, cleanup: complete
npm run verify:ghidra:aarch64-jump-table Fail, exit 1, 30 s, same error as the first row Pass, exit 0, 361 s. 7 fixtures, cleanup: complete
Bad output through a reused checker, Node 22, 24, 26 Not applicable. Main stops first Pass on all three. 145 of 145 bad payloads rejected
Existing tests near this code, not changed by #1527, on Node 22, 24 and 26 8 of 8 pass on each 8 of 8 pass on each

In each #1527 lane, 286 real tool calls had their output checked, and 52 bad requests were refused on purpose. The three Ghidra lanes ran on Node 24.18.0 only.

Bad output is still rejected. I built the checkers from the real tool list with the exact loop from #1527. The 8 groups of tools that share an id each get one shared checker. A valid payload passes through every tool in a group. Five kinds of bad payload fail through every tool, including the second tool in a group. Details are in the fold below.

Hopper. #1527 also changes real-hopper-boundaries.mjs, with the same line. By reading, Hopper takes its tool list from the same server and builds Ajv the same way, so it had the same bug. I could not run Hopper.

Windows. No workflow calls these checks. real-ghidra-windows.yml runs verify:ghidra:windows (lines 62 to 74) and verify:ghidra:windows:package (lines 78 to 90). Neither script imports a boundary file. No workflow runs the three Linux Ghidra lanes. Only real-hopper.yml:35 and real-hopper-linux.yml:56 reach the Hopper copy, and both are manual.

Not run

  • Hopper lanes. Hopper is not on this box.
  • macOS and Windows.
  • The Ghidra lanes on Node 22 and 26.
  • Other Ghidra lanes: swift, switch, dos, com, nativeaot and windows.
  • The full test suite and npm run check.
  • Tests added by fix(verification): reuse validators for shared output schema ids #1527. It adds none and changes no tests.
  • Main has since moved to f9ce32b47, a commit that only changes tests. I did not rerun on f9ce32b47.

Thanks, peter-research, for the small fix that keeps the check honest.

Edge check: what it did
  • The tool list has 139 tools, 118 distinct ids, and 8 groups that share an id (29 tools in all).
  • Control: the old loop, which compiles every schema, throws the same already exists error on this list.
  • In every group, the second tool gets the same checker object as the first.
  • Valid payloads come from a small generator that reads each schema. A fresh Ajv with no reuse accepts each one.
  • Bad payloads: an empty object, a missing key, an extra key, a wrong type, and a plain string. A fresh Ajv rejects each one, and so does every tool in every group: 5 shapes times 29 tools is 145.
  • Example: comment, which reuses the checker built for address_name, rejects the empty object with must have required property 'evidence_id'.
  • A two-tool test with one shared id also rejects {"n": "x"} through the second tool (must be integer).
  • Same result on Node 22.23.1, 24.18.0 and 26.11.1 (363 assertions each).
  • One note. The reuse trusts the id. That is safe while the id is a hash of the schema (toolSchemaMetadata.ts:90). Say two different schemas shared an id. Then the second tool would be checked against the first schema. Main would stop with an error instead. I tested this with a made-up pair, not with the real list.
Commands
# each tree built from git archive, then, on Node 24.18.0:
npm ci && npm run build
npm run verify:ghidra:cross-format
npm run verify:ghidra
npm run verify:ghidra:aarch64-jump-table
# on Node 22, 24 and 26:
npm run test:focused -- tests/boundary/mcp/outputValidatorReuse.test.ts src/contracts/toolOutputReferences.test.ts

One Ghidra lane at a time, with GHIDRA_INSTALL_DIR and JAVA_HOME set.

Test setup
  • Box: Nobara 44 (Fedora 44 base), Linux 7.2.9 (7.2.9-200.nobara.fc44.x86_64)
  • CPU: AMD Ryzen 9 5900X, 12 cores / 24 threads
  • RAM: 32 GB
  • Node: 22.23.1, 24.18.0, 26.11.1
  • Ghidra: 12.1.4 (Apache-2.0)
  • JDK: Temurin 21.0.12.1
  • JDK license: GPL-2.0-only, with the Classpath exception
  • Head / base: e707642c2 / 46c9cba03

🤖 Generated with Claude Code (Sonnet 5.5, verified by Opus 5.5) 🦊

@morluto morluto added bug Something isn't working area: native Native analysis and decompilation, including Ghidra, IDA, and Hopper analysis behavior labels Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: native Native analysis and decompilation, including Ghidra, IDA, and Hopper analysis behavior bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Ghidra boundary verifier fails on duplicate output schema ids since #1433

4 participants