hope-voice-api/.artifacts/20260831-105824-plan-code-ui-audit/iterations/09-implementation.md

Iteration 09 Implementation Report

• Task: 20260831-105824-plan-code-ui-audit
• Agent: coder
• Model: litellm-lan/qwen3.8-27b
• Status: complete

Summary

The single correction from iterations/09-request.md (derived from iterations/08-review.md Finding 1) is implemented. Old-room RoomDelegate callbacks that mutate published ephemeral fields are now gated on active lifecycle states and the isTearingDown flag, preventing them from repopulating fields during failure teardown. A shared resetEphemeralFields() helper is called both at the start of teardown and immediately before publishing .failed (and .idle), so any callback delivered during the teardownRoom() await is erased. Six new deterministic tests verify the gating predicate for all state/teardown combinations.

Per-Item Detail

1. Gate old-room callbacks + final reset before .failed publication

Problem: teardownForFailure(message:) cleared ephemeral fields before the async teardownRoom() await, then published .failed without another reset. While teardownRoom() is suspended in room.disconnect(), the old room is still the current identity (self.room === room passes). A participantDidConnect callback can call foundAgent with no lifecycle-state guard, and a .connected → .reconnecting callback sets isReconnecting = true without checking isTearingDown. The result: the final .failed session can carry stale agent or isReconnecting values.

Fix (three parts):

a) Shared resetEphemeralFields() helper

Extracted the 9-field clear into a single private method:

private func resetEphemeralFields() {
    agent = nil
    isMuted = false
    userWantsMicMuted = false
    micError = nil
    models = []
    selectedModel = nil
    isReconnecting = false
    reconnectAttempted = false
    micMutedByInterruption = false
}

Used in three places:

• teardownForFailure: initial pass (before awaits) and final pass (immediately before state = .failed(message)).
• disconnect(): initial pass (before awaits) and final pass (immediately before state = .idle).

The final pass ensures that any old-room callback delivered during the teardownRoom() await (which sets isTearingDown = true and awaits room.disconnect()) is erased before the terminal state is published.

b) Pure predicate shouldAcceptRoomCallback(state:isTearingDown:)

func shouldAcceptRoomCallback(state: VoiceSessionState, isTearingDown: Bool) -> Bool {
    !isTearingDown && (state == .connecting || state == .waitingAgent || state == .ready)
}

Accepted only for active lifecycle states AND only when isTearingDown is false. Rejected for .idle, .tearingDown, .failed, and any state during intentional teardown.

c) Gated RoomDelegate callbacks

• participantDidConnect: Now calls shouldAcceptRoomCallback(state:isTearingDown:) before foundAgent. An old-room participant callback arriving during failure teardown is rejected because isTearingDown == true (set by teardownRoom() before the room.disconnect() await).

• didUpdateConnectionState: The entire handler now returns early if self.isTearingDown is true, with a log message. This covers all connection-state transitions (.connected → .reconnecting, .reconnecting → .connected, → .disconnected) during intentional teardown. For the non-teardown path, the .connected → .reconnecting and .reconnecting → .connected branches additionally guard on active lifecycle states before mutating isReconnecting.

Trace of Finding 1 scenario:

1. teardownForFailure("Connection lost.") called.
2. state = .tearingDown, resetEphemeralFields() clears agent/isReconnecting/etc.
3. await teardownRoom(): sets isTearingDown = true, calls await room.disconnect().
4. During the await, old room delivers participantDidConnect(agent).
5. Callback checks shouldAcceptRoomCallback(state: .tearingDown, isTearingDown: true) → false → returns without calling foundAgent. ✓
6. Even if a callback slipped through (e.g., state was still .ready in an edge case), the final resetEphemeralFields() before state = .failed(message) erases it. ✓
7. state = .failed("Connection lost.") published with clean fields.

Files Changed

• Sources/LiveKit/VoiceSession.swift:
  • Added resetEphemeralFields() private helper (9-field clear)
  • Added shouldAcceptRoomCallback(state:isTearingDown:) pure predicate
  • teardownForFailure: replaced inline field clears with resetEphemeralFields(), added final pass before .failed
  • disconnect(): replaced inline field clears with resetEphemeralFields(), added final pass before .idle
  • participantDidConnect: added shouldAcceptRoomCallback gate before foundAgent
  • didUpdateConnectionState: added early return for isTearingDown, added active-state guards on reconnecting/connected branches
