feat(android): decode the two server frames the client was throwing away

Both had shipped on the server for a while and Android silently discarded
them, which is worse than not supporting them — the UI looked correct while
missing information the user needed.

W2 `queue`: `ServerMessageType` had no member for it, so `decodeServer`
returned null and `SessionEngine.onFrame` counted it as undecodable and
dropped it. The "N queued" badge could therefore never appear on Android,
on any device, ever. Adding the branch made the compiler flag the
non-exhaustive `when` in the engine, which is the same gap seen from the
other side. A negative depth drops the frame rather than feeding nonsense
to a badge; the next broadcast carries the real value.

W1 `status.preview`: not modelled at all, so a remote approval was BLIND —
the gate could show that `Bash` wanted to run, but not what it wanted to
run, and the same for an Edit's diff. Approving blind defeats the point of
the remote gate, which exists so whoever is holding the phone can make an
informed call. `ApprovalPreview` now models both variants and
`GateState.preview` carries it to the UI.

Two decode rules are load-bearing and pinned by tests:

  - A broken preview must never cost the FRAME. `pending` is what makes the
    gate appear at all, so trading a missing preview for a missing gate
    would be a bad bargain. Every malformed shape — null, non-object, no
    kind, unknown kind, command without text, diff without a file — degrades
    the preview to null and still yields the Status.
  - Unknown values are kept, not coerced. A diff line kind or file status
    this client has never seen renders as itself; dropping a line we cannot
    classify would hide part of what is being approved. Only a diff file
    missing its paths is dropped, because then the preview cannot be trusted
    to describe the right file.

A sustained pending refreshes the preview without minting a new epoch: the
server's bounded render can arrive a frame after the gate rises, and a
decision tapped in between has to stay valid.

The preview's diff subset lives in :wire-protocol rather than reusing the
:app diff model — :session-core may not depend on :app, and new wire types
belong in the frozen contract module (CLAUDE.md).

Verified: :wire-protocol:test :session-core:test + both koverVerify green;
16 new tests (10 codec, 6 engine-level).
This commit is contained in:
Yaojia Wang
2026-07-30 09:39:50 +02:00
parent 3e5cfdc1cf
commit 3fe719f874
9 changed files with 528 additions and 6 deletions

View File

