morluto/rea

Fork 0

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

[GH-ISSUE #5845] [Bug] CLI requiredness test confuses exact-optional key omission with undefined #3542

New issue

Closed

opened 2026-10-31 00:41:40 +00:00 by bsakaguchi · 2 comments

bsakaguchi commented

2026-10-31 00:41:40 +00:00

Owner

Originally created by @gensokyo-dev on GitHub (Oct 21, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/5845

Before submitting

Searched open/closed reports for related exact-optional/CLI requiredness failures.
No credentials, private paths or sensitive target data.

Area / interface / earliest failure stage

CLI; Other (test oracle); request parsing or validation assertions, not an observed runtime parsing failure.

Observed behavior

tests/boundary/cli/cliInputSchemaRequired.test.ts reports:

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

The actual registered options parser accepts {} and rejects {commandTimeoutMs: undefined}. A standalone exact-optional field rejects undefined, which the test incorrectly treats as proof that the object key is required. This is a false requiredness assertion, not evidence that the public CLI requires the timeout option.

Reproduction actually performed

At WABT commit e9cdf873c58bd32e8f43b69f8a9a6d1ceb50d645 (before the SQLite changes in #5818), ran:

npm run test:focused -- tests/boundary/cli/cliInputSchemaRequired.test.ts

Observed 1 failed, 5 passed with the mismatch above. CI also reproduced it in test shard 4 at SQLite head 200f493d9; that PR does not edit the WABT contract or this test.

Separately inspected the actual registered WABT options schema on the built runtime:

import { Cli } from "incur";
import { createCli } from "./dist/cli.js";
const options = Cli.toCommands.get(createCli({}))
  .get("inspect-wasm-artifact").options;
console.log({
  standalone: options.shape.commandTimeoutMs.safeParse(undefined).success,
  omitted: options.safeParse({}).success,
  explicitUndefined: options.safeParse({commandTimeoutMs: undefined}).success,
});
// observed: { standalone: false, omitted: true, explicitUndefined: false }

Expected behavior

Infer requiredness from object-key omission semantics, including ordinary optional/default/exact-optional fields. Keep detecting genuinely required inputs and real advertisement/parser mismatches. Do not relax the production WABT contract to make a faulty oracle pass. A suitable helper/regression must also preserve issue paths/refinement meaning rather than accepting every object with unrelated invalid values.

Impact and scope

Observed local focused failure and portable CI failure; no WABT invocation, target change or Evidence integrity failure in this reproduction. Trigger became visible after #5798 added the exact-optional per-call deadline. This report isolates test semantics; no exhaustive CI audit, other-platform reproduction or natural runtime timeout claim.

REA version / runtime and platform / target

Source revisions above; Linux x64, Node 24.18.0. Target: registered CLI schema only, no analysis artifact or provider execution.

Typed outcome / remaining unknowns

Not applicable: no analysis request reaches a provider. NOT_ENOUGH_INFO: occurrence with other exact-optional fields and other runtime versions; proposed checks are not completed results. Related timeout/limit work stays in #5808; keep this focused test correction separate from SQLite #5818.

Originally created by @gensokyo-dev on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5845 ### Before submitting - [x] Searched open/closed reports for related exact-optional/CLI requiredness failures. - [x] No credentials, private paths or sensitive target data. ### Area / interface / earliest failure stage CLI; Other (test oracle); request parsing or validation assertions, not an observed runtime parsing failure. ### Observed behavior `tests/boundary/cli/cliInputSchemaRequired.test.ts` reports: ```text inspect-wasm-artifact options: advertised [], parser rejects [commandTimeoutMs] ``` The actual registered options parser accepts `{}` and rejects `{commandTimeoutMs: undefined}`. A standalone exact-optional field rejects `undefined`, which the test incorrectly treats as proof that the object key is required. This is a **false requiredness assertion**, not evidence that the public CLI requires the timeout option. ### Reproduction actually performed At WABT commit `e9cdf873c58bd32e8f43b69f8a9a6d1ceb50d645` (before the SQLite changes in #5818), ran: ```sh npm run test:focused -- tests/boundary/cli/cliInputSchemaRequired.test.ts ``` Observed **1 failed, 5 passed** with the mismatch above. CI also reproduced it in [test shard 4](https://github.com/morluto/rea/actions/runs/39567199853/job/119113187319) at SQLite head `200f493d9`; that PR does not edit the WABT contract or this test. Separately inspected the actual registered WABT options schema on the built runtime: ```js import { Cli } from "incur"; import { createCli } from "./dist/cli.js"; const options = Cli.toCommands.get(createCli({})) .get("inspect-wasm-artifact").options; console.log({ standalone: options.shape.commandTimeoutMs.safeParse(undefined).success, omitted: options.safeParse({}).success, explicitUndefined: options.safeParse({commandTimeoutMs: undefined}).success, }); // observed: { standalone: false, omitted: true, explicitUndefined: false } ``` ### Expected behavior Infer requiredness from object-key omission semantics, including ordinary optional/default/exact-optional fields. Keep detecting genuinely required inputs and real advertisement/parser mismatches. Do not relax the production WABT contract to make a faulty oracle pass. A suitable helper/regression must also preserve issue paths/refinement meaning rather than accepting every object with unrelated invalid values. ### Impact and scope Observed local focused failure and portable CI failure; no WABT invocation, target change or Evidence integrity failure in this reproduction. Trigger became visible after #5798 added the exact-optional per-call deadline. This report isolates test semantics; no exhaustive CI audit, other-platform reproduction or natural runtime timeout claim. ### REA version / runtime and platform / target Source revisions above; Linux x64, Node 24.18.0. Target: registered CLI schema only, no analysis artifact or provider execution. ### Typed outcome / remaining unknowns Not applicable: no analysis request reaches a provider. NOT_ENOUGH_INFO: occurrence with other exact-optional fields and other runtime versions; proposed checks are not completed results. Related timeout/limit work stays in #5808; keep this focused test correction separate from SQLite #5818.

bsakaguchi closed this issue

2026-10-31 00:41:41 +00:00

bsakaguchi commented

2026-10-31 00:41:41 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 21, 2026):

Agreed — the oracle is asserting the wrong semantics: for an exact-optional field, rejecting an explicitly-undefined value is normal strict-schema behavior, while omission succeeding is the actual requiredness signal. Using the former to flag the option as required produces false positives every time an exact-optional field is added (as #5798 did). Testing omission-based requiredness while keeping the standalone exact-optional checks for refinement guarantees (invalid values still rejected) keeps the test honest without weakening the production contract.

@XinwangTao commented on GitHub (Oct 21, 2026): Agreed — the oracle is asserting the wrong semantics: for an exact-optional field, rejecting an explicitly-undefined value is normal strict-schema behavior, while omission succeeding is the actual requiredness signal. Using the former to flag the option as required produces false positives every time an exact-optional field is added (as #5798 did). Testing omission-based requiredness while keeping the standalone exact-optional checks for refinement guarantees (invalid values still rejected) keeps the test honest without weakening the production contract.

bsakaguchi commented

2026-10-31 00:41:42 +00:00

Author

Owner

@kaoru0822-kitauji commented on GitHub (Oct 21, 2026):

Status: fixed on main. The oracle fix landed in 19f2969c5, and 5eebe2fdf extracted the helper. omissionRejected now judges requiredness with z.object({ [key]: field }).safeParse({}) rather than field.safeParse(undefined). No PR was opened for this, because it would duplicate what is already on main.

Verification (local, c324d31b4, macOS arm64):

npm run test:focused -- tests/boundary/cli/cliInputSchemaRequired.test.ts: 7/7 passed.
Mutation A: removed .exactOptional() from command_timeout_ms. The advertisement and the parser then both mark it required, so the consistency test agrees (correct). The WASM regression test fails ({} rejected).
Mutation B: restored the old field.safeParse(undefined) oracle. This reproduces inspect-wasm-artifact options: advertised [], parser rejects [commandTimeoutMs].
7 registered option groups have object-level refinements (packet, signal and Frida commands). Whole-object {} parse issues match the per-field oracle exactly for each of them, so isolating each field doesn't hide any requiredness in the current catalog.
Hosted CI on 19f2969c5: Test shard (4/8) passed, and cliInputSchemaRequired passed (7 tests) inside Portable tests. Portable tests still failed there on environmentExport.test.ts, a separate cause that c324d31b4 addresses.

Open PRs whose only real failure is this issue (Portable tests plus Test shard (4/8), both on cliInputSchemaRequired, besides the aggregate CI required). They need main merged in; a plain rerun reuses the old merge ref:
#5785 (@8rulerstar), #5803, #5810, #5858, #5860, #5862, #5874 (@kaoru0822-kitauji), #5816, #5854 (@XiaoBinGan)

Failing on this issue plus something else:

#5852: also environmentExport.test.ts (fixed on main) and clientConfigurationDiagnosticsCli.test.ts
#5867, #5875: also a 30 s timeout in tests/acceptance/applications/peResources.test.ts. The same timeout appears on main c324d31b4 Test shard (7/8), so this is a separate issue, not this one.
#2876, #3463, #5807: also Validate author declaration

#5857 targeted this too. It now conflicts with main and is superseded by 19f2969c5.

This issue can be closed as fixed by 19f2969c5.

@kaoru0822-kitauji commented on GitHub (Oct 21, 2026): **Status: fixed on `main`.** The oracle fix landed in 19f2969c5, and 5eebe2fdf extracted the helper. `omissionRejected` now judges requiredness with `z.object({ [key]: field }).safeParse({})` rather than `field.safeParse(undefined)`. No PR was opened for this, because it would duplicate what is already on `main`. **Verification (local, `c324d31b4`, macOS arm64):** - `npm run test:focused -- tests/boundary/cli/cliInputSchemaRequired.test.ts`: 7/7 passed. - Mutation A: removed `.exactOptional()` from `command_timeout_ms`. The advertisement and the parser then both mark it required, so the consistency test agrees (correct). The WASM regression test fails (`{}` rejected). - Mutation B: restored the old `field.safeParse(undefined)` oracle. This reproduces `inspect-wasm-artifact options: advertised [], parser rejects [commandTimeoutMs]`. - 7 registered option groups have object-level refinements (packet, signal and Frida commands). Whole-object `{}` parse issues match the per-field oracle exactly for each of them, so isolating each field doesn't hide any requiredness in the current catalog. - Hosted CI on `19f2969c5`: Test shard (4/8) passed, and `cliInputSchemaRequired` passed (7 tests) inside Portable tests. Portable tests still failed there on `environmentExport.test.ts`, a separate cause that c324d31b4 addresses. **Open PRs whose only real failure is this issue** (Portable tests plus Test shard (4/8), both on `cliInputSchemaRequired`, besides the aggregate `CI required`). They need `main` merged in; a plain rerun reuses the old merge ref: #5785 (@8rulerstar), #5803, #5810, #5858, #5860, #5862, #5874 (@kaoru0822-kitauji), #5816, #5854 (@XiaoBinGan) Failing on this issue **plus** something else: - #5852: also `environmentExport.test.ts` (fixed on `main`) and `clientConfigurationDiagnosticsCli.test.ts` - #5867, #5875: also a 30 s timeout in `tests/acceptance/applications/peResources.test.ts`. The same timeout appears on `main` `c324d31b4` Test shard (7/8), so this is a separate issue, not this one. - #2876, #3463, #5807: also `Validate author declaration` #5857 targeted this too. It now conflicts with `main` and is superseded by 19f2969c5. This issue can be closed as fixed by 19f2969c5.

bsakaguchi referenced this issue

2026-10-31 01:09:39 +00:00

[PR #3549] [MERGED] fix(ci): require portable inventory reports on selected runs #5817

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

No description provided.

Rows
Columns