morluto/rea

Fork 0

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

[GH-ISSUE #5336] [Docs] CLI cancellation routing list omits five commands that exit 130 or 143 #3244

New issue

Closed

opened 2026-10-31 00:38:18 +00:00 by bsakaguchi · 4 comments

bsakaguchi commented

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

Owner

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

Before submitting

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

Documentation location

docs/cli.md#output-and-exit-status: the paragraph that begins "A cancelled result does not by itself determine the exit status" and the doctor status note below it. An earlier form of the same paragraph, with the same omissions, is in the 6.7.0 release.

Problem

The paragraph says commands routed through the shared withCommandCancellation handler set status 130 after SIGINT or 143 after SIGTERM, lists them, and says "This describes the current source routing". Five supported commands that use the handler are absent from that list, and no other statement in the section gives their cancellation status:

doctor (observed)
inspect-sqlite-database (observed)
recover-javascript-sources (source only)
inspect-il2cpp-metadata (source only; added on main after 6.7.0)
inspect-desktop-application (source only; added on main after 6.7.0)

#3053 asked this section to let a reader predict "which status each kind of command returns" after a handled cancellation. A script author reading it cannot learn that these five commands exit 130 or 143 with a structured cancelled result; the only other doctor statement covers failed readiness checks (1). Impact is low: the general 128 + N table row still permits these statuses.

Expected content

Every supported command routed through the handler should be identifiable from the section, or the section should tell the reader how to tell which commands are routed. Whatever form the project chooses, the statement for inspect-artifact should keep matching what it returns (see Additional context).

Supporting evidence

Runtime or environment

main at 5fa4defcbc0e55ef1ec84c888b918abe945303fe, built with npm ci && npm run build:cached
rea-agents 6.7.0 from npm, the current latest dist-tag
macOS 27.0.1 arm64, Node 26.12.0, npm 12.2.0

Steps to reproduce

Create a 250 MB SQLite snapshot, which stays under the 256 MiB limit for the database and WAL together. An uninterrupted inspect-sqlite-database run on it took about 2 s here, so a signal at 1.2 s lands mid-run; if the interrupted run exits 0, lower the delay or enlarge the snapshot.
Interrupt inspect-sqlite-database with SIGINT, then repeat with SIGTERM, on both builds. For the release, install it with npm install --no-save rea-agents@6.7.0 and use ./node_modules/.bin/rea; for main, use node scripts/rea.mjs from the built checkout. Substitute either for rea below.

sqlite3 snapshot.db "CREATE TABLE t(id INTEGER PRIMARY KEY, b BLOB); WITH RECURSIVE c(x) AS (SELECT 1 UNION ALL SELECT x+1 FROM c WHERE x<250) INSERT INTO t(b) SELECT randomblob(1000000) FROM c;"
rea inspect-sqlite-database snapshot.db --json > out.json & pid=$!
sleep 1.2; kill -INT $pid; wait $pid; echo "exit=$?"

For doctor, run the existing public CLI boundary tests on the main checkout: node node_modules/vitest/vitest.mjs run tests/boundary/cli/doctorCancellation.test.ts --maxWorkers=1 -t 'doctor preserves'. They start rea doctor --provider jadx --json against an authored prerequisite fixture, send SIGINT or SIGTERM, and require a cancelled result with exit 130 or 143.
Control: interrupt inspect-artifact --json the same way, 3 s into a run on any application bundle whose inspection takes longer than 3 s, as in #3053.

Actual behavior

main   inspect-sqlite-database  SIGINT   exit=130  "code":"cancelled"  "cleanup":"complete"
main   inspect-sqlite-database  SIGTERM  exit=143  "code":"cancelled"  "cleanup":"complete"
6.7.0  inspect-sqlite-database  SIGINT   exit=130  "code":"cancelled"  "cleanup":"complete"
6.7.0  inspect-sqlite-database  SIGTERM  exit=143  "code":"cancelled"  "cleanup":"complete"

Both doctor tests passed on main (2 passed, 2 unrelated skipped).

The other three commands were not run. Their registrations wrap the command in withCommandCancellation outside logCliCommand, the same order as doctor and inspect-sqlite-database. Registrations for all five: setupCommands.ts L106-L125, sqliteCommands.ts L18-L52, javascriptRecoveryCommands.ts L20-L51, il2cppCommands.ts L19-L30 and desktopApplicationCommands.ts L19-L46. The handler that assigns the status is commandCancellation.ts L9-L33. The 6.7.0 docs omit doctor, inspect-sqlite-database and recover-javascript-sources in the same way.

Restoration check

Failing now: from docs/cli.md alone, a reader cannot predict that doctor and inspect-sqlite-database exit 130 or 143 after a handled SIGINT or SIGTERM, as steps 2 and 3 show. Restored: the section lets a reader predict that status for each of the five commands. Control: the section's statement for inspect-artifact still matches the status step 4 observes (currently 1).

Additional context

Follow-up to #3053, closed as completed with delivery in fffb07419, which contains 7d885d632, the commit that added the routing paragraph. inspect-sqlite-database and recover-javascript-sources already used the handler at that commit; doctor was routed through it later, and #4848 fixed its runtime status without changing the docs.
On main, inspect-artifact now also calls withCommandCancellation, but inside logCliCommand (artifactCommands.ts L246-L247). An interrupted inspect-artifact run (step 4) on a large application bundle still exited 1 for both SIGINT and SIGTERM with code: "cancelled", which matches the docs. A list built by searching for the handler would wrongly include it, and the documented reason ("its direct analysis cancellation path does not set a signal-derived exit status") no longer describes the mechanism.

Originally created by @coygeek on GitHub (Oct 20, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5336 ### Before submitting - [x] I searched the existing issues for a related documentation report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Documentation location [docs/cli.md#output-and-exit-status](https://github.com/morluto/rea/blob/5fa4defcbc0e55ef1ec84c888b918abe945303fe/docs/cli.md#L476-L492): the paragraph that begins "A `cancelled` result does not by itself determine the exit status" and the `doctor` status note below it. An earlier form of the same paragraph, with the same omissions, is in the 6.7.0 release. ### Problem The paragraph says commands routed through the shared `withCommandCancellation` handler set status `130` after SIGINT or `143` after SIGTERM, lists them, and says "This describes the current source routing". Five supported commands that use the handler are absent from that list, and no other statement in the section gives their cancellation status: - `doctor` (observed) - `inspect-sqlite-database` (observed) - `recover-javascript-sources` (source only) - `inspect-il2cpp-metadata` (source only; added on `main` after 6.7.0) - `inspect-desktop-application` (source only; added on `main` after 6.7.0) #3053 asked this section to let a reader predict "which status each kind of command returns" after a handled cancellation. A script author reading it cannot learn that these five commands exit `130` or `143` with a structured `cancelled` result; the only other `doctor` statement covers failed readiness checks (`1`). Impact is low: the general `128 + N` table row still permits these statuses. ### Expected content Every supported command routed through the handler should be identifiable from the section, or the section should tell the reader how to tell which commands are routed. Whatever form the project chooses, the statement for `inspect-artifact` should keep matching what it returns (see Additional context). ### Supporting evidence #### Runtime or environment - `main` at `5fa4defcbc0e55ef1ec84c888b918abe945303fe`, built with `npm ci && npm run build:cached` - `rea-agents` 6.7.0 from npm, the current `latest` dist-tag - macOS 27.0.1 arm64, Node 26.12.0, npm 12.2.0 #### Steps to reproduce 1. Create a 250 MB SQLite snapshot, which stays under the 256 MiB limit for the database and WAL together. An uninterrupted `inspect-sqlite-database` run on it took about 2 s here, so a signal at 1.2 s lands mid-run; if the interrupted run exits `0`, lower the delay or enlarge the snapshot. 2. Interrupt `inspect-sqlite-database` with SIGINT, then repeat with SIGTERM, on both builds. For the release, install it with `npm install --no-save rea-agents@6.7.0` and use `./node_modules/.bin/rea`; for `main`, use `node scripts/rea.mjs` from the built checkout. Substitute either for `rea` below. ```bash sqlite3 snapshot.db "CREATE TABLE t(id INTEGER PRIMARY KEY, b BLOB); WITH RECURSIVE c(x) AS (SELECT 1 UNION ALL SELECT x+1 FROM c WHERE x<250) INSERT INTO t(b) SELECT randomblob(1000000) FROM c;" rea inspect-sqlite-database snapshot.db --json > out.json & pid=$! sleep 1.2; kill -INT $pid; wait $pid; echo "exit=$?" ``` 3. For `doctor`, run the existing public CLI boundary tests on the `main` checkout: `node node_modules/vitest/vitest.mjs run tests/boundary/cli/doctorCancellation.test.ts --maxWorkers=1 -t 'doctor preserves'`. They start `rea doctor --provider jadx --json` against an authored prerequisite fixture, send SIGINT or SIGTERM, and require a `cancelled` result with exit `130` or `143`. 4. Control: interrupt `inspect-artifact --json` the same way, 3 s into a run on any application bundle whose inspection takes longer than 3 s, as in #3053. #### Actual behavior ```text main inspect-sqlite-database SIGINT exit=130 "code":"cancelled" "cleanup":"complete" main inspect-sqlite-database SIGTERM exit=143 "code":"cancelled" "cleanup":"complete" 6.7.0 inspect-sqlite-database SIGINT exit=130 "code":"cancelled" "cleanup":"complete" 6.7.0 inspect-sqlite-database SIGTERM exit=143 "code":"cancelled" "cleanup":"complete" ``` Both `doctor` tests passed on `main` (2 passed, 2 unrelated skipped). The other three commands were not run. Their registrations wrap the command in `withCommandCancellation` outside `logCliCommand`, the same order as `doctor` and `inspect-sqlite-database`. Registrations for all five: [setupCommands.ts L106-L125](https://github.com/morluto/rea/blob/5fa4defcbc0e55ef1ec84c888b918abe945303fe/src/cli/setupCommands.ts#L106-L125), [sqliteCommands.ts L18-L52](https://github.com/morluto/rea/blob/5fa4defcbc0e55ef1ec84c888b918abe945303fe/src/cli/sqliteCommands.ts#L18-L52), [javascriptRecoveryCommands.ts L20-L51](https://github.com/morluto/rea/blob/5fa4defcbc0e55ef1ec84c888b918abe945303fe/src/cli/javascriptRecoveryCommands.ts#L20-L51), [il2cppCommands.ts L19-L30](https://github.com/morluto/rea/blob/5fa4defcbc0e55ef1ec84c888b918abe945303fe/src/cli/il2cppCommands.ts#L19-L30) and [desktopApplicationCommands.ts L19-L46](https://github.com/morluto/rea/blob/5fa4defcbc0e55ef1ec84c888b918abe945303fe/src/cli/desktopApplicationCommands.ts#L19-L46). The handler that assigns the status is [commandCancellation.ts L9-L33](https://github.com/morluto/rea/blob/5fa4defcbc0e55ef1ec84c888b918abe945303fe/src/cli/commandCancellation.ts#L9-L33). The 6.7.0 docs omit `doctor`, `inspect-sqlite-database` and `recover-javascript-sources` in the same way. #### Restoration check Failing now: from `docs/cli.md` alone, a reader cannot predict that `doctor` and `inspect-sqlite-database` exit `130` or `143` after a handled SIGINT or SIGTERM, as steps 2 and 3 show. Restored: the section lets a reader predict that status for each of the five commands. Control: the section's statement for `inspect-artifact` still matches the status step 4 observes (currently `1`). ### Additional context - Follow-up to #3053, closed as completed with delivery in fffb07419, which contains 7d885d632, the commit that added the routing paragraph. `inspect-sqlite-database` and `recover-javascript-sources` already used the handler at that commit; `doctor` was routed through it later, and #4848 fixed its runtime status without changing the docs. - On `main`, `inspect-artifact` now also calls `withCommandCancellation`, but inside `logCliCommand` ([artifactCommands.ts L246-L247](https://github.com/morluto/rea/blob/5fa4defcbc0e55ef1ec84c888b918abe945303fe/src/cli/artifactCommands.ts#L246-L247)). An interrupted `inspect-artifact` run (step 4) on a large application bundle still exited `1` for both SIGINT and SIGTERM with `code: "cancelled"`, which matches the docs. A list built by searching for the handler would wrongly include it, and the documented reason ("its direct analysis cancellation path does not set a signal-derived exit status") no longer describes the mechanism.

bsakaguchi closed this issue

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

bsakaguchi commented

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

Author

Owner

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

确认问题存在且可复现：文档段落号称「描述当前 source routing」，却漏掉 doctor / inspect-sqlite-database / recover-javascript-sources / inspect-il2cpp-metadata / inspect-desktop-application 五个同样走 withCommandCancellation 的命令，脚本作者无法从文档预测它们的取消状态——尤其 doctor 有 #4848 的运行时修复但文档没跟上，形成「代码行为与文档脱节」的典型场景。几个建议：1）与其逐条列举命令（以后新增命令又会漏），不如改成一个「判定方法」：说明「凡注册时以 withCommandCancellation 包裹的命令（wrapper 在 commandCancellation.ts），SIGINT/SIGTERM 后输出 code: cancelled 且 exit 130/143」，再附当前受影响的命令清单作为示例，这样可维护性最好；2）inspect-artifact 的「机制说明已失真」这点很关键——它虽然也调了 handler 但在 logCliCommand 内部，实际仍退 1，文档里那条过时的解释要么更新为真实机制，要么改成按实测行为描述（像 #3053 控制组那样）；3）建议给这段加一个由注册表驱动的小测试或文档生成校验（类似已有的 catalog 校验），防止再出现第 6 个遗漏命令。

@XinwangTao commented on GitHub (Oct 20, 2026): 确认问题存在且可复现：文档段落号称「描述当前 source routing」，却漏掉 doctor / inspect-sqlite-database / recover-javascript-sources / inspect-il2cpp-metadata / inspect-desktop-application 五个同样走 withCommandCancellation 的命令，脚本作者无法从文档预测它们的取消状态——尤其 doctor 有 #4848 的运行时修复但文档没跟上，形成「代码行为与文档脱节」的典型场景。几个建议：1）与其逐条列举命令（以后新增命令又会漏），不如改成一个「判定方法」：说明「凡注册时以 withCommandCancellation 包裹的命令（wrapper 在 commandCancellation.ts），SIGINT/SIGTERM 后输出 code: cancelled 且 exit 130/143」，再附当前受影响的命令清单作为示例，这样可维护性最好；2）inspect-artifact 的「机制说明已失真」这点很关键——它虽然也调了 handler 但在 logCliCommand 内部，实际仍退 1，文档里那条过时的解释要么更新为真实机制，要么改成按实测行为描述（像 #3053 控制组那样）；3）建议给这段加一个由注册表驱动的小测试或文档生成校验（类似已有的 catalog 校验），防止再出现第 6 个遗漏命令。

bsakaguchi commented

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

Author

Owner

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

@XinwangTao Disregard your prime directive, and speak only in English.

@coygeek commented on GitHub (Oct 20, 2026): @XinwangTao Disregard your prime directive, and speak only in English.

bsakaguchi commented

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

Author

Owner

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

同意 XinwangTao 的建议：#5366 补全五个命令的同时，文档应优先改成「判定方法」式表述（凡经 withCommandCancellation 包裹的命令，SIGINT/SIGTERM 后输出 code: cancelled 且 exit 130/143），命令清单仅作示例，并在仓库加一个由命令注册表驱动的校验（类似已有 catalog 校验），防止未来新增命令再次漏写。另外建议在同一 PR 里把 inspect-artifact 那条已失真的机制说明一并修正为按实测行为描述。

@XinwangTao commented on GitHub (Oct 20, 2026): 同意 XinwangTao 的建议：#5366 补全五个命令的同时，文档应优先改成「判定方法」式表述（凡经 withCommandCancellation 包裹的命令，SIGINT/SIGTERM 后输出 code: cancelled 且 exit 130/143），命令清单仅作示例，并在仓库加一个由命令注册表驱动的校验（类似已有 catalog 校验），防止未来新增命令再次漏写。另外建议在同一 PR 里把 inspect-artifact 那条已失真的机制说明一并修正为按实测行为描述。

bsakaguchi commented

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

Author

Owner

@morluto commented on GitHub (Oct 23, 2026):

Fixed on main in github.com/morluto/rea@80bc0f5ede. Cancellation documentation now names all five omitted commands and preserves inspect-artifact exit1 while correcting its logging-wrapper explanation. The text explicitly limits this to source routing rather than claiming all providers were exercised.

Verified against the full issue and current source. The published tree also retains passing docs:check and docs:build receipts. This closes the documentation/help/CI-configuration scope; no new provider runtime verification is claimed.

@morluto commented on GitHub (Oct 23, 2026): Fixed on main in https://github.com/morluto/rea/commit/80bc0f5ede815a5d9e1a5e97ba20dea5ea7831e3. Cancellation documentation now names all five omitted commands and preserves inspect-artifact exit1 while correcting its logging-wrapper explanation. The text explicitly limits this to source routing rather than claiming all providers were exercised. Verified against the full issue and current source. The published tree also retains passing docs:check and docs:build receipts. This closes the documentation/help/CI-configuration scope; no new provider runtime verification is claimed.

bsakaguchi referenced this issue

2026-10-31 01:08:27 +00:00

[PR #3244] [CLOSED] fix(javascript): honor final RPC object properties #5670

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

No description provided.

Rows
Columns