From a96e16fd1ab0d46cfc78c77825589a239bdcb4e1 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 31 Aug 2026 00:55:22 -0700 Subject: [PATCH] test(relay): pin PTY source boundary cleanup and guard ascending sends The early-`break` in advanceCredit is only correct while sentBoundaries is inserted in ascending sentEndSu order. Turn that implicit invariant into a throw at the sole live write site (commitPtySourceSend), and assert the post-state directly instead of inferring it from an iteration budget: - assert the surviving boundary set after the 1,023-ACK benchmark - cover the jump-ahead cumulative ACK that must delete many boundaries in one pass (the case an over-eager `break` would get wrong) - cover the settleReservedPtySourceAck -> advanceCredit entry point - drop an arithmetically-implied assertion and CI benchmark log noise --- src/relay/pty-source-credit-ledger.test.ts | 30 ++++++++++++++++++---- src/relay/pty-source-credit-settlement.ts | 4 +++ 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/src/relay/pty-source-credit-ledger.test.ts b/src/relay/pty-source-credit-ledger.test.ts index 638676354aa..d2624cafc82 100644 --- a/src/relay/pty-source-credit-ledger.test.ts +++ b/src/relay/pty-source-credit-ledger.test.ts @@ -328,12 +328,31 @@ describe('RelayPtySourceCreditLedger', () => { expect(legacyVisits).toBe(524_799) expect(countedBoundaries.visits).toBe(2_046) - expect(legacyVisits - countedBoundaries.visits).toBe(522_753) - console.log( - `[bench] ${boundaryCount} sent boundaries: ACK iterator visits ${legacyVisits} -> ${countedBoundaries.visits} ` + - `(-${(((legacyVisits - countedBoundaries.visits) / legacyVisits) * 100).toFixed(2)}%)` - ) expect(ledger.retentionSnapshot()).toEqual({ sourceSu: 0, dataBytes: 0, spans: 0 }) + expect([...record.sentBoundaries]).toEqual([spanCount]) + }) + + it('deletes every boundary skipped by a jump-ahead cumulative ACK', () => { + const ledger = new RelayPtySourceCreditLedger() + const owner = identity() + ledger.open(owner, 16) + append(ledger, owner, 'abcdefgh') + for (let index = 0; index < 8; index += 1) { + ledger.commitSend(ledger.reserveNextSend(owner, 1)!) + } + const record = getBoundaryRecord(ledger, owner) + expect([...record.sentBoundaries]).toEqual([0, 1, 2, 3, 4, 5, 6, 7, 8]) + + expect( + ledger.acknowledge(owner, { + id: owner.id, + clientGeneration: owner.clientGeneration, + ownerGeneration: owner.ownerGeneration, + deliveryToken: owner.deliveryToken, + creditedEndSu: 8 + }) + ).toBe('advanced') + expect([...record.sentBoundaries]).toEqual([8]) }) it('never exceeds a token source window across generated send/ACK sequences', () => { @@ -487,6 +506,7 @@ describe('RelayPtySourceCreditLedger', () => { ledger.commitSend(pending) expect(ledger.snapshot(owner)).toMatchObject({ sentEndSu: 4, creditedEndSu: 4 }) + expect([...getBoundaryRecord(ledger, owner).sentBoundaries]).toEqual([4]) expect(ledger.retentionSnapshot()).toEqual({ sourceSu: 0, dataBytes: 0, spans: 0 }) }) diff --git a/src/relay/pty-source-credit-settlement.ts b/src/relay/pty-source-credit-settlement.ts index 55196110e79..8a0d724fec9 100644 --- a/src/relay/pty-source-credit-settlement.ts +++ b/src/relay/pty-source-credit-settlement.ts @@ -101,6 +101,10 @@ export function commitPtySourceSend( if (record.pendingSend !== reservation) { throw new Error('PTY source send reservation is stale') } + // advanceCredit breaks on the first uncredited boundary, so sentBoundaries inserts must ascend. + if (reservation.span.sourceEndSu <= record.sentEndSu) { + throw new Error('PTY source send reservation regresses the sent boundary') + } record.pendingSend = null record.sentEndSu = reservation.span.sourceEndSu record.sentBoundaries.add(record.sentEndSu)