morluto/rea

Fork 0

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

[PR #6021] [MERGED] fix(ci): validate artifact MCP replies against named schemas #7126

New issue

Closed

opened 2026-11-01 21:45:35 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:45:35 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6021
Author: @gensokyo-dev
Created: 10/21/2026
Status: ✅ Merged
Merged: 10/21/2026
Merged by: @gensokyo-dev

Base: main ← Head: fix/artifact-mcp-output-validation-20261021

📝 Commits (2)

3e250f3 fix(ci): validate artifact MCP replies against named schemas
f734d46 fix(ci): reject artifact MCP schema overrides

📊 Changes

10 files changed (+447 additions, -24 deletions)

View changed files

📝 docs/testing.md (+16 -0)
📝 scripts/lib/artifact-e2e.mjs (+75 -4)
📝 scripts/verify-native-calls.mjs (+3 -1)
📝 scripts/verify/apple/asset-catalog.mjs (+2 -1)
📝 scripts/verify/apple/keyed-archive.mjs (+2 -1)
📝 scripts/verify/apple/macho-occurrences.mjs (+2 -1)
📝 scripts/verify/apple/macos-dylib-e2e.mjs (+2 -1)
📝 scripts/verify/apple/native-provenance.mjs (+10 -15)
➕ tests/boundary/process/artifactMcpVerifierOutput.test.ts (+126 -0)
➕ tests/fixtures/artifactMcpVerifierProbe.mjs (+209 -0)

📄 Description

Summary

A fresh MCP SDK client has no held tools/list, so the artifact verification helper can skip output-schema validation. A real stdio boundary probe changed result.canonical_path from string to number while retaining the caller path: the old helper passed, although the advertised contract requires a string.

Discover actual tool definitions once per negotiated REA connection and bind lifecycle/analysis calls to the named SDK schema. Reject absent schemas, duplicate names and caller-supplied schema overrides. Share concurrent discovery, discard failed loads and rebind on reconnect. Preserve request options and attempt both owned cleanup operations while retaining the primary error. Raw artifact calls share this owner; the duplicated native-provenance definition index is removed. Existing provider/parity/identity assertions remain. Verification scope only; no new provider capability or authority.

Validation

PR head f734d463c9fe881887d454ee20e5ded2b614b11b; full PR CI tested merge preview 760a75011 with main 2319166c2. Node 24.18.0/npm 11.16.0; four-core affinity, two compiler checkers, one test worker and serial Turbo.

Regression guards fail on the old behavior for corrupted ordinary/space/Unicode paths and a permissive supplied schema, then pass with the fix. All 11 cases cover healthy output, discovery reuse/concurrency, same-client reconnect, absent/ambiguous schemas, override rejection and timeout preservation. Faults alter only the client boundary after real REA requests.
On this rebased integration:235 tests passed across 15 files;12 macOS-only native plist CLI cases skipped locally and are selected in the real Apple CI lane. This includes all changed test files from five intervening main fixes plus this PR's guard. Completed in 150.8s,201.3 CPU seconds,1684MiB sampled family RSS. npm run check passed (28.3s,54.0 CPU seconds,1530MiB); diff checks passed. The earlier cleanup/SDK/routing guard set passed 284 tests with no skips.
Complete PR CI passed on its first attempt:40 jobs passed,1 intentional Source-test skip;24,246 portable tests passed with200 named skips (1900 files passed,24 skipped). Real Apple, Windows, browser and all native-package lanes passed. Apple boundary regressions:271 passed,4 conditional skips. Greptile reviewed this head at5/5; Codex completed review of the actual merge commit f86206535 without findings. The actionable supplied-schema finding is fixed and resolved.
Main then advanced to 8e3bf56e1. Its conflict-free integration with this PR has tree ce5264688d5ed5198e6607edd061a6ad041629cd:complete static checks passed, and287 tests passed with1 named skip across11 files (all ten newly changed test files plus this PR guard). This ran391.8s,406.7 CPU seconds,1999MiB sampled family RSS. Full qualification of the actual merge commit passed on its first attempt:24,272 portable tests passed,200 named skips,1901 files passed and24 skipped, with all selected real-platform lanes passing. The earlier main feedback failure at 8e3bf56e1 exposed an unrelated fluctuating heap-observation fixture; its reproduced structural fix is #6081.

Two-session admission/close measurements on the earlier 6d356b843 runtime: warm filesystem, fresh parent process, separate initially empty child compile-cache roots, equivalent replies with SDK validation active:

Workflow	Elapsed	CPU	Sampled family RSS
SDK validates all listed tools	21.9s	27.5s	1045MiB
Actual helper selects named schemas	12.2s	16.7s	666MiB

The original unvalidated helper took 4.4s and is not an equivalent baseline. RSS samples can double-count shared pages/miss short children. Agent tokens and whole-Apple-job speedup are unmeasured; the historical Apple 15m cancellation's cause is not established by this fix. Linux resource-only admission does not prove native provider behavior; current real-platform CI must pass before merge.

Author readiness

Human owner: @gensokyo-dev

Declarations entered under the user's explicit delegation; independent human review was not observed in this session.

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.

_{🔄 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/6021 **Author:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Created:** 10/21/2026 **Status:** ✅ Merged **Merged:** 10/21/2026 **Merged by:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Base:** `main` ← **Head:** `fix/artifact-mcp-output-validation-20261021` --- ### 📝 Commits (2) - [`3e250f3`](https://github.com/morluto/rea/commit/3e250f39486dbb39e64e59c4fa99b7b49949ed3f) fix(ci): validate artifact MCP replies against named schemas - [`f734d46`](https://github.com/morluto/rea/commit/f734d463c9fe881887d454ee20e5ded2b614b11b) fix(ci): reject artifact MCP schema overrides ### 📊 Changes **10 files changed** (+447 additions, -24 deletions) <details> <summary>View changed files</summary> 📝 `docs/testing.md` (+16 -0) 📝 `scripts/lib/artifact-e2e.mjs` (+75 -4) 📝 `scripts/verify-native-calls.mjs` (+3 -1) 📝 `scripts/verify/apple/asset-catalog.mjs` (+2 -1) 📝 `scripts/verify/apple/keyed-archive.mjs` (+2 -1) 📝 `scripts/verify/apple/macho-occurrences.mjs` (+2 -1) 📝 `scripts/verify/apple/macos-dylib-e2e.mjs` (+2 -1) 📝 `scripts/verify/apple/native-provenance.mjs` (+10 -15) ➕ `tests/boundary/process/artifactMcpVerifierOutput.test.ts` (+126 -0) ➕ `tests/fixtures/artifactMcpVerifierProbe.mjs` (+209 -0) </details> ### 📄 Description ## Summary A fresh MCP SDK client has no held tools/list, so the artifact verification helper can skip output-schema validation. A real stdio boundary probe changed result.canonical_path from string to number while retaining the caller path: the old helper passed, although the advertised contract requires a string. Discover actual tool definitions once per negotiated REA connection and bind lifecycle/analysis calls to the named SDK schema. Reject absent schemas, duplicate names and caller-supplied schema overrides. Share concurrent discovery, discard failed loads and rebind on reconnect. Preserve request options and attempt both owned cleanup operations while retaining the primary error. Raw artifact calls share this owner; the duplicated native-provenance definition index is removed. Existing provider/parity/identity assertions remain. Verification scope only; no new provider capability or authority. ## Validation PR head `f734d463c9fe881887d454ee20e5ded2b614b11b`; full PR CI tested merge preview `760a75011` with main `2319166c2`. Node 24.18.0/npm 11.16.0; four-core affinity, two compiler checkers, one test worker and serial Turbo. - Regression guards fail on the old behavior for corrupted ordinary/space/Unicode paths and a permissive supplied schema, then pass with the fix. All 11 cases cover healthy output, discovery reuse/concurrency, same-client reconnect, absent/ambiguous schemas, override rejection and timeout preservation. Faults alter only the client boundary after real REA requests. - On this rebased integration:235 tests passed across 15 files;12 macOS-only native plist CLI cases skipped locally and are selected in the real Apple CI lane. This includes all changed test files from five intervening main fixes plus this PR's guard. Completed in 150.8s,201.3 CPU seconds,1684MiB sampled family RSS. `npm run check` passed (28.3s,54.0 CPU seconds,1530MiB); diff checks passed. The earlier cleanup/SDK/routing guard set passed 284 tests with no skips. - [Complete PR CI](https://github.com/morluto/rea/actions/runs/39621286564) passed on its first attempt:40 jobs passed,1 intentional Source-test skip;24,246 portable tests passed with200 named skips (1900 files passed,24 skipped). Real Apple, Windows, browser and all native-package lanes passed. Apple boundary regressions:271 passed,4 conditional skips. Greptile reviewed this head at5/5; Codex completed review of the actual merge commit `f86206535` without findings. The actionable supplied-schema finding is fixed and resolved. - Main then advanced to `8e3bf56e1`. Its conflict-free integration with this PR has tree `ce5264688d5ed5198e6607edd061a6ad041629cd`:complete static checks passed, and287 tests passed with1 named skip across11 files (all ten newly changed test files plus this PR guard). This ran391.8s,406.7 CPU seconds,1999MiB sampled family RSS. [Full qualification of the actual merge commit](https://github.com/morluto/rea/actions/runs/39626254139) passed on its first attempt:24,272 portable tests passed,200 named skips,1901 files passed and24 skipped, with all selected real-platform lanes passing. The earlier main feedback failure at `8e3bf56e1` exposed an unrelated fluctuating heap-observation fixture; its reproduced structural fix is #6081. Two-session admission/close measurements on the earlier `6d356b843` runtime: warm filesystem, fresh parent process, separate initially empty child compile-cache roots, equivalent replies with SDK validation active: | Workflow | Elapsed | CPU | Sampled family RSS | | --- | --- | --- | --- | | SDK validates all listed tools |21.9s|27.5s|1045MiB| | Actual helper selects named schemas |12.2s|16.7s|666MiB| The original unvalidated helper took 4.4s and is not an equivalent baseline. RSS samples can double-count shared pages/miss short children. Agent tokens and whole-Apple-job speedup are unmeasured; the historical Apple 15m cancellation's cause is not established by this fix. Linux resource-only admission does not prove native provider behavior; current real-platform CI must pass before merge. ## Author readiness Human owner: @gensokyo-dev Declarations entered under the user's explicit delegation; independent human review was not observed in this session. - [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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:45:35 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-03 13:39:26 +00:00

[PR #7126] [MERGED] fix(javascript): evaluate each short-circuit left operand once #7658

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#7126

No description provided.

Rows
Columns