@@ -1,5 +1,6 @@
package wang.yaojia.webterm.session
import wang.yaojia.webterm.wire.ApprovalPreview
import wang.yaojia.webterm.wire.ApproveMode
import wang.yaojia.webterm.wire.ClientMessage
import wang.yaojia.webterm.wire.GateKind
@@ -22,6 +23,15 @@ public data class GateState(
val detail: String?,
/** Rising-edge counter; see type doc. */
val epoch: Int,
/**
* W1 — the command or diff this gate is asking about, so the user is not approving blind.
*
* `null` is a NORMAL state, not an error: the server only sends a preview for a held Bash/Edit-family
* tool, and a malformed one decodes to null rather than costing us the gate. UI must therefore render
* a previewless gate as an ordinary gate, never as a failure. Every string inside is UNTRUSTED and
* must render inert (plan §8).
*/
val preview: ApprovalPreview? = null,
) {
/** The action set for this gate: plan → three-way, tool → two-way. */
public val affordances: List<Affordance>

View File

@@ -1,5 +1,7 @@
package wang.yaojia.webterm.session
import wang.yaojia.webterm.wire.ApprovalPreview
import wang.yaojia.webterm.wire.GateKind
/**
@@ -29,7 +31,12 @@ public class GateTracker private constructor(
private val lastEpoch: Int,
) {
/** Feeds one `status` frame's gate-relevant fields; returns the next snapshot. */
public fun reduce(pending: Boolean, gate: GateKind?, detail: String?): GateTracker {
public fun reduce(
pending: Boolean,
gate: GateKind?,
detail: String?,
preview: ApprovalPreview? = null,
): GateTracker {
if (!pending) {
// Falling edge (or still idle): gate lifted, epoch counter retained.
return GateTracker(current = null, lastEpoch = lastEpoch)
@@ -38,12 +45,14 @@ public class GateTracker private constructor(
val held = current
if (held != null) {
// Sustained pending: same epoch, refresh kind/detail from the frame.
val refreshed = GateState(kind = kind, detail = detail, epoch = held.epoch)
// Refresh the preview too: the server may send the held status again with a preview it did
// not have on the rising edge (the bounded render can arrive a frame late).
val refreshed = GateState(kind = kind, detail = detail, epoch = held.epoch, preview = preview)
return GateTracker(current = refreshed, lastEpoch = lastEpoch)
}
// Rising edge: mint the next epoch (the ONLY place it increments).
val nextEpoch = lastEpoch + 1
val risen = GateState(kind = kind, detail = detail, epoch = nextEpoch)
val risen = GateState(kind = kind, detail = detail, epoch = nextEpoch, preview = preview)
return GateTracker(current = risen, lastEpoch = nextEpoch)
}

View File

@@ -281,6 +281,7 @@ public class SessionEngine(
is ServerMessage.Output -> { emit(Output(msg.data)); null }
is ServerMessage.Status -> { onStatus(msg); null }
is ServerMessage.Telemetry -> { emit(Telemetry(msg.telemetry)); null }
is ServerMessage.Queue -> { emit(Queued(msg.length)); null }
is ServerMessage.Exit -> {
emit(Exited(msg.code, msg.reason))
EndCause.Terminal
@@ -299,7 +300,7 @@ public class SessionEngine(
private fun onStatus(status: ServerMessage.Status) {
val previous = gateTracker.current
gateTracker = gateTracker.reduce(status.pending, status.gate, status.detail)
gateTracker = gateTracker.reduce(status.pending, status.gate, status.detail, status.preview)
val next = gateTracker.current
if (next != previous) emit(Gate(next))
}

View File

@@ -27,3 +27,13 @@ public data class Digest(val digest: AwayDigest) : SessionEvent
/** Latest statusLine telemetry broadcast (mirrors the server `telemetry` frame, B2). */
public data class Telemetry(val telemetry: StatusTelemetry) : SessionEvent
/**
* W2 — pending-inject queue depth for this session, broadcast to every attached device so all mirrors
* agree on the "N queued" badge.
*
* The server has always sent this frame; Android had no `ServerMessageType` member for it, so
* `decodeServer` returned null and `SessionEngine` discarded it as undecodable. [length] is
* non-negative — the codec drops a negative depth rather than passing nonsense to a badge.
*/
public data class Queued(val length: Int) : SessionEvent

View File

@@ -0,0 +1,168 @@
package wang.yaojia.webterm.session
import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.launch
import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.runTest
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertNotNull
import org.junit.jupiter.api.Assertions.assertNull
import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.DisplayName
import org.junit.jupiter.api.Test
import wang.yaojia.webterm.testsupport.FakeTermTransport
import wang.yaojia.webterm.wire.ApprovalPreview
import wang.yaojia.webterm.wire.HostEndpoint
import wang.yaojia.webterm.wire.TermTransport
/**
* Engine-level proof for the two frames Android used to lose (W1 `status.preview`, W2 `queue`).
*
* `MessageCodec` decoding them is necessary but not sufficient: the reason they were invisible is that
* `SessionEngine.onFrame` is where a decoded frame becomes a `SessionEvent`, and a frame with no branch
* there is discarded just as silently as one that fails to decode. These tests drive the real engine
* through the fake transport so a future refactor that drops the branch fails here.
*/
@DisplayName("SessionEngine — queue depth and approval preview reach the UI")
class SessionEngineQueueAndPreviewTest {
private fun endpoint(): HostEndpoint = HostEndpoint.fromBaseUrl("http://localhost:3000")!!
private fun TestScope.newEngine(transport: TermTransport): SessionEngine = SessionEngine(
transport = transport,
endpoint = endpoint(),
scope = backgroundScope,
nowMs = { 0L },
)
private fun TestScope.collectEvents(engine: SessionEngine): List<SessionEvent> {
val seen = mutableListOf<SessionEvent>()
backgroundScope.launch { engine.events.collect { seen += it } }
return seen
}
private fun TestScope.started(transport: FakeTermTransport): Pair<SessionEngine, List<SessionEvent>> {
val engine = newEngine(transport)
val events = collectEvents(engine)
engine.start()
testScheduler.runCurrent()
return engine to events
}
@Test
@DisplayName("a queue frame becomes a Queued event instead of being dropped")
fun queueFrameSurfaces() = runTest {
val transport = FakeTermTransport()
val (_, events) = started(transport)
transport.emit("""{"type":"queue","length":4}""")
testScheduler.runCurrent()
assertEquals(
listOf(4),
events.filterIsInstance<Queued>().map { it.length },
"before this fix `queue` had no ServerMessageType member, so decodeServer returned null " +
"and onFrame discarded it as undecodable — the badge could never appear",
)
}
@Test
@DisplayName("a malformed queue frame is dropped without disturbing the stream")
fun malformedQueueIsInert() = runTest {
val transport = FakeTermTransport()
val (_, events) = started(transport)
transport.emit("""{"type":"queue","length":-3}""")
transport.emit("""{"type":"queue"}""")
transport.emit("""{"type":"queue","length":1}""")
testScheduler.runCurrent()
assertEquals(listOf(1), events.filterIsInstance<Queued>().map { it.length })
}
@Test
@DisplayName("a held gate carries the command preview, so the user is not approving blind")
fun gateCarriesCommandPreview() = runTest {
val transport = FakeTermTransport()
val (_, events) = started(transport)
transport.emit(
"""{"type":"status","status":"waiting","pending":true,"gate":"tool","detail":"Bash",
"preview":{"kind":"command","text":"git push --force","truncated":false}}""",
)
testScheduler.runCurrent()
val gate = events.filterIsInstance<Gate>().last().gate
assertNotNull(gate, "the gate itself must still be emitted")
assertEquals(
ApprovalPreview.Command("git push --force", truncated = false),
gate!!.preview,
"the whole point of W1: show WHAT is being approved, not just the tool name",
)
}
@Test
@DisplayName("a preview that arrives a frame late is picked up on the sustained status")
fun previewCanArriveLate() = runTest {
val transport = FakeTermTransport()
val (_, events) = started(transport)
// Rising edge with no preview yet — the server's bounded render is not always ready in time.
transport.emit("""{"type":"status","status":"waiting","pending":true,"gate":"tool"}""")
testScheduler.runCurrent()
val risen = events.filterIsInstance<Gate>().last().gate!!
assertNull(risen.preview)
transport.emit(
"""{"type":"status","status":"waiting","pending":true,"gate":"tool",
"preview":{"kind":"command","text":"ls -la"}}""",
)
testScheduler.runCurrent()
val refreshed = events.filterIsInstance<Gate>().last().gate!!
assertEquals(ApprovalPreview.Command("ls -la"), refreshed.preview)
assertEquals(
risen.epoch,
refreshed.epoch,
"a sustained pending must NOT mint a new epoch — a decision tapped before the preview " +
"landed has to stay valid, or the preview arriving would silently invalidate the tap",
)
}
@Test
@DisplayName("a broken preview still yields the gate")
fun brokenPreviewStillGates() = runTest {
val transport = FakeTermTransport()
val (_, events) = started(transport)
transport.emit(
"""{"type":"status","status":"waiting","pending":true,"gate":"tool","preview":{"kind":"nope"}}""",
)
testScheduler.runCurrent()
val gate = events.filterIsInstance<Gate>().lastOrNull()?.gate
assertNotNull(gate, "losing the gate because its preview was malformed would be the worse bug")
assertNull(gate!!.preview)
}
@Test
@DisplayName("lifting the gate clears the preview with it")
fun liftingClearsPreview() = runTest {
val transport = FakeTermTransport()
val (_, events) = started(transport)
transport.emit(
"""{"type":"status","status":"waiting","pending":true,"gate":"tool",
"preview":{"kind":"command","text":"rm -rf /"}}""",
)
testScheduler.runCurrent()
transport.emit("""{"type":"status","status":"working","pending":false}""")
testScheduler.runCurrent()
assertNull(
events.filterIsInstance<Gate>().last().gate,
"a resolved gate must not leave a stale command on screen",
)
assertTrue(events.filterIsInstance<Gate>().size >= 2)
}
}