morluto/rea

Fork 0

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

[PR #3755] [MERGED] fix(doctor): validate Hopper startup timeout in readiness checks #5877

New issue

Closed

opened 2026-10-31 01:10:07 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:10:07 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3755
Author: @xiajiahang
Created: 10/14/2026
Status: ✅ Merged
Merged: 10/20/2026
Merged by: @gensokyo-dev

Base: main ← Head: fix/doctor-hopper-startup-timeout

📝 Commits (6)

1d3b330 fix(doctor): validate Hopper startup timeout in readiness checks
963de3a Merge remote-tracking branch 'origin/main' into sweep/3755
4674d2e Merge remote-tracking branch 'origin/main' into followup/3755-current
0e550c2 Merge remote-tracking branch 'origin/pr-3755' into fix/3306-hopper-doctor-timeout
2ab2964 test(doctor): isolate Hopper timeout regression and cover Linux CLI
d39962f fix(doctor): separate timeout validation from Hopper configuration

📊 Changes

5 files changed (+185 additions, -1 deletions)

View changed files

📝 src/application/Doctor.test.ts (+96 -1)
📝 src/application/Doctor.ts (+7 -0)
📝 src/application/DoctorDiagnostics.ts (+26 -0)
📝 src/setupEnvironment.test.ts (+2 -0)
📝 tests/boundary/cli/doctorHopperDiscovery.test.ts (+54 -0)

📄 Description

Summary

Fixes #3306. doctor --provider hopper now diagnoses invalid REA_HOPPER_STARTUP_TIMEOUT_MS using the same parser as runtime configuration. A timeout of 0 produces an unsuccessful hopper-startup-timeout check with config_drift and remediation; unset and valid values retain their existing range. Audit health validates the supplied timeout independently of provider discovery. Invalid settings cannot be hidden by an available peer; valid tuning settings do not imply that Hopper is configured. Outside Hopper scope the diagnostic remains informational.

Keep the original contribution by @xiajiahang. Follow-up qualification merges current main d532b1812, isolates the audit test from unrelated local registrations, adds a healthy audit control and extends the compiled CLI regression to Linux. The actual comparison contains five focused files, including the existing setup regression extended to preserve the same setting.

Scope

Existing read-only readiness/configuration check; additional maintainer scope decision is not applicable. Reuse providerStartupTimeoutSchema and shared Doctor application workflow; no independent CLI parser, new timer range, target execution, installer, analysis cache or Evidence format. One bounded diagnostic is returned when the setting is supplied. Existing installation documentation already defines the same positive-decimal range, 1–2,147,483,647 ms.

Executed validation

Linux, pinned Node 24.18.0 / npm 11.16.0:

Before the production fix, 10 regression cases failed. The initial audit test also passed on the base because unrelated local registrations made it unhealthy; the final isolated test and healthy control remove that false pass.
npm run test:focused -- src/application/Doctor.test.ts src/config.test.ts src/doctorRuntime.test.ts src/setupEnvironment.test.ts tests/boundary/cli/doctorHopperDiscovery.test.ts tests/boundary/filesystem/doctorSystemHostFilesystem.test.ts: 144 passed across 6 files. Covers invalid/valid/unset values, signed timer boundary, selected/unselected scope, available peers and public CLI diagnostics. The launcher was not executed.
Public compiled CLI workflow, before/after: providers --json and doctor --provider hopper --json, each with 0 and 120000. Invalid runtime configuration still exits 1, valid configuration exits 0. Doctor now includes the correct failed/successful timeout check. Linux independently rejects the authored shell fixture as an unsupported Hopper build, so its overall doctor exits 1 on both settings; this is diagnostic verification, not a healthy real-Hopper claim.
Final npm run check and pre-push check:fast: passed typecheck, formatting, lint, knip, metadata, module boundaries and architecture guards.
Review regression: valid timeout with Hopper absent and an available Ghidra peer failed before the correction and passes now. Audit coverage includes present/absent Hopper with invalid/valid/unset timeout. Setup keeps the timeout registration without treating it as provider configuration.

CI qualification: the first final-head run failed only DSH CLI previews, approves, repairs, launches MCP, repeats and uninstalls while preserving caller YAML at its 30-second test limit. Its file is unchanged (blob b544c5816a69b85bbf3d5be9773cdc82a3703a47) and supplies no Hopper timeout. The identical timeout reproduces on exact base d532b1812 and candidate. With invocation-only --testTimeout=90000, both complete (base 30.5 s; candidate 31.8 s in the final interleaved sample). One candidate sample took 58.0 s; it is retained as uncontrolled execution variance. No test or CI budget was changed. Only failed CI jobs were rerun: the DSH case passed in 19.638 s, and that shard passed 1,783 tests with 51 skips. Final-head CI is now 17 successful / 19 skipped checks. Earlier diagnostics used equivalent Doctor code on an older runtime and are excluded from the equivalent performance comparison.

Prior contributor evidence records controlled-launcher macOS CLI exit alignment (invalid 1/valid 0). That platform was not rerun here. No real Hopper installation/session or Windows verification was available; simulated provider or platform fixtures do not establish that coverage. No analysis Evidence is created or changed.

Performance

Equivalent four-call CLI workflow: exact PR base d532b1812 vs final d39962f5d, each with initially empty/reused Node compile cache.

Revision/cache	Elapsed	Family CPU	Peak family memory	Output token estimate
Before / cold	10.293 s	9.701 s	371.5 MiB	11,011
Before / warm	10.089 s	9.495 s	367.3 MiB	11,011
After / cold	10.195 s	9.654 s	370.9 MiB	11,272
After / warm	10.092 s	9.582 s	366.1 MiB	11,272

Output-only cl100k_base estimates include all four replies; the new invalid/valid doctor check accounts for the added context. Actual model usage, CLI inputs/shell framing and initial pre-push hook resources are unmeasured. Final bounded push/check:fast peaked at 5.36 GiB, no OOM and joined cleanup. OS cache/load are uncontrolled; single samples do not establish a speedup. No result cache or repeated engine analysis is introduced. Measured tests and workflows use one CPU, 8-GiB hard / 7-GiB early memory limits and no swap; all completed without OOM and with owned processes cleared. Final 144-test build/workflow: 111.730 s elapsed, 110.082 s family CPU, 3.20 GiB peak, no OOM and joined cleanup.

Review attribution

Four older Greptile threads concern inherited Kuna, reference-comparison and JavaScript files. Their blobs are identical to current main; refreshed GitHub and local comparisons contain only the five related timeout/setup files. Replies record this attribution without claiming those unrelated runtime reports were reproduced or fixed.

Author readiness

Human owner: @xiajiahang

The original readiness declaration is retained. Later qualification was performed by the agent for @gensokyo-dev under the task owner's explicit delegation to validate, update ready PRs and merge; no new separate human review or declaration by the original author 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.

Before merge

Focused existing-boundary fix; no unresolved scope or authority questions.
Final diff checked for unrelated changes and credentials; no generated/binary artifacts committed.
Final local checks and final-head CI/review complete: d39962f5de88e1ee7ccfaaa9a96816bdfcb77fdf, 17 successful / 19 skipped checks after the unchanged failed-shard retry; Codex no findings; Greptile 5/5; no unresolved threads.

_{🔄 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/3755 **Author:** [@xiajiahang](https://github.com/xiajiahang) **Created:** 10/14/2026 **Status:** ✅ Merged **Merged:** 10/20/2026 **Merged by:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Base:** `main` ← **Head:** `fix/doctor-hopper-startup-timeout` --- ### 📝 Commits (6) - [`1d3b330`](https://github.com/morluto/rea/commit/1d3b330ae8846a8bd6f0818281c28e2d342d5f92) fix(doctor): validate Hopper startup timeout in readiness checks - [`963de3a`](https://github.com/morluto/rea/commit/963de3a22b38945891c61411b61858443acad82f) Merge remote-tracking branch 'origin/main' into sweep/3755 - [`4674d2e`](https://github.com/morluto/rea/commit/4674d2e2f7ac411c037d039715c1bdb8a8a0aece) Merge remote-tracking branch 'origin/main' into followup/3755-current - [`0e550c2`](https://github.com/morluto/rea/commit/0e550c2f296aaf4e0bb74151f32c8a6b6f6dc9e8) Merge remote-tracking branch 'origin/pr-3755' into fix/3306-hopper-doctor-timeout - [`2ab2964`](https://github.com/morluto/rea/commit/2ab2964316a6ba0aa9f281bd50ae89622bc30815) test(doctor): isolate Hopper timeout regression and cover Linux CLI - [`d39962f`](https://github.com/morluto/rea/commit/d39962f5de88e1ee7ccfaaa9a96816bdfcb77fdf) fix(doctor): separate timeout validation from Hopper configuration ### 📊 Changes **5 files changed** (+185 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/application/Doctor.test.ts` (+96 -1) 📝 `src/application/Doctor.ts` (+7 -0) 📝 `src/application/DoctorDiagnostics.ts` (+26 -0) 📝 `src/setupEnvironment.test.ts` (+2 -0) 📝 `tests/boundary/cli/doctorHopperDiscovery.test.ts` (+54 -0) </details> ### 📄 Description ## Summary Fixes #3306. `doctor --provider hopper` now diagnoses invalid `REA_HOPPER_STARTUP_TIMEOUT_MS` using the same parser as runtime configuration. A timeout of `0` produces an unsuccessful `hopper-startup-timeout` check with `config_drift` and remediation; unset and valid values retain their existing range. Audit health validates the supplied timeout independently of provider discovery. Invalid settings cannot be hidden by an available peer; valid tuning settings do not imply that Hopper is configured. Outside Hopper scope the diagnostic remains informational. Keep the original contribution by @xiajiahang. Follow-up qualification merges current main `d532b1812`, isolates the audit test from unrelated local registrations, adds a healthy audit control and extends the compiled CLI regression to Linux. The actual comparison contains five focused files, including the existing setup regression extended to preserve the same setting. ## Scope Existing read-only readiness/configuration check; additional maintainer scope decision is not applicable. Reuse `providerStartupTimeoutSchema` and shared Doctor application workflow; no independent CLI parser, new timer range, target execution, installer, analysis cache or Evidence format. One bounded diagnostic is returned when the setting is supplied. Existing installation documentation already defines the same positive-decimal range, 1–2,147,483,647 ms. ## Executed validation Linux, pinned Node 24.18.0 / npm 11.16.0: - Before the production fix, **10 regression cases failed**. The initial audit test also passed on the base because unrelated local registrations made it unhealthy; the final isolated test and healthy control remove that false pass. - `npm run test:focused -- src/application/Doctor.test.ts src/config.test.ts src/doctorRuntime.test.ts src/setupEnvironment.test.ts tests/boundary/cli/doctorHopperDiscovery.test.ts tests/boundary/filesystem/doctorSystemHostFilesystem.test.ts`: **144 passed across 6 files**. Covers invalid/valid/unset values, signed timer boundary, selected/unselected scope, available peers and public CLI diagnostics. The launcher was not executed. - Public compiled CLI workflow, before/after: `providers --json` and `doctor --provider hopper --json`, each with `0` and `120000`. Invalid runtime configuration still exits 1, valid configuration exits 0. Doctor now includes the correct failed/successful timeout check. Linux independently rejects the authored shell fixture as an unsupported Hopper build, so its overall doctor exits 1 on both settings; this is diagnostic verification, not a healthy real-Hopper claim. - Final `npm run check` and pre-push `check:fast`: passed typecheck, formatting, lint, knip, metadata, module boundaries and architecture guards. - Review regression: valid timeout with Hopper absent and an available Ghidra peer failed before the correction and passes now. Audit coverage includes present/absent Hopper with invalid/valid/unset timeout. Setup keeps the timeout registration without treating it as provider configuration. CI qualification: the first final-head run failed only `DSH CLI previews, approves, repairs, launches MCP, repeats and uninstalls while preserving caller YAML` at its 30-second test limit. Its file is unchanged (blob `b544c5816a69b85bbf3d5be9773cdc82a3703a47`) and supplies no Hopper timeout. The identical timeout reproduces on exact base `d532b1812` and candidate. With invocation-only `--testTimeout=90000`, both complete (base 30.5 s; candidate 31.8 s in the final interleaved sample). One candidate sample took 58.0 s; it is retained as uncontrolled execution variance. No test or CI budget was changed. Only failed CI jobs were rerun: the DSH case passed in 19.638 s, and that shard passed 1,783 tests with 51 skips. Final-head CI is now 17 successful / 19 skipped checks. Earlier diagnostics used equivalent Doctor code on an older runtime and are excluded from the equivalent performance comparison. Prior contributor evidence records controlled-launcher macOS CLI exit alignment (invalid 1/valid 0). That platform was not rerun here. No real Hopper installation/session or Windows verification was available; simulated provider or platform fixtures do not establish that coverage. No analysis Evidence is created or changed. ## Performance Equivalent four-call CLI workflow: exact PR base `d532b1812` vs final `d39962f5d`, each with initially empty/reused Node compile cache. | Revision/cache | Elapsed | Family CPU | Peak family memory | Output token estimate | | --- | ---: | ---: | ---: | ---: | | Before / cold | 10.293 s | 9.701 s | 371.5 MiB | 11,011 | | Before / warm | 10.089 s | 9.495 s | 367.3 MiB | 11,011 | | After / cold | 10.195 s | 9.654 s | 370.9 MiB | 11,272 | | After / warm | 10.092 s | 9.582 s | 366.1 MiB | 11,272 | Output-only `cl100k_base` estimates include all four replies; the new invalid/valid doctor check accounts for the added context. Actual model usage, CLI inputs/shell framing and initial pre-push hook resources are unmeasured. Final bounded push/check:fast peaked at 5.36 GiB, no OOM and joined cleanup. OS cache/load are uncontrolled; single samples do not establish a speedup. No result cache or repeated engine analysis is introduced. Measured tests and workflows use one CPU, 8-GiB hard / 7-GiB early memory limits and no swap; all completed without OOM and with owned processes cleared. Final 144-test build/workflow: 111.730 s elapsed, 110.082 s family CPU, 3.20 GiB peak, no OOM and joined cleanup. ## Review attribution Four older Greptile threads concern inherited Kuna, reference-comparison and JavaScript files. Their blobs are identical to current main; refreshed GitHub and local comparisons contain only the five related timeout/setup files. Replies record this attribution without claiming those unrelated runtime reports were reproduced or fixed. ## Author readiness Human owner: @xiajiahang The original readiness declaration is retained. Later qualification was performed by the agent for @gensokyo-dev under the task owner's explicit delegation to validate, update ready PRs and merge; no new separate human review or declaration by the original author 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. ## Before merge - [x] Focused existing-boundary fix; no unresolved scope or authority questions. - [x] Final diff checked for unrelated changes and credentials; no generated/binary artifacts committed. - [x] Final local checks and final-head CI/review complete: `d39962f5de88e1ee7ccfaaa9a96816bdfcb77fdf`, 17 successful / 19 skipped checks after the unchanged failed-shard retry; Codex no findings; Greptile 5/5; no unresolved threads. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:10:07 +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#5877

No description provided.

Rows
Columns