Stop PumpUntil waits from destroying evidence later assertions need (#64)

[Segfaultd]

Partial fix for #64. Resolves the part that turned out to be an actually-blind test; the remaining static-mode item is called out below and left open.

The finding

InteractiveAcceptGatesBothConnectionPackets is commented as "The load-bearing assertion" — neither peer may report a connection until the server answers the session request. It could not detect that violation at all.

PumpUntil() deallocates every packet that is not the id it waits for — on the waited-on peer, not just on alsoPump:

for (p = wanted->Receive(); p; wanted->DeallocatePacket(p), p = wanted->Receive())
{
    if (p->data[0] == (unsigned char)wantedId)
        return p;
}

The test's first action waits for ID_SESSION_CONFIG_REQUEST on the server. A connection packet produced early is drained and freed right there, before the negative assertion that looks for it ever runs.

Proved with a library mutant that reports the connection instead of withholding it:

[probe] PumpUntil(want=128) DISCARDED id=19 from WANTED peer
[  PASSED  ] 1 test.

128 = ID_SESSION_CONFIG_REQUEST, 19 = ID_NEW_INCOMING_CONNECTION. The gating contract was broken and the test said nothing, 10/10 runs.

Second defect in the same test: its two negative assertions were successive SawWithin() probes, which swallow each other's evidence — exactly the hazard CollectIds()'s own comment forbids.

Fix

• PumpUntil() takes two optional ledgers recording the ids it drains and discards, from the waited-on peer and from alsoPump. Default-null, so none of the other call sites change.
• CollectIdsBoth() drains two peers in one loop, appending, so a paired negative assertion can't pass because the other probe ate its packet.
• The gating test carries both ledgers across the initial wait and asserts over them.

No library change.

Scope

An audit found exactly 2 at-risk negative assertions, both in this one test; every other negative assertion in the suite already uses the collect-once pattern. So the helper hazard is real but the damage was contained — narrower than #64 implies, and I've noted that on the issue.

Verification (macOS arm64)

Check	Before	After
Gating test vs withholding mutant	passes 10/10 (blind)	fails 5/5, ~630 ms, names the side
Gating test vs clean library	passes	passes
Payload-storing mutant (regression check)	caught	still caught
SessionConfigLive.* ×10, clean library	—	21 × 10 green
Full ctest	—	276/276

The new failure message is specific: server reported a connection before answering the session request.

Still open on #64

A static-mode gating assertion. Static mode answers the session request the moment it arrives, so there is no window in which loopback can observe withholding — it needs control over message arrival order, i.e. the hermetic two-layer fake-socket harness per CLAUDE.md. Out of scope here; #64 stays open for it. Note the property is now covered deterministically by a test whose stated purpose is gating, rather than incidentally by two tests about rejection and client departure.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 30 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: bb6a2a21-7dde-440e-a0d8-d7735ea4b6c1

📥 Commits

Reviewing files that changed from the base of the PR and between 93aad47 and f5beffe.

📒 Files selected for processing (1)

• Tests/Integration/SessionConfigLiveTests.cpp

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}