Skip to content

Commit d5110fb

Browse files
sirtimidclaude
andcommitted
fix: address cursor review findings
- Set streamInactivityTimeoutMs to 6s (above the 5s floor) in the E2E test so the inactivity timeout actually fires - Remove redundant giveUp() call in #handleAckTimeout — RemoteManager is the sole caller via #onGiveUp callback Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 877fb80 commit d5110fb

2 files changed

Lines changed: 7 additions & 6 deletions

File tree

packages/kernel-node-runtime/test/e2e/libp2p-v3-features.test.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,11 @@ describe.sequential('libp2p v3 Features E2E', () => {
166166
'recovers communication after idle period exceeds inactivity timeout',
167167
async () => {
168168
// Use a short inactivity timeout to test the auto-abort behavior.
169+
// Must be >= MIN_STREAM_INACTIVITY_TIMEOUT_MS (5 s) since the
170+
// transport clamps lower values.
169171
const shortTimeoutOptions = {
170172
...testBackoffOptions,
171-
streamInactivityTimeoutMs: 2_000,
173+
streamInactivityTimeoutMs: 6_000,
172174
};
173175

174176
const { aliceRef, bobURL } = await setupAliceAndBob(
@@ -190,10 +192,10 @@ describe.sequential('libp2p v3 Features E2E', () => {
190192
);
191193
expect(initial).toContain('vat Bob got "hello" from Alice');
192194

193-
// Wait longer than the inactivity timeout (2s + buffer).
195+
// Wait longer than the inactivity timeout (6s + buffer).
194196
// The stream should auto-abort due to inactivityTimeout,
195197
// triggering connection loss handling and reconnection.
196-
await delay(4_000);
198+
await delay(8_000);
197199

198200
// Send another message — the transport layer should
199201
// reconnect since the previous stream was aborted by inactivity.

packages/ocap-kernel/src/remotes/kernel/RemoteHandle.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -376,12 +376,11 @@ export class RemoteHandle implements EndpointHandle {
376376
}
377377

378378
if (this.#retryCount >= MAX_RETRIES) {
379-
// Give up - notify RemoteManager which calls giveUp() to reject
380-
// pending messages, URL redemptions, and clear the ACK timer.
379+
// Notify RemoteManager, which calls giveUp() to reject pending
380+
// messages, URL redemptions, and clear the ACK timer.
381381
this.#logger.log(
382382
`${this.#peerId.slice(0, 8)}:: gave up after ${MAX_RETRIES} retries, rejecting ${this.#getPendingCount()} pending messages`,
383383
);
384-
this.giveUp(`not acknowledged after ${MAX_RETRIES} retries`);
385384
this.#onGiveUp?.(this.#peerId);
386385
return;
387386
}

0 commit comments

Comments
 (0)