morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #4064] [CLOSED] fix(mcp): advertise boolean literals as typed booleans #6029

New issue

Closed

opened 2026-10-31 01:11:18 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:11:18 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4064
Author: @yankikucuk
Created: 10/15/2026
Status: ❌ Closed

Base: main ← Head: fix/advertised-boolean-literals

📝 Commits (4)

4d9d2cc fix(mcp): advertise boolean literals as typed booleans
783c52e fix(mcp): keep literal-discriminated unions inclusive and retain folded examples
104e51d fix(mcp): detect referenced boolean discriminants and keep union examples
ae57a54 fix(mcp): follow reference chains to boolean discriminants and keep literals exact under not and if

📊 Changes

4 files changed (+452 additions, -12 deletions)

View changed files

📝 docs/mcp-contracts.md (+21 -0)
📝 src/contracts/inputSchemaPresentation.ts (+139 -10)
📝 src/contracts/toolInputReferences.test.ts (+285 -1)
📝 tests/boundary/mcp/toolSchemaValidity.test.ts (+7 -1)

📄 Description

Summary

Fixes #4025.

Advertised tool input schemas emitted boolean const values on five tools. Clients that render const as a one-member enum then meet Gemini function declarations, which accept only string enum members, and the provider rejects the whole request with HTTP 400 while REA is connected. The advertised projection now presents boolean literals as plain boolean fields. Canonical Zod validation is unchanged.

Problem and expected behavior

Trigger: connect REA over MCP to a Gemini-backed client (reported with opencode 1.18.32). inspect_electron_page.include_script_sources projects as anyOf: [{const: false, …}, {const: true}]; the client renders enum: [false] / enum: [true], and Google rejects enum[0] as not TYPE_STRING. One offending declaration takes down every tool, not only the one that owns it.

Expected: every advertised input schema survives every supported dialect, as established for dependentRequired in #2832. Boolean literals should advertise as their declared type with their default and description, while the canonical contract keeps enforcing the exact literal and the union branch it selects.

Change and scope

presentInputJsonSchema is the single owner of the wire projection for both the full and the compact profile (the compact reduction runs on its output), so the presentation is changed once there:

A schema node with a boolean const drops the const and keeps type: "boolean", default, description and title.
anyOf / oneOf whose alternatives are plain boolean schemas that differ only in the literal fold into one boolean field (the first default, description and title found are kept). This is the shape the root-union flattening produces for include_script_sources.
Nested object alternatives keep their structure; only the discriminating field becomes type: "boolean" (as in compare_web_captures … source.oneOf[*].properties.included).
String literals stay const; every supported dialect accepts them.

Not included: changes to canonical contracts, the compact budget, or output schemas (output schemas are not sent as function declarations). Contribution scope: ordinary fix within existing boundaries; maintainer scope decision not applicable.

Contract and boundary impact

Semantic owner and earliest changed stage: src/contracts/inputSchemaPresentation.ts, describeSchema (advertised input projection).
Contribution scope fit and linked maintainer decision: existing MCP advertisement boundary; not applicable.
CLI and MCP/tool-catalog contract: advertised input JSON for inspect_web_page, inspect_electron_page, analyze_web_bundle, compare_web_captures, evaluate_reconstruction_coverage changes shape (boolean literal fields become typed booleans). Accepted inputs are unchanged; the canonical schema still rejects the wrong literal. The tool-schema digest in the catalog identity changes accordingly.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: none.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: the generated catalog (docs/public/product-catalog.json) will reflect the new advertised shape; no authored skill instructions change.

Expected effects:

Documentation or usability only; no target interaction.

Evidence and regression coverage

Tests added or updated: src/contracts/toolInputReferences.test.ts
- advertises no non-string const or enum values in any profile walks every TOOL_CONTRACTS entry in the full and compact projection. On main it fails with 43 paths (the five tools above); it passes with this change.
- presents boolean literals as typed booleans and keeps their defaults covers the flattened root union and checks Ajv acceptance against the canonical parser.
- keeps a boolean literal that discriminates nested object alternatives covers the oneOf object case.

Base reproduction or other evidence (executed): a real stdio scan started scripts/rea.mjs mcp from the built checkout with an isolated HOME, listed all 207 tools and walked every inputSchema for non-string const/enum:

revision	full profile	compact profile
main `c61c108b5`	38 values in `inspect_web_page`, `analyze_web_bundle`, `compare_web_captures`, `inspect_electron_page`, `evaluate_reconstruction_coverage`	5 values in `inspect_web_page`, `analyze_web_bundle`, `inspect_electron_page`
this branch	0	0

User-visible CLI/MCP output: inspect_electron_page.inputSchema.properties.include_script_sources advertised over stdio after the change:
```
{ "default": false, "type": "boolean", "description": "Whether to include script sources in the result." }
```
Remaining proof gaps: not verified against a live Gemini endpoint or the reporting client; the claim that Gemini now accepts the catalog is derived from the error paths in the report lining up with the removed const positions.

Validation performed

