[PR #1643] [CLOSED] test(hopper): recognize signal-terminated diagnostic fixtures #4720

New issue

Closed

opened 2026-10-31 00:54:19 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1643
Author: @evenafterallisaid
Created: 10/10/2026
Status: ❌ Closed

Base: main ← Head: test/hopper-diagnostic-signal-exit

📝 Commits (1)

320a792 test(hopper): recognize signal-terminated diagnostic fixtures

📊 Changes

1 file changed (+14 additions, -4 deletions)

View changed files

📝 tests/boundary/providers/hopper/hopperClient.diagnostics.test.ts (+14 -4)

📄 Description

summary

superseded by #1640, merged as ac9677bd: current main already checks that a diagnostic fixture child exists and recognizes termination by either an exit code or a signal. closing this pr to avoid duplicating the correction. this pr's old shard failure was the deep-return scope migration, repaired by merged #1644.

the Hopper diagnostic tests now recognize a fixture child terminated by a signal as exited, and fail explicitly if cleanup is unsuccessful or no child was launched.

problem and expected behavior

both tests required child.exitCode !== null after cleanup. Node's ChildProcess contract permits exitCode: null with a non-null signalCode after signal termination. HopperFixtureLauncher.cleanup already accepts either termination form. the diagnostics assertions should use the same liveness rule.

the exit-code-only assertion failed in an earlier ci run. a separate Windows reproduction using the repository's actual fakeHopper.mjs produced exitCode: null and signalCode: "SIGTERM" after emitting 8,388,615 diagnostic bytes. this confirms the faulty assertion; it does not establish the precise termination event in that Linux run.

no issue is linked because this was found while inspecting the failed check; existing issues and prs were checked for overlap.

change and scope

update the two existing diagnostics tests only. propagate an unsuccessful client.close() result, require exactly one actual child, and accept an exit code or signal code. output bounds, bridge usability, timeout details, credential redaction, and temporary-directory removal assertions remain intact.

contract and boundary impact

semantic owner and earliest changed stage: test assertions after fixture cleanup.
CLI and MCP/tool-catalog contract: none.
provider, bridge, target-format, or platform compatibility: no production changes; tests accept both documented Node termination forms.
evidence, artifact, provenance, or reconstruction contract: none.
process execution, authorization, cleanup, or containment impact: no production changes; failed cleanup and a missing child now fail the test explicitly.
generated metadata, package, or installation impact: none.

evidence and regression coverage

tests updated: the two existing Hopper diagnostics cases; no duplicate test or helper added.
reproduction: actual repository fixture subprocess on Windows, pinned Node 24.18.0, terminated with SIGTERM after all diagnostic bytes were observed. the previous assertion evaluates false, the corrected termination assertion evaluates true.
user-visible CLI/MCP output: not applicable; test-only change.
remaining proof gaps: the full diagnostics suite cannot start its private runtime on this Windows checkout because bundled native controls are absent. real Hopper and the precise Linux ci termination event were not verified locally.

validation performed

actual fakeHopper.mjs subprocess reproduction — observed exitCode: null, signalCode: "SIGTERM"; previous assertion failed, corrected assertion passed.
npm run test:focused -- tests/boundary/providers/hopper/hopperClient.diagnostics.test.ts — attempted; both tests stop at startup before the changed assertions because native/windows/build/manifest.json is unavailable. the underlying cause is PrivateRuntimeRootUnavailableError.
npm run check — passed, including typecheck, lint, formatting, module boundaries, metadata, knip, and architecture guards; 637 lint warnings, zero errors.
git diff --check — passed.

compatibility, safety, and release

breaking changes or migration steps: none.
real Hopper/Ghidra, browser, or OS coverage: Windows Node subprocess semantics reproduced; no real Hopper run. Unix fixture integration is left to ci.
package or release metadata impact: none.
security, privacy, process, or containment review: no runtime changes. the count assertion compares the number of children without dumping child-process objects or fixture tokens.

review checklist

this pr addresses a concrete problem.
the pr has one focused outcome and a conventional title.
the reason for not linking an issue is stated above.
changed termination behavior is reproduced; unavailable integration coverage is explicit.
no docs, contracts, or generated metadata require changes.
no user-visible CLI/MCP output changes.
the final diff was checked for secrets, unrelated cleanup, and unsupported claims.

_{🔄 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/1643 **Author:** [@evenafterallisaid](https://github.com/evenafterallisaid) **Created:** 10/10/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `test/hopper-diagnostic-signal-exit` --- ### 📝 Commits (1) - [`320a792`](https://github.com/morluto/rea/commit/320a792740f5d2917e6c8b2fdd77118341ac5943) test(hopper): recognize signal-terminated diagnostic fixtures ### 📊 Changes **1 file changed** (+14 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `tests/boundary/providers/hopper/hopperClient.diagnostics.test.ts` (+14 -4) </details> ### 📄 Description ## summary superseded by [#1640](https://github.com/morluto/rea/pull/1640), merged as `ac9677bd`: current main already checks that a diagnostic fixture child exists and recognizes termination by either an exit code or a signal. closing this pr to avoid duplicating the correction. this pr's old shard failure was the deep-return scope migration, repaired by merged [#1644](https://github.com/morluto/rea/pull/1644). the Hopper diagnostic tests now recognize a fixture child terminated by a signal as exited, and fail explicitly if cleanup is unsuccessful or no child was launched. ## problem and expected behavior both tests required `child.exitCode !== null` after cleanup. [Node's ChildProcess contract](https://nodejs.org/api/child_process.html#subprocessexitcode) permits `exitCode: null` with a non-null `signalCode` after signal termination. `HopperFixtureLauncher.cleanup` already accepts either termination form. the diagnostics assertions should use the same liveness rule. the exit-code-only assertion failed in [an earlier ci run](https://github.com/morluto/rea/actions/runs/38079492920/job/114294225534). a separate Windows reproduction using the repository's actual `fakeHopper.mjs` produced `exitCode: null` and `signalCode: "SIGTERM"` after emitting 8,388,615 diagnostic bytes. this confirms the faulty assertion; it does not establish the precise termination event in that Linux run. no issue is linked because this was found while inspecting the failed check; existing issues and prs were checked for overlap. ## change and scope update the two existing diagnostics tests only. propagate an unsuccessful `client.close()` result, require exactly one actual child, and accept an exit code or signal code. output bounds, bridge usability, timeout details, credential redaction, and temporary-directory removal assertions remain intact. ## contract and boundary impact - semantic owner and earliest changed stage: test assertions after fixture cleanup. - CLI and MCP/tool-catalog contract: none. - provider, bridge, target-format, or platform compatibility: no production changes; tests accept both documented Node termination forms. - evidence, artifact, provenance, or reconstruction contract: none. - process execution, authorization, cleanup, or containment impact: no production changes; failed cleanup and a missing child now fail the test explicitly. - generated metadata, package, or installation impact: none. ## evidence and regression coverage - tests updated: the two existing Hopper diagnostics cases; no duplicate test or helper added. - reproduction: actual repository fixture subprocess on Windows, pinned Node 24.18.0, terminated with SIGTERM after all diagnostic bytes were observed. the previous assertion evaluates false, the corrected termination assertion evaluates true. - user-visible CLI/MCP output: not applicable; test-only change. - remaining proof gaps: the full diagnostics suite cannot start its private runtime on this Windows checkout because bundled native controls are absent. real Hopper and the precise Linux ci termination event were not verified locally. ## validation performed - actual `fakeHopper.mjs` subprocess reproduction — observed `exitCode: null`, `signalCode: "SIGTERM"`; previous assertion failed, corrected assertion passed. - `npm run test:focused -- tests/boundary/providers/hopper/hopperClient.diagnostics.test.ts` — attempted; both tests stop at startup before the changed assertions because `native/windows/build/manifest.json` is unavailable. the underlying cause is `PrivateRuntimeRootUnavailableError`. - `npm run check` — passed, including typecheck, lint, formatting, module boundaries, metadata, knip, and architecture guards; 637 lint warnings, zero errors. - `git diff --check` — passed. ## compatibility, safety, and release - breaking changes or migration steps: none. - real Hopper/Ghidra, browser, or OS coverage: Windows Node subprocess semantics reproduced; no real Hopper run. Unix fixture integration is left to ci. - package or release metadata impact: none. - security, privacy, process, or containment review: no runtime changes. the count assertion compares the number of children without dumping child-process objects or fixture tokens. ## review checklist - [x] this pr addresses a concrete problem. - [x] the pr has one focused outcome and a conventional title. - [x] the reason for not linking an issue is stated above. - [x] changed termination behavior is reproduced; unavailable integration coverage is explicit. - [x] no docs, contracts, or generated metadata require changes. - [x] no user-visible CLI/MCP output changes. - [x] the final diff was checked for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

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