morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 07:47:29 +00:00

[PR #4182] [CLOSED] fix(cli): reject empty and non-decimal text for integer positional arguments #6111

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/cli-strict-integer-args

📝 Commits (1)

281d040 fix(cli): reject empty and non-decimal text for integer positional arguments

📊 Changes

6 files changed (+78 additions, -22 deletions)

View changed files

📝 src/cli/cutterCommands.ts (+4 -5)
📝 src/cli/options.ts (+13 -0)
📝 src/cli/webModuleCommands.ts (+4 -5)
📝 src/cli/webNetworkCaptureCommands.ts (+2 -1)
📝 src/cli/webSourceLocationCommands.ts (+8 -11)
➕ tests/boundary/cli/cliIntegerArguments.test.ts (+47 -0)

📄 Description

Summary

Integer CLI arguments accept text that is not a number. rea cutter-command <session> "$GEN" pdi with GEN unset sends expected_generation: 0, and a fresh Cutter bridge is at generation 0, so the stale-document guard passes without the caller ever supplying a generation. rea trace-web-module-imports <manifest> "" likewise traces script 0 and exits 0. Hex, binary, exponent, signed and fractional spellings (0x1, 0b1, 1e0, +1, 1.0) are also accepted.

Problem and expected behavior

These arguments were declared as z.coerce.number().int().nonnegative() (or .min(0) / .min(1)). z.coerce.number() calls Number(value), and Number("") and Number(" ") are 0, which passes both checks. Incur passes an empty token through as a positional, so the empty value reaches the schema. Expected: anything other than a plain decimal integer is a VALIDATION_ERROR naming the argument, as it already is for other malformed positionals. MCP is unaffected because its schemas use strict z.number().

Change and scope

src/cli/options.ts adds cliIntegerArgument(min): a string that must match ^(?:0|[1-9][0-9]*)$, piped into z.coerce.number<string>().int().safe().min(min). A pipe keeps the schema exportable, which a transform would not. It replaces the coerced schemas at every CLI integer site: cutter-command expectedGeneration, trace-web-module-imports scriptIndex, the web source-location scriptIndex, line (min 1) and column, and the web network capture --record ordinals. Valid decimal values parse to the same numbers as before. Not included: MCP schemas, which already reject strings.

Contract and boundary impact

Semantic owner and earliest changed stage: CLI argument parsing (src/cli/*Commands.ts, src/cli/options.ts).
Contribution scope fit and linked maintainer decision: ordinary fix within existing boundaries; not applicable.
CLI and MCP/tool-catalog contract: CLI rejects inputs it previously coerced to numbers; accepted values are unchanged. MCP unchanged.
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): cutter-command no longer runs against session generation 0 when the generation argument is empty.
Generated metadata (docs/public/product-catalog.json), package, or installation impact: none committed; CI regenerates catalogs.
Documentation or usability only; no target interaction.

Evidence and regression coverage

Tests added or updated: tests/boundary/cli/cliIntegerArguments.test.ts runs cutter-command and trace-web-module-imports with "", " ", 0x1, 0b1, 1e0, +1 and 1.0, and expects exit 1 with a VALIDATION_ERROR on expectedGeneration / scriptIndex.
Base reproduction or other evidence: on main all 14 cases exit 0 (executed).
User-visible CLI/MCP output (if applicable): {"code":"VALIDATION_ERROR","fieldErrors":[{"path":"expectedGeneration","message":"Expected a decimal integer",...}]}.
Remaining proof gaps: --record is covered by the shared schema but has no dedicated case.

Validation performed

npm run test:prepare && npx vitest run --project cli-boundary — 88 files, 498 passed, 2 skipped
npx vitest run --project mcp-boundary — 138 files, 776 passed, 5 skipped
npx vitest run tests/acceptance/webModuleCli.test.ts — 1 passed
npx tsc --noEmit -p . — no errors
npx prettier --check and npx oxlint on the changed files — clean apart from the existing max-params warning at options.ts:69

_{🔄 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/4182 **Author:** [@arjunprabhulal](https://github.com/arjunprabhulal) **Created:** 10/15/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/cli-strict-integer-args` --- ### 📝 Commits (1) - [`281d040`](https://github.com/morluto/rea/commit/281d0400b8e9926e6cfabc05cd47a4a67de43aa6) fix(cli): reject empty and non-decimal text for integer positional arguments ### 📊 Changes **6 files changed** (+78 additions, -22 deletions) <details> <summary>View changed files</summary> 📝 `src/cli/cutterCommands.ts` (+4 -5) 📝 `src/cli/options.ts` (+13 -0) 📝 `src/cli/webModuleCommands.ts` (+4 -5) 📝 `src/cli/webNetworkCaptureCommands.ts` (+2 -1) 📝 `src/cli/webSourceLocationCommands.ts` (+8 -11) ➕ `tests/boundary/cli/cliIntegerArguments.test.ts` (+47 -0) </details> ### 📄 Description ## Summary Integer CLI arguments accept text that is not a number. `rea cutter-command <session> "$GEN" pdi` with `GEN` unset sends `expected_generation: 0`, and a fresh Cutter bridge is at generation 0, so the stale-document guard passes without the caller ever supplying a generation. `rea trace-web-module-imports <manifest> ""` likewise traces script 0 and exits 0. Hex, binary, exponent, signed and fractional spellings (`0x1`, `0b1`, `1e0`, `+1`, `1.0`) are also accepted. ## Problem and expected behavior These arguments were declared as `z.coerce.number().int().nonnegative()` (or `.min(0)` / `.min(1)`). `z.coerce.number()` calls `Number(value)`, and `Number("")` and `Number(" ")` are `0`, which passes both checks. Incur passes an empty token through as a positional, so the empty value reaches the schema. Expected: anything other than a plain decimal integer is a `VALIDATION_ERROR` naming the argument, as it already is for other malformed positionals. MCP is unaffected because its schemas use strict `z.number()`. ## Change and scope `src/cli/options.ts` adds `cliIntegerArgument(min)`: a string that must match `^(?:0|[1-9][0-9]*)$`, piped into `z.coerce.number<string>().int().safe().min(min)`. A pipe keeps the schema exportable, which a transform would not. It replaces the coerced schemas at every CLI integer site: `cutter-command` `expectedGeneration`, `trace-web-module-imports` `scriptIndex`, the web source-location `scriptIndex`, `line` (min 1) and `column`, and the web network capture `--record` ordinals. Valid decimal values parse to the same numbers as before. Not included: MCP schemas, which already reject strings. ## Contract and boundary impact - Semantic owner and earliest changed stage: CLI argument parsing (`src/cli/*Commands.ts`, `src/cli/options.ts`). - Contribution scope fit and linked maintainer decision: ordinary fix within existing boundaries; not applicable. - CLI and MCP/tool-catalog contract: CLI rejects inputs it previously coerced to numbers; accepted values are unchanged. MCP unchanged. - 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): `cutter-command` no longer runs against session generation 0 when the generation argument is empty. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none committed; CI regenerates catalogs. - [x] Documentation or usability only; no target interaction. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/cli/cliIntegerArguments.test.ts` runs `cutter-command` and `trace-web-module-imports` with `""`, `" "`, `0x1`, `0b1`, `1e0`, `+1` and `1.0`, and expects exit 1 with a `VALIDATION_ERROR` on `expectedGeneration` / `scriptIndex`. - Base reproduction or other evidence: on `main` all 14 cases exit 0 (executed). - User-visible CLI/MCP output (if applicable): `{"code":"VALIDATION_ERROR","fieldErrors":[{"path":"expectedGeneration","message":"Expected a decimal integer",...}]}`. - Remaining proof gaps: `--record` is covered by the shared schema but has no dedicated case. ## Validation performed - `npm run test:prepare && npx vitest run --project cli-boundary` — 88 files, 498 passed, 2 skipped - `npx vitest run --project mcp-boundary` — 138 files, 776 passed, 5 skipped - `npx vitest run tests/acceptance/webModuleCli.test.ts` — 1 passed - `npx tsc --noEmit -p .` — no errors - `npx prettier --check` and `npx oxlint` on the changed files — clean apart from the existing `max-params` warning at `options.ts:69` --- <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:57 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

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

[PR #6143] [MERGED] fix(cancellation): preserve caller abort reasons through late finalization #7158

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

No description provided.

Rows
Columns