[PR #4937] [MERGED] fix(setup): read signed 64-bit TOML integers losslessly #6681

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4937
Author: @kaoru0822-kitauji
Created: 10/18/2026
Status: ✅ Merged
Merged: 10/18/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/codex-toml-int64


📝 Commits (1)

  • 3cbb879 fix(setup): read signed 64-bit TOML integers losslessly

📊 Changes

4 files changed (+163 additions, -7 deletions)

View changed files

📝 src/application/ClientConfigurationDocument.ts (+32 -3)
📝 src/application/ClientRegistrationStatus.ts (+10 -3)
📝 src/application/SetupClientConfiguration.ts (+3 -1)
➕ tests/acceptance/setup/tomlConfigurationIntegerRangeCli.test.ts (+118 -0)

📄 Description

Summary

Fixes #4917

Codex and Grok Build setup, doctor and uninstall rejected valid TOML files whose integers were outside JavaScript's safe Number range, such as [history] max_bytes = 9223372036854775807. They reported the file as malformed (integer value cannot be represented losslessly) even though Codex itself accepts it. With this change those integers are read exactly and preserved. Values outside signed Int64 are still rejected before any write.

Problem and expected behavior

  • Trigger: any Codex/Grok config.toml integer with magnitude above 2^53 - 1, for example 9007199254740993 or 9223372036854775807.
  • Violated invariant: TOML 1.0 requires lossless signed 64-bit integers, and setup must preserve unrelated configuration. Instead, ClientConfigurationDocument called smol-toml parse with its default options, and that reader throws on any integer beyond the safe Number range.
  • Expected: Int64 values parse, setup adds REA's entry while keeping them byte-for-byte, and 9223372036854775808 / -9223372036854775809 fail before writes.

Change and scope

  • ClientConfigurationDocument.ts now owns a single parseClientToml reader. It calls parse(text, { integersAsBigInt: "asNeeded" }), so safe integers stay number and larger ones become exact bigint. It then walks the parsed tree iteratively, without argument spreads, and throws a SyntaxError for any bigint outside [-2^63, 2^63 - 1]. smol-toml does not enforce that bound itself.
  • Every TOML parse in this owner now uses that reader: the document read, inline string decoding, and the tomlTextHolds check that compares an edited document against the original values. Without that last one, a preserving edit could fall back to a whole-document rewrite.
  • The inline-table writer (tomlInlineValue) writes bigint values exactly. Before, it wrote them as "".
  • Consumers that see the new value type: setup's positiveTimeout keeps a positive bigint startup_timeout_sec/tool_timeout_sec instead of replacing it with the default. Doctor's registration schema accepts a positive bigint for those two Codex keys, and the timeout advisory compares via Number(...). That comparison is safe because any bigint value is above 2^53 and so above the allowance.
  • Not included: Kimi timeouts are JSON and never bigint, so they are unchanged. There is no dependency, schema, registration identity, transaction or backup change.
  • Contribution scope: ordinary fix within existing boundaries. A maintainer scope decision is not applicable.

Contract and boundary impact

  • Semantic owner and earliest changed stage: src/application/ClientConfigurationDocument.ts TOML parse, the boundary where client config text is read.
  • Contribution scope fit and linked maintainer decision: in scope for existing setup behavior. Not applicable.
  • CLI and MCP/tool-catalog contract: unchanged. Setup now returns planned/ready instead of needs_human for these valid files. The out-of-range diagnostic now reads integer N is outside the signed 64-bit range.
  • Provider, bridge, target-format, or platform compatibility: Codex and Grok Build TOML clients. Other formats are unchanged.
  • Evidence, artifact, provenance, or reconstruction contract: none.
  • Target selection, authority, upstream behavior, and cleanup details: not applicable.
  • Generated metadata, package, or installation impact: none.

Expected effects:

  • 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. (Existing setup/uninstall client config writes. The writes themselves are unchanged.)
  • 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.

Evidence and regression coverage

  • Tests added: tests/acceptance/setup/tomlConfigurationIntegerRangeCli.test.ts runs the real CLI:
    • Int64 max, Int64 min and an underscored hex value under [history]. Covers the setup dry-run (planned, file unchanged), apply (exact backup, original prefix kept), a repeated apply with no actions and identical bytes, and uninstall restoring the original bytes.
    • An inline mcp_servers table holding a foreign 9007199254740993 and an existing REA entry with tool_timeout_sec = 9223372036854775807. The comment and both exact integers are kept, and REA's command is replaced.
    • 9223372036854775808 and -9223372036854775809 produce needs_human with the Int64 diagnostic, exit 1, and leave the file unchanged.
  • Base reproduction: with this PR's three source files reverted to origin/main and the new test kept, all 4 tests failed. The valid cases failed with needs_human. The out-of-range cases still failed closed on main, but with the old "losslessly" diagnostic rather than the Int64 range message.
  • Remaining proof gaps: no native Codex or Grok binary was run here. Doctor's bigint timeout path is covered by type checking and the existing doctor boundary tests, but no new doctor test targets it.

For evidence-bearing changes: not applicable (setup configuration only).

Validation performed

  • npm run test:focused -- tests/acceptance/setup/tomlConfigurationIntegerRangeCli.test.ts tests/acceptance/setup/tomlConfigurationCapacityCli.test.ts src/application/ClientConfigurationDocument.capacity.test.ts src/application/ClientConfigurationDocument.shape.test.ts tests/boundary/filesystem/clientRegistrationStatus.test.ts tests/boundary/filesystem/clientLifecycle.test.ts: 6 files, 99 tests passed (includes the build).
  • The same new test file run against the origin/main sources: 4 failed (expected).
  • npm run check:fast (typecheck + lint): passed.
  • npx oxfmt --check on the changed files: passed. npx oxlint on the changed files: no errors, only existing size/complexity warnings.

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable. Tested on macOS arm64 only.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: out-of-range input still fails before any write. There is no new I/O.

Author readiness

Human owner:

  • 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.

🤖 Generated with Claude Code


🔄 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/4937 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/18/2026 **Status:** ✅ Merged **Merged:** 10/18/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/codex-toml-int64` --- ### 📝 Commits (1) - [`3cbb879`](https://github.com/morluto/rea/commit/3cbb8793ff9352b33cfc7f81177f250a866e7274) fix(setup): read signed 64-bit TOML integers losslessly ### 📊 Changes **4 files changed** (+163 additions, -7 deletions) <details> <summary>View changed files</summary> 📝 `src/application/ClientConfigurationDocument.ts` (+32 -3) 📝 `src/application/ClientRegistrationStatus.ts` (+10 -3) 📝 `src/application/SetupClientConfiguration.ts` (+3 -1) ➕ `tests/acceptance/setup/tomlConfigurationIntegerRangeCli.test.ts` (+118 -0) </details> ### 📄 Description ## Summary Fixes #4917 Codex and Grok Build setup, doctor and uninstall rejected valid TOML files whose integers were outside JavaScript's safe Number range, such as `[history] max_bytes = 9223372036854775807`. They reported the file as malformed (`integer value cannot be represented losslessly`) even though Codex itself accepts it. With this change those integers are read exactly and preserved. Values outside signed Int64 are still rejected before any write. ## Problem and expected behavior - Trigger: any Codex/Grok `config.toml` integer with magnitude above 2^53 - 1, for example `9007199254740993` or `9223372036854775807`. - Violated invariant: TOML 1.0 requires lossless signed 64-bit integers, and setup must preserve unrelated configuration. Instead, `ClientConfigurationDocument` called `smol-toml` `parse` with its default options, and that reader throws on any integer beyond the safe Number range. - Expected: Int64 values parse, setup adds REA's entry while keeping them byte-for-byte, and `9223372036854775808` / `-9223372036854775809` fail before writes. ## Change and scope - `ClientConfigurationDocument.ts` now owns a single `parseClientToml` reader. It calls `parse(text, { integersAsBigInt: "asNeeded" })`, so safe integers stay `number` and larger ones become exact `bigint`. It then walks the parsed tree iteratively, without argument spreads, and throws a `SyntaxError` for any bigint outside `[-2^63, 2^63 - 1]`. smol-toml does not enforce that bound itself. - Every TOML parse in this owner now uses that reader: the document read, inline string decoding, and the `tomlTextHolds` check that compares an edited document against the original values. Without that last one, a preserving edit could fall back to a whole-document rewrite. - The inline-table writer (`tomlInlineValue`) writes `bigint` values exactly. Before, it wrote them as `""`. - Consumers that see the new value type: setup's `positiveTimeout` keeps a positive bigint `startup_timeout_sec`/`tool_timeout_sec` instead of replacing it with the default. Doctor's registration schema accepts a positive bigint for those two Codex keys, and the timeout advisory compares via `Number(...)`. That comparison is safe because any bigint value is above 2^53 and so above the allowance. - Not included: Kimi timeouts are JSON and never bigint, so they are unchanged. There is no dependency, schema, registration identity, transaction or backup change. - Contribution scope: ordinary fix within existing boundaries. A maintainer scope decision is not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `src/application/ClientConfigurationDocument.ts` TOML parse, the boundary where client config text is read. - Contribution scope fit and linked maintainer decision: in scope for existing setup behavior. Not applicable. - CLI and MCP/tool-catalog contract: unchanged. Setup now returns `planned`/`ready` instead of `needs_human` for these valid files. The out-of-range diagnostic now reads `integer N is outside the signed 64-bit range`. - Provider, bridge, target-format, or platform compatibility: Codex and Grok Build TOML clients. Other formats are unchanged. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. Expected effects: - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [x] Change process memory, registers, files, or configuration. (Existing setup/uninstall client config writes. The writes themselves are unchanged.) - [ ] 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. ## Evidence and regression coverage - Tests added: `tests/acceptance/setup/tomlConfigurationIntegerRangeCli.test.ts` runs the real CLI: - Int64 max, Int64 min and an underscored hex value under `[history]`. Covers the setup dry-run (`planned`, file unchanged), apply (exact backup, original prefix kept), a repeated apply with no actions and identical bytes, and uninstall restoring the original bytes. - An inline `mcp_servers` table holding a foreign `9007199254740993` and an existing REA entry with `tool_timeout_sec = 9223372036854775807`. The comment and both exact integers are kept, and REA's command is replaced. - `9223372036854775808` and `-9223372036854775809` produce `needs_human` with the Int64 diagnostic, exit 1, and leave the file unchanged. - Base reproduction: with this PR's three source files reverted to `origin/main` and the new test kept, all 4 tests failed. The valid cases failed with `needs_human`. The out-of-range cases still failed closed on main, but with the old "losslessly" diagnostic rather than the Int64 range message. - Remaining proof gaps: no native Codex or Grok binary was run here. Doctor's bigint timeout path is covered by type checking and the existing doctor boundary tests, but no new doctor test targets it. For evidence-bearing changes: not applicable (setup configuration only). ## Validation performed - `npm run test:focused -- tests/acceptance/setup/tomlConfigurationIntegerRangeCli.test.ts tests/acceptance/setup/tomlConfigurationCapacityCli.test.ts src/application/ClientConfigurationDocument.capacity.test.ts src/application/ClientConfigurationDocument.shape.test.ts tests/boundary/filesystem/clientRegistrationStatus.test.ts tests/boundary/filesystem/clientLifecycle.test.ts`: 6 files, 99 tests passed (includes the build). - The same new test file run against the `origin/main` sources: 4 failed (expected). - `npm run check:fast` (typecheck + lint): passed. - `npx oxfmt --check` on the changed files: passed. `npx oxlint` on the changed files: no errors, only existing size/complexity warnings. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. Tested on macOS arm64 only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: out-of-range input still fails before any write. There is no new I/O. ## Author readiness Human owner: - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:42:16 +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#6681
No description provided.