morluto/rea

Fork 0

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

[PR #6903] [MERGED] test(cli): repair Markdown capacity recovery coverage #7435

New issue

Closed

opened 2026-11-01 21:47:51 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:47:51 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6903
Author: @gensokyo-dev
Created: 10/23/2026
Status: ✅ Merged
Merged: 10/23/2026
Merged by: @gensokyo-dev

Base: main ← Head: test/cli-markdown-capacity

📝 Commits (2)

c4f9325 test(cli): repair Markdown capacity recovery coverage
81ff238 test(cli): assert the full Markdown recovery envelope

📊 Changes

1 file changed (+60 additions, -11 deletions)

View changed files

📝 tests/acceptance/cliFormattingCapacity.test.ts (+60 -11)

📄 Description

Problem and outcome

The CLI capacity regression injects failures only into Incur, but REA now renders Markdown itself. Both Markdown cases fail on main@3787ec5f without ever reaching the expected fault. This directly repairs observed CI; no separate issue was opened.

Inject a single string-materialization failure while the real Markdown formatter converts the operation or error-code scalar. The hook runs only in the fixture's CLI child and restores the original String before recovery. It follows the actual rendering path without matching generated bundle names or compiled function text. Keep Incur injection for its formats and the SIGPIPE scenario.

Give scenarios explicit format/human fields: format selection, hook selection and YAML assertions no longer interpret display names. Add full Markdown and Markdown token-window cases for both successful and failed operations. Product runtime code is unchanged.

Validation performed

Executed on Linux x64 with Node 24.22.1 / npm 11.16.0; the compiled public CLI reads a real 4 KiB managed PE fixture or a missing target:

Baseline npm run test:focused -- tests/acceptance/cliFormattingCapacity.test.ts — 2 Markdown failures, 17 passes.
Real CLI conversion probe — both operation/error-code conversions occur inside formatMarkdown called by formatCliOutput.
Controlled Incur-only hook — both original Markdown cases fail again; source restored before final checks.
npm run test:focused -- tests/acceptance/cliFormattingCapacity.test.ts tests/boundary/cli/formattingRecovery.test.ts — 31 passed at c4f9325, including SIGPIPE. 49.55 s elapsed, 58.91 s user+system CPU, 847.1 MiB sampled process-family RSS peak.
Final npm run check after the review follow-up — passed (typecheck, lint, formatting, Knip, metadata and architecture guards). 21.58 s elapsed, 39.59 s CPU, 2829.1 MiB sampled family RSS peak.
git diff --check — passed.

Final runs used two CPUs (12/15), elapsed guards and an 8 GiB sampled-memory stop. The build cache was warm; CLI children and fixture workspaces were fresh. Summed descendant RSS was sampled every 100 ms; shared pages may double-count and short-lived/reparented children may be missed. These are validation costs, not a speedup comparison; baseline CPU placement and workload differ. Agent-token cost was not measured. Windows/macOS and real analysis engines were not run locally; this change is confined to test fixtures.

Review follow-up

Greptile correctly identified that the new full Markdown cases only checked error text, so they could miss a dropped envelope. They now require root ok: false, resource_constraint under error.code, and the command row inside meta. A controlled removal of --full-output makes both full Markdown cases fail on the envelope assertion; the source was restored afterward. All six Markdown cases were then rechecked with node scripts/run-exact-tests.mjs run tests/acceptance/cliFormattingCapacity.test.ts --testNamePattern=Markdown: 6 passed, 17 deliberately excluded, 14.16 s elapsed / 16.69 s CPU / 696.5 MiB sampled family RSS peak. The other cases and production code are unchanged; no directly related cleanup remains.

Current-head CI and review

Final CI tests 81ff238aa as merge ed245f2 against main@34563fcc9. The changed acceptance file passes all 23 tests in CI, including the new envelope assertions. Static checks, package build, title and readiness pass. Codex and Greptile both completed review of the final commit with no new findings; the earlier Greptile finding is fixed and resolved.

CI remains not green because of independently demonstrated baseline failures:

Source CI has 11 failing cases. The same four source files in a clean detached main@34563fcc9 worktree reproduce exactly those 11 failures (155 other cases pass); failure names were compared as sets. Command: npm run test:local -- src/application/frida/FridaInstrumentationService.test.ts src/domain/javascript/javascriptStaticAnalysis.mapEndpoints.test.ts src/domain/javascript/javascriptStaticAnalysisReturnedInvocation.test.ts src/server/BundleResourcesMcp.test.ts.
Built shard 2 has three setup environment-validation failures and the existing CDP resource-completeness failure. All four also occur in main's full qualification. The full main qualification uses coverage; the PR feedback lane does not.
PR fail-fast cancels the other seven shards, so their complete coverage is unavailable. No gate, assertion or retry policy was relaxed. The owner explicitly authorized proceeding past demonstrated main failures.

The earlier PR run was cancelled to release runners while addressing the review finding; the linked run is the final commit's result.

Post-merge verification

Merged as bded9c628. Main advanced by five other commits while CI ran, so the complete merged tree differs from the CI snapshot; both changed test files and the CLI formatter/output sources match the tested versions. A fresh detached worktree at the actual merged commit installed the pinned dependencies and rebuilt/prepared the runtime, then ran npm run test:focused -- src/ida/IdaNativeProtocol.test.ts tests/acceptance/cliFormattingCapacity.test.ts tests/boundary/cli/formattingRecovery.test.ts: 54 passed. Including preparation: 68.48 s elapsed / 91.30 s CPU / 1364.1 MiB sampled family RSS peak. npm run check also passed on that merged tree: 39.43 s / 72.54 s CPU / 2527.2 MiB peak. The same two-CPU and sampled-memory guards applied. These scoped checks do not replace the unavailable cancelled-shard coverage above.

Author readiness

Human owner: @gensokyo-dev

The owner explicitly delegated readiness declarations and agent-managed review in the task. Code inspection and command execution above were performed by Codex; no separate human inspection is claimed.

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.

_{🔄 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/6903 **Author:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Created:** 10/23/2026 **Status:** ✅ Merged **Merged:** 10/23/2026 **Merged by:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Base:** `main` ← **Head:** `test/cli-markdown-capacity` --- ### 📝 Commits (2) - [`c4f9325`](https://github.com/morluto/rea/commit/c4f932583c3f3525cd09b744921e0aaf3fd2fe43) test(cli): repair Markdown capacity recovery coverage - [`81ff238`](https://github.com/morluto/rea/commit/81ff238aa8e8862fed393f8a2e25ec3f67a5cf16) test(cli): assert the full Markdown recovery envelope ### 📊 Changes **1 file changed** (+60 additions, -11 deletions) <details> <summary>View changed files</summary> 📝 `tests/acceptance/cliFormattingCapacity.test.ts` (+60 -11) </details> ### 📄 Description ## Problem and outcome The CLI capacity regression injects failures only into Incur, but REA now renders Markdown itself. Both Markdown cases fail on `main@3787ec5f` without ever reaching the expected fault. This directly repairs observed CI; no separate issue was opened. Inject a single string-materialization failure while the real Markdown formatter converts the operation or error-code scalar. The hook runs only in the fixture's CLI child and restores the original `String` before recovery. It follows the actual rendering path without matching generated bundle names or compiled function text. Keep Incur injection for its formats and the SIGPIPE scenario. Give scenarios explicit format/human fields: format selection, hook selection and YAML assertions no longer interpret display names. Add full Markdown and Markdown token-window cases for both successful and failed operations. Product runtime code is unchanged. ## Validation performed Executed on Linux x64 with Node 24.22.1 / npm 11.16.0; the compiled public CLI reads a real 4 KiB managed PE fixture or a missing target: - Baseline `npm run test:focused -- tests/acceptance/cliFormattingCapacity.test.ts` — 2 Markdown failures, 17 passes. - Real CLI conversion probe — both operation/error-code conversions occur inside `formatMarkdown` called by `formatCliOutput`. - Controlled Incur-only hook — both original Markdown cases fail again; source restored before final checks. - `npm run test:focused -- tests/acceptance/cliFormattingCapacity.test.ts tests/boundary/cli/formattingRecovery.test.ts` — **31 passed** at `c4f9325`, including SIGPIPE. 49.55 s elapsed, 58.91 s user+system CPU, 847.1 MiB sampled process-family RSS peak. - Final `npm run check` after the review follow-up — passed (typecheck, lint, formatting, Knip, metadata and architecture guards). 21.58 s elapsed, 39.59 s CPU, 2829.1 MiB sampled family RSS peak. - `git diff --check` — passed. Final runs used two CPUs (12/15), elapsed guards and an 8 GiB sampled-memory stop. The build cache was warm; CLI children and fixture workspaces were fresh. Summed descendant RSS was sampled every 100 ms; shared pages may double-count and short-lived/reparented children may be missed. These are validation costs, not a speedup comparison; baseline CPU placement and workload differ. Agent-token cost was not measured. Windows/macOS and real analysis engines were not run locally; this change is confined to test fixtures. ## Review follow-up Greptile correctly identified that the new full Markdown cases only checked error text, so they could miss a dropped envelope. They now require root `ok: false`, `resource_constraint` under `error.code`, and the command row inside `meta`. A controlled removal of `--full-output` makes both full Markdown cases fail on the envelope assertion; the source was restored afterward. All six Markdown cases were then rechecked with `node scripts/run-exact-tests.mjs run tests/acceptance/cliFormattingCapacity.test.ts --testNamePattern=Markdown`: **6 passed, 17 deliberately excluded**, 14.16 s elapsed / 16.69 s CPU / 696.5 MiB sampled family RSS peak. The other cases and production code are unchanged; no directly related cleanup remains. ## Current-head CI and review [Final CI](https://github.com/morluto/rea/actions/runs/40007125176) tests `81ff238aa` as merge `ed245f2` against `main@34563fcc9`. The changed acceptance file passes **all 23 tests in CI**, including the new envelope assertions. Static checks, package build, title and readiness pass. Codex and Greptile both completed review of the final commit with no new findings; the earlier Greptile finding is fixed and resolved. CI remains **not green** because of independently demonstrated baseline failures: - Source CI has 11 failing cases. The same four source files in a clean detached `main@34563fcc9` worktree reproduce exactly those 11 failures (155 other cases pass); failure names were compared as sets. Command: `npm run test:local -- src/application/frida/FridaInstrumentationService.test.ts src/domain/javascript/javascriptStaticAnalysis.mapEndpoints.test.ts src/domain/javascript/javascriptStaticAnalysisReturnedInvocation.test.ts src/server/BundleResourcesMcp.test.ts`. - Built shard 2 has three setup environment-validation failures and the existing CDP resource-completeness failure. All four also occur in [main's full qualification](https://github.com/morluto/rea/actions/runs/40005078330). The full main qualification uses coverage; the PR feedback lane does not. - PR fail-fast cancels the other seven shards, so their complete coverage is unavailable. No gate, assertion or retry policy was relaxed. The owner explicitly authorized proceeding past demonstrated main failures. The earlier PR run was cancelled to release runners while addressing the review finding; the linked run is the final commit's result. ## Post-merge verification Merged as `bded9c628`. Main advanced by five other commits while CI ran, so the complete merged tree differs from the CI snapshot; both changed test files and the CLI formatter/output sources match the tested versions. A fresh detached worktree at the actual merged commit installed the pinned dependencies and rebuilt/prepared the runtime, then ran `npm run test:focused -- src/ida/IdaNativeProtocol.test.ts tests/acceptance/cliFormattingCapacity.test.ts tests/boundary/cli/formattingRecovery.test.ts`: **54 passed**. Including preparation: 68.48 s elapsed / 91.30 s CPU / 1364.1 MiB sampled family RSS peak. `npm run check` also passed on that merged tree: 39.43 s / 72.54 s CPU / 2527.2 MiB peak. The same two-CPU and sampled-memory guards applied. These scoped checks do not replace the unavailable cancelled-shard coverage above. ## Author readiness Human owner: @gensokyo-dev The owner explicitly delegated readiness declarations and agent-managed review in the task. Code inspection and command execution above were performed by Codex; no separate human inspection is claimed. - [x] 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. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:47:51 +00:00

closed this issue
added the
pull-request
label

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

No description provided.

Rows
Columns