[xianshijing-lk]

Port of livekit/client-sdk-js#2114.

The bug

Transport.setRemoteDescription applied every answer it was handed, with no check that one was still wanted:

try {
  await pc.setRemoteDescription(sd);
} catch (e) {
  logger.warning('[$objectId] setRemoteDescription() failed with error: $e');
}

A proxy duplicating the signal message, or a server retry, therefore applies the same answer twice. The second apply happens with no local offer left to answer, which is an invalid transition. Because the failure is swallowed into a warning, the method then carries on and flushes _pendingCandidates and clears restartingIce a second time — side effects for a message that should have been dropped.

Repro

Added a Transport.setRemoteDescription duplicate answers group to transport_negotiate_test.dart. Against the unfixed code:

ignores a second copy of an answer that was already applied [E]
  Expected: v=0\r\nfirst
    Actual: v=0\r\nduplicate
  duplicate answer was applied to the peer connection

The fix

Offer ids are not plumbed through this SDK, so the numbered-answer half of the upstream fix has no analogue here. What does apply is the rule JS falls back to for unnumbered answers: in stable there is no local offer left to answer, so an answer arriving then is a duplicate. An _answerInFlight flag covers a copy that arrives while the first is still being applied, and is released in a finally however the apply ends, so a genuine retry still works.

This is consistent with how the transport already reasons about negotiation — createAndSendOffer likewise gates on getSignalingState() == RTCSignalingStateHaveLocalOffer.

Only answers are gated; remote offers (normal subscriber traffic, which do arrive in stable) are untouched, and there is a test for that.

The ICE-restart path at transport.dart:368 calls pc.setRemoteDescription directly rather than going through this method, so it is unaffected.

Verification

flutter test 471/471, flutter analyze lib/src/core/transport.dart clean. The three new cases cover the duplicate, the still-wanted answer, and the remote-offer-in-stable case.

One thing worth a reviewer's eye

I could not get an automated test to cover a second negotiation round (offer → answer → offer → answer). MockPeerConnection.signalingState only reports HaveLocalOffer when _remoteDescription is null, so once any answer has been applied the mock reports stable forever, even after a fresh setLocalDescription(offer).

Real WebRTC moves to have-local-offer there, so the second answer is accepted in production — and no existing test exercises that path through Transport.setRemoteDescription, which is why the suite stays green. But that means the mock cannot currently prove it. If you'd rather, I can tighten the mock's state model in a follow-up so this is actually pinned down.

🤖 Generated with Claude Code

[changt]

Why can't we do the same as we did in JS to ensure the message is indeed duplicated?

[xianshijing-lk]

(Reposting here so it is attached to your question rather than sitting at the bottom of the PR.)

Short answer: this SDK has no offer ids to match against yet.

The proto field exists (SessionDescription.id), but Flutter never populates it — SignalClient.sendOffer sends offer.toPBType() with no id, so every offer goes out as id 0 and the server echoes that back. SignalAnswerEvent then carries only sd and drops msg.answer.id anyway.

So the numbered half of the JS fix has nothing to key on here. What this PR implements is specifically JS's own fallback for that case — isDuplicateAnswer returns signalingState === 'stable' && !hasPendingInitialOffer when offerId is 0, which is the branch legacy servers take. Flutter is permanently in that regime, so this matches JS rather than diverging from it. The _answerInFlight flag covers the mid-apply race that JS handles with answerInFlight.

Making it id-based the way JS is would mean teaching this SDK to assign offer ids: a counter in Transport.createAndSendOffer, threading it through sendOffer, and carrying it back on SignalAnswerEvent. That is strictly more precise — it would catch a duplicate arriving after a new offer is already pending, which the state check cannot — and I am happy to do it as a follow-up. I kept this PR to the minimal fix since the state check covers the reported failure.

One caveat either way: I could not get automated coverage of a second negotiation round, because MockPeerConnection.signalingState reports stable forever once any answer is applied. Id-based matching would sidestep that mock limitation too.

[xianshijing-lk]

Good question — the short answer is that this SDK has no offer ids to match against yet.

The proto field exists (SessionDescription.id), but Flutter never populates it. SignalClient.sendOffer sends offer.toPBType() with no id, so every offer goes out as id 0, and the server echoes that back. SignalAnswerEvent then carries only sd and drops msg.answer.id anyway.

So the numbered half of the JS fix has nothing to key on here. What I implemented is specifically JS's own fallback for that case — isDuplicateAnswer returns signalingState === 'stable' && !hasPendingInitialOffer when offerId is 0, which is the branch legacy servers take. Flutter is permanently in that regime, so the behaviour matches JS rather than diverging from it. The _answerInFlight flag covers the mid-apply race that JS handles with answerInFlight.

Making it id-based the way JS is would mean teaching this SDK to assign offer ids: a counter in Transport.createAndSendOffer, threading it through sendOffer, and carrying it back on SignalAnswerEvent so the transport can compare. That's very doable and it would be strictly more precise than the signaling-state inference — it would catch a duplicate that arrives after a new offer is already pending, which the current check cannot. Happy to do it as a follow-up if you want parity; I kept this PR to the minimal fix since the state check covers the reported failure.

One caveat worth flagging either way: I could not get automated coverage of a second negotiation round, because MockPeerConnection.signalingState reports stable forever once any answer is applied. Id-based matching would sidestep that mock limitation too.

[cloudwebrtc]

lgtm!