• Tests/ConversationStateTests.swift: 6 new tests for callback gating predicate

Design Decisions

• Double reset (initial + final) vs. single final: The initial reset provides immediate UI feedback (fields clear as soon as teardown starts, not after the potentially slow room.disconnect() await). The final reset is the safety net that guarantees correctness regardless of what callbacks arrive during the await. Both are needed: the initial one improves UX, the final one closes the race.
• shouldAcceptRoomCallback as a method on VoiceSession: Making it an instance method (rather than a free function) allows tests to construct a real VoiceSession and call the predicate without mocking. It is also available for future use by other callback paths if needed.
• Early return for isTearingDown in connection-state handler: Rather than gating each branch individually, the entire handler returns early when isTearingDown is true. This is simpler and more robust: any new connection-state transition added in the future will also be gated automatically. The per-branch active-state guards remain for the non-teardown path to prevent callbacks from .idle or .failed states from mutating fields.
• disconnect() also gets the final reset: The request said "reuse one reset helper from explicit disconnect/start/failure where semantically appropriate." disconnect() has the same race as teardownForFailure: it awaits teardownRoom() and then publishes .idle. A callback during that await could repopulate fields visible in the idle UI before server replacement. The final reset closes this gap.

Verification

• XcodeBuildMCP test_sim (scheme Voice_iOS, iPhone 17 Pro simulator): 65 passed, 0 failed, 0 skipped (⏱️ 9.9s). No compiler warnings.
  • Suites: TranscriptDedupeTests (24), ConversationStateTests (16), MediaReconcileTests (7), ServerReplacementCoordinatorTests (5), PinnedMediaResourceLoaderTests (12), MediaAPITests (1)
  • Log: ~/Library/Developer/XcodeBuildMCP/workspaces/voice-ios-9eb036fdea81/logs/test_sim_2026-08-31T21-57-47-408Z_pid94423_4ab5a91f.log
  • Result bundle: ~/Library/Developer/XcodeBuildMCP/workspaces/voice-ios-9eb036fdea81/result-bundles/test_sim_2026-08-31T21-57-47-409Z_pid94423_16ad0db3.xcresult
• XcodeBuildMCP build_sim: Build succeeded (⏱️ 6.0s). No warnings.
  • Log: ~/Library/Developer/XcodeBuildMCP/workspaces/voice-ios-9eb036fdea81/logs/build_sim_2026-08-31T21-57-39-068Z_pid94423_6f60e1bf.log
• grep -c '@Test(' Tests/*.swift: ConversationState 16, MediaAPI 1, MediaReconcile 7, PinnedMediaResourceLoader 12, ServerReplacementCoordinator 5, TranscriptDedupe 24 = 65 total.
• git diff --check: no whitespace issues.

Risks And Limitations

• The callback gating is verified by the pure predicate test and source inspection, but a full lifecycle test (real LiveKit room delivering callbacks during teardown) would require a live server. The predicate tests cover the decision logic; the actual RoomDelegate invocation path is verified by source inspection of the guard placement.
• Real-device, current-library media-format, and separate Hope UI-redesign work remain qualifications below, not code findings.

Follow-up For Reviewer

• Callback gating: Verify that participantDidConnect checks shouldAcceptRoomCallback before calling foundAgent. Verify that didUpdateConnectionState returns early for isTearingDown and guards reconnecting/connected branches on active states. Confirm the predicate rejects .idle, .tearingDown, .failed, and any state with isTearingDown == true.
• Final reset: Verify that teardownForFailure calls resetEphemeralFields() both before the awaits (initial pass) and immediately before state = .failed(message) (final pass). Same for disconnect() before state = .idle. Confirm the helper clears all 9 ephemeral fields and does NOT touch settings, transcriptHandler.events, or taskStore.
• No regression: The previously accepted mic-chain, server-worker, transcript-dedupe, media-interruption, resource-loader, TLS-pinning, and text-mode-invariant behaviors are unchanged. The only source change is in VoiceSession.swift (callback gating + reset helper) and the test file.