morluto/rea

Fork 0

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

[GH-ISSUE #3862] [Bug] rea doctor marks a Codex registration invalid when it has per-tool approval settings #1482

New issue

Closed

opened 2026-10-16 21:06:00 +00:00 by bsakaguchi · 3 comments

bsakaguchi commented

2026-10-16 21:06:00 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 15, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/3862

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

Setup, packaging, or upgrade

Observed behavior

rea doctor --client codex reports an otherwise aligned Codex registration as invalid (exit 1, empty command, advice to rerun setup) as soon as the registration contains a per-tool table such as [mcp_servers.rea.tools.binary_session] with approval_mode="approve". REA's own Codex verifier writes this exact setting shape, so the diagnostic is false.

The first run exits 0 with the registration aligned. After the per-tool table is appended, the second run exits 1 with state: "invalid", an empty command, and remediation Run rea setup to refresh this registration, then restart the client. The registration:codex scope check changes to ok: false.

Reproduction

Run from the root of a built source checkout. The helper isolates every client configuration directory in a disposable home. npm_command=exec and the npm user agent make REA treat the pinned npx launcher as its own package-runner launcher; neither invokes npm nor contacts a registry.

REA_CHECKOUT="$PWD"
trial="$(mktemp -d)"
mkdir -p "$trial/.codex"
cat > "$trial/.codex/config.toml" <<'TOML'
[mcp_servers.rea]
command="npx"
args=["-y", "rea-agents@6.5.0", "mcp"]
startup_timeout_sec=30
tool_timeout_sec=600
TOML
run_rea() {
  env -i PATH="$(dirname "$(command -v node)"):/usr/bin:/bin:/usr/sbin:/sbin" \
    HOME="$trial" XDG_CONFIG_HOME="$trial/.config" \
    CODEX_HOME="$trial/.codex" CLAUDE_CONFIG_DIR="$trial/.claude" \
    npm_config_prefix="$trial/.npm-global" \
    npm_command=exec npm_config_user_agent=npm/12.0.0 \
    node "$REA_CHECKOUT/scripts/rea.mjs" "$@"
}
cd "$trial"
run_rea doctor --client codex --json; echo "exit=$?"
cat >> "$trial/.codex/config.toml" <<'TOML'
[mcp_servers.rea.tools.binary_session]
approval_mode="approve"
TOML
run_rea doctor --client codex --json; echo "exit=$?"

Expected behavior

Both doctor runs report the Codex registration as aligned with exit 0. docs/installation.md "Check readiness for your task" describes doctor as checking one agent registration, and the appended table changes no launcher, version, environment, or timeout. REA itself configures Codex with mcp_servers.rea.tools.<name>.approval_mode="approve" at scripts/verify-agent-experience.mjs:495-497, so this is a supported Codex configuration shape.

Impact and scope

Codex users who set per-tool approval policy get a false readiness failure from doctor and, per source, lose automatic setup and update selection for that registration; no data is lost.

Workaround: available. Doctor did not modify the configuration in the reproduction, so users can keep the existing registration and disregard the diagnostic; running setup as remediation is not safe because it deletes the per-tool settings.

Reporter's impact estimate: P2 (supported).

REA version

6.5.0, built from source at main commit 034342ba28 (latest npm release is 6.5.0)

Runtime and platform

Node.js v26.10.0; macOS 27.0.1 arm64

Target and artifact context

Codex MCP client config.toml registration

Interface

CLI

Earliest failure stage

Setup or provider admission

Relevant output or evidence

Expected source: scripts/verify-agent-experience.mjs:495-497 (REA's own Codex per-tool approval setting) and docs/installation.md "Check readiness for your task"
Failure source: observed rea doctor --client codex --json output before and after appending the tools table; schema at src/application/ClientRegistrationStatus.ts:92
Evidence provenance: observed
Local verification: reproduced
Reproduction completeness: complete

Relevant fields from the two runs:

before: exit 0  "command":["npx","-y","rea-agents@6.5.0","mcp"],"package_reference":"rea-agents@6.5.0","state":"aligned","remediation":null
after:  exit 1  "command":[],"state":"invalid","remediation":"Run rea setup to refresh this registration, then restart the client."

Source-derived analysis: the shared registration schema declares tools: z.array(z.string()).optional() at src/application/ClientRegistrationStatus.ts:92, which matches another client's list-of-names field rather than Codex's per-tool table. Codex registrations go through the generic registrationSchema.parse at line 454, and the catch at lines 303-309 turns the schema rejection into an invalid status. Because setup defaults (src/application/SetupPlan.ts:111-120) and update maintenance (src/application/UpdateMaintenance.ts:65-70) admit only aligned or stale registrations, such registrations are probably also dropped from automatic setup and update selection; that consequence was not run.

Typed outcome or recovery expected

Doctor should report the Codex registration as aligned (exit 0) with its launcher command, ignoring or accepting Codex's per-tool tools table.

Restoration check:

On the same fixture after the tools table is appended, rea doctor --client codex --json currently exits 1 with state: "invalid" and an empty command; it should exit 0 with state: "aligned" and the original npx command, matching the run before the append. The first run without the table remains the control.

Analysis or suspected cause

Source-derived: src/application/ClientRegistrationStatus.ts:92 types tools as an array of strings; Codex registrations are parsed by the shared schema at line 454 and the catch at lines 303-309 maps the rejection to invalid. Setup defaults (src/application/SetupPlan.ts:111-120) and update maintenance (src/application/UpdateMaintenance.ts:65-70) admit only aligned or stale registrations.

Additional context

Following the doctor remediation does not preserve the setting: refreshing with rea setup --client codex deletes the per-tool table, which is reported separately. Closed #2016 mentions the verifier's per-tool approval configuration but not this parser failure. Closed #1163 concerns TOML comment loss, not rejection of a tools table. No open issue reports this failure.

This report came from AI-assisted source review with local reproduction where stated above; the evidence labels distinguish observed runs from source-only reasoning.

Originally created by @coygeek on GitHub (Oct 15, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/3862 ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area Setup, packaging, or upgrade ### Observed behavior `rea doctor --client codex` reports an otherwise aligned Codex registration as `invalid` (exit 1, empty command, advice to rerun setup) as soon as the registration contains a per-tool table such as `[mcp_servers.rea.tools.binary_session]` with `approval_mode="approve"`. REA's own Codex verifier writes this exact setting shape, so the diagnostic is false. The first run exits 0 with the registration `aligned`. After the per-tool table is appended, the second run exits 1 with `state: "invalid"`, an empty `command`, and remediation `Run rea setup to refresh this registration, then restart the client.` The `registration:codex` scope check changes to `ok: false`. ### Reproduction Run from the root of a built source checkout. The helper isolates every client configuration directory in a disposable home. `npm_command=exec` and the npm user agent make REA treat the pinned `npx` launcher as its own package-runner launcher; neither invokes npm nor contacts a registry. ```sh REA_CHECKOUT="$PWD" trial="$(mktemp -d)" mkdir -p "$trial/.codex" cat > "$trial/.codex/config.toml" <<'TOML' [mcp_servers.rea] command="npx" args=["-y", "rea-agents@6.5.0", "mcp"] startup_timeout_sec=30 tool_timeout_sec=600 TOML run_rea() { env -i PATH="$(dirname "$(command -v node)"):/usr/bin:/bin:/usr/sbin:/sbin" \ HOME="$trial" XDG_CONFIG_HOME="$trial/.config" \ CODEX_HOME="$trial/.codex" CLAUDE_CONFIG_DIR="$trial/.claude" \ npm_config_prefix="$trial/.npm-global" \ npm_command=exec npm_config_user_agent=npm/12.0.0 \ node "$REA_CHECKOUT/scripts/rea.mjs" "$@" } cd "$trial" run_rea doctor --client codex --json; echo "exit=$?" cat >> "$trial/.codex/config.toml" <<'TOML' [mcp_servers.rea.tools.binary_session] approval_mode="approve" TOML run_rea doctor --client codex --json; echo "exit=$?" ``` ### Expected behavior Both doctor runs report the Codex registration as `aligned` with exit 0. `docs/installation.md` "Check readiness for your task" describes doctor as checking one agent registration, and the appended table changes no launcher, version, environment, or timeout. REA itself configures Codex with `mcp_servers.rea.tools.<name>.approval_mode="approve"` at `scripts/verify-agent-experience.mjs:495-497`, so this is a supported Codex configuration shape. ### Impact and scope Codex users who set per-tool approval policy get a false readiness failure from doctor and, per source, lose automatic setup and update selection for that registration; no data is lost. Workaround: available. Doctor did not modify the configuration in the reproduction, so users can keep the existing registration and disregard the diagnostic; running setup as remediation is not safe because it deletes the per-tool settings. Reporter's impact estimate: P2 (supported). ### REA version 6.5.0, built from source at main commit 034342ba28c8e56c3746be92b78aa7299afa02bd (latest npm release is 6.5.0) ### Runtime and platform Node.js v26.10.0; macOS 27.0.1 arm64 ### Target and artifact context Codex MCP client config.toml registration ### Interface CLI ### Earliest failure stage Setup or provider admission ### Relevant output or evidence - Expected source: `scripts/verify-agent-experience.mjs:495-497` (REA's own Codex per-tool approval setting) and `docs/installation.md` "Check readiness for your task" - Failure source: observed `rea doctor --client codex --json` output before and after appending the tools table; schema at `src/application/ClientRegistrationStatus.ts:92` - Evidence provenance: observed - Local verification: reproduced - Reproduction completeness: complete Relevant fields from the two runs: ```text before: exit 0 "command":["npx","-y","rea-agents@6.5.0","mcp"],"package_reference":"rea-agents@6.5.0","state":"aligned","remediation":null after: exit 1 "command":[],"state":"invalid","remediation":"Run rea setup to refresh this registration, then restart the client." ``` Source-derived analysis: the shared registration schema declares `tools: z.array(z.string()).optional()` at `src/application/ClientRegistrationStatus.ts:92`, which matches another client's list-of-names field rather than Codex's per-tool table. Codex registrations go through the generic `registrationSchema.parse` at line 454, and the catch at lines 303-309 turns the schema rejection into an `invalid` status. Because setup defaults (`src/application/SetupPlan.ts:111-120`) and update maintenance (`src/application/UpdateMaintenance.ts:65-70`) admit only `aligned` or `stale` registrations, such registrations are probably also dropped from automatic setup and update selection; that consequence was not run. ### Typed outcome or recovery expected Doctor should report the Codex registration as aligned (exit 0) with its launcher command, ignoring or accepting Codex's per-tool tools table. Restoration check: On the same fixture after the tools table is appended, `rea doctor --client codex --json` currently exits 1 with `state: "invalid"` and an empty command; it should exit 0 with `state: "aligned"` and the original `npx` command, matching the run before the append. The first run without the table remains the control. ### Analysis or suspected cause Source-derived: src/application/ClientRegistrationStatus.ts:92 types tools as an array of strings; Codex registrations are parsed by the shared schema at line 454 and the catch at lines 303-309 maps the rejection to invalid. Setup defaults (src/application/SetupPlan.ts:111-120) and update maintenance (src/application/UpdateMaintenance.ts:65-70) admit only aligned or stale registrations. ### Additional context Following the doctor remediation does not preserve the setting: refreshing with `rea setup --client codex` deletes the per-tool table, which is reported separately. Closed #2016 mentions the verifier's per-tool approval configuration but not this parser failure. Closed #1163 concerns TOML comment loss, not rejection of a tools table. No open issue reports this failure. This report came from AI-assisted source review with local reproduction where stated above; the evidence labels distinguish observed runs from source-only reasoning.

bsakaguchi commented

2026-10-16 21:06:02 +00:00

Author

Owner

@yankikucuk commented on GitHub (Oct 15, 2026):

Working on this together with #3863, same root cause. Reproduced through rea doctor --client codex --json on a built checkout: aligned before the per-tool table, invalid with an empty command after it. The shared registration schema typed tools as a list of names; it now accepts Codex's table of per-tool settings as well, and the registration reads aligned with its launcher command. Draft PR follows shortly.

@yankikucuk commented on GitHub (Oct 15, 2026): Working on this together with #3863, same root cause. Reproduced through `rea doctor --client codex --json` on a built checkout: aligned before the per-tool table, invalid with an empty command after it. The shared registration schema typed `tools` as a list of names; it now accepts Codex's table of per-tool settings as well, and the registration reads aligned with its launcher command. Draft PR follows shortly.

bsakaguchi commented

2026-10-16 21:06:02 +00:00

Author

Owner

@SoGeRiEn commented on GitHub (Oct 15, 2026):

Fixed on fix/reliability-pack-20261015 at 12309d27. Doctor now accepts Codex per-tool approval tables.

@SoGeRiEn commented on GitHub (Oct 15, 2026): Fixed on `fix/reliability-pack-20261015` at 12309d27. Doctor now accepts Codex per-tool approval tables.

bsakaguchi referenced this issue

2026-10-17 21:11:51 +00:00

[PR #1482] [MERGED] feat(artifacts): inspect PE resource identities and payload ranges #2920

bsakaguchi closed this issue

2026-10-31 00:30:58 +00:00

bsakaguchi commented

2026-10-31 00:30:59 +00:00

Author

Owner

@coygeek commented on GitHub (Oct 24, 2026):

Checked the reported fix on main at d87e73f2db.

Policy validation is client-specific: Codex tools is a map of per-tool settings including approval_mode, while Copilot keeps its array dialect. Generic launcher normalization no longer rejects the table. A production-source probe admits the reported binary_session approval table and retains npx args.

Implementation: ClientRegistration.ts, ClientRegistration.ts.

The relevant regression checks in cliSetup.test.ts passed in the focused test run on this revision. This closure is based on the current source and those checks; provider fixtures do not establish a real-engine run. Closing as completed on main.

@coygeek commented on GitHub (Oct 24, 2026): Checked the reported fix on `main` at d87e73f2db50be72d33a4b3c7199af35518b441c. Policy validation is client-specific: Codex tools is a map of per-tool settings including approval_mode, while Copilot keeps its array dialect. Generic launcher normalization no longer rejects the table. A production-source probe admits the reported binary_session approval table and retains npx args. Implementation: [ClientRegistration.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/application/ClientRegistration.ts#L59), [ClientRegistration.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/application/ClientRegistration.ts#L111). The relevant regression checks in [cliSetup.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/tests/boundary/cli/cliSetup.test.ts) passed in the focused test run on this revision. This closure is based on the current source and those checks; provider fixtures do not establish a real-engine run. Closing as completed on main.

bsakaguchi referenced this issue

2026-10-31 00:54:38 +00:00

[PR #1678] [MERGED] fix(native): bound PE resource RVAs by section virtual size #4747

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

No description provided.

Rows
Columns