npm run test:local -- src/contracts/toolInputReferences.test.ts — 23 passed (new catalog-wide test failed on main with 43 paths before the fix)
npm run test:local -- <all 15 test files under src/contracts and src/server> — 15 files, 127 passed, 3 skipped
npx tsc --noEmit — clean
npm run lint — 0 errors; verify:module-boundaries clean
npm run build:cached — ok; npm run docs:check — 5 passed; npm run verify:test-discovery — 1329 files discovered once
Real stdio tools/list scan in both profiles on main and on this branch — table above

Compatibility, safety, and release

Breaking changes or migration steps: none. Previously valid calls remain valid; only the advertised JSON Schema shape of boolean literal fields changes.
Real Hopper/Ghidra, browser, or OS coverage: not applicable (schema advertisement only). macOS arm64, Node 26.
Package or release metadata impact: generated catalog and tool-schema digest change; no package change.
Security, privacy, process, or containment review: none; no target interaction.

Author readiness

Human owner: @yankikucuk

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

Review follow-up (`783c52e3e`)

oneOf object unions discriminated by a boolean literal are advertised as anyOf, so clients that validate the advertised schema accept every input the canonical union accepts (Greptile P1).
Folded boolean alternatives keep their merged examples (Codex P2).
tests/boundary/mcp/toolSchemaValidity.test.ts now states the intentional broadening for analyze_web_bundle (Codex P1); the three schema files pass with 49 tests and the stdio scan is still 0/0.
The full tests/boundary/mcp folder shows 21 failures in eight Ghidra and analysis-view files that fail identically on upstream main in this environment; they do not touch input schema presentation.
The real OpenCode/Gemini conversion is not runnable here; it remains the stated proof gap.

Review follow-up 2

