[PR #1119] [MERGED] fix(hopper): enforce caller-supplied request deadlines #2664

New issue

Closed

opened 2026-10-17 21:08:47 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-17 21:08:47 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1119
Author: @vilmerfrost
Created: 10/8/2026
Status: ✅ Merged
Merged: 10/8/2026
Merged by: @morluto

Base: main ← Head: fix/hopper-request-deadlines

📝 Commits (7)

5d1f403 fix(hopper): enforce caller-supplied request deadlines
0cd66c3 chore: merge current main into Hopper deadline fix
c66eb06 fix(hopper): correct idle deadline state and name timeout bound
1024460 fix(hopper): enforce elapsed deadlines before dispatch and replies
dd828b7 chore: retain contributor deadline diagnostics fixes
98478a8 fix(hopper): distinguish unsent requests in timeout diagnostics
14a0d9e test(hopper): exercise request deadlines against native Hopper

📊 Changes

7 files changed (+688 additions, -4 deletions)

View changed files

📝 docs/testing.md (+9 -0)
📝 package.json (+1 -0)
➕ scripts/verify-real-hopper-deadlines.mjs (+126 -0)
📝 src/hopper/HopperClient.ts (+5 -0)
➕ src/hopper/HopperRequestQueue.test.ts (+300 -0)
📝 src/hopper/HopperRequestQueue.ts (+96 -4)
➕ tests/boundary/providers/hopper/hopperClient.deadlines.test.ts (+151 -0)

📄 Description

Summary

Enforce optional caller-supplied Hopper request deadlines. Previously callTool(..., { timeoutMs }) silently waited indefinitely because the client dropped the option before the FIFO.

Related to #340. This implements caller deadlines and cleanup; it does not satisfy that issue's separate native-analysis interruption requirement.

Problem and expected behavior

A bridge that withholds its response must not keep the caller pending beyond a supplied deadline. Expired queued requests must never be sent. An already transmitted request must retain Hopper's serial wire slot until its late reply or verified teardown, so timeout cannot corrupt request correlation.

Change and scope

Expose typed timeoutMs on callTool, propagate it into HopperRequestQueue, and schedule a deadline from FIFO admission. Remove expired queued entries; settle active callers with contextual HopperTimeoutError while retaining native work. Clear timers and abort observers on every settlement. First completion/cancellation/timeout wins. Handle synchronous progress-observer cancellation before transmission without retaining an empty slot.

Zero expires without sending. Reject negative, fractional, nonfinite, and overflowing values instead of allowing Node timer clamping. No implicit operation deadline is added.

Contract and boundary impact

Semantic owner/earliest stage: Hopper client FIFO admission; response serialization unchanged.
CLI/MCP/tool catalog: unchanged; additive typed client option. Existing internal handshake/shutdown deadlines now reach the queue.
Provider/platform compatibility: serial correlation preserved; timer logic is host-independent, real workflow tested on macOS ARM64.
Evidence/artifact/provenance: unchanged; timeout carries operation, request ID, busy state, and stage.
Execution/authorization/cleanup: timeout settles the caller, not Hopper's synchronous Python operation. Close still uses existing verified cleanup. No concurrent native dispatch after active timeout.
Generated metadata/package/installation: none.

Evidence and regression coverage

Thirteen deterministic queue tests cover success, active/queued expiry, zero/invalid values, explicit/pre-admission cancellation, late response, first-settlement races, progress reentrancy, teardown, and timer/listener cleanup. Three socket-client tests exercise propagation, nontransmission of expired FIFO work, and process/runtime cleanup after timeout.

On unchanged base d1c904bac7977d44b06bf2f823587baedcc88b9e, the initial deadline regressions failed: two queue tests and one bounded socket test. The preserved original reproduction also stayed pending beyond its supplied 25 ms deadline. With the built fix, it settles as HopperTimeoutError, accepts a late reply, and closes with idle activity. This is executed protocol evidence, not a claim of native interruption.

Observed, inferred, and unverified claims remain distinct.
Reproduction, before/after logs, and failed attempts preserved locally.
Native cancellation limitations remain explicit.

Validation performed

Built source with npm run build:cached — passed.
Focused queue/deadline/protocol/progress/startup/restart/health suites — 62 passed across 7 files.
npm run check — passed (TypeScript, lint, formatting, Knip, metadata).
npm run verify:test-discovery — all 631 files discovered once.
Preserved protocol mock against built source — timeout, late reply, cleanup passed.
Broader client suite — 72 passed, 2 existing native startup/ownership test cases failed. Those same cases also fail with original upstream client and queue restored in this environment; assertions vary. No tests were weakened or excluded from that recorded run.
npm run verify:hopper using installed launcher — passed on macOS ARM64 with source-built manifest fixtures, target switching, unrelated-process survival, and clean shutdown.
git diff --check and pre-push check:fast — passed.

Compatibility, safety, and release

Deadlines start after client startup; startup retains its separate bounded policy. A timed-out native operation can continue to occupy the serial slot until reply/teardown. There is no new native interruption or default-timeout policy. Linux real-Hopper verification was unavailable on this macOS host. No schema migration, installed-provider edits, or release metadata changes.

Review checklist

One focused outcome; conventional title; related issue linked.
Regression, cancellation, race, and cleanup coverage added.
No authored contract/generated metadata changes needed.
Final diff checked for unrelated files, secrets, 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/1119 **Author:** [@vilmerfrost](https://github.com/vilmerfrost) **Created:** 10/8/2026 **Status:** ✅ Merged **Merged:** 10/8/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/hopper-request-deadlines` --- ### 📝 Commits (7) - [`5d1f403`](https://github.com/morluto/rea/commit/5d1f403a45045ac9f8af339ea62c48996607f74c) fix(hopper): enforce caller-supplied request deadlines - [`0cd66c3`](https://github.com/morluto/rea/commit/0cd66c381d0ec51e8b8eb3439ad158591d0afd7a) chore: merge current main into Hopper deadline fix - [`c66eb06`](https://github.com/morluto/rea/commit/c66eb067a847d0a0b03d01dbbc16b6c6f2e48291) fix(hopper): correct idle deadline state and name timeout bound - [`1024460`](https://github.com/morluto/rea/commit/10244606b675a96eb26417f7c1b9b29265d04d07) fix(hopper): enforce elapsed deadlines before dispatch and replies - [`dd828b7`](https://github.com/morluto/rea/commit/dd828b75c6bc6c4df7a15fe15df1bdfd7805b58b) chore: retain contributor deadline diagnostics fixes - [`98478a8`](https://github.com/morluto/rea/commit/98478a88438883cfe8751490fa6d621d646890fb) fix(hopper): distinguish unsent requests in timeout diagnostics - [`14a0d9e`](https://github.com/morluto/rea/commit/14a0d9e04d9fe748644acba38643f90e8d1fa781) test(hopper): exercise request deadlines against native Hopper ### 📊 Changes **7 files changed** (+688 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `docs/testing.md` (+9 -0) 📝 `package.json` (+1 -0) ➕ `scripts/verify-real-hopper-deadlines.mjs` (+126 -0) 📝 `src/hopper/HopperClient.ts` (+5 -0) ➕ `src/hopper/HopperRequestQueue.test.ts` (+300 -0) 📝 `src/hopper/HopperRequestQueue.ts` (+96 -4) ➕ `tests/boundary/providers/hopper/hopperClient.deadlines.test.ts` (+151 -0) </details> ### 📄 Description ## Summary Enforce optional caller-supplied Hopper request deadlines. Previously `callTool(..., { timeoutMs })` silently waited indefinitely because the client dropped the option before the FIFO. Related to #340. This implements caller deadlines and cleanup; it does **not** satisfy that issue's separate native-analysis interruption requirement. ## Problem and expected behavior A bridge that withholds its response must not keep the caller pending beyond a supplied deadline. Expired queued requests must never be sent. An already transmitted request must retain Hopper's serial wire slot until its late reply or verified teardown, so timeout cannot corrupt request correlation. ## Change and scope Expose typed `timeoutMs` on `callTool`, propagate it into `HopperRequestQueue`, and schedule a deadline from FIFO admission. Remove expired queued entries; settle active callers with contextual `HopperTimeoutError` while retaining native work. Clear timers and abort observers on every settlement. First completion/cancellation/timeout wins. Handle synchronous progress-observer cancellation before transmission without retaining an empty slot. Zero expires without sending. Reject negative, fractional, nonfinite, and overflowing values instead of allowing Node timer clamping. No implicit operation deadline is added. ## Contract and boundary impact - Semantic owner/earliest stage: Hopper client FIFO admission; response serialization unchanged. - CLI/MCP/tool catalog: unchanged; additive typed client option. Existing internal handshake/shutdown deadlines now reach the queue. - Provider/platform compatibility: serial correlation preserved; timer logic is host-independent, real workflow tested on macOS ARM64. - Evidence/artifact/provenance: unchanged; timeout carries operation, request ID, busy state, and stage. - Execution/authorization/cleanup: timeout settles the caller, not Hopper's synchronous Python operation. Close still uses existing verified cleanup. No concurrent native dispatch after active timeout. - Generated metadata/package/installation: none. ## Evidence and regression coverage Thirteen deterministic queue tests cover success, active/queued expiry, zero/invalid values, explicit/pre-admission cancellation, late response, first-settlement races, progress reentrancy, teardown, and timer/listener cleanup. Three socket-client tests exercise propagation, nontransmission of expired FIFO work, and process/runtime cleanup after timeout. On unchanged base `d1c904bac7977d44b06bf2f823587baedcc88b9e`, the initial deadline regressions failed: two queue tests and one bounded socket test. The preserved original reproduction also stayed pending beyond its supplied 25 ms deadline. With the built fix, it settles as `HopperTimeoutError`, accepts a late reply, and closes with idle activity. This is executed protocol evidence, not a claim of native interruption. - [x] Observed, inferred, and unverified claims remain distinct. - [x] Reproduction, before/after logs, and failed attempts preserved locally. - [x] Native cancellation limitations remain explicit. ## Validation performed - Built source with `npm run build:cached` — passed. - Focused queue/deadline/protocol/progress/startup/restart/health suites — 62 passed across 7 files. - `npm run check` — passed (TypeScript, lint, formatting, Knip, metadata). - `npm run verify:test-discovery` — all 631 files discovered once. - Preserved protocol mock against built source — timeout, late reply, cleanup passed. - Broader client suite — 72 passed, 2 existing native startup/ownership test cases failed. Those same cases also fail with original upstream client and queue restored in this environment; assertions vary. No tests were weakened or excluded from that recorded run. - `npm run verify:hopper` using installed launcher — passed on macOS ARM64 with source-built manifest fixtures, target switching, unrelated-process survival, and clean shutdown. - `git diff --check` and pre-push `check:fast` — passed. ## Compatibility, safety, and release Deadlines start after client startup; startup retains its separate bounded policy. A timed-out native operation can continue to occupy the serial slot until reply/teardown. There is no new native interruption or default-timeout policy. Linux real-Hopper verification was unavailable on this macOS host. No schema migration, installed-provider edits, or release metadata changes. ## Review checklist - [x] One focused outcome; conventional title; related issue linked. - [x] Regression, cancellation, race, and cleanup coverage added. - [x] No authored contract/generated metadata changes needed. - [x] Final diff checked for unrelated files, secrets, 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-17 21:08:47 +00:00