[PR #5880] [CLOSED] test(cli): treat omitted exactOptional inputs as optional #7098

Closed
opened 2026-11-01 21:45:22 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5880
Author: @morluto
Created: 10/21/2026
Status: ❌ Closed

Base: main ← Head: cursor/fix-cli-schema-omission-10a8


📝 Commits (1)

  • cc339e5 test(cli): treat omitted exactOptional inputs as optional

📊 Changes

1 file changed (+8 additions, -1 deletions)

View changed files

📝 tests/boundary/cli/cliInputSchemaRequired.test.ts (+8 -1)

📄 Description

Summary

The CLI advertised-input check treated inspect-wasm-artifact's commandTimeoutMs as required. Callers can omit it, and --schema already advertises no required options. The check now decides omission by parsing an object that leaves the key out.

Problem and expected behavior

tests/boundary/cli/cliInputSchemaRequired.test.ts failed on main with:

inspect-wasm-artifact options: advertised [], parser rejects [commandTimeoutMs]

That assertion is in the portable suite and fails Test shard 4/8 on PRs based on main (for example #5819 and #5821), from 524159759 through f5410bdc4.

command_timeout_ms is z.number().int().min(1).max(2147483647).exactOptional(), wired in src/cli/wasmCommands.ts as commandTimeoutMs. A field parse of undefined fails. An object parse of {} succeeds, and the advertised schema does not list the option as required. Those two sides are the contract: omission is accepted, and an explicit undefined is not.

Change and scope

The mismatch became observable in a5f982de8 (feat(wasm): make command deadlines adjustable with recovery guidance, #5798), which added this .exactOptional() input. The probe itself is from b4927a0a4 (#1175) and calls safeParse(undefined) on each field. That matches .optional() and defaulted fields. It does not match .exactOptional(), which is used intentionally for missing-key values elsewhere, including this input.

#5798 was merged by maintainer override with the portable suite unfinished. Test shard 4/8 was cancelled, so this assertion did not run on the introducing PR.

The probe now builds a one-key object and parses {}. An inventory of every primary command's args and options found no other field where field-level undefined and object-level omission disagree, and none where the advertised required list disagreed with object omission. SQLite's worker_timeout_ms input stays .optional(); changing the WASM field was set aside because .optional() accepts explicit undefined, and the WASM service stores the parsed input as Evidence parameters without SQLite's undefined strip.

Follow the contribution scope.
Unresolved scope, authority, side effects, or required verification block
integration. Link the required maintainer scope decision for new providers or
capabilities involving target execution, injection, mutation, or protection
recovery; ordinary fixes within existing boundaries may state "not applicable".
Draft PRs may leave the scope decision pending and the readiness and merge checklists
unchecked. Maintainers can help resolve open questions.

Contract and boundary impact

  • Semantic owner and earliest changed stage: the CLI advertised-input boundary test owns the omission comparison. No parser, provider, or schema producer changes.
  • Contribution scope fit and linked maintainer decision: not applicable. Test-only correction of an existing check; no new provider or execution authority.
  • CLI and MCP/tool-catalog contract: unchanged. inspect-wasm-artifact still advertises commandTimeoutMs as optional and still accepts omission.
  • Provider, bridge, target-format, or platform compatibility: none.
  • Evidence, artifact, provenance, or reconstruction contract: none.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs and skills/; edit authored instructions in .agents/skills/reverse-engineer-anything/ and never commit build outputs): none.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

No target interaction. The change is the boundary assertion only.

Evidence and regression coverage

  • Tests added or updated: tests/boundary/cli/cliInputSchemaRequired.test.ts (existing inventory assertion; probe only).
  • Base reproduction or other evidence: on the unchanged test, npm run test:focused -- tests/boundary/cli/cliInputSchemaRequired.test.ts failed 1/6 with the commandTimeoutMs mismatch above. After the probe change, the same command passed 6/6. A separate inventory compared field-level undefined, one-key object omission, and JSON Schema required for every primary command; only commandTimeoutMs disagreed, and it disagreed only on the field-level probe.
  • User-visible CLI/MCP output (if applicable): none.
  • Remaining proof gaps: the full portable shard was not rerun locally. No WABT workflow was run; command behavior did not change. Parent-object refinements are outside both this probe and advertised required.

For evidence-bearing changes:

  • Observed, derived, and inferred claims remain distinguishable.
  • Artifact identity, source provenance, and failed attempts remain preserved.
  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

  • npm run test:focused -- tests/boundary/cli/cliInputSchemaRequired.test.ts — before the change: 1 failed, 5 passed, mismatch inspect-wasm-artifact options: advertised [], parser rejects [commandTimeoutMs]. After the change: 6 passed.
  • npm run check:fast — typecheck and lint succeeded (2 tasks).
  • npm run format:check — all matched files already formatted.
  • Real WABT CLI/MCP — not run; this change does not alter the command.

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable. No provider or target behavior changed.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: none. Test-only.

Author readiness

Human owner: @morluto

  • 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.
Open in Web Open in Cursor 

🔄 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/5880 **Author:** [@morluto](https://github.com/morluto) **Created:** 10/21/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `cursor/fix-cli-schema-omission-10a8` --- ### 📝 Commits (1) - [`cc339e5`](https://github.com/morluto/rea/commit/cc339e5246eda00fbf24ea7f38251dd3117f6774) test(cli): treat omitted exactOptional inputs as optional ### 📊 Changes **1 file changed** (+8 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `tests/boundary/cli/cliInputSchemaRequired.test.ts` (+8 -1) </details> ### 📄 Description <!-- CURSOR_AGENT_PR_BODY_BEGIN --> ## Summary The CLI advertised-input check treated `inspect-wasm-artifact`'s `commandTimeoutMs` as required. Callers can omit it, and `--schema` already advertises no required options. The check now decides omission by parsing an object that leaves the key out. ## Problem and expected behavior `tests/boundary/cli/cliInputSchemaRequired.test.ts` failed on main with: `inspect-wasm-artifact options: advertised [], parser rejects [commandTimeoutMs]` That assertion is in the portable suite and fails Test shard 4/8 on PRs based on main (for example #5819 and #5821), from `524159759` through `f5410bdc4`. `command_timeout_ms` is `z.number().int().min(1).max(2147483647).exactOptional()`, wired in `src/cli/wasmCommands.ts` as `commandTimeoutMs`. A field parse of `undefined` fails. An object parse of `{}` succeeds, and the advertised schema does not list the option as required. Those two sides are the contract: omission is accepted, and an explicit `undefined` is not. ## Change and scope The mismatch became observable in `a5f982de8` (`feat(wasm): make command deadlines adjustable with recovery guidance`, #5798), which added this `.exactOptional()` input. The probe itself is from `b4927a0a4` (#1175) and calls `safeParse(undefined)` on each field. That matches `.optional()` and defaulted fields. It does not match `.exactOptional()`, which is used intentionally for missing-key values elsewhere, including this input. #5798 was merged by maintainer override with the portable suite unfinished. Test shard 4/8 was cancelled, so this assertion did not run on the introducing PR. The probe now builds a one-key object and parses `{}`. An inventory of every primary command's args and options found no other field where field-level `undefined` and object-level omission disagree, and none where the advertised `required` list disagreed with object omission. SQLite's `worker_timeout_ms` input stays `.optional()`; changing the WASM field was set aside because `.optional()` accepts explicit `undefined`, and the WASM service stores the parsed input as Evidence parameters without SQLite's undefined strip. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Unresolved scope, authority, side effects, or required verification block integration. Link the required maintainer scope decision for new providers or capabilities involving target execution, injection, mutation, or protection recovery; ordinary fixes within existing boundaries may state "not applicable". Draft PRs may leave the scope decision pending and the readiness and merge checklists unchecked. Maintainers can help resolve open questions. ## Contract and boundary impact - Semantic owner and earliest changed stage: the CLI advertised-input boundary test owns the omission comparison. No parser, provider, or schema producer changes. - Contribution scope fit and linked maintainer decision: not applicable. Test-only correction of an existing check; no new provider or execution authority. - CLI and MCP/tool-catalog contract: unchanged. `inspect-wasm-artifact` still advertises `commandTimeoutMs` as optional and still accepts omission. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact (CI generates and validates ignored catalogs and `skills/`; edit authored instructions in `.agents/skills/reverse-engineer-anything/` and never commit build outputs): none. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [ ] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. No target interaction. The change is the boundary assertion only. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/cli/cliInputSchemaRequired.test.ts` (existing inventory assertion; probe only). - Base reproduction or other evidence: on the unchanged test, `npm run test:focused -- tests/boundary/cli/cliInputSchemaRequired.test.ts` failed 1/6 with the `commandTimeoutMs` mismatch above. After the probe change, the same command passed 6/6. A separate inventory compared field-level `undefined`, one-key object omission, and JSON Schema `required` for every primary command; only `commandTimeoutMs` disagreed, and it disagreed only on the field-level probe. - User-visible CLI/MCP output (if applicable): none. - Remaining proof gaps: the full portable shard was not rerun locally. No WABT workflow was run; command behavior did not change. Parent-object refinements are outside both this probe and advertised `required`. For evidence-bearing changes: - [ ] Observed, derived, and inferred claims remain distinguishable. - [ ] Artifact identity, source provenance, and failed attempts remain preserved. - [ ] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run test:focused -- tests/boundary/cli/cliInputSchemaRequired.test.ts` — before the change: 1 failed, 5 passed, mismatch `inspect-wasm-artifact options: advertised [], parser rejects [commandTimeoutMs]`. After the change: 6 passed. - `npm run check:fast` — typecheck and lint succeeded (2 tasks). - `npm run format:check` — all matched files already formatted. - Real WABT CLI/MCP — not run; this change does not alter the command. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. No provider or target behavior changed. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none. Test-only. ## Author readiness Human owner: @morluto - [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 - [ ] 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. <!-- CURSOR_AGENT_PR_BODY_END --> <div><a href="https://cursor.com/agents/bc-671cf507-d468-5f08-ba55-a47d3c2110a8?cursor_ref=pr_footer&cursor_cta=open_in_web"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-web-light.png"><img alt="Open in Web" width="114" height="28" src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a>&nbsp;<a href="https://cursor.com/background-agent?bcId=bc-671cf507-d468-5f08-ba55-a47d3c2110a8&cursor_ref=pr_footer&cursor_cta=open_in_cursor"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img alt="Open in Cursor" width="131" height="28" src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a>&nbsp;</div> --- <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:22 +00:00
Sign in to join this conversation.
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#7098
No description provided.