Boolean discriminants reached through local $refs (shared projection) are now recognised before a oneOf is kept, so such unions are advertised as anyOf too (Codex P2).
Folding merges the union's own examples with the branch examples instead of letting the outer annotation overwrite them (Codex P2).
50 tests across the three schema files, lint clean, stdio scan 0/0 in both profiles.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/4064 **Author:** [@yankikucuk](https://github.com/yankikucuk) **Created:** 10/15/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/advertised-boolean-literals` --- ### 📝 Commits (4) - [`4d9d2cc`](https://github.com/morluto/rea/commit/4d9d2cc7fdfd6380dff112d1af0363778f6fa909) fix(mcp): advertise boolean literals as typed booleans - [`783c52e`](https://github.com/morluto/rea/commit/783c52e3e774c50573b7522a1a46f404000224cc) fix(mcp): keep literal-discriminated unions inclusive and retain folded examples - [`104e51d`](https://github.com/morluto/rea/commit/104e51dfd08549193f569534cdb7562575360681) fix(mcp): detect referenced boolean discriminants and keep union examples - [`ae57a54`](https://github.com/morluto/rea/commit/ae57a54f4f3b2bef2525df0a7a3f56e6b07a68fb) fix(mcp): follow reference chains to boolean discriminants and keep literals exact under not and if ### 📊 Changes **4 files changed** (+452 additions, -12 deletions) <details> <summary>View changed files</summary> 📝 `docs/mcp-contracts.md` (+21 -0) 📝 `src/contracts/inputSchemaPresentation.ts` (+139 -10) 📝 `src/contracts/toolInputReferences.test.ts` (+285 -1) 📝 `tests/boundary/mcp/toolSchemaValidity.test.ts` (+7 -1) </details> ### 📄 Description ## Summary Fixes #4025. Advertised tool input schemas emitted boolean `const` values on five tools. Clients that render `const` as a one-member `enum` then meet Gemini function declarations, which accept only string enum members, and the provider rejects the whole request with HTTP 400 while REA is connected. The advertised projection now presents boolean literals as plain `boolean` fields. Canonical Zod validation is unchanged. ## Problem and expected behavior Trigger: connect REA over MCP to a Gemini-backed client (reported with opencode 1.18.32). `inspect_electron_page.include_script_sources` projects as `anyOf: [{const: false, …}, {const: true}]`; the client renders `enum: [false]` / `enum: [true]`, and Google rejects `enum[0]` as not `TYPE_STRING`. One offending declaration takes down every tool, not only the one that owns it. Expected: every advertised input schema survives every supported dialect, as established for `dependentRequired` in #2832. Boolean literals should advertise as their declared `type` with their `default` and description, while the canonical contract keeps enforcing the exact literal and the union branch it selects. ## Change and scope `presentInputJsonSchema` is the single owner of the wire projection for both the full and the compact profile (the compact reduction runs on its output), so the presentation is changed once there: - A schema node with a boolean `const` drops the `const` and keeps `type: "boolean"`, `default`, `description` and `title`. - `anyOf` / `oneOf` whose alternatives are plain boolean schemas that differ only in the literal fold into one boolean field (the first `default`, `description` and `title` found are kept). This is the shape the root-union flattening produces for `include_script_sources`. - Nested object alternatives keep their structure; only the discriminating field becomes `type: "boolean"` (as in `compare_web_captures … source.oneOf[*].properties.included`). - String literals stay `const`; every supported dialect accepts them. Not included: changes to canonical contracts, the compact budget, or output schemas (output schemas are not sent as function declarations). Contribution scope: ordinary fix within existing boundaries; maintainer scope decision not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `src/contracts/inputSchemaPresentation.ts`, `describeSchema` (advertised input projection). - Contribution scope fit and linked maintainer decision: existing MCP advertisement boundary; not applicable. - CLI and MCP/tool-catalog contract: advertised input JSON for `inspect_web_page`, `inspect_electron_page`, `analyze_web_bundle`, `compare_web_captures`, `evaluate_reconstruction_coverage` changes shape (boolean literal fields become typed booleans). Accepted inputs are unchanged; the canonical schema still rejects the wrong literal. The tool-schema digest in the catalog identity changes accordingly. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: the generated catalog (`docs/public/product-catalog.json`) will reflect the new advertised shape; no authored skill instructions change. Expected effects: - [x] Documentation or usability only; no target interaction. ## Evidence and regression coverage - Tests added or updated: `src/contracts/toolInputReferences.test.ts` - `advertises no non-string const or enum values in any profile` walks every `TOOL_CONTRACTS` entry in the full and compact projection. On main it fails with 43 paths (the five tools above); it passes with this change. - `presents boolean literals as typed booleans and keeps their defaults` covers the flattened root union and checks Ajv acceptance against the canonical parser. - `keeps a boolean literal that discriminates nested object alternatives` covers the `oneOf` object case. - Base reproduction or other evidence (executed): a real stdio scan started `scripts/rea.mjs mcp` from the built checkout with an isolated `HOME`, listed all 207 tools and walked every `inputSchema` for non-string `const`/`enum`: | revision | full profile | compact profile | |---|---|---| | main `c61c108b5` | 38 values in `inspect_web_page`, `analyze_web_bundle`, `compare_web_captures`, `inspect_electron_page`, `evaluate_reconstruction_coverage` | 5 values in `inspect_web_page`, `analyze_web_bundle`, `inspect_electron_page` | | this branch | 0 | 0 | - User-visible CLI/MCP output: `inspect_electron_page.inputSchema.properties.include_script_sources` advertised over stdio after the change: ```json { "default": false, "type": "boolean", "description": "Whether to include script sources in the result." } ``` - Remaining proof gaps: not verified against a live Gemini endpoint or the reporting client; the claim that Gemini now accepts the catalog is derived from the error paths in the report lining up with the removed `const` positions. ## Validation performed - `npm run test:local -- src/contracts/toolInputReferences.test.ts` — 23 passed (new catalog-wide test failed on main with 43 paths before the fix) - `npm run test:local -- <all 15 test files under src/contracts and src/server>` — 15 files, 127 passed, 3 skipped - `npx tsc --noEmit` — clean - `npm run lint` — 0 errors; `verify:module-boundaries` clean - `npm run build:cached` — ok; `npm run docs:check` — 5 passed; `npm run verify:test-discovery` — 1329 files discovered once - Real stdio `tools/list` scan in both profiles on main and on this branch — table above ## Compatibility, safety, and release - Breaking changes or migration steps: none. Previously valid calls remain valid; only the advertised JSON Schema shape of boolean literal fields changes. - Real Hopper/Ghidra, browser, or OS coverage: not applicable (schema advertisement only). macOS arm64, Node 26. - Package or release metadata impact: generated catalog and tool-schema digest change; no package change. - Security, privacy, process, or containment review: none; no target interaction. ## Author readiness Human owner: @yankikucuk - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [ ] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. ## Review follow-up (783c52e3e) - `oneOf` object unions discriminated by a boolean literal are advertised as `anyOf`, so clients that validate the advertised schema accept every input the canonical union accepts (Greptile P1). - Folded boolean alternatives keep their merged `examples` (Codex P2). - `tests/boundary/mcp/toolSchemaValidity.test.ts` now states the intentional broadening for `analyze_web_bundle` (Codex P1); the three schema files pass with 49 tests and the stdio scan is still 0/0. - The full `tests/boundary/mcp` folder shows 21 failures in eight Ghidra and analysis-view files that fail identically on upstream main in this environment; they do not touch input schema presentation. - The real OpenCode/Gemini conversion is not runnable here; it remains the stated proof gap. ## Review follow-up 2 - Boolean discriminants reached through local `$ref`s (shared projection) are now recognised before a `oneOf` is kept, so such unions are advertised as `anyOf` too (Codex P2). - Folding merges the union's own `examples` with the branch examples instead of letting the outer annotation overwrite them (Codex P2). - 50 tests across the three schema files, lint clean, stdio scan 0/0 in both profiles. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:11:18 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:46:39 +00:00

[PR #6313] [MERGED] fix(web): refuse an existing script-export destination as invalid input #7274

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#6029

No description provided.

Rows
Columns