diff --git a/.claude/rules/turma-sessions.md b/.claude/rules/turma-sessions.md index d358edb9..caa63467 100644 --- a/.claude/rules/turma-sessions.md +++ b/.claude/rules/turma-sessions.md @@ -100,7 +100,10 @@ Split out of `.claude/rules/turma.md` (which covers the rest of the hub UI) to k session is marked (🏠, warn colour) β€” it is a weaker model, and nobody should have to wonder which one wrote a turn. Like the mode switch it paints from a MEMO, never an optimistic write onto `sess`, so a stale beat can't flash the old value back; the memo ages out so a switch that never - lands doesn't pin the chip. Tests: the `model source:` cases in `chat.test.js`. + lands doesn't pin the chip. **`normalizeLocalModel` coerces the block at ingest** β€” the block is + typed on Android and `/api/agents` decodes atomically there, so one host's `available:"yes"` hid + the whole fleet from the phone; see CLAUDE.md's heartbeat contract. Tests: the `model source:` + cases in `chat.test.js`, `normalizeLocalModel` in `server.test.js`. - The compose footer's agent-mode / model selectors are joined by a compact **PR status chip** (`prFooterChip`) when it has one, and a `jira-chip` when the session has a ticket. - The **model selector is accurate** (XERK-33) β€” never a hardcoded menu, and never rewriting the diff --git a/CLAUDE.md b/CLAUDE.md index d794c788..a2e82ed6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -226,6 +226,33 @@ Rules spanning more than one component, so no `paths:`-scoped file can carry the it every client). A field older agents don't send must degrade, never break: clients gate on the capability flag the agent reports (`inputMaxChars`, `uploadMaxBytes`, `github.available`, `capacity`), and an absent flag means "that agent can't do it", not "unlimited". + - **A full `/api/agents` decode is ATOMIC on Android**, so one host's wrong-typed field throws for + the whole array β€” the poll fails silently while the app keeps its last snapshot and the tile + still says "N / N online". Per-agent SSE events decode individually, so the bad host is simply + missing from the list while SSE is healthy; with SSE down too, the raw decoder exception + replaces the screen. + - **A field becomes decode-fatal the moment a client TYPES it** β€” until then `ignoreUnknownKeys` + skips it and any value is harmless. So typing one on `SessionInfo`/`AgentInfo` and adding its + hub-side coercion are the SAME change; `normalizeRecord` is where it goes, and it runs on both + the heartbeat ingest and the `state.json` restore (a restart is when a coercion ships, and the + restore is the first thing it serves). Coerce to the "can't tell you" value every client already + handles, never to a plausible default. + - **The whole record is held to that shape by `turma/wire-shape.js`** (XERK-259), a table mirroring + `Models.kt` β€” per-block `normalize*`s covered only what someone had got to, and `repoUsage:[null]` + from one host stopped the phone signing in at all. A LIST's ELEMENTS are as fatal as its type, + and `typeof [] === "object"`, so element tests go through `isPlainObject`. + - It coerces **IN PLACE, touching only the keys it names**, which is what keeps it from being a + whitelist: a sub-key a newer agent adds rides through untouched, where rebuilding an object + drops it fleet-wide until the table catches up (`normalizeLimits`/`normalizeLocalModel` DO + rebuild, so a new sub-key of theirs must be added to them). + - **A block those two rebuild is in the table anyway** β€” a rebuild is only as good as its own + gates, and `limits` shipped one gating its epoch fields on `Number.isFinite`, so a fractional + `resetsAt` (Kotlin `Long` takes no fraction) went out raw. Nothing agent-authored is exempt. + - Its own module because the restore runs at `server.js` module init: a `const` declared below + that point is in its temporal dead zone, and the ReferenceError dies in the restore's own + `catch {}`, leaving records half-coerced with nothing logged. + - Typing a field in `Models.kt` without adding it there **fails the hub's suite** β€” the test + parses `Models.kt` and walks it, so this pairing is enforced, not remembered. - **A hub refusal must reach the operator, in the hub's own words** (XERK-264). The hub refuses commands with a status and a JSON `{error}` body (409 org mismatch / unsupported agent, 503 host offline, 404 stale attachment, 413 too long, 429 queue full); a client that reads the body and diff --git a/android/PARITY.md b/android/PARITY.md index da51d02a..969c0181 100644 --- a/android/PARITY.md +++ b/android/PARITY.md @@ -28,6 +28,14 @@ are recorded under "Deliberate differences" below, not left to look like gaps. - **Hub-URL field on Login.** The web is same-origin; a phone app must point at any hub, so Login has an extra Hub-URL field. - **Voice dictation** into the spawn/compose fields β€” a phone-only addition. +- **No hover tooltips on the two compose-bar model chips** (XERK-246); a phone has no hover, so the + web's `title=` text goes to the accessibility layer or nowhere. + - The **"run against"** chip: nowhere. Its tooltip only names the self-hosted model, which is + already the chip's own text. + - The **fixed-model** chip on a local session: it carries the web's wording as a + `contentDescription` instead. That tooltip is not redundant β€” it explains why the chip is inert + and names the way out β€” so dropping it entirely would leave a dead-looking control beside two + live ones with no account of itself. - **Ticket-detail fields tap-to-change** (XERK-138 follow-up). The web detail panel shows each editable field's value beside a separate "Change" link/control that swaps in a ``. [optionLabel] separates what a row READS from the + * value it SENDS, for a field whose wire values aren't labels ("subscription" β†’ + * "Claude subscription"); by default they are the same string. + */ @OptIn(ExperimentalMaterial3Api::class) @Composable -fun DropdownField(label: String, options: List, selected: String, onSelect: (String) -> Unit) { +fun DropdownField( + label: String, + options: List, + selected: String, + optionLabel: (String) -> String = { it }, + onSelect: (String) -> Unit, +) { var expanded by remember { mutableStateOf(false) } ExposedDropdownMenuBox(expanded = expanded, onExpandedChange = { expanded = it }) { OutlinedTextField( - value = selected, + value = optionLabel(selected), onValueChange = {}, readOnly = true, label = { Text(label) }, @@ -104,7 +157,7 @@ fun DropdownField(label: String, options: List, selected: String, onSele ) ExposedDropdownMenu(expanded = expanded, onDismissRequest = { expanded = false }) { options.forEach { opt -> - DropdownMenuItem(text = { Text(opt) }, onClick = { onSelect(opt); expanded = false }) + DropdownMenuItem(text = { Text(optionLabel(opt)) }, onClick = { onSelect(opt); expanded = false }) } } } diff --git a/android/app/src/main/java/com/xerktech/turma/ui/FleetScreen.kt b/android/app/src/main/java/com/xerktech/turma/ui/FleetScreen.kt index df4dcd35..100a3289 100644 --- a/android/app/src/main/java/com/xerktech/turma/ui/FleetScreen.kt +++ b/android/app/src/main/java/com/xerktech/turma/ui/FleetScreen.kt @@ -155,9 +155,14 @@ fun FleetScreen( spawnFor?.let { (host, repo, isRoot) -> SpawnDialog( host = host, repo = repo, isRoot = isRoot, + // The TARGET host's own local model, not the fleet's: the failover + // is configured per host, so only the one being spawned on can + // offer it. Pure + tested, because "the wrong loop" is a shape this + // repo has shipped before. + localModel = com.xerktech.turma.core.ModelSource.hostLocalModel(fleet.agents, host), onDismiss = { spawnFor = null }, - onSpawn = { prompt, label, baseRef, model, mode -> - vm.spawn(host, repo, prompt, label, baseRef, model, mode); spawnFor = null + onSpawn = { prompt, label, baseRef, model, mode, source -> + vm.spawn(host, repo, prompt, label, baseRef, model, mode, source); spawnFor = null }, ) } diff --git a/android/app/src/main/java/com/xerktech/turma/ui/SessionsScreen.kt b/android/app/src/main/java/com/xerktech/turma/ui/SessionsScreen.kt index 5ba1d3c0..ec331294 100644 --- a/android/app/src/main/java/com/xerktech/turma/ui/SessionsScreen.kt +++ b/android/app/src/main/java/com/xerktech/turma/ui/SessionsScreen.kt @@ -34,13 +34,13 @@ import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Scaffold +import androidx.compose.material3.SnackbarHost +import androidx.compose.material3.SnackbarHostState import androidx.compose.material3.Text import androidx.compose.material3.TopAppBar import androidx.compose.material3.VerticalDivider import androidx.compose.runtime.Composable import androidx.compose.runtime.DisposableEffect -import androidx.compose.material3.SnackbarHost -import androidx.compose.material3.SnackbarHostState import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.key @@ -475,6 +475,9 @@ fun SessionsListPane( // dashboard used to collect: a refusal raised here reached nobody, which is // the same "it looked like it worked" bug the hub's `{error}` text exists to // prevent (XERK-264). One host, like FleetScreen's. + // XERK-246 needs it for the same reason from the other direction: the spawn + // composer's "Run against" row gives this pane a FIRST-CLASS refusal (409 + // "host has no local model configured"), not just a network failure. val snackbar = remember { SnackbarHostState() } LaunchedEffect(Unit) { vm.messages.collect { snackbar.showSnackbar(it) } } val fleet by vm.fleet.collectAsStateWithLifecycle() @@ -519,7 +522,6 @@ fun SessionsListPane( // New-session picker: pick an online host + repo, then the spawn composer. var pickerOpen by remember { mutableStateOf(false) } var spawnFor by remember { mutableStateOf?>(null) } - Box(modifier.fillMaxSize()) { Column(Modifier.fillMaxSize()) { ScreenHeader("Sessions") { @@ -724,9 +726,14 @@ fun SessionsListPane( spawnFor?.let { (host, repo, isRoot) -> SpawnDialog( host = host, repo = repo, isRoot = isRoot, + // The TARGET host's own local model, not the fleet's: the failover + // is configured per host, so only the one being spawned on can + // offer it. Pure + tested, because "the wrong loop" is a shape this + // repo has shipped before. + localModel = com.xerktech.turma.core.ModelSource.hostLocalModel(fleet.agents, host), onDismiss = { spawnFor = null }, - onSpawn = { prompt, label, baseRef, model, mode -> - vm.spawn(host, repo, prompt, label, baseRef, model, mode); spawnFor = null + onSpawn = { prompt, label, baseRef, model, mode, source -> + vm.spawn(host, repo, prompt, label, baseRef, model, mode, source); spawnFor = null }, ) } diff --git a/android/app/src/main/java/com/xerktech/turma/vm/ChatViewModel.kt b/android/app/src/main/java/com/xerktech/turma/vm/ChatViewModel.kt index 0c66d1dd..edfd96d2 100644 --- a/android/app/src/main/java/com/xerktech/turma/vm/ChatViewModel.kt +++ b/android/app/src/main/java/com/xerktech/turma/vm/ChatViewModel.kt @@ -10,6 +10,7 @@ import android.net.Uri import android.provider.OpenableColumns import com.xerktech.turma.core.AttachStatus import com.xerktech.turma.core.Attachment +import com.xerktech.turma.core.ModelSource import com.xerktech.turma.core.Uploads import com.xerktech.turma.core.Verbosity import com.xerktech.turma.core.VerbosityPrefs @@ -17,6 +18,7 @@ import com.xerktech.turma.core.entryTruncated import com.xerktech.turma.core.mergeTail import com.xerktech.turma.core.prependHistory import com.xerktech.turma.core.tunnelOnlineOf +import com.xerktech.turma.model.AgentInfo import com.xerktech.turma.model.SessionInfo import com.xerktech.turma.model.TailEntry import com.xerktech.turma.model.TurnStatus @@ -27,6 +29,8 @@ import com.xerktech.turma.net.InputRequest import com.xerktech.turma.net.LiveEvent import com.xerktech.turma.net.ModeRequest import com.xerktech.turma.net.ModelRequest +import com.xerktech.turma.net.ModelSourceRequest +import com.xerktech.turma.net.OkResponse import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext import okhttp3.MediaType.Companion.toMediaType @@ -74,6 +78,11 @@ data class ChatUiState( // The host this session's agent runs on, shown in the header (XERK-121): // the agent's device name, falling back to its registration key. val hostLabel: String = "", + // This host's self-hosted model (XERK-246) and an unconfirmed switch onto or + // off it. Null localModel = its agent can't fail over, which is what hides + // the "run against" chip rather than offering a command the host would drop. + val localModel: com.xerktech.turma.model.LocalModelInfo? = null, + val modelSourcePending: ModelSource.Pending? = null, ) { val prefs: VerbosityPrefs get() = VerbosityPrefs.forPreset(verbosity) val question: String get() = session?.session?.question ?: "" @@ -86,6 +95,35 @@ data class ChatUiState( // Attaching is off while a question is pending: the draft then routes to // POST .../answer, which carries no files (web chat.js renderAttachments). val canAttach: Boolean get() = Uploads.canAttach(uploadMaxBytes) && question.isBlank() + + /** + * Which model this session runs against, taking an unconfirmed switch at its + * word until the heartbeat agrees. Read with a caller-supplied clock so the + * memo can age out on a repaint rather than only on the next state change. + */ + fun modelSource(now: Long = System.currentTimeMillis()): String = + ModelSource.current(session, modelSourcePending, now) + + fun canSwitchModelSource(now: Long = System.currentTimeMillis()): Boolean = + ModelSource.offered(localModel, modelSource(now)) + + /** + * Everything this screen takes from a fleet beat, in one place. + * + * Extracted from the two callers (the poll collector and the initial seed) + * so the set of carried fields is pinned by a test rather than by whoever + * last resolved a merge on that `copy(...)`. A field silently dropped there + * disables its whole feature β€” [localModel] going missing hides both + * local-model controls forever β€” and nothing else in the suite notices, + * because a Composable's body has no gate at all. + */ + fun fromFleet(agent: AgentInfo?, session: SessionInfo?, host: String): ChatUiState = copy( + session = session, + hostLabel = agent?.device?.ifBlank { host } ?: host, + tunnelOnline = tunnelOnlineOf(agent), + uploadMaxBytes = agent?.uploadMaxBytes ?: 0, + localModel = agent?.localModel, + ) } class ChatViewModel( @@ -115,11 +153,64 @@ class ChatViewModel( */ private val draft = container.drafts.of(host, sessionId) + /** + * The in-flight model-source switch, held in the container for the SAME + * reason as [draft] (XERK-246): this ViewModel is scoped to the chat's nav + * entry, so a memo kept here died the moment you walked back to the session + * list β€” mid-switch, which is when it is doing its job. + */ + private val modelSwitch = container.modelSwitches.of(host, sessionId) + init { // Collected on viewModelScope (not the onEnter/onLeave jobs): the mirror // must survive a detail-pane swap, or a re-entry would paint an empty box // until the next keystroke. viewModelScope.launch { draft.collect { text -> _state.update { it.copy(draft = text) } } } + viewModelScope.launch { + modelSwitch.collect { p -> + _state.update { it.copy(modelSourcePending = p) } + armMemoExpiry(p) + } + } + } + + /** + * Wake once when the outstanding memo ages out and retire it (XERK-246). + * + * Without this the TTL is only ever observed by whoever next reads the clock, + * and on a quiet fleet nobody does β€” Compose skips recomposition while the + * state compares equal, so an expired memo stays painted indefinitely. The + * heartbeat's own `settle` cannot cover it either: it runs on fleet + * emissions, which is exactly what a quiet fleet does not produce. + * + * Collected off the STORE rather than set alongside each POST, so a memo + * carried in from another nav entry is armed too β€” that is the case the + * store exists for. Self-cancelling and bounded: one alarm per memo, none at + * all when there is no memo. + */ + private var memoExpiryJob: Job? = null + + private fun armMemoExpiry(pending: ModelSource.Pending?) { + memoExpiryJob?.cancel() + if (pending == null) return + memoExpiryJob = viewModelScope.launch { + val left = ModelSource.SWITCH_SETTLE_MS - (System.currentTimeMillis() - pending.at) + if (left > 0) delay(left) + // Retire by IDENTITY, never by re-asking the clock. `delay` measures + // elapsed UPTIME while `expired` re-reads the WALL clock, and a + // backward wall-clock jump between the two makes the re-check false: + // `settle` then returns the same instance, `MutableStateFlow` does + // not emit an equal value, the collector never runs, and no new + // alarm is armed β€” so the memo is pinned until the clock catches up. + // Measured with a 10-minute backward jump: the chip still claimed + // the subscription at t+190s while the record said `local`. + // + // This alarm's only job is the TTL, so it does not need `settle`'s + // other rules: the heartbeat-agreement case is handled by the fleet + // collector, and `compareAndSet` no-ops if a newer switch (a + // different `at`) or a settle has already replaced this memo. + modelSwitch.compareAndSet(pending, null) + } } private var liveJob: Job? = null @@ -160,12 +251,12 @@ class ChatViewModel( container.fleet.state.collect { fleet -> val agent = fleet.agents.firstOrNull { it.key == host } val session = agent?.sessions?.firstOrNull { it.id == sessionId } - val label = agent?.device?.ifBlank { host } ?: host - _state.update { - it.copy(session = session, hostLabel = label, - tunnelOnline = tunnelOnlineOf(agent), - uploadMaxBytes = agent?.uploadMaxBytes ?: 0) - } + // Retire a memo the heartbeat has caught up with, through the + // store β€” the state copy is a mirror, so clearing only that + // would let the next emission paint the stale memo back. + modelSwitch.value = + ModelSource.settle(modelSwitch.value, session, System.currentTimeMillis()) + _state.update { it.fromFleet(agent, session, host) } session?.session?.tail?.takeIf { it.isNotEmpty() }?.let { seed -> _state.update { it.copy(entries = mergeTail(it.entries, seed)) } } @@ -176,13 +267,11 @@ class ChatViewModel( private fun seedFromFleet() { val agent = container.fleet.state.value.agents.firstOrNull { it.key == host } val session = agent?.sessions?.firstOrNull { it.id == sessionId } - val label = agent?.device?.ifBlank { host } ?: host val seed = session?.session?.tail ?: emptyList() _state.update { - it.copy(session = session, hostLabel = label, - tunnelOnline = tunnelOnlineOf(agent), - uploadMaxBytes = agent?.uploadMaxBytes ?: 0, - entries = mergeTail(it.entries, seed)) + it.fromFleet(agent, session, host) + .copy(modelSourcePending = modelSwitch.value, + entries = mergeTail(it.entries, seed)) } } @@ -451,11 +540,30 @@ class ChatViewModel( * unconditional "βœ“ queued" (XERK-264), so a command that never ran read as * one that did. Retrofit throws a non-2xx as an HttpException carrying the * body, which [hubErrorMessage] reads; only a transport failure is generic. + * + * XERK-246 arrived at the same helper independently and merged INTO this one + * rather than beside it β€” two report paths is how the halves drift. It adds + * the `OkResponse.error` read (a refusal the hub chose to answer 200 with, + * which a status-only branch calls a success β€” the same bug one layer in, + * and the one `FleetViewModel.run` already avoided) and an optional + * route-specific [failed], since "hub unreachable" is wrong for a request + * the hub answered. Every caller here, `kill()` included, gets both. */ - private suspend fun report(ok: String, block: suspend () -> Unit) { + private suspend fun report( + ok: String, + failed: String = "hub unreachable", + block: suspend () -> OkResponse, + ) { val r = runCatching { block() } - if (r.isSuccess) _messages.tryEmit("βœ“ $ok") - else _messages.tryEmit("βœ— " + (r.exceptionOrNull()?.let { hubErrorMessage(it) } ?: "hub unreachable")) + _messages.tryEmit( + ModelSource.outcomeMessage( + ok = r.isSuccess, + bodyError = r.getOrNull()?.error, + hubMessage = r.exceptionOrNull()?.let { hubErrorMessage(it) }, + queued = "βœ“ $ok", + failed = failed, + ) + ) } /** Interrupt the in-flight turn (web "β—Ό Stop" β€” POST .../interrupt). */ @@ -489,11 +597,50 @@ class ChatViewModel( } fun setModel(model: String) = viewModelScope.launch { - report("model queued") { client.api.setModel(host, sessionId, ModelRequest(model)) } + report("model queued", "could not set the model") { + client.api.setModel(host, sessionId, ModelRequest(model)) + } } fun setMode(mode: String) = viewModelScope.launch { - report("mode queued") { client.api.setMode(host, sessionId, ModeRequest(mode)) } + report("mode queued", "could not set the mode") { + client.api.setMode(host, sessionId, ModeRequest(mode)) + } + } + + /** + * Move this session between the subscription and the host's self-hosted + * model (XERK-246). The agent relaunches Claude with `--resume`, so the + * conversation, worktree and branch carry over β€” but that takes several + * beats, hence the memo the chip paints from meanwhile. + * + * A refusal DROPS the memo instead of letting it age out: the hub 409s when + * the host has no local model, and a chip that keeps claiming a switch that + * was rejected is worse than one that never moved. + */ + fun setModelSource(source: String) = viewModelScope.launch { + if (source == _state.value.modelSource()) return@launch + modelSwitch.value = ModelSource.Pending(sessionId, source, System.currentTimeMillis()) + val res = runCatching { client.api.setModelSource(host, sessionId, ModelSourceRequest(source)) } + val bodyError = res.getOrNull()?.error + // ONE verdict drives both the wording and the memo. A refusal the hub + // answered 200 with has to drop the memo exactly as a 409 does β€” a chip + // that keeps claiming a switch the hub refused is the lie this whole + // memo is bounded to prevent, and it must not depend on how the refusal + // was spelled on the wire. + val ok = ModelSource.accepted(res.isSuccess, bodyError) + _messages.tryEmit( + ModelSource.outcomeMessage( + ok = res.isSuccess, + bodyError = bodyError, + hubMessage = res.exceptionOrNull()?.let { hubErrorMessage(it) }, + queued = if (source == ModelSource.LOCAL) "βœ“ switching to the local model…" + else "βœ“ switching back to the subscription…", + failed = "could not switch model", + ) + ) + if (ok) container.fleet.nudge() + modelSwitch.value = ModelSource.afterAttempt(modelSwitch.value, ok) } // ---- voice dictation into the draft -------------------------------------- diff --git a/android/app/src/main/java/com/xerktech/turma/vm/FleetViewModel.kt b/android/app/src/main/java/com/xerktech/turma/vm/FleetViewModel.kt index 55268dfd..9ea5c305 100644 --- a/android/app/src/main/java/com/xerktech/turma/vm/FleetViewModel.kt +++ b/android/app/src/main/java/com/xerktech/turma/vm/FleetViewModel.kt @@ -4,6 +4,7 @@ import android.app.Application import androidx.lifecycle.AndroidViewModel import androidx.lifecycle.viewModelScope import com.xerktech.turma.TurmaApplication +import com.xerktech.turma.core.ModelSource import com.xerktech.turma.net.AnswerRequest import com.xerktech.turma.net.CloneRequest import com.xerktech.turma.net.InputRequest @@ -11,6 +12,7 @@ import com.xerktech.turma.net.MigrateRequest import com.xerktech.turma.net.ModeRequest import com.xerktech.turma.net.ModelRequest import com.xerktech.turma.net.OkResponse +import com.xerktech.turma.net.hubErrorMessage import com.xerktech.turma.net.ResumeRequest import com.xerktech.turma.net.SpawnRequest import com.xerktech.turma.net.SummaryRequest @@ -83,6 +85,10 @@ class FleetViewModel(app: Application) : AndroidViewModel(app) { } catch (e: Exception) { "βœ— " + (hubErrorMessage(e) ?: "hub unreachable") } + // Main landed this same `hubErrorMessage` line independently under + // XERK-264, so the two sides agree on the wording; the docstring + // above now carries the reason this branch wrote it for. What is + // ONLY on main's side is the pendKeys clearing β€” keep it. if (msg.startsWith("βœ—") && pendKeys.isNotEmpty()) { _pending.value = _pending.value - pendKeys.toSet() } @@ -105,17 +111,11 @@ class FleetViewModel(app: Application) : AndroidViewModel(app) { fun spawn( host: String, repo: String, prompt: String? = null, label: String? = null, baseRef: String? = null, model: String? = null, permissionMode: String? = null, + modelSource: String? = null, ) = run("session queued") { container.client.api.spawnSession( host, - SpawnRequest( - repo = repo, - prompt = prompt?.ifBlank { null }, - label = label?.ifBlank { null }, - baseRef = baseRef?.ifBlank { null }, - model = model?.ifBlank { null }, - permissionMode = permissionMode?.ifBlank { null }, - ), + spawnRequest(repo, prompt, label, baseRef, model, permissionMode, modelSource), ) } @@ -178,6 +178,32 @@ class FleetViewModel(app: Application) : AndroidViewModel(app) { companion object { fun pendKey(host: String, id: String) = "$host::$id" + /** + * The body a "New session" spawn posts. Pure, and separate from [spawn] + * so the wire shape is pinned by a test rather than only by driving the + * app: every blank optional is omitted, so a bare one-click spawn queues + * exactly `{repo}` as it always did. + * + * `modelSource` is sent ONLY for the local model (XERK-246) β€” + * "subscription" is what a spawn already meant. `model` is sent whatever + * the source, matching the web composer: the agent drops `--model` for a + * local session itself, and the alias is what that session goes back to + * if it is later switched to the subscription. + */ + fun spawnRequest( + repo: String, prompt: String? = null, label: String? = null, + baseRef: String? = null, model: String? = null, permissionMode: String? = null, + modelSource: String? = null, + ) = SpawnRequest( + repo = repo, + prompt = prompt?.ifBlank { null }, + label = label?.ifBlank { null }, + baseRef = baseRef?.ifBlank { null }, + model = model?.ifBlank { null }, + permissionMode = permissionMode?.ifBlank { null }, + modelSource = ModelSource.spawnValue(modelSource), + ) + /** The in-flight action kind for a session, or null (web sessPending). */ fun sessPending(pending: Map, host: String, id: String): String? = pending[pendKey(host, id)]?.kind diff --git a/android/app/src/test/java/com/xerktech/turma/core/ModelSourceTest.kt b/android/app/src/test/java/com/xerktech/turma/core/ModelSourceTest.kt new file mode 100644 index 00000000..a91ed8e5 --- /dev/null +++ b/android/app/src/test/java/com/xerktech/turma/core/ModelSourceTest.kt @@ -0,0 +1,217 @@ +package com.xerktech.turma.core + +import com.xerktech.turma.model.AgentInfo +import com.xerktech.turma.model.LocalModelInfo +import com.xerktech.turma.model.SessionInfo +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The local-model failover control (XERK-246), ported from web `chat.js`. + * The rules that matter are the two gates β€” a host that can't fail over must not + * be offered the switch, and a session already on the local model must always + * keep a way back β€” plus the memo that stops a slow relaunch reading as a dead + * button without letting it lie forever. + */ +class ModelSourceTest { + + private val configured = LocalModelInfo(available = true, model = "gpt-oss:120b", contextTokens = 81920) + private val unconfigured = LocalModelInfo(available = false) + + @Test fun `the control follows the host's capability flag`() { + assertTrue(ModelSource.offered(configured, ModelSource.SUBSCRIPTION)) + assertFalse(ModelSource.offered(unconfigured, ModelSource.SUBSCRIPTION)) + // An agent predating the failover reports no block at all β€” "cannot", + // never "assume it can". + assertFalse(ModelSource.offered(null, ModelSource.SUBSCRIPTION)) + } + + @Test fun `a session already local keeps the control after its host loses the config`() { + // Otherwise it is stranded on the weaker model with no way back. + assertTrue(ModelSource.offered(null, ModelSource.LOCAL)) + assertTrue(ModelSource.offered(unconfigured, ModelSource.LOCAL)) + } + + @Test fun `a blank model source reads as the subscription`() { + assertEquals(ModelSource.SUBSCRIPTION, ModelSource.current(SessionInfo(id = "s1"), null, 0)) + assertEquals(ModelSource.SUBSCRIPTION, ModelSource.current(null, null, 0)) + assertEquals( + ModelSource.LOCAL, + ModelSource.current(SessionInfo(id = "s1", modelSource = "local"), null, 0), + ) + } + + @Test fun `an unconfirmed switch paints its own value until it settles`() { + val sess = SessionInfo(id = "s1", modelSource = "subscription") + val pending = ModelSource.Pending("s1", ModelSource.LOCAL, at = 1_000) + // The relaunch takes several beats; without the memo the chip springs + // back and reads as a control that did nothing. + assertEquals(ModelSource.LOCAL, ModelSource.current(sess, pending, 1_500)) + // ...but it ages out, so a switch that never lands can't pin it on a lie. + // The boundary is a LITERAL, not `at + SWITCH_SETTLE_MS + 1`: derived + // from the constant it only bounds the TTL from below, and raising it to + // 16.7 hours β€” a chip pinned on a lie for the rest of the day, the exact + // failure the constant exists to prevent β€” would keep the test green. + assertEquals(60_000L, ModelSource.SWITCH_SETTLE_MS) + assertEquals(ModelSource.LOCAL, ModelSource.current(sess, pending, 60_999)) + assertEquals(ModelSource.SUBSCRIPTION, ModelSource.current(sess, pending, 61_001)) + } + + @Test fun `a memo for another session never paints this one`() { + val sess = SessionInfo(id = "s1", modelSource = "subscription") + val other = ModelSource.Pending("s2", ModelSource.LOCAL, at = 1_000) + assertEquals(ModelSource.SUBSCRIPTION, ModelSource.current(sess, other, 1_100)) + } + + @Test fun `a memo retires on the heartbeat agreeing, not on a blind timer`() { + val p = ModelSource.Pending("s1", ModelSource.LOCAL, at = 1_000) + val inside = 1_500L + // Heartbeat still reports the old value: hold. + assertEquals(p, ModelSource.settle(p, SessionInfo(id = "s1", modelSource = "subscription"), inside)) + // Heartbeat caught up: retire, well inside the TTL. + assertNull(ModelSource.settle(p, SessionInfo(id = "s1", modelSource = "local"), inside)) + // A memo is never judged by a DIFFERENT session's record. + assertEquals(p, ModelSource.settle(p, SessionInfo(id = "s2", modelSource = "local"), inside)) + assertEquals(p, ModelSource.settle(p, null, inside)) + assertNull(ModelSource.settle(null, SessionInfo(id = "s1"), inside)) + } + + @Test fun `an expired memo is retired from the store, not merely ignored on read`() { + // The TTL cannot live only in `current`, which is read from a Composable + // body: Compose skips recomposition while the state compares equal, so on + // a quiet fleet nothing re-reads the clock and the expired value stays on + // screen. Measured as the whole "run against" control vanishing on an + // unconfirmed switch and never returning β€” t+120s and still gone. + // Retiring the memo is a STATE CHANGE, which is what repaints it. + val p = ModelSource.Pending("s1", ModelSource.LOCAL, at = 1_000) + val held = SessionInfo(id = "s1", modelSource = "subscription") + assertEquals(p, ModelSource.settle(p, held, 60_999)) + assertNull(ModelSource.settle(p, held, 61_001)) + // Ageing out is about the MEMO, so it applies even when this session's + // record cannot speak to it β€” otherwise a memo left by a switch on + // another session would never be collected at all. + assertEquals(p, ModelSource.settle(p, SessionInfo(id = "s2"), 60_999)) + assertNull(ModelSource.settle(p, SessionInfo(id = "s2"), 61_001)) + assertNull(ModelSource.settle(p, null, 61_001)) + // `expired` is the same boundary `current` honours, so the memo can never + // be retired while it is still being painted, nor painted after it is + // retired. Both directions asserted at the boundary. + assertEquals(false, ModelSource.expired(p, 60_999)) + assertEquals(true, ModelSource.expired(p, 61_001)) + assertEquals(false, ModelSource.expired(null, 61_001)) + // The EXACT boundary, not just either side of it: asserting 60_999 and + // 61_001 alone leaves `>=` vs `>` a free choice, and that mutation + // survived a battery. The TTL is inclusive β€” at exactly one full + // SWITCH_SETTLE_MS the memo is spent. + assertEquals(true, ModelSource.expired(p, 61_000)) + assertNull(ModelSource.settle(p, held, 61_000)) + assertEquals(ModelSource.SUBSCRIPTION, ModelSource.current(held, p, 61_000)) + assertEquals(ModelSource.LOCAL, ModelSource.current(held, p, 60_999)) + assertEquals(ModelSource.SUBSCRIPTION, ModelSource.current(held, p, 61_001)) + } + + @Test fun `a refused command reports the hub's own words, never a blanket queued`() { + // setModel/setMode discarded their result and always said "βœ“ queued", so + // the 409 the hub added for a session on the self-hosted model β€” added + // precisely so an out-of-parity client could not silently drop the + // command β€” painted as a success. + val q = "βœ“ model queued" + val f = "could not set the model" + assertEquals(q, ModelSource.outcomeMessage(true, null, null, q, f)) + assertEquals(q, ModelSource.outcomeMessage(true, "", "", q, f)) + assertEquals("βœ— session runs on the self-hosted model", + ModelSource.outcomeMessage(false, null, "session runs on the self-hosted model", q, f)) + // A genuinely unanswered request has no hub words: fall back, and do NOT + // reach for a network phrase the hub never said. + assertEquals("βœ— $f", ModelSource.outcomeMessage(false, null, null, q, f)) + assertEquals("βœ— $f", ModelSource.outcomeMessage(false, null, " ", q, f)) + // A refusal the hub answered 200 with. Branching on the HTTP status + // alone is what made this bug the first time; `FleetViewModel.run` + // already reads `OkResponse.error` and this side must agree. + assertEquals("βœ— agent refused the model", + ModelSource.outcomeMessage(true, "agent refused the model", null, q, f)) + // Body error outranks the transport message when somehow both exist. + assertEquals("βœ— agent refused the model", + ModelSource.outcomeMessage(false, "agent refused the model", "Bad Request", q, f)) + } + + @Test fun `a refused switch drops the memo instead of letting it age out`() { + val p = ModelSource.Pending("s1", ModelSource.LOCAL, at = 1_000) + // The hub 409s a host with no local model. Holding the memo for a full + // minute with the answer already in hand is the same lie the TTL bounds. + assertEquals(p, ModelSource.afterAttempt(p, ok = true)) + assertNull(ModelSource.afterAttempt(p, ok = false)) + assertNull(ModelSource.afterAttempt(null, ok = true)) + } + + @Test fun `a spawn sends the source only when it is local`() { + // "subscription" is what a spawn already meant, so omitting it keeps a + // bare spawn byte-identical to what it was before the failover existed. + assertEquals("local", ModelSource.spawnValue(ModelSource.LOCAL)) + assertNull(ModelSource.spawnValue(ModelSource.SUBSCRIPTION)) + assertNull(ModelSource.spawnValue("")) + assertNull(ModelSource.spawnValue(null)) + } + + @Test fun `the spawn composer offers the row only on a host reporting one`() { + assertTrue(ModelSource.composerOffers(configured)) + assertFalse(ModelSource.composerOffers(unconfigured)) + assertFalse(ModelSource.composerOffers(null)) + } + + @Test fun `the composer reads the TARGET host's model, not the fleet's first`() { + // "The wrong loop" is a shape this repo has shipped before, and the + // composer's dialog only ever sees one host: offering another host's + // model would queue a `local` spawn the target 409s or drops. + val other = LocalModelInfo(available = true, model = "qwen3-coder:30b") + val fleet = listOf( + AgentInfo(key = "h0", localModel = other), + AgentInfo(key = "h1", localModel = configured), + AgentInfo(key = "h2", localModel = null), + ) + assertEquals(configured, ModelSource.hostLocalModel(fleet, "h1")) + assertEquals(other, ModelSource.hostLocalModel(fleet, "h0")) + assertNull(ModelSource.hostLocalModel(fleet, "h2")) + assertNull(ModelSource.hostLocalModel(fleet, "nosuchhost")) + assertNull(ModelSource.hostLocalModel(emptyList(), "h1")) + } + + @Test fun `a memo with no session id is never honoured`() { + // Else it would paint every record-less session at once. + val blank = ModelSource.Pending("", ModelSource.LOCAL, at = 0) + assertEquals(ModelSource.SUBSCRIPTION, ModelSource.current(null, blank, 1)) + assertEquals(ModelSource.SUBSCRIPTION, ModelSource.current(SessionInfo(id = ""), blank, 1)) + } + + @Test fun `the chip carries the web's cloud-or-house glyph`() { + // The colour alone can't answer "which model wrote this turn" for a + // colour-blind reader. + assertEquals("🏠", ModelSource.glyph(ModelSource.LOCAL)) + assertEquals("☁", ModelSource.glyph(ModelSource.SUBSCRIPTION)) + } + + @Test fun `a local session reads as the model name, not the word local`() { + // It is a weaker model than Claude; nobody should have to wonder which + // one wrote a turn. + assertEquals("gpt-oss:120b", ModelSource.label(ModelSource.LOCAL, configured)) + assertEquals("Subscription", ModelSource.label(ModelSource.SUBSCRIPTION, configured)) + // A host that stopped reporting a name still labels the row honestly. + assertEquals("local model", ModelSource.label(ModelSource.LOCAL, null)) + assertEquals("Self-hosted model", ModelSource.options(null)[1].second) + assertEquals("gpt-oss:120b", ModelSource.options(configured)[1].second) + assertEquals( + listOf(ModelSource.SUBSCRIPTION, ModelSource.LOCAL), + ModelSource.options(configured).map { it.first }, + ) + } + + @Test fun `the Claude model picker is hidden on the local model`() { + // Every alias it could offer β€” "default" included, since that resolves to + // the shared login's default β€” is one the self-hosted endpoint refuses. + assertFalse(ModelSource.modelPickable(ModelSource.LOCAL)) + assertTrue(ModelSource.modelPickable(ModelSource.SUBSCRIPTION)) + } +} diff --git a/android/app/src/test/java/com/xerktech/turma/data/ModelSwitchStoreTest.kt b/android/app/src/test/java/com/xerktech/turma/data/ModelSwitchStoreTest.kt new file mode 100644 index 00000000..04c91c84 --- /dev/null +++ b/android/app/src/test/java/com/xerktech/turma/data/ModelSwitchStoreTest.kt @@ -0,0 +1,50 @@ +package com.xerktech.turma.data + +import com.xerktech.turma.core.ModelSource +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotSame +import org.junit.Assert.assertNull +import org.junit.Assert.assertSame +import org.junit.Test + +/** + * XERK-246: the model-source memo has to be the SAME object every time a chat + * asks for it, or leaving the chat screen and coming back mid-switch destroys it + * β€” the chip then springs back to the old value and reads as a control that did + * nothing, which is the whole reason the memo exists. + * + * Same shape and same reasoning as [DraftStoreTest]; a fresh flow per call is a + * change nothing else in the suite would notice. + */ +class ModelSwitchStoreTest { + + private fun pending(v: String) = ModelSource.Pending("s1", v, at = 1_000) + + @Test + fun `re-entering a chat gets the same memo object back`() { + val store = ModelSwitchStore() + val first = store.of("hostA", "sess1") + first.value = pending(ModelSource.LOCAL) + + // What a leave-and-return does: ask the store again. + val afterReturn = store.of("hostA", "sess1") + assertSame(first, afterReturn) + assertEquals(ModelSource.LOCAL, afterReturn.value?.value) + } + + @Test + fun `memos are per session, and per host`() { + val store = ModelSwitchStore() + store.of("hostA", "sess1").value = pending(ModelSource.LOCAL) + // Session ids are unique per host, not fleet-wide, so the host has to be + // part of the key β€” else one host's switch paints another's session. + assertNull(store.of("hostA", "sess2").value) + assertNull(store.of("hostB", "sess1").value) + assertNotSame(store.of("hostA", "sess1"), store.of("hostB", "sess1")) + } + + @Test + fun `a session that never switched starts with no memo`() { + assertNull(ModelSwitchStore().of("hostA", "fresh").value) + } +} diff --git a/android/app/src/test/java/com/xerktech/turma/model/AgentDecodeTest.kt b/android/app/src/test/java/com/xerktech/turma/model/AgentDecodeTest.kt index 1c89d7ec..6c1f62be 100644 --- a/android/app/src/test/java/com/xerktech/turma/model/AgentDecodeTest.kt +++ b/android/app/src/test/java/com/xerktech/turma/model/AgentDecodeTest.kt @@ -2,6 +2,7 @@ package com.xerktech.turma.model import kotlinx.serialization.decodeFromString import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Assert.assertTrue @@ -97,6 +98,60 @@ class AgentDecodeTest { assertEquals(0, closed.prs.size) } + // The local-model failover block (XERK-246), in the exact shape hub-agent + // reports it: a host with no LOCAL_MODEL_* env sends available:false with + // BOTH other fields explicitly null, which must decode as "cannot fail over" + // rather than throw and hide the whole fleet. + @Test fun `the localModel block decodes both configured and not`() { + val body = """ + { "now": 1, "agents": [ + { "key": "on", "device": "on", "online": true, + "localModel": { "available": true, "model": "gpt-oss:120b", "contextTokens": 81920 }, + "sessions": [ { "id": "s1", "modelSource": "local", + "modelSourceAt": "2026-08-11T02:30:00Z" } ] }, + { "key": "off", "device": "off", "online": true, + "localModel": { "available": false, "model": null, "contextTokens": null }, + "sessions": [ { "id": "s2", "modelSource": "subscription" } ] } + ] } + """.trimIndent() + val resp = TurmaJson.decodeFromString(body) + val on = resp.agents[0] + assertEquals(true, on.localModel?.available) + assertEquals("gpt-oss:120b", on.localModel?.model) + assertEquals(81920, on.localModel?.contextTokens) + assertEquals("local", on.sessions[0].modelSource) + assertEquals("2026-08-11T02:30:00Z", on.sessions[0].modelSourceAt) + val off = resp.agents[1] + assertEquals(false, off.localModel?.available) + assertNull(off.localModel?.model) + assertEquals("subscription", off.sessions[0].modelSource) + } + + // What hub-agent actually emits for a session that never moved: + // `_session_payload` sends `modelSourceAt: sess.get("modelSourceAt")`, i.e. + // a JSON null, on EVERY such session. A non-nullable field would throw and + // take the whole fleet's decode with it. + @Test fun `a null modelSourceAt does not break the decode`() { + val body = """ + { "now": 1, "agents": [ { "key": "h", "device": "h", "online": true, + "sessions": [ { "id": "s", "modelSource": "subscription", "modelSourceAt": null } ] } ] } + """.trimIndent() + val resp = TurmaJson.decodeFromString(body) + assertEquals("", resp.agents[0].sessions[0].modelSourceAt) + } + + // An agent predating the failover reports neither field. Absent must mean + // "that host can't do it", which is what hides the control. + @Test fun `an agent predating the failover decodes with no local model`() { + val body = """ + { "now": 1, "agents": [ { "key": "h", "device": "h", "online": true, + "sessions": [ { "id": "s" } ] } ] } + """.trimIndent() + val resp = TurmaJson.decodeFromString(body) + assertNull(resp.agents[0].localModel) + assertEquals("", resp.agents[0].sessions[0].modelSource) + } + // The live-status frame (XERK-75): tunnel-agent.js scrapes up/down/elapsed as // DISPLAY STRINGS ("1.2k", "12s") and attaches an optional agents[] list. These // were typed Long, so decodeFromString threw on every real status @@ -235,4 +290,93 @@ class AgentDecodeTest { assertTrue((plain as ToolUseBlock).files.isEmpty()) assertEquals("", plain.caption) } + + // ---- the shapes the hub MUST coerce (XERK-259) -------------------------- + // + // The client half of the contract `turma/wire-shape.js` holds up. Each of + // these was served raw by the hub, and each one is fatal for the WHOLE + // fleet β€” measured on a phone as sign-in itself failing with "Could not + // reach the hub β€” check the URL", because the login probe decodes + // /api/agents. They are pinned HERE so the hub's table can never be + // "simplified" back past them: if one of these stops throwing, the client + // got more tolerant and the note beside the coercion is what needs + // updating, not the coercion. + + private fun decodes(host: String): Boolean = try { + TurmaJson.decodeFromString("""{"now":1,"agents":[$host]}""") + true + } catch (e: Exception) { + false + } + + @Test fun `a bad list ELEMENT is as fatal as a bad list`() { + val h = """"key":"h","device":"h"""" + // The four shapes XERK-259 was filed for. + assertFalse(decodes("""{$h,"repoUsage":[null]}""")) + assertFalse(decodes("""{$h,"repoUsage":["nope"]}""")) + // `typeof [] === "object"` β€” the element shape a careless hub-side + // predicate lets straight through. + assertFalse(decodes("""{$h,"repoUsage":[[1]]}""")) + assertFalse(decodes("""{$h,"repoUsage":[{"repo":{"a":1}}]}""")) + // Not a repoUsage quirk: every typed list behaves this way. + assertFalse(decodes("""{$h,"sessions":[[1,2]]}""")) + assertFalse(decodes("""{$h,"closedSessions":[null]}""")) + assertFalse(decodes("""{$h,"repos":[{"name":"r","resumable":["x"]}]}""")) + assertFalse(decodes("""{$h,"jira":{"tickets":[{"key":"X-1","labels":[null]}]}}""")) + } + + @Test fun `the other shapes the hub coerces are fatal too`() { + val h = """"key":"h","device":"h"""" + assertFalse(decodes("""{$h,"agentVersion":{"a":1}}""")) // String <- object + assertFalse(decodes("""{$h,"github":"nope"}""")) // object <- string + assertFalse(decodes("""{$h,"uploadMaxBytes":1.5}""")) // Long <- fraction + assertFalse(decodes("""{$h,"capacity":{"maxSessions":99999999999999}}""")) // Int is 32-bit + assertFalse(decodes("""{$h,"claudeAuth":{"present":"yes"}}""")) // Boolean <- word + // A map VALUE has no `coerceInputValues` fallback, unlike a field. + assertFalse(decodes("""{$h,"usage":{"days":{"2026-08-12":null}}}""")) + // A block whose discriminator is not a string reaches neither a known + // block nor the UnknownBlock fallback. + assertFalse(decodes( + """{$h,"sessions":[{"session":{"tail":[{"blocks":[{"t":{"a":1}}]}]}}]}""")) + // ...while an unknown or absent `t` is fine, which is why the hub can + // coerce a bad one to "" instead of dropping the block. + assertTrue(decodes( + """{$h,"sessions":[{"session":{"tail":[{"blocks":[{"t":""},{"text":"hi"}]}]}}]}""")) + } + + @Test fun `the record the hub now serves for those inputs decodes`() { + // Exactly what `normalizeRecord` emits for the poisoned beat above (the + // hub suite asserts the same values from the other side), plus a good + // host beside it β€” the point being that neither disappears. + val poisoned = """ + { "key": "bad", "device": "", "online": true, + "repoUsage": [ { "repo": "" }, { "repo": "good", "remoteKey": "gh:o/r" } ], + "sessions": [ { "id": "s1", "ttydPort": 0, + "work": { "pushed": null, "aheadOfBase": null } } ], + "capacity": { "maxSessions": 0 }, "github": null, + "usage": { "days": { } } } + """.trimIndent() + val resp = TurmaJson.decodeFromString( + """{"now":1,"agents":[$plainHost,$poisoned]}""") + assertEquals(listOf("mxh-t16", "bad"), resp.agents.map { it.key }) + assertEquals(2, resp.agents[1].repoUsage.size) + assertEquals("good", resp.agents[1].repoUsage[1].repo) + assertNull(resp.agents[1].sessions[0].work!!.pushed) + } + + @Test fun `the limits block's epoch fields are Longs, and a fraction is fatal`() { + val h = """"key":"h","device":"h"""" + // Both are `Long` on this class, and a Long cannot take a fractional + // literal β€” so the hub gating them on "is a finite number" served this + // straight through and hid the whole fleet. Pinned here because that + // block is rebuilt hub-side by its own function, where the shape is + // easy to assume rather than check. + assertFalse(decodes( + """{$h,"limits":{"fiveHour":{"usedPct":50,"resetsAt":1.5},"capturedAt":1}}""")) + assertFalse(decodes( + """{$h,"limits":{"fiveHour":{"usedPct":50,"resetsAt":1e21},"capturedAt":1}}""")) + // A fractional PERCENTAGE is fine β€” that one is a Double. + assertTrue(decodes( + """{$h,"limits":{"fiveHour":{"usedPct":50.5},"capturedAt":1700000000}}""")) + } } diff --git a/android/app/src/test/java/com/xerktech/turma/vm/ChatUiStateTest.kt b/android/app/src/test/java/com/xerktech/turma/vm/ChatUiStateTest.kt index 34a9ff6a..bb26b9a6 100644 --- a/android/app/src/test/java/com/xerktech/turma/vm/ChatUiStateTest.kt +++ b/android/app/src/test/java/com/xerktech/turma/vm/ChatUiStateTest.kt @@ -1,7 +1,14 @@ package com.xerktech.turma.vm +import com.xerktech.turma.core.ModelSource import com.xerktech.turma.core.Verbosity +import com.xerktech.turma.model.AgentInfo +import com.xerktech.turma.model.LocalModelInfo +import com.xerktech.turma.model.SessionInfo import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue import org.junit.Test class ChatUiStateTest { @@ -11,4 +18,91 @@ class ChatUiStateTest { assertEquals(Verbosity.CONCISE, ChatUiState().verbosity) assertEquals(Verbosity.CONCISE.ordinal, 0) // the SharedPreferences fallback in ChatViewModel } + + // --- local-model failover (XERK-246) ------------------------------------- + // The compose bar reads BOTH of these every repaint, so a wrong answer here + // is a control that either lies about the model or isn't offered at all. + + private val configured = LocalModelInfo(available = true, model = "gpt-oss:120b") + + @Test + fun `the run-against chip follows the host's capability flag`() { + val sess = SessionInfo(id = "s1", modelSource = "subscription") + assertTrue(ChatUiState(session = sess, localModel = configured).canSwitchModelSource()) + // No block at all β€” an agent predating the failover. "Cannot", never + // "assume it can", or the hub 409s a button the operator just pressed. + assertFalse(ChatUiState(session = sess, localModel = null).canSwitchModelSource()) + assertFalse( + ChatUiState(session = sess, localModel = LocalModelInfo(available = false)) + .canSwitchModelSource() + ) + } + + @Test + fun `a session already local keeps the chip after its host loses the config`() { + // Otherwise it is stranded on the weaker model with no way back. + val local = SessionInfo(id = "s1", modelSource = "local") + assertTrue(ChatUiState(session = local, localModel = null).canSwitchModelSource()) + } + + @Test + fun `an unconfirmed switch paints over the heartbeat until it settles`() { + val sess = SessionInfo(id = "s1", modelSource = "subscription") + val state = ChatUiState( + session = sess, + localModel = configured, + modelSourcePending = ModelSource.Pending("s1", ModelSource.LOCAL, at = 1_000), + ) + assertEquals(ModelSource.LOCAL, state.modelSource(now = 1_500)) + // Literal, not `at + SWITCH_SETTLE_MS + 1` β€” see ModelSourceTest: a + // boundary derived from the constant under test only bounds it below. + assertEquals(ModelSource.SUBSCRIPTION, state.modelSource(now = 61_001)) + // Another session's memo must never paint this one. + val other = state.copy(modelSourcePending = ModelSource.Pending("s2", ModelSource.LOCAL, 1_000)) + assertEquals(ModelSource.SUBSCRIPTION, other.modelSource(now = 1_100)) + } + + @Test + fun `no session record yet reads as the subscription`() { + assertEquals(ModelSource.SUBSCRIPTION, ChatUiState().modelSource(now = 1)) + assertFalse(ChatUiState().canSwitchModelSource(now = 1)) + } + + @Test + fun `a fleet beat carries EVERY field this screen reads off it`() { + // This is the line a merge resolution silently truncates, and it must + // name every field or it does not do its job: XERK-246 and XERK-252 both + // landed a field in this one `copy(...)`, so the conflict was over the + // list itself. Dropping `localModel` hides both local-model controls + // forever; dropping `tunnelOnline` is worse than a lost warning, because + // it defaults TRUE β€” the header then asserts the tunnel is up while the + // hub says it is down. Neither shows up anywhere else in the suite: a + // Composable's body has no gate at all. Add an assert here whenever you + // add a field there. + val sess = SessionInfo(id = "s1", modelSource = "local") + val agent = AgentInfo( + key = "h1", device = "maxai", online = true, terminalOnline = false, + uploadMaxBytes = 5_000, localModel = configured, sessions = listOf(sess), + ) + val s = ChatUiState().fromFleet(agent, sess, host = "h1") + assertEquals(sess, s.session) + assertEquals("maxai", s.hostLabel) // device name, not the key + assertFalse(s.tunnelOnline) // drives the ⚠ header marker + assertEquals(5_000L, s.uploadMaxBytes) // gates the πŸ“Ž + assertEquals(configured, s.localModel) // gates BOTH new controls + assertTrue(s.canSwitchModelSource()) + } + + @Test + fun `a beat from a host with no local model clears the capability`() { + // Not merely "leaves it alone": a host that lost its configuration must + // stop offering the switch, and a stale carried-over block would keep it. + val before = ChatUiState(localModel = configured, tunnelOnline = false) + val after = before.fromFleet( + AgentInfo(key = "h1", online = true, terminalOnline = true), null, host = "h1") + assertNull(after.localModel) + assertEquals("h1", after.hostLabel) // no device name: fall back to the key + assertTrue(after.tunnelOnline) // recovers, not just degrades + assertFalse(after.canSwitchModelSource()) + } } diff --git a/android/app/src/test/java/com/xerktech/turma/vm/SpawnRequestTest.kt b/android/app/src/test/java/com/xerktech/turma/vm/SpawnRequestTest.kt new file mode 100644 index 00000000..4b22d2dc --- /dev/null +++ b/android/app/src/test/java/com/xerktech/turma/vm/SpawnRequestTest.kt @@ -0,0 +1,87 @@ +package com.xerktech.turma.vm + +import com.xerktech.turma.core.ModelSource +import com.xerktech.turma.model.TurmaJson +import com.xerktech.turma.vm.FleetViewModel.Companion.spawnRequest +import kotlinx.serialization.encodeToString +import org.junit.Assert.assertEquals +import org.junit.Test + +/** + * The exact body a "New session" spawn puts on the wire. + * + * Pinned as JSON rather than as field reads because the hub validates the BODY: + * `turma/server.js` rejects an unknown-typed field and 409s a `modelSource: + * "local"` at a host with no local model, and an accidentally-present field + * would change what an ordinary spawn queues on every host in the fleet. + */ +class SpawnRequestTest { + + private fun json(r: com.xerktech.turma.net.SpawnRequest) = TurmaJson.encodeToString(r) + + @Test fun `a bare spawn is exactly what it always was`() { + // No modelSource key at all β€” the pre-XERK-246 body, byte for byte. + assertEquals("""{"repo":"Turma"}""", json(spawnRequest("Turma"))) + } + + @Test fun `blank optionals are omitted, not sent empty`() { + assertEquals( + """{"repo":"Turma"}""", + json(spawnRequest("Turma", prompt = "", label = "", baseRef = "", model = "", + permissionMode = "", modelSource = "")), + ) + } + + @Test fun `a subscription spawn sends no model source`() { + // "subscription" is what a spawn already meant; sending it would change + // the body every existing host receives for no behavioural gain. + assertEquals( + """{"repo":"Turma","model":"opus","permissionMode":"auto"}""", + json(spawnRequest("Turma", model = "opus", permissionMode = "auto", + modelSource = ModelSource.SUBSCRIPTION)), + ) + } + + @Test fun `a local spawn carries the source AND the Claude alias`() { + // The alias goes too, matching the web composer (sessions.html). The + // agent drops `--model` for a local session itself, and this is the + // model that session returns to if it is later switched back β€” so + // dropping it here would give an Android-spawned session a different + // model from a web-spawned one. + assertEquals( + """{"repo":"Turma","model":"sonnet","permissionMode":"auto","modelSource":"local"}""", + json(spawnRequest("Turma", model = "sonnet", permissionMode = "auto", + modelSource = ModelSource.LOCAL)), + ) + } + + /** + * The switch's own wire contract β€” the ONE thing between the chip and a + * hub route, and the only part of it not covered by anything else. Renaming + * either the field or the path leaves the whole suite green while the + * feature is dead on the wire (a 400 "modelSource must be subscription or + * local", or a 404). The sibling `SpawnRequest.modelSource` is pinned above + * for the same reason; this closes the pair. + */ + @Test fun `the model-source switch posts the field and path the hub expects`() { + assertEquals( + """{"modelSource":"local"}""", + TurmaJson.encodeToString(com.xerktech.turma.net.ModelSourceRequest("local")), + ) + // By name, not signature: a `suspend fun` carries a trailing + // Continuation parameter that getMethod(...) won't match. + val m = com.xerktech.turma.net.HubApi::class.java.methods + .single { it.name == "setModelSource" } + assertEquals("api/agents/{host}/sessions/{id}/model-source", + m.getAnnotation(retrofit2.http.POST::class.java)!!.value) + } + + @Test fun `the full composer body keeps its field order and content`() { + assertEquals( + """{"repo":"Turma","prompt":"do the thing","label":"lbl","baseRef":"main",""" + + """"model":"haiku","permissionMode":"plan","modelSource":"local"}""", + json(spawnRequest("Turma", prompt = "do the thing", label = "lbl", baseRef = "main", + model = "haiku", permissionMode = "plan", modelSource = ModelSource.LOCAL)), + ) + } +} diff --git a/qa-findings.md b/qa-findings.md index 316ac84c..6626727d 100644 --- a/qa-findings.md +++ b/qa-findings.md @@ -1,8 +1,10 @@ # qa-findings.md β€” the defects a QA pass actually found -The case-study half of `qa.md` Β§5, split out to keep that file readable and -under the 40,000-character ceiling this repo holds its instruction files to. -`qa.md` is the method; this is the evidence. Ranked by what each would have +The case-study half of `qa.md` Β§5, split out to keep that file readable. +Neither file is auto-loaded into a session, so the 40,000-character ceiling that +governs `CLAUDE.md` and `.claude/rules/**` does not apply to them and CI does not +gate them β€” `qa.md` has since grown past it, and that is a readability question +rather than a context-budget one. `qa.md` is the method; this is the evidence. Ranked by what each would have cost in the field, and every one was found by RUNNING the thing, not reading it. Use them as a hunting guide, not a checklist β€” the shapes repeat. @@ -230,3 +232,73 @@ Staging and driving them, which every case above needed: ownership, writes under `/root`, or timing. - A **running** claude survives the package swap (one static ELF, the kernel keeps the inode); only a new exec in the window fails. + +### 5.10 What the ELEVENTH round found β€” again in the previous round's own fix + +- **`typeof [] === "object"`, in a validator whose whole job is "is this an + object".** `normalizeSessions` dropped a `null` and a bare string from + `sessions` and served a nested ARRAY element raw, because its predicate was + `!s || typeof s !== "object"`. The comment above it named the two cases the + fixture used, and the fixture used the two cases the comment named β€” so the + third non-object shape existed in neither, and the fix read as complete from + every angle except running it. Measured as the phone unable to SIGN IN, since + the login probe decodes `/api/agents` and reads the throw as "Could not reach + the hub". **When you write an is-an-object test in JS, write `Array.isArray` + in the same breath, and put every non-object shape in the fixture β€” `null`, + a string, a number, `[]`, and a non-empty array β€” not a representative one.** + This is Β§5.3 again, one round later, in the code that fixed Β§5.3. +- **A TTL read from a Composable body is a timer that never fires.** The + model-switch memo aged out inside `canSwitchModelSource()`, evaluated at + composition time from `System.currentTimeMillis()`. Compose skips + recomposition while the state compares equal, so on a quiet fleet nothing + re-read the clock: the control vanished on an unconfirmed switch and was still + gone at t+120s, with the bar naming a model the session was not running. + **Expiry has to change STATE, not merely change what a read would return** β€” + retire the value from the store on a bounded alarm. The web had no such bug + because its poll recomputes the same predicate unconditionally every beat; + when porting a per-beat web computation to Compose, ask what re-runs it. +- **A discarded `Result` turns every refusal into a success.** `setModel` and + `setMode` ran `runCatching { … }` and then emitted "βœ“ queued" unconditionally. + Harmless while those routes only failed on the network β€” and then a sibling + commit gave the hub a first-class 409 for them, which the bar reported as a + success. **Grep for `runCatching` whose result is not bound**; each one is a + silent success waiting for someone to add a refusal to that route. The same + shape one layer in: branching on the HTTP status alone and ignoring + `OkResponse.error`, so a `200 {ok:false,error:…}` reads as success. + +### 5.11 What the TWELFTH round found β€” in the eleventh round's fixes + +The eleventh round's three fixes all held. Every finding below is a defect in +one of those fixes, which is the pattern Β§5.7 named and this file keeps proving. + +- **Fixing the instance instead of the class.** The `typeof [] === "object"` fix + landed on `normalizeSessions`' element filter, and the identical predicate sat + **five lines below it**, on `s.session` β€” where `"agents" in []` is also false, + so an array fell through both halves of the guard. One heartbeat of + `sessions:[{id,session:[]}]` reproduced the original symptom exactly: the app + could not sign in. **When a bug is a wrong predicate, `grep` the predicate and + fix every hit in the same commit**, then hoist it into one named helper so + there is nothing left to grep. (When you hoist, use a `function` declaration β€” + a `const` used above its own line is the TDZ bug Β§5.10's siblings already + cost this file once.) Note which of the remaining hits are safe and WHY: here + `normalizeLimits` and `sanitizeLiveAgents` also test `typeof x === "object"`, + but both rebuild a whitelist from scratch, so an array falls out at the next + field read rather than reaching a client. +- **Skipping a bad value is not coercing it.** The first attempt at the above + guarded the *sanitize* with the fixed predicate β€” and left the raw array sitting + in the record, which is the thing that gets served. A coercion has to REWRITE + (here, to `null`, the "can't tell you" value every client handles), not decline + to touch. Caught only because the test asserted the served VALUE rather than + that the code did not throw. +- **A wall-clock deadline slept through with an uptime timer.** The memo alarm + computed its delay from `System.currentTimeMillis()`, slept on `delay` (which + measures uptime), then re-checked the wall clock β€” so a backward clock jump + made the re-check false, `settle` returned the same instance, `MutableStateFlow` + did not emit an equal value, the collector never ran, and no new alarm was + armed. The memo pinned a lie for the length of the skew (measured at t+190s + after a 10-minute jump). **After a `delay`, retire by IDENTITY** + (`compareAndSet(theSameMemo, null)`) rather than re-deriving the decision from + a clock that may have moved under you. +- **Asserting either side of a boundary is not asserting the boundary.** The new + TTL test checked 60_999 and 61_001 and never 61_000, so `>=` β†’ `>` survived a + mutation battery. Always assert the exact edge. diff --git a/qa.md b/qa.md index 9a625a94..20594229 100644 --- a/qa.md +++ b/qa.md @@ -14,17 +14,20 @@ actually run on, where several of its recipes do not apply as written. Turma runs its fleet two ways, and they are not interchangeable: -| | agent container | TrueNAS native (this host) | -|---|---|---| -| `npm` / `npx` | on PATH | **not on PATH** β€” see below | -| `java`, `gradle`, Android SDK | bundled | **absent** | -| `apt`, writable `/usr` | yes | **no** β€” read-only, no sudo | -| `ps` / `pkill` | absent | present | -| `~/.claude`, `~/.turma` | bind mounts | the operator's real ones | - -Check with `which java gradle npm` before you plan anything. Assuming the +| | agent container | TrueNAS native | WSL workstation (`MaxAI`) | +|---|---|---|---| +| `npm` / `npx` | on PATH | **not on PATH** β€” see below | on PATH | +| `java`, `gradle`, Android SDK | bundled | **absent** | **installed** β€” see Β§2.5 | +| emulator / `adb` | only the `:emulator` tag | absent | **a running AVD** | +| `apt`, writable `/usr` | yes | **no** β€” read-only, no sudo | yes, with sudo | +| `ps` / `pkill` | absent | present | present | +| `~/.claude`, `~/.turma` | bind mounts | the operator's real ones | the operator's real ones | + +Check with `which java gradle npm adb` before you plan anything. Assuming the container's toolchain on a native host is the single most common way to waste -a QA session. +a QA session β€” and assuming the TrueNAS host's *absences* on the WSL +workstation is the second, because it sends you into a docker pull and an +emulator download you don't need. ### Native-host facts @@ -326,7 +329,154 @@ Driving it: There is **no committed gradle wrapper** β€” CI generates it. `.github/workflows/android-ci.yml` is the reliable spec for how this is really -built; read it before inventing your own invocation. +built; read it before inventing your own invocation. That workflow runs +`:app:testDebugUnitTest` + `:app:assembleDebug` and **nothing else** β€” no lint, +no ktlint, no instrumented source set β€” so a rule living inside a `@Composable` +or a ViewModel call site has **no gate at all**. Read the count out of +`app/build/test-results/testDebugUnitTest/TEST-*.xml`; it moves every ticket. + +#### On the WSL workstation, build and drive natively β€” no container + +```bash +export JAVA_HOME=~/tools/jdk-17.0.20+8 ANDROID_HOME=~/Android/Sdk +export PATH=~/tools/gradle-8.11.1/bin:$JAVA_HOME/bin:~/Android/Sdk/platform-tools:$PATH +cd android && gradle --no-daemon :app:testDebugUnitTest --rerun :app:assembleDebug +adb -s install -r app/build/outputs/apk/debug/app-debug.apk +``` + +`--rerun` is not optional for a mutation test: without it Gradle says +`UP-TO-DATE`, nothing executes, and **every mutation reads as caught**. Same +trap as the container's `FROM-CACHE`, different wording. (`assembleDebug` on its +own is safe to trust: Gradle hashes CONTENT, so a mutated-then-reverted file +reports `UP-TO-DATE` correctly even though its mtime moved.) + +**Run a throwaway probe test without editing the repo** β€” an init script can add +a scratch source dir, which is how you measure "is this wire shape actually +decode-fatal?" against the app's own `TurmaJson` and data classes: + +```bash +cat > /tmp/qa-init.gradle <<'EOF' +gradle.projectsEvaluated { gradle.rootProject.allprojects.each { p -> + if (p.plugins.hasPlugin('com.android.application')) + p.android.sourceSets.getByName('test').java.srcDir('/tmp/qa-kt') } } +EOF +gradle --no-daemon --init-script /tmp/qa-init.gradle \ + :app:testDebugUnitTest --tests "qa.MyProbeTest" -i | grep PROBE +``` + +Measured that way: `coerceInputValues` saves a `null` and a wrong-typed +PRIMITIVE (`modelSource: 5` and `: true` decode fine, `device: 5` too), but an +object or an array where a `String`/`Boolean`/`Int` is declared always throws, +as does any non-object element of a typed `List<…>`. So "it is typed" is not the +test β€” "an object or array can land there" is. + +**Stand up your own AVD.** The shared `turma228` is used by other sessions whose +apps steal the foreground every 30–60s and which have force-stopped +`com.xerktech.turma` outright (`adb logcat -b events | grep am_kill`) β€” that +looks exactly like your app crashing. + +```bash +echo no | avdmanager create avd -n qa- -k "system-images;android-35;google_apis;x86_64" -d pixel_6 +emulator -avd qa- -no-window -no-audio -no-boot-anim -port 5556 +``` + +A second instance of an AVD already running is refused unless the first was +started `-read-only`, which is why you cannot simply reuse the shared one. + +- **Re-focus with `am start -f 0x20000`** (REORDER_TO_FRONT), which returns you + to the last SCREEN, not a tab. A plain `am start` pushes a new `MainActivity` + and resets the Compose nav stack to the dashboard, silently losing the screen + under test. +- **Guard every tap on `dumpsys activity activities | grep topResumedActivity`.** + A tap resolved from one app's dump and delivered to another lands wherever + that app put it β€” one such stray tap opened a package-manager "Uninstall + Turma?" dialog. +- Resolve targets from `uiautomator dump`, never screenshot pixels: the + compose-bar chips reflow as their labels change width. Match `content-desc` + where there is one, and **match exactly** β€” a substring `Sessions` hits + `RUNNING SESSIONS` first. Snackbars live ~3s, so poll at t+2s. + - **A dump costs ~2s normally, but can stall for tens of seconds** β€” it waits + for window idle, so a screen the ~1s fleet beat keeps repainting can hold it + off. Measured on emulator-5556 with a second emulator running and the fleet + beating: 1.94–1.97s, so do NOT plan around a fixed 30–60s budget. When one + screen does hang, `exec-out screencap -p` costs ~2s and never blocks: drive + from fixed coordinates read off one screenshot, and spend a dump only where + you need `content-desc`. + - A `content-desc` containing a `"` is emitted in SINGLE quotes, so + `grep 'content-desc="…'` misses it. Grep the value, not the attribute. + - **A row that appears/disappears reflows the buttons under it.** Hiding the + composer's "Run against" row moved `Spawn` up ~100px, and the stale + coordinate hit dead space β€” re-screenshot after any state change that can + add or drop a field. +- `ExposedDropdownMenuBox` opens on a tap **anywhere in the field**, caret or + body β€” measured on the spawn composer's Model row (bounds `[183,1277]`– + `[897,1445]`, tapped at x=325, menu opened). An earlier note here said the + caret only; that is wrong for this build, so resolve the field's own bounds + and tap its centre. +- A destructive row **arms and re-disarms**: `Kill` becomes `Confirm kill` and + reverts in ~2s, so the two taps must be one `adb shell "input tap …; sleep + 0.4; input tap …"`. A dump between them loses the arm. +- **Every list/chat screen collects the VM's `messages` into a snackbar** β€” + `SessionsListPane` (its own `SnackbarHost`, bottom of the pane; in the wide + two-pane layout that is inside the 360dp list column), `FleetScreen`, + `ChatScreen`, and `BoardScreen`/`OrgControl` as toasts. So an error-wording + check can be driven from any of them. Measured: a refused local spawn from the + session list reads `βœ— host has no local model configured`, a good one + `βœ“ session queued`. +- **XML-illegal characters in the payload make `uiautomator dump` die** β€” + a 0-byte file, and every tap resolved from it misses, while the app itself + renders the string fine. THREE classes do it, all measured on emulator-5556: + **lone surrogates**, **C0/DEL controls**, and **U+FFFE/U+FFFF** (the two + noncharacters XML 1.0 also excludes). Everything else survives β€” `U+FDD0` and + `U+1FFFE` dump fine, so do not blame "noncharacters" generally. + `normalizeLocalModel` closes ALL THREE for `localModel.model`, in both + directions (manufactured by its own 60-code-point cut, and arriving in a rogue + agent's input); every other agent-supplied string reaching the UI is unguarded + entirely, so re-probe per field rather than assuming the guard travels. + Screenshot instead when a dump goes + empty for no reason, suspect the payload before the tooling, and always run + the same screen with a benign name as the control before filing. +- **`ChatViewModel` is scoped to the chat's nav back-stack entry**, so all of its + in-memory state dies when you leave that screen (backgrounding does not). + Anything that must survive lives in `AppContainer` β€” `container.drafts`, + `container.modelSwitches`. Check which before calling a "the value springs + back" report a logic bug. + +#### Standing a hub up for the app to talk to + +Run the real `turma/server.js` on scratch `STATE_FILE`/`ARCHIVE_DIR`/ +`ARCHIVE_DB`, POST synthetic `/api/heartbeat`s every ~3s to keep hosts online, +and point the app at `http://10.0.2.2:`. + +- **Put a logging HTTP proxy in front of it.** The heartbeat RESPONSE drains the + command queue, so polling `/api/agents` for `commands` misses what a tap + actually sent; a proxy logging method + path + body is the only reliable + record, and it is how you inject 409/500/socket-drop to reach the error paths. + It must handle `upgrade` (raw socket pipe) or the live-tail WebSocket dies, + and must `pipe()` responses or SSE hangs. +- Delay the first heartbeat ~2.5s; `server.js` is not listening instantly. +- **Kill previous passes' rigs first** (`/proc/*/cmdline` matching + `turma/server.js` under your worktree). Stale beat loops from an earlier pass + keep overwriting your hosts and quietly contaminate the evidence. +- A host the app has decoded once stays in its `byKey` map, so **re-probing a + malformed payload under a name the app has already seen hides the failure** β€” + use a fresh host name for every decode probe. +- Kill the rig by PID, not `pkill -f`, and not by grepping `/proc/*/cmdline` + either: BOTH match your own shell, whose command line quotes the pattern you + are searching for. `pkill` kills the caller (exit 144); the `/proc` version + kills it too, and the replacement rig then dies on `EADDRINUSE` while the OLD + one keeps serving β€” so the whole A/B runs against the unmutated code and reads + as "no difference". Match `argv[0] == "node"`, skip your own PID's ancestry + (`scratchpad/qa8/killrig.py`), and assert the port is free before restarting. + The tell is subtler than a dead rig: the NEW rig keeps beating happily into the + OLD rig's server, so any control your new rig added (an env var, a file toggle) + silently does nothing and the app looks like it ignored the change. `ps -eo + pid,lstart,args | grep "[r]ig.js"` must show exactly ONE before you believe a + negative result. +- **Refuse `/api/events` at the proxy to see what a decode failure really + costs.** With SSE healthy a bad host loses only itself (per-agent events + decode in `runCatching`); poll-only, the whole fleet is replaced by the raw + kotlinx exception text. Test both β€” they look like different bugs. --- @@ -593,6 +743,131 @@ act on every time. a session only when its LAST `/live` viewer disconnects, so one browser socket the page has lost track of pins the agent's ~1s transcript tail on forever. Count sockets opened and still-open after leaving the stage. +- **Whether the branch still merges, and what the careless resolution costs.** + `git merge-tree --write-tree HEAD origin/main` finds the conflicts (it exits 1 + on one); then run the *wrong* side of each hunk as a mutation. XERK-246 + conflicted on a single `_state.update` line where taking `main`'s side drops + one field and permanently hides two controls β€” with the whole suite green. + A branch verified only at its own tip is not verified. `qa.md` is the file + most likely to conflict; a "take theirs" there silently deletes a pass's notes. +- **Boundaries asserted relative to the constant they test.** A TTL check written + `now = at + SETTLE_MS + 1` bounds the constant from below only: raising it to + 16.7 hours keeps the test green, which is the exact failure the constant + exists to prevent. Assert one side with a literal. +- **Which heartbeat fields are actually coerced.** Read `normalizeRecord` β€” it is + the one list, and both the ingest and the `state.json` restore call it. Today: + `normalizeUsage`, `normalizeLimits`, `normalizeLocalModel`, `normalizeSessions` + (which reshapes `sessions[].session.agents` via `sanitizeLiveAgents`, and + coerces `modelSource`/`modelSourceAt`). Everything else is raw: every other + host-level block Android types (`capacity`, `github`, `models`, `jira`, + `claudeAuth`, `closedSessions`, …) and every other per-session field (`prs`, + `id`, …), where an object or array throws for the WHOLE `/api/agents` array. + `normalizeSessions` DROPS a non-object element and REWRITES a non-array + `sessions` to `[]` (both used to hide that host from the phone β€” and the + non-array one stopped the app signing in at all, see below). + **`normalizeRecord` runs PAST the `AGENT_RECORD_MAX` gate**, which is what + makes rewriting safe: placed before it, a coercion shrinks away the very + amplifier the gate exists to refuse (an 8 MiB string `sessions` rewritten to + `[]` turned a 413 into a 200) and walks an oversized record field by field + before throwing it out β€” which is how a 24 MiB model name reached a + per-code-point spread and OOM-killed the hub. So: **anything running before + the gate may only SHRINK; put a rewrite after it.** `sanitizeHeartbeat` is + pre-gate and is held to the shrink-only half. + - Measure what an uncoerced field costs before calling it acceptable. A host + with `sessions:"x"` does not merely vanish: the phone **cannot sign in at + all**, reporting `Could not reach the hub β€” check the URL`, because the + login probe decodes `/api/agents`. A/B it by dropping the bad host and + re-pressing Sign in. (That one is coerced as of XERK-246; the point is the + method β€” the cost of an uncoerced field is rarely the one you assume.) + Verify by probing, not by reading any doc: this list has been wrong in + `CLAUDE.md` and here more than once. +- **Check the state.json RESTORE, not just the ingest path**, and check it + against the LIST above rather than against what the loader looks like it does. + A hub restart is when a new coercion ships and the restore is the first thing + it serves, so a coercion applied only at ingest is a hole straight through + itself β€” until that host's next beat, which for an OFFLINE host is never + (records live 7 days). The loader is a bare `for` over the parsed blob; + `grep -n "(a);"` the slice between `agents = JSON.parse` and + `first boot or no volume` and compare it to the four, one by one. + Repro that costs 30 seconds and needs no agent: write a state file with the + suspect record, boot `turma/server.js` on it with `STATE_FILE=` pointed there, + and `curl -u … /api/agents` β€” the record has no beat behind it, so whatever + comes back is what the restore did. Point the phone at the same hub to see + what the raw record costs it. + A `normalize*` is also a WHITELIST: a sub-key a newer agent adds is dropped + fleet-wide with nothing failing. +- **The restore runs inside `try { … } catch {}`, so ANY throw in it is silent** β€” + the record loads half-coerced, no log line, every suite green. `console.log` + the `loaded N agents from …` line is the only tell; its ABSENCE with a + non-empty state file means the loop threw. Two things cause it: a module + `const` the restore path reads that is declared BELOW the restore (temporal + dead zone at module init β€” function declarations hoist, `const`s do not), and + a shape the coercions don't guard. Check the first mechanically rather than by + eye: BFS the call graph from `normalizeRecord`, collect the top-level + `const`/`let` each reached function references, and flag any declared after the + loader (`scratchpad/qa8/tdz_scan.py`). A line-order assert naming two constants + by hand does not see the third. +- **The hub's deployed memory ceiling is `mem_limit: 256m`** (DockerOps + `compose/turma.yaml`), so ANY per-request allocation over ~200 MB is a + one-request kill of the whole fleet's control plane, not a slow page. + `HEARTBEAT_MAX` is 32 MiB and `AGENT_RECORD_MAX` (8 MiB) is checked AFTER + `normalizeRecord`, so a coercion that expands an agent-controlled string before + bounding it β€” `[...s]` spreads to one array element per code point β€” routes + straight around that guard. **Bound with `slice()` BEFORE any spread/split/ + match over an agent string.** Reproduce at the deployed shape rather than + arguing about it: + ```bash + docker run -d --name qa-hub -m 256m --memory-swap 256m -p 127.0.0.1:8993:8993 \ + -e PORT=8993 -e TURMA_USER=qa -e TURMA_PASSWORD=qa-pass -e TURMA_AGENT_TOKEN=t \ + -e STATE_FILE=/tmp/state.json -e ARCHIVE_DIR=/tmp/a -e ARCHIVE_DB=/tmp/a.db \ + -v "$PWD/turma:/app:ro" -w /app node:24-bookworm-slim node server.js + # then POST one beat with a ~24 MiB string in the field under test and read + docker inspect -f '{{.State.Status}} {{.State.OOMKilled}}' qa-hub + ``` + +Mutation-testing mechanics, since a broken harness reads as a passing one: + +- Force the tests to actually run (`--rerun`, or `--rerun-tasks` in the + container) β€” an `UP-TO-DATE`/`FROM-CACHE` result makes every mutation "caught". +- **Run a mutation the way CI runs the suite, and run it more than once.** A + guard asserted as a TIME or `heapUsed` budget is order-dependent: whether the + GC ran just before the measurement decides the number. `node --test + turma/tests/server.test.js` caught the un-bounded-spread mutation 5 times in 8 + identical runs and MISSED it 3 β€” alone (`--test-name-pattern`) it failed every + time. A single green run against a mutation proves nothing; a resource budget + needs a margin of ~10x, not ~1.02x (measured 51ms against a 50ms limit and + 71MB against 64MB). +- `git checkout -- ` resolves relative to the `-C` root, not your cwd. A + silently failed revert leaves the previous mutation in the tree and the NEXT + one then reads as caught. Verify with `git status` after every revert, and + mutate a scratch copy rather than the repo. +- On `android/`, expect roughly HALF the battery to survive, and not at random: + 60 mutations over XERK-246 left 30 alive, all in the same three places β€” + **Composable bodies** (19: `ChatScreen.kt`, `FleetDialogs.kt`, the two + `SpawnDialog` call sites, and `SessionsListPane`'s snackbar collector + host, + no instrumented source set), **ViewModel call sites** (10: `ChatViewModel.kt`, + `FleetViewModel.kt`, no Robolectric or coroutine-test harness), and + **`@Serializable` field defaults** (1) that no fixture exercises. A pure + `core/` rule with a test is gated; the CALL to it is not, and neither is which + value a Composable passes it. Spot-checked independently: 3/3 `core/` mutations + caught, 9/9 VM-call-site and Composable-body mutations escaped. Judge an + Android change on where its rule lives, and quote the survivor count rather + than "a few". +- A hub-side rule genuinely is gated by comparison. Over `turma/server.js`'s + coercion path a 12-mutation battery is now caught 12/12 by `server.test.js` β€” + including the TDZ one (the behavioural child-process restore test sees it) and + the un-bounded spread, whose guard pairs a structural assertion with a + resource budget precisely because the budget alone was flaky. `sessions:{a:1}` + (any non-iterable) used to make `normalizeUsage`'s `for…of` throw and abort the + whole restore into its own `catch {}` with `loaded N agents` never printing; + `normalizeSessions` now runs first and rewrites it. The asymmetry with Android + is structural, not effort: "0 escapes" on a UI layer usually means the battery + was too small, while on the hub it means the tests are real. +- `gradle --no-daemon --offline :app:testDebugUnitTest --rerun` is ~13s per + mutation once the first compile is warm, so a 50-mutation battery is ~12 + minutes; run it in the background and do UI work meanwhile (the emulator does + not see tree edits). Do NOT read a source file while it runs β€” you will read a + mutation and think it is the branch. - Type validation on hub routes: `!body.repo` passes an object; a non-string `model` coerced to `""` silently *released* a pin. - The two mirrors of any rule (`liveState` in `index.html` vs `sessions.html`, @@ -659,6 +934,28 @@ are holding. Re-confirm with the same script on both before you spend time. Deleting the call leaves all 455 vitest tests green while a scrolled-up view gets yanked down by a growing live turn. The behaviour is correct on both `main` and HEAD β€” it is the coverage that is missing. +- **One malformed host costs the phone every OTHER host, silently.** Decoding + `/api/agents` is atomic on Android, so an object/array in any uncoerced field + throws for the whole array; `FleetRepository.refresh` catches it into + `FleetState.error`, but `ui/FleetScreen.kt` renders that error ONLY when the + fleet is empty, so the app keeps painting its last good snapshot and still + claims "N / N online". Per-agent SSE events decode inside `runCatching{}`, so + while SSE is healthy a bad host loses only itself β€” but every full refresh + throws, and a cold start then shows the reduced count with no error at all. + Two-line repro, no app changes needed: + ``` + POST /api/heartbeat {"device":"capbad","online":true, + "capacity":{"maxSessions":"eight","running":1.5,"queued":0,"free":0}} + ``` + Hub serves it raw; the phone silently drops `capbad`. Lenient decoding absorbs + a number or bool in a String field, so only object/array values do it. Use a + host name the app has never decoded, or its `byKey` entry hides the failure. +- **`ChatViewModel` never starts the fleet poll**, so a process-death restore + straight into a chat shows no session record β€” header "Session", chips at + their defaults, no PR/ticket/source chips β€” and never recovers until you back + out to a list and re-enter. `FleetRepository.start()` is called only from + `FleetViewModel.start()`, i.e. from a list screen. It degrades in the safe + direction (controls hidden, not wrongly enabled). - **The board drag's edge auto-scroll is dead on a phone (≀560px).** `edgeScroll` nudges `scrollLeft += 18` per pointermove and `scroll-snap-type: x proximity` snaps it back, so a card only reaches a column already on screen (a peek is diff --git a/turma/Dockerfile b/turma/Dockerfile index 12e45c3a..d30c1ef5 100644 --- a/turma/Dockerfile +++ b/turma/Dockerfile @@ -11,7 +11,7 @@ RUN npm install -g "npm@${NPM_VERSION}" && npm cache clean --force ENV NODE_ENV=production WORKDIR /app -COPY server.js archive.js push.js ./ +COPY server.js archive.js push.js wire-shape.js ./ COPY public ./public LABEL org.opencontainers.image.source="https://github.com/xerktech/DockerOps" \ diff --git a/turma/server.js b/turma/server.js index 01363550..5cc130ae 100644 --- a/turma/server.js +++ b/turma/server.js @@ -33,6 +33,10 @@ const archive = require("./archive.js"); // Mobile push (FCM) fan-out for the alert bus. Lazily/gracefully no-ops when // FCM_SERVICE_ACCOUNT_JSON is unset, so requiring it is side-effect-free. const push = require("./push.js"); +// The wire-shape coercion (XERK-259). Required, not inlined: the state.json +// restore below runs at module init and calls it, so its table must already be +// evaluated β€” see the header of wire-shape.js. +const { normalizeWireShapes, AGENT_WIRE_SHAPE } = require("./wire-shape.js"); const PORT = parseInt(process.env.PORT || "8300", 10); const STATE_FILE = process.env.STATE_FILE || "/data/state.json"; @@ -90,6 +94,15 @@ const PRUNE_AFTER_MS = 7 * 24 * 3600 * 1000; // drop entries gone for a week const HISTORY_FRESH_MS = 5 * 60 * 1000; // serve cached session history under this age const HISTORY_MAX_AGE_MS = 10 * 60 * 1000; // evict cache entries older than this const HISTORY_MAX_SESSIONS = 8; // cap per-host cache; oldest fetchedAt evicted first +// Bounds for a session's live agent rows (sanitizeLiveAgents, far below). They +// live UP HERE, away from their function, because the state.json restore runs +// at module init and calls that function: a `const` declared later is in its +// temporal dead zone then, so reading one throws a ReferenceError that the +// restore's own `catch {}` swallows β€” the record loads half-coerced and says +// nothing. Any constant a restore-path function reads has to be declared above +// the restore. +const LIVE_AGENTS_MAX = 32; +const LIVE_AGENT_FIELD_MAX = 400; // How long a message typed into a session may be (XERK-227). The operator pastes // logs and specs into the chat composer and the raw terminal takes them at any // size, so this is a payload backstop β€” the agent delivers the text to the pane @@ -363,10 +376,14 @@ const liveClients = {}; // for the first heartbeat interval; losing it is harmless) ------------------- try { agents = JSON.parse(fs.readFileSync(STATE_FILE, "utf8")); - // Records written before the ingest-side coercion below (and any host that is - // OFFLINE, so no beat will ever rewrite its record) still carry the legacy - // per-model usage shape β€” normalize what we load, not just what arrives. - for (const a of Object.values(agents)) normalizeUsage(a); + // Records written before a coercion existed β€” and any host that is OFFLINE, + // so no beat will ever rewrite its record β€” carry whatever shape was current + // when they were saved. Normalize what we LOAD, not just what arrives: the + // first `/api/agents` after a restart serves the raw record, and a hub + // restart is exactly when a new coercion ships. `normalizeRecord` is shared + // with the ingest path so the two cannot drift; adding a coercion in one + // place covers both. Tests: `the state.json restore coerces too`. + for (const a of Object.values(agents)) normalizeRecord(a); console.log(`loaded ${Object.keys(agents).length} agents from ${STATE_FILE}`); } catch { /* first boot or no volume mounted */ @@ -1042,8 +1059,22 @@ function normalizeModelUsage(usage) { function normalizeUsage(payload) { if (!payload || typeof payload !== "object") return; normalizeModelUsage(payload.usage); - for (const r of payload.repoUsage || []) normalizeModelUsage(r && r.usage); - for (const s of payload.sessions || []) normalizeModelUsage(s && s.usage); + // `Array.isArray`, not `|| []`: a non-iterable `repoUsage`/`sessions` (an + // OBJECT, say) makes a bare `for…of` THROW, and a throw here is uniquely + // costly β€” on the restore path it lands in a silent `catch {}` and abandons + // every host after this one, uncoerced, on every boot. + // Both are typed LISTS on Android, so a non-array is decode-fatal for the + // WHOLE fleet payload, not just this host β€” rewrite it rather than merely + // stepping around it. (Safe because normalizeRecord runs past the raw-size + // gate; before it, this would have shrunk away an amplifier.) + if (!Array.isArray(payload.repoUsage)) { + if ("repoUsage" in payload) payload.repoUsage = []; + } else { + for (const r of payload.repoUsage) normalizeModelUsage(r && r.usage); + } + if (Array.isArray(payload.sessions)) { + for (const s of payload.sessions) normalizeModelUsage(s && s.usage); + } } // The subscription-limit snapshot (XERK-247), coerced to numbers or dropped, for @@ -1060,6 +1091,12 @@ function normalizeLimits(payload) { return; } const num = (v) => (typeof v === "number" && Number.isFinite(v) ? v : undefined); + // The two epoch fields are `Long` on Android, and a Long cannot take a + // FRACTIONAL literal β€” `resetsAt: 1.5` and a value past 2^63 both pass + // `Number.isFinite` and then throw for the whole fleet payload, exactly the + // failure this function exists to prevent. `usedPct` is a Double, so it keeps + // the looser check. + const epoch = (v) => (typeof v === "number" && Number.isSafeInteger(v) ? v : undefined); const out = {}; for (const key of ["fiveHour", "sevenDay"]) { const win = lim[key]; @@ -1067,11 +1104,11 @@ function normalizeLimits(payload) { const pct = num(win.usedPct); if (pct === undefined) continue; // a window with no percentage draws nothing const clean = { usedPct: Math.min(100, Math.max(0, pct)) }; - const resets = num(win.resetsAt); + const resets = epoch(win.resetsAt); if (resets !== undefined) clean.resetsAt = resets; out[key] = clean; } - const captured = num(lim.capturedAt); + const captured = epoch(lim.capturedAt); if (!Object.keys(out).length || captured === undefined) { payload.limits = null; return; @@ -1081,6 +1118,68 @@ function normalizeLimits(payload) { payload.limits = out; } +// Coerce the local-model block at ingest, for exactly the reason normalizeLimits +// above does it (XERK-246): this fans out to web, Android and glasses, and +// Android decodes it into TYPED fields β€” `available: Boolean`, `contextTokens: +// Int?` β€” so an `available` of "yes" or a contextTokens past 2^31 from ONE buggy +// host fails the decode of the WHOLE /api/agents array, and every other host +// silently vanishes from that phone's fleet. +// +// Anything unusable becomes null, which every client already reads as "this host +// cannot fail over" β€” the same degradation as an agent too old to report it. +function normalizeLocalModel(payload) { + if (!payload || typeof payload !== "object") return; + const lm = payload.localModel; + if (!lm || typeof lm !== "object" || Array.isArray(lm)) { + if ("localModel" in payload) payload.localModel = null; + return; + } + // Strictly boolean: a truthy string would turn a host that cannot fail over + // into one the UI offers the switch on, and the command would be dropped. + if (lm.available !== true) { + payload.localModel = { available: false, model: null, contextTokens: null }; + return; + } + // Nothing XML-ILLEGAL may leave here, from either direction β€” the whole class, + // not just the one case that bit us. A lone surrogate, a C0 control and the + // noncharacters U+FFFE/U+FFFF are all unencodable in XML, and each kills + // Android's `uiautomator dump` outright (`KXmlSerializer: Illegal character` + // / a 0-byte file), i.e. the tool a QA pass drives the app with. Cutting with + // `slice(60)` through an astral pair MANUFACTURES a lone surrogate, so that + // cut is on CODE POINTS and runs after the strip. Only a rogue agent reaches + // this β€” a real one is bounded by LOCAL_MODEL_NAME_RE β€” which is this + // function's whole threat model. + // + // BOUND FIRST, THEN SPREAD. `[...s]` materialises one array element per code + // point over the WHOLE string, and this runs BEFORE the AGENT_RECORD_MAX check + // that refuses an oversized beat β€” so spreading the raw value let a single + // agent-authed heartbeat with a 24 MiB name OOM-kill the hub at its deployed + // `mem_limit: 256m` (32M chars measured at 288 MB heap), which + // `restart: unless-stopped` turns into an outage loop of the fleet's whole + // control plane. 512 UTF-16 units is far more than the 60 code points that + // survive, and cutting there can only split an astral pair β€” which the + // surrogate replace immediately below then handles. + const name = typeof lm.model === "string" + ? [...lm.model.slice(0, 512) + .replace(/\p{Surrogate}/gu, "οΏ½") // unpaired halves -> replacement + .replace(/[\x00-\x08\x0b\x0c\x0e-\x1f\x7f\ufffe\uffff]/g, "") // XML-illegal + .trim()] + .slice(0, 60).join("") + : ""; + const ctx = lm.contextTokens; + payload.localModel = { + available: true, + // The name is display-only here (the agent validates its own charset before + // launching), but it must be A STRING or the decode dies. + model: name || null, + // Int-safe or nothing: the field is unused by the UI, so dropping a bad one + // costs nothing and keeps the fleet decodable. + contextTokens: + typeof ctx === "number" && Number.isSafeInteger(ctx) && + ctx > 0 && ctx <= 2_147_483_647 ? ctx : null, + }; +} + // Merge the agent's on-demand history deliveries (heartbeat `historyResults`) // into the host's per-session cache, then bound its memory: drop entries older // than HISTORY_MAX_AGE_MS and cap the cache at HISTORY_MAX_SESSIONS, evicting @@ -1736,10 +1835,15 @@ function sanitizeHeartbeat(payload, key) { // session's live agent rows come from a pane scrape and are re-shaped and // bounded here for the same reason the `turn` frame's are β€” the clients turn // this list into a count and a label, and nothing else bounds it. + // Pre-ceiling, so this may only ever SHRINK (see normalizeRecord, which runs + // past the gate and is free to rewrite). Live agent rows come from a pane + // scrape and are re-shaped and bounded here for the same reason the `turn` + // frame's are β€” clients turn this list into a count and a label, and nothing + // else bounds it. if (Array.isArray(payload.sessions)) { for (const s of payload.sessions) { - const live = s && typeof s === "object" ? s.session : null; - if (live && typeof live === "object" && "agents" in live) { + const live = objectish(s) ? s.session : null; + if (objectish(live) && "agents" in live) { live.agents = sanitizeLiveAgents(live.agents) || []; } } @@ -1747,6 +1851,114 @@ function sanitizeHeartbeat(payload, key) { return payload; } +/** + * Is this a plain object β€” the thing a client that TYPED a field will accept? + * + * The one predicate every shape check in the coercion path goes through, because + * the obvious spelling is wrong in the same way every time: `typeof [] === + * "object"`, so `!x || typeof x !== "object"` passes arrays straight through, + * and the follow-on `"k" in []` is false so an array is neither coerced nor + * rejected. Both escapes were measured the same way β€” the Android login probe + * decodes /api/agents, so one raw array anywhere in the payload reads as "Could + * not reach the hub" and the app cannot sign in at all. + * + * A `function` declaration, not a `const`: it is used ~70 lines ABOVE this point + * by `sanitizeHeartbeat`, and this file has already shipped a coercion that + * threw a ReferenceError at module init from exactly that pattern (a const in + * its temporal dead zone, swallowed by the restore's `catch {}` β€” see + * `normalizeRecord`'s ordering comment). Declarations hoist; consts do not. + */ +function objectish(x) { + return !!x && typeof x === "object" && !Array.isArray(x); +} + +/** + * Every coercion an agent record needs before a client sees it, in ONE place. + * + * Called from the heartbeat ingest and from the `state.json` restore, because a + * coercion applied at only one of those is a hole straight through itself: the + * restore serves whatever was persisted, for one beat on a live host and up to + * the record's whole life on an offline one. Add a new `normalize*` HERE and + * both paths get it. + * + * The stakes are Android's: `/api/agents` decodes atomically into typed fields, + * so one host's wrong-typed value throws for the whole array and every OTHER + * host silently disappears from that phone. + */ +function normalizeRecord(a) { + // Each of these guards its own input shape (`Array.isArray`, not `|| []`), + // because a throw anywhere in here lands in the restore's silent `catch {}` + // and abandons every host after this one, uncoerced, on every boot. + // + // The block-specific passes run FIRST and `normalizeWireShapes` LAST, and that + // order IS load-bearing: the shape sweep coerces `usage.models` to the objects + // Android types, so run before normalizeModelUsage it would DROP an old + // agent's bare model-name strings instead of letting that pass rewrite them. + // A block one of them REBUILDS (`limits`, `localModel`) is in the table too, + // not exempted from it: a rebuild is only as good as its own gates, and + // `limits` shipped one that let a fractional `resetsAt` β€” decode-fatal for the + // whole fleet β€” straight through. Each pass keeps its own semantics; the sweep + // is the type backstop under all of them. + normalizeSessions(a); + normalizeUsage(a); + normalizeLimits(a); + normalizeLocalModel(a); + normalizeWireShapes(a); +} + +// The per-SESSION coercions (see normalizeRecord). +// +// `sessions` is a KNOWN key, so sanitizeHeartbeat's unknown-field sweep never +// looks inside it β€” everything typed under a session has to be handled here. +// Android decodes /api/agents into typed fields, so an object or array where it +// expects a String throws for the WHOLE array and every other host disappears +// from that phone; a field only becomes this dangerous once a client TYPES it, +// which is why `modelSource`/`modelSourceAt` are here from the commit that +// declared them on `SessionInfo`. +function normalizeSessions(payload) { + if (!payload || typeof payload !== "object") return; + // A non-array `sessions` is REWRITTEN, not skipped. It is decode-fatal on + // Android β€” measured as the app failing to sign in at all, reporting "Could + // not reach the hub" β€” and on the restore path it also throws out of + // normalizeUsage's `for … of`, silently abandoning the record. Rewriting is + // safe only because this now runs PAST the AGENT_RECORD_MAX gate: doing it + // before would have erased the amplifier that gate exists to refuse. + if (!Array.isArray(payload.sessions)) { + if ("sessions" in payload) payload.sessions = []; + return; + } + // DROP a non-object element, never skip past it: `sessions` is typed + // `List` on Android, so a `null` or a bare string in the array is + // as fatal as a wrong-typed field inside one β€” measured as a host silently + // missing from the phone while the tile still counted it. + if (!payload.sessions.every(objectish)) payload.sessions = payload.sessions.filter(objectish); + for (const s of payload.sessions) { + // Re-bounded here as well as in sanitizeHeartbeat, because the restore path + // never goes through that: idempotent, so running twice costs nothing. + // + // REWRITE a non-object `session`, never merely skip past it. `session` is + // typed `LiveSignals?` on Android, so any non-object is decode-fatal for + // the WHOLE /api/agents array β€” measured as the app unable to sign in at + // all, since the login probe decodes it and reads the throw as "Could not + // reach the hub". Skipping the sanitize is NOT enough: the raw value stays + // in the record and is what gets served. `null` is the "can't tell you" + // value every client already handles. + // + // An array is the case a bare `typeof live === "object"` guard misses, and + // `"agents" in []` is false, so it fell through both halves of the old + // test. Rewriting is safe here only because normalizeSessions runs PAST the + // AGENT_RECORD_MAX gate β€” see normalizeRecord's ordering comment. + if ("session" in s && !objectish(s.session)) s.session = null; + const live = s.session; + if (live && "agents" in live) { + live.agents = sanitizeLiveAgents(live.agents) || []; + } + for (const k of ["modelSource", "modelSourceAt"]) { + if (k in s && typeof s[k] !== "string") s[k] = ""; + } + } +} + // Thrown past the cap so a route can answer 413 instead of leaking a generic // 400 (or, worse, nothing at all). class BodyTooLarge extends Error { @@ -2054,8 +2266,6 @@ function fmtDur(ms) { // and cost that repaint; the cap matches the agent's own PANE_AGENTS_MAX. // `null` (not `[]`) for a frame with no `agents` key at all, so the chat can // tell "this agent can't report them" from "no agents are running". -const LIVE_AGENTS_MAX = 32; -const LIVE_AGENT_FIELD_MAX = 400; function sanitizeLiveAgents(raw) { if (!Array.isArray(raw)) return null; const out = []; @@ -3415,8 +3625,6 @@ const server = http.createServer(async (req, res) => { const payload = sanitizeHeartbeat(raw, (raw && raw.device) || "unknown host"); // Coerce an old agent's per-model usage lists to the current shape before // anything (the record, the cache, every client) sees them. - normalizeUsage(payload); - normalizeLimits(payload); // Identity is the physical host name (`device`); with one container per // host the container name is no longer meaningful. agentId is a last-resort // fallback if the host name couldn't be read. @@ -3512,6 +3720,40 @@ const server = http.createServer(async (req, res) => { // and then restoring `prev` restored an object the ingests had already // mutated β€” a refused beat still poisoned the caches and its content came // back out of /history with a 413 on the wire. + const refuseOversized = (size) => { + if (prev && Object.keys(prev).length) agents[key] = prev; + else delete agents[key]; + console.error( + `heartbeat from ${key}: record is ${size} bytes, over the ` + + `${AGENT_RECORD_MAX} limit β€” beat refused` + ); + return json(res, 413, { error: "agent record too large", limit: AGENT_RECORD_MAX }); + }; + // TWO measurements, one ceiling. The RAW size is the amplifier check: a + // coercion that discards junk must not be able to shrink an oversized + // beat into an accepted one (rewriting an 8 MiB string `sessions` to `[]` + // did exactly that, turning a 413 into a 200). The COERCED size is what + // actually gets stored and served, and a coercion can EXPAND β€” + // normalizeModelUsage rewrites `"m"` to `{model:"m"}`, ~3.5x, so an 8 MiB + // beat of model names parked 28 MiB per host for a week. Neither + // measurement alone holds the ceiling. + const rawSize = agentRecordSize(next); + if (rawSize > AGENT_RECORD_MAX) return refuseOversized(rawSize); + // Coercion sits BETWEEN the two, so it never walks an unbounded record β€” + // that is how a 24 MiB model name reached a per-code-point spread and + // OOM-killed the hub. A throw here would leave the RAW record installed + // (`agents[key] = next` is already done), which is worse than refusing: + // it defeats every gate downstream, including localModelAvailable's + // strict-boolean check. The coercions are written not to throw; this is + // the backstop that makes that not matter. + try { + normalizeRecord(next); + } catch (e) { + console.error(`heartbeat from ${key}: coercion failed (${e.message}) β€” beat refused`); + if (prev && Object.keys(prev).length) agents[key] = prev; + else delete agents[key]; + return json(res, 400, { error: "malformed heartbeat" }); + } const recordSize = agentRecordSize(next); // Visible BEFORE it 413s. Measured against the operator's real fleet the // largest record is 0.30 MiB, so half the ceiling means something has @@ -3527,15 +3769,7 @@ const server = http.createServer(async (req, res) => { ); } recordSizeWarned.set(key, overHalf); - if (recordSize > AGENT_RECORD_MAX) { - if (prev && Object.keys(prev).length) agents[key] = prev; - else delete agents[key]; - console.error( - `heartbeat from ${key}: record is ${recordSize} bytes, over the ` + - `${AGENT_RECORD_MAX} limit β€” beat refused` - ); - return json(res, 413, { error: "agent record too large", limit: AGENT_RECORD_MAX }); - } + if (recordSize > AGENT_RECORD_MAX) return refuseOversized(recordSize); ingestHistory(next, historyResults); ingestSubagentHistory(next, subagentHistoryResults); ingestJiraIssues(next, jiraIssueResults); @@ -5146,6 +5380,15 @@ if (process.env.TURMA_TEST) { // and the suite stayed green, so they are exported to be pinned. sanitizeHeartbeat, agentRecordSize, safeAgentsCache, HEARTBEAT_UNKNOWN_MAX, AGENT_RECORD_MAX, + // Ingest coercion, exported for the same reason as the rest of this group: + // Android decodes /api/agents atomically, so one host's wrong-typed field + // hides the WHOLE fleet from that phone (XERK-246). `normalizeRecord` is + // the one both the ingest path and the state.json restore call. + normalizeRecord, + normalizeLocalModel, + // The shape table itself, so a test can walk it rather than re-listing what + // it covers β€” a hand-copied list of fields is what drifts (XERK-259). + AGENT_WIRE_SHAPE, queueCommand, findSession, diff --git a/turma/tests/server.test.js b/turma/tests/server.test.js index 1198be0c..c9fc403a 100644 --- a/turma/tests/server.test.js +++ b/turma/tests/server.test.js @@ -6455,3 +6455,793 @@ test("heartbeat: localModel is a known key, not an unknown-field remnant", async assert.ok(hub.HEARTBEAT_KNOWN_KEYS.has("localModel")); }); +test("normalizeLocalModel coerces the block so one host cannot hide the fleet", () => { + // Android decodes /api/agents ATOMICALLY into typed fields, so a wrong-typed + // localModel from ONE host throws for the whole array and every other host + // silently disappears from that phone. Same contract, and same reason, as + // normalizeLimits beside it. + const norm = (localModel) => { + const p = { device: "h", localModel }; + hub.normalizeLocalModel(p); + return p.localModel; + }; + + // A good block passes through unchanged. + assert.deepEqual( + norm({ available: true, model: "gpt-oss:120b", contextTokens: 81920 }), + { available: true, model: "gpt-oss:120b", contextTokens: 81920 }, + ); + + // `available` is STRICTLY boolean: a truthy string would offer the switch on + // a host that cannot do it, and the command would be acked and dropped. + assert.deepEqual(norm({ available: "yes", model: "x" }), + { available: false, model: null, contextTokens: null }); + assert.deepEqual(norm({ available: 1 }), + { available: false, model: null, contextTokens: null }); + + // A non-string model and an out-of-Int contextTokens degrade to null rather + // than failing the decode. contextTokens is unused by the UI, so this is free. + assert.deepEqual(norm({ available: true, model: 12345, contextTokens: 9999999999 }), + { available: true, model: null, contextTokens: null }); + assert.deepEqual(norm({ available: true, model: "m", contextTokens: 1.5 }), + { available: true, model: "m", contextTokens: null }); + + // The name is BOUNDED, and cut on code points. A UTF-16 `slice` through an + // astral pair emits a lone surrogate β€” unencodable, and it kills Android's + // uiautomator outright. Nothing else pins this length. + const long = norm({ available: true, model: "x".repeat(500) }); + assert.equal(long.model.length, 60); + const astral = norm({ available: true, model: "x".repeat(59) + "πŸ˜€" + "tail" }); + assert.equal([...astral.model].length, 60); + assert.ok(astral.model.isWellFormed(), "the cut manufactured a lone surrogate"); + // ...and one that ARRIVES that way is replaced, not passed through. Either + // direction kills uiautomator, so the guarantee has to cover both. + for (const evil of ["qwen\uD83Dcoder", "abc\uDE00def", "x".repeat(59) + "\uD83Dtail"]) { + assert.ok(norm({ available: true, model: evil }).model.isWellFormed(), + `a lone surrogate survived: ${JSON.stringify(evil)}`); + } + // The guarantee is the whole XML-ILLEGAL class, not just surrogates: a C0 + // control and the noncharacters U+FFFE/U+FFFF each kill `uiautomator dump` + // the same way (a 0-byte file), and closing only the case that bit us leaves + // the next one to be rediscovered β€” which is how the second and third of + // these were found, one pass apart. + const ctl = norm({ available: true, model: "qwen\x01ctl\x00nul\x0bvt\x7fdel" }); + assert.equal(ctl.model, "qwenctlnulvtdel"); + assert.equal(norm({ available: true, model: "qwen\uffffbad\ufffemore" }).model, "qwenbadmore"); + // ...but U+FDD0 and U+1FFFE are LEGAL XML and must survive: over-stripping + // would mangle a name for no reason. + assert.equal(norm({ available: true, model: "a\ufdd0b\u{1FFFE}c" }).model, "a\ufdd0b\u{1FFFE}c"); + // Tab/newline/CR are legal XML and are only trimmed at the edges. + assert.equal(norm({ available: true, model: " a\tb " }).model, "a\tb"); + + // Not an object at all -> null, which every client reads as "cannot fail over". + assert.equal(norm("yes"), null); + assert.equal(norm([1, 2]), null); + assert.equal(norm(null), null); + + // An agent predating the failover sends nothing; the key must stay absent + // rather than become an explicit null, so the payload is byte-identical. + const old = { device: "h" }; + hub.normalizeLocalModel(old); + assert.ok(!("localModel" in old)); +}); + +test("the state.json restore coerces too, not just the ingest path", () => { + // A hub restart is exactly when a new coercion ships, and the restore is the + // FIRST thing it serves. A record written before it β€” or belonging to an + // OFFLINE host, where no beat will ever rewrite it β€” would otherwise reach + // the phone raw and throw for the whole fleet. Held here rather than by + // booting a second hub: the loader is a bare `for` over the parsed blob, so + // what matters is that all three coercions are applied to a loaded record. + const restored = { + device: "old", + localModel: { available: "yes", model: 12345, contextTokens: 9999999999 }, + limits: { fiveHour: { usedPct: "lots" } }, + }; + hub.normalizeLocalModel(restored); + assert.deepEqual(restored.localModel, + { available: false, model: null, contextTokens: null }); + + // The durable form of "the restore can't fall behind the ingest": both go + // through ONE function, so there is no list here to keep in step. A previous + // version of this test named three coercions and therefore could not notice + // the fourth (`sanitizeLiveAgents`) missing from the restore β€” enumerating is + // exactly the shape that let the hole exist. + const src = fs.readFileSync(path.join(__dirname, "..", "server.js"), "utf8"); + const loader = src.slice(src.indexOf("agents = JSON.parse"), src.indexOf("first boot or no volume")); + assert.ok(/normalizeRecord\(a\)/.test(loader), + "the state.json restore must go through normalizeRecord, like the ingest path"); + const ingest = src.slice(src.indexOf('url.pathname === "/api/heartbeat"'), + src.indexOf("ingestHistory(next, historyResults)")); + assert.ok(/normalizeRecord\(next\)/.test(ingest), + "the heartbeat ingest must go through the same normalizeRecord"); + // ...and BETWEEN the two size measurements. Before the raw check it could + // shrink away the amplifier the ceiling exists to refuse (an 8 MiB string + // `sessions` became `[]` and the beat 200'd); after the coerced check, an + // EXPANDING coercion escapes the ceiling entirely (normalizeModelUsage is + // ~3.5x, and an 8 MiB beat parked 28 MiB per host for a week). + const iRaw = ingest.indexOf("rawSize > AGENT_RECORD_MAX"); + const iCoerce = ingest.indexOf("normalizeRecord(next)"); + const iStored = ingest.indexOf("recordSize > AGENT_RECORD_MAX"); + assert.ok(iRaw > -1 && iCoerce > iRaw && iStored > iCoerce, + "the ingest must measure raw size, THEN coerce, THEN measure the stored size"); +}); + +test("the restore actually RUNS β€” it must not throw into its own catch", () => { + // The restore sits at module init inside `try { … } catch {}`, so anything it + // throws is swallowed: the record loads HALF-coerced, with no log line and no + // error anywhere. That is not hypothetical β€” `sanitizeLiveAgents` read two + // module `const`s declared 1700 lines BELOW the restore, i.e. in their + // temporal dead zone at that moment, so the localModel half was applied and + // the session half silently was not, with every suite green. + // + // Held BEHAVIOURALLY, by loading the real module against a fixture in a child + // process. An earlier version asserted the line order of two constants BY + // NAME, which a third constant walks straight past β€” the same enumerate-the- + // instances mistake that let the original hole exist. + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "restore-")); + const state = { + h1: { + device: "h1", online: true, + localModel: { available: "yes", model: 12345, contextTokens: 9999999999 }, + limits: { fiveHour: { usedPct: "lots" } }, + sessions: [ + null, // decode-fatal element + { id: "s1", modelSource: { a: 1 }, modelSourceAt: ["x"], + session: { agents: [{ sel: "yes", type: { a: 1 }, label: ["x"] }] } }, + "not-a-session", + ], + }, + }; + fs.writeFileSync(path.join(dir, "state.json"), JSON.stringify(state)); + const out = require("child_process").execFileSync(process.execPath, ["-e", ` + process.env.TURMA_TEST = "1"; + const hub = require(${JSON.stringify(path.join(__dirname, "..", "server.js"))}); + // Marker-delimited: the loader logs "loaded N agents …" to stdout on the + // way past, and that line landing here is itself the proof it ran. + process.stdout.write("<<<" + JSON.stringify(hub.agents) + ">>>"); + process.exit(0); + `], { + env: { ...process.env, TURMA_TEST: "1", STATE_FILE: path.join(dir, "state.json"), + ARCHIVE_DIR: path.join(dir, "a"), ARCHIVE_DB: path.join(dir, "a.db"), + NODE_NO_WARNINGS: "1" }, + encoding: "utf8", stdio: ["ignore", "pipe", "ignore"], + }); + assert.match(out, /loaded 1 agents from/, + "the restore did not run β€” it threw into its own catch"); + const rec = JSON.parse(out.slice(out.indexOf("<<<") + 3, out.lastIndexOf(">>>"))).h1; + assert.deepEqual(rec.localModel, { available: false, model: null, contextTokens: null }); + assert.equal(rec.limits, null); + assert.equal(rec.sessions.length, 1, "non-object session elements must be dropped"); + assert.equal(rec.sessions[0].modelSource, ""); + assert.equal(rec.sessions[0].modelSourceAt, ""); + assert.deepEqual(rec.sessions[0].session.agents, + [{ sel: true, type: "[object Object]", label: "x" }]); +}); + +test("normalizeLocalModel bounds the name BEFORE spreading it", () => { + // `[...s]` allocates per code point over the WHOLE string, so an unbounded + // spread let one agent-authed heartbeat with a 24 MiB name OOM-kill the hub + // at its deployed `mem_limit: 256m`, on repeat. + // + // Two assertions, because neither is sufficient alone. A RESOURCE BUDGET is + // the honest behavioural check but is nondeterministic at close margins β€” an + // earlier version used 8 MiB against 50ms/64MB and caught the reintroduced + // bug only 5 runs in 8, decided by whether a GC landed between the samples. + // So the budget runs at 32 MiB (`HEARTBEAT_MAX`, the largest a beat can carry) + // with ~10x headroom, and a STRUCTURAL assertion holds the ordering exactly. + const src = fs.readFileSync(path.join(__dirname, "..", "server.js"), "utf8"); + const spread = src.slice(src.indexOf("const name = typeof lm.model"), src.indexOf(".slice(0, 60)")); + assert.match(spread, /\[\.\.\.lm\.model\.slice\(/, + "the model name must be bounded BEFORE the per-code-point spread"); + + const huge = { device: "h", localModel: { available: true, model: "x".repeat(32 << 20) } }; + const t0 = process.hrtime.bigint(); + hub.normalizeRecord(huge); + const ms = Number(process.hrtime.bigint() - t0) / 1e6; + assert.equal(huge.localModel.model, "x".repeat(60)); + // Bounded: ~0.1ms. Unbounded at this size: several hundred ms and ~600 MB. + assert.ok(ms < 60, `coercing a 32 MiB name took ${ms.toFixed(1)}ms β€” is it spreading first?`); +}); + +test("http: an EXPANDING coercion cannot escape the record ceiling", async () => { + // normalizeModelUsage rewrites `"m"` to `{model:"m"}` β€” ~3.5x. Measuring only + // the raw size let an 8 MiB beat of bare model names park 28 MiB per host for + // a week, in state.json, in every /api/agents response and every SSE frame: + // exactly the amplification the ceiling was added to stop. + const host = "fat-expand"; + const models = new Array(2_000_000).fill("m"); + const res = await request("POST", "/api/heartbeat", { + headers: agentHeaders, + body: { device: host, sessions: [], usage: { models } }, + }); + assert.equal(res.status, 413); + assert.equal(agents[host], undefined, "a refused beat must not install a record"); +}); + +test("http: a beat whose coercion throws is refused, never installed raw", async () => { + // The coercion runs AFTER `agents[key] = next`, so a throw inside it would + // leave the RAW record installed β€” worse than refusing, because every gate + // downstream then reads uncoerced values (`localModelAvailable` treats the + // string "yes" as true and hands out a switch the host cannot honour), and + // the poison reaches state.json where the restore chokes on it forever. + const host = "throw-host"; + const res = await request("POST", "/api/heartbeat", { + headers: agentHeaders, + // A non-iterable `repoUsage` used to throw out of normalizeUsage's `for…of`. + body: { device: host, sessions: [], repoUsage: { a: 1 }, + localModel: { available: "yes" } }, + }); + // Either it coerces cleanly (the guards below) or it is refused β€” never both + // 4xx AND installed. + if (res.status !== 200) { + assert.equal(agents[host], undefined, "a refused beat must not install a record"); + } else { + // Accepted means fully coerced β€” never accepted-and-raw, which is the state + // that defeats every gate downstream and poisons state.json. + assert.equal(agents[host].localModel.available, false, "served uncoerced"); + assert.deepEqual(agents[host].repoUsage, [], "a non-array repoUsage must not be served"); + } + // And the capability gate must not be fooled by the raw string either way. + const spawn = await request("POST", `/api/agents/${host}/sessions`, { + headers: userHeaders, body: { repo: "Turma", modelSource: "local" }, + }); + assert.equal(spawn.status, 409, "an uncoerced `available` must not pass the gate"); +}); + +test("normalizeUsage survives a non-iterable repoUsage or sessions", () => { + // A bare `for (… of payload.repoUsage || [])` throws on an object, and on the + // restore path that throw aborts EVERY host after this one, silently. + for (const bad of [{ a: 1 }, "str", 7, true]) { + const p = { device: "h", repoUsage: bad, sessions: bad, usage: { models: ["m"] } }; + hub.normalizeRecord(p); // must not throw + // BOTH are typed lists on Android, so both are rewritten β€” not merely + // stepped around. Guarding the loop alone turned a 400 (which never + // installed the host) into a 200 serving a fleet-killing shape. + assert.deepEqual(p.sessions, []); + assert.deepEqual(p.repoUsage, []); + } +}); + +test("normalizeSessions coerces the per-session fields Android types", () => { + // Typing a field on `SessionInfo` is what makes it decode-fatal: before that + // `ignoreUnknownKeys` skipped it and any value was harmless. `modelSource` + // and `modelSourceAt` were typed by XERK-246, so they are coerced from it. + const payload = { + device: "h", + sessions: [ + { id: "s1", modelSource: { a: 1 }, modelSourceAt: ["x"] }, + { id: "s2", modelSource: "local", modelSourceAt: "2026-08-11T00:00:00Z" }, + { id: "s3", session: { agents: [{ sel: "yes", type: { a: 1 }, label: ["x"] }] } }, + // `session` itself, not just its `agents`. `"agents" in []` is false, so + // a bare `typeof live === "object"` guard neither coerces nor rejects an + // ARRAY here and serves it raw β€” `LiveSignals?` is typed on Android, so + // that is decode-fatal for the whole payload and blocks sign-in. + { id: "s5", session: [] }, + { id: "s6", session: [1, 2] }, + { id: "s7", session: "busy" }, + { id: "s8", session: 7 }, + // Every non-object shape, not a representative one. An ARRAY element is + // the case a `typeof s !== "object"` predicate misses (`typeof [] === + // "object"`), and it is decode-fatal exactly like the other two: measured + // as the Android app unable to SIGN IN, because the login probe decodes + // /api/agents and reads the throw as "Could not reach the hub". + null, + "nope", + [1, 2], + [], + ], + }; + hub.normalizeRecord(payload); + assert.equal(payload.sessions.length, 7, "every non-object ELEMENT is dropped"); + assert.deepEqual(payload.sessions.map((s) => s.id), + ["s1", "s2", "s3", "s5", "s6", "s7", "s8"]); + // ...and every non-object `session` is REWRITTEN to null, not left raw. + for (const id of ["s5", "s6", "s7", "s8"]) { + assert.equal(payload.sessions.find((s) => s.id === id).session, null, `${id}.session`); + } + // A session that never carried `session` must not gain the key. + assert.equal("session" in payload.sessions.find((s) => s.id === "s1"), false); + assert.equal(payload.sessions[0].modelSource, ""); + assert.equal(payload.sessions[0].modelSourceAt, ""); + assert.equal(payload.sessions[1].modelSource, "local"); // good values untouched + assert.equal(payload.sessions[1].modelSourceAt, "2026-08-11T00:00:00Z"); + assert.deepEqual(payload.sessions[2].session.agents, + [{ sel: true, type: "[object Object]", label: "x" }]); + // A session that never carried the keys must not gain them β€” an older agent's + // payload has to stay byte-identical. + hub.normalizeRecord({ device: "h", sessions: [{ id: "s4" }] }); +}); + +// ---- the wire-shape sweep (XERK-259) --------------------------------------- +// `/api/agents` decodes ATOMICALLY on Android, so any one of these shapes hides +// the WHOLE fleet from the phone β€” measured as the app refusing to sign in at +// all ("Could not reach the hub β€” check the URL"), because the login probe +// decodes that payload. + +test("repoUsage ELEMENTS are coerced, not just the field's type (XERK-259)", () => { + // The four shapes the QA pass drove through a real heartbeat. Coercing the + // field to `[]` when it isn't an array, and then serving `[null]` unchanged, + // fixes the shape nobody sends and keeps the one that breaks the phone. + const p = { + device: "h", + repoUsage: [null, "nope", [1], { repo: { a: 1 } }, + { repo: "good", remoteKey: "gh:o/r" }], + }; + hub.normalizeRecord(p); + // Non-objects DROPPED (a null element is fatal even though a null FIELD is + // fine β€” `coerceInputValues` reaches the field's default, not the element). + // `[1]` is the case a `typeof el !== "object"` test lets through. + assert.deepEqual(p.repoUsage, [{ repo: "" }, { repo: "good", remoteKey: "gh:o/r" }]); +}); + +test("http: repoUsage:[null] no longer hides the fleet from the phone", async () => { + await request("POST", "/api/heartbeat", { + headers: agentHeaders, + body: { device: "ru-null", sessions: [], repoUsage: [null] }, + }); + const res = await request("GET", "/api/agents", { headers: userHeaders }); + const host = res.body.agents.find((a) => a.device === "ru-null"); + assert.deepEqual(host.repoUsage, [], "the fatal element must not be served"); + assert.ok(res.body.agents.length > 1, "and the rest of the fleet is still there"); +}); + +test("the shape sweep drops a bad list element in EVERY typed list, not just sessions", () => { + // One field at a time is how this bug got filed twice. Everything a client + // types as a list is swept the same way, and `[]` (whose `typeof` is + // "object") is the element shape a careless predicate serves raw. + const p = { + device: "h", + repos: [[], { name: "r", resumable: [null, { transcriptId: "t" }] }], + closedSessions: ["x", { id: "c1", prs: [[], { url: "u", number: 3 }] }], + sessions: [[1, 2], { id: "s1", prs: [null] }], + clones: [null, { repo: "o/r" }], + gitSources: [7, { source: "azure", repos: ["nope", { name: "r" }] }], + jira: { tickets: [null, { key: "X-1", labels: [null, "ok", { a: 1 }] }], + repoOptions: [[], { name: "r" }] }, + github: { repos: [null, { name: "r" }] }, + models: { available: [null, "sonnet", { a: 1 }] }, + }; + hub.normalizeRecord(p); + assert.deepEqual(p.repos, [{ name: "r", resumable: [{ transcriptId: "t" }] }]); + assert.deepEqual(p.closedSessions, [{ id: "c1", prs: [{ url: "u", number: 3 }] }]); + assert.deepEqual(p.sessions, [{ id: "s1", prs: [] }]); + assert.deepEqual(p.clones, [{ repo: "o/r" }]); + assert.deepEqual(p.gitSources, [{ source: "azure", repos: [{ name: "r" }] }]); + assert.deepEqual(p.jira.tickets, [{ key: "X-1", labels: ["ok"] }]); + assert.deepEqual(p.jira.repoOptions, [{ name: "r" }]); + assert.deepEqual(p.github.repos, [{ name: "r" }]); + assert.deepEqual(p.models.available, ["sonnet"]); +}); + +test("a wrong-typed field becomes its client's can't-tell value, never a plausible one", () => { + const p = { + device: { a: 1 }, // String <- object: decode-fatal + agentVersion: ["1.2"], // String <- array: decode-fatal + uploadMaxBytes: 1.5, // Long cannot take a fractional literal + capacity: { maxSessions: 99999999999999, running: "3", free: 2 }, // Int is 32-bit + github: "nope", // object <- string + claudeAuth: { present: "yes", refreshExpiresAt: "soon" }, + sessions: [{ + id: "s1", ttydPort: 2.5, restartCount: -1, + work: { pushed: "maybe", aheadOfBase: 9e99, baseRef: { a: 1 } }, + session: { paneBusy: "busy", transcriptAgeSec: "3", questionIndex: 1.5 }, + ticket: { key: "X-1", branch: ["b"] }, + }], + }; + hub.normalizeRecord(p); + assert.equal(p.device, ""); + assert.equal(p.agentVersion, ""); + assert.equal(p.uploadMaxBytes, 0); + assert.deepEqual(p.capacity, { maxSessions: 0, running: 0, free: 2 }); + assert.equal(p.github, null); + assert.deepEqual(p.claudeAuth, { present: false, refreshExpiresAt: null }); + const s = p.sessions[0]; + assert.equal(s.ttydPort, 0); + assert.equal(s.restartCount, -1, "a legitimate value is untouched"); + // A NULLABLE field coerces to null, not to a zero β€” `pushed: false` would read + // as "this branch is definitely not pushed" and `aheadOfBase: 0` as "nothing + // to lose", which is the opposite of what the work-risk line is for. + assert.deepEqual(s.work, { pushed: null, aheadOfBase: null, baseRef: null }); + assert.deepEqual(s.session, { paneBusy: null, transcriptAgeSec: null, + questionIndex: null }); // no `agents` key invented + assert.deepEqual(s.ticket, { key: "X-1", branch: null }); +}); + +test("a map VALUE that would throw is dropped, unlike a field's", () => { + // `usage.days` is a Map: `coerceInputValues` does not + // reach map values, so a null there is fatal where a null FIELD is not. + const p = { device: "h", usage: { days: { a: null, b: "x", c: [1], + "2026-08-12": { input: 5 } } } }; + hub.normalizeRecord(p); + assert.deepEqual(p.usage.days, { "2026-08-12": { input: 5 } }); + // ...and a non-object `days` becomes the empty map, not an array. + const q = { device: "h", usage: { days: [1, 2] } }; + hub.normalizeRecord(q); + assert.deepEqual(q.usage.days, {}); +}); + +test("a transcript block whose `t` is not a string decodes as unknown, not as a throw", () => { + // Blocks are polymorphic on `t`. An unknown NAME falls back to UnknownBlock, + // but a non-string discriminator throws out of the decoder, so it is coerced + // to "" β€” which lands on that same fallback. + const p = { device: "h", sessions: [{ id: "s1", session: { tail: [ + null, + { id: "e1", blocks: [null, "x", { t: { a: 1 } }, { t: "text", text: { a: 1 } }, + { t: "tool_use", input: { any: ["shape", 1, null] } }] }, + ] } }] }; + hub.normalizeRecord(p); + const tail = p.sessions[0].session.tail; + assert.equal(tail.length, 1, "a null tail entry is fatal too"); + assert.deepEqual(tail[0].blocks, [ + { t: "" }, { t: "text", text: "" }, + // A tool_use block's `input` is typed JsonElement? β€” any shape is legal, so + // it must survive verbatim. + { t: "tool_use", input: { any: ["shape", 1, null] } }, + ]); +}); + +test("the sweep is not a whitelist: untyped keys ride through untouched", () => { + // Rebuilding these objects instead of coercing them in place would drop every + // sub-key a newer agent adds, fleet-wide, until this table caught up β€” the + // failure mode that makes a whitelist worse than the hole it closes. + const p = { + device: "h", futureField: { deep: [1, { x: null }] }, + sessions: [{ id: "s1", futureSessionField: { a: [null] } }], + jira: { tickets: [{ key: "X-1", futureTicketField: ["anything"] }] }, + }; + hub.normalizeRecord(p); + assert.deepEqual(p.futureField, { deep: [1, { x: null }] }); + assert.deepEqual(p.sessions[0].futureSessionField, { a: [null] }); + assert.deepEqual(p.jira.tickets[0].futureTicketField, ["anything"]); +}); + +test("an older agent's payload comes out byte-identical", () => { + // The sweep must never ADD a key: a field the record does not carry means + // "this agent can't tell you", and inventing "" or 0 for it turns that into a + // measurement. Nulls are left alone for the same reason β€” every client already + // reads one as its default. + const before = JSON.stringify({ + device: "old", agentVersion: "0.1", repoUsage: [{ repo: "r" }], + sessions: [{ id: "s1", session: { paneBusy: null }, usage: null }], + jira: { tickets: [{ key: "X-1", dueDate: null }] }, + }); + const p = JSON.parse(before); + hub.normalizeRecord(p); + assert.equal(JSON.stringify(p), before); +}); + +test("the sweep runs AFTER normalizeModelUsage, so a legacy model list survives", () => { + // Old agents report `usage.models` as bare model-name STRINGS, which + // normalizeModelUsage rewrites into objects. Sweeping first would drop them as + // non-objects and silently zero that host's usage page. + const p = { device: "h", usage: { models: ["sonnet", "opus"] }, + repoUsage: [{ repo: "r", usage: { models: ["haiku"] } }] }; + hub.normalizeRecord(p); + assert.deepEqual(p.usage.models, [{ model: "sonnet" }, { model: "opus" }]); + assert.deepEqual(p.repoUsage[0].usage.models, [{ model: "haiku" }]); +}); + +test("limits' two epoch fields are INTEGER-bounded, not merely finite", () => { + // `resetsAt`/`capturedAt` are `Long` on Android and a Long cannot take a + // fractional literal, so `Number.isFinite` was never the right gate: a + // `resetsAt: 1.5` from one host passed it, was served raw, and threw for the + // whole fleet payload β€” the exact failure normalizeLimits exists to prevent, + // inside normalizeLimits. `usedPct` is a Double and stays loosely checked. + const p = { device: "h", limits: { fiveHour: { usedPct: 50.5, resetsAt: 1.5 }, + sevenDay: { usedPct: 10, resetsAt: 1e21 }, + capturedAt: 1700000000 } }; + hub.normalizeRecord(p); + assert.deepEqual(p.limits, { + fiveHour: { usedPct: 50.5 }, // the window survives; the bad stamp is dropped + sevenDay: { usedPct: 10 }, + capturedAt: 1700000000, + }); + // A snapshot with no usable capturedAt is the whole block's own "can't tell + // you" β€” every reader ages the snapshot against that stamp. + const q = { device: "h", limits: { fiveHour: { usedPct: 50 }, capturedAt: 2.5 } }; + hub.normalizeRecord(q); + assert.equal(q.limits, null); +}); + +// Blank out the parts of a source file that a pattern matching CODE must not +// read as code, LENGTH-PRESERVINGLY so offsets into the original stay valid. +// Comment bodies always; string bodies only when asked (the require walk below +// needs its strings intact, the Kotlin parser must not read a default's +// contents as syntax). Both guards below got this wrong in opposite directions: +// one read a `//` inside a string as a comment, the other read `"/*"` as one. +function blankNonCode(src, { blankStrings = false, regexLiterals = false } = {}) { + let out = ""; + const pad = (s) => s.replace(/[^\n]/g, " "); // keep line numbering intact + // Whether a `/` here opens a REGEX or is a division, by the previous + // significant token β€” the standard JS lexer rule. `regexLiterals` is off for + // Kotlin, which has no such literal and where `a / b / c` would otherwise read + // as one. + const opensRegex = () => { + const before = out.replace(/\s+$/, ""); + if (!before) return true; + const last = before[before.length - 1]; + if ("(,=:[!&|?{};+-*%~^<>".includes(last)) return true; + return /\b(return|typeof|case|in|of|do|else|yield|await|delete|void|new)$/.test(before); + }; + for (let i = 0; i < src.length; ) { + if (regexLiterals && src[i] === "/" && src[i + 1] !== "/" && src[i + 1] !== "*" && + opensRegex()) { + // A regex literal cannot span a line, so scan to the closing unescaped `/` + // on this one, stepping over a `[...]` class. Consuming it is the point: a + // quote or BACKTICK inside a character class (`` /`([^`]+)`/g ``, which + // glasses and veiller both contain verbatim) otherwise opens a phantom + // literal that runs to the next matching one β€” swallowing every comment in + // between and un-blanking them, which is the false positive this whole + // helper exists to prevent. + let j = i + 1, inClass = false, closed = false; + for (; j < src.length; j++) { + const c = src[j]; + if (c === "\\") { j++; continue; } + if (c === "\n") break; // unterminated: not a regex + if (inClass) { if (c === "]") inClass = false; continue; } + if (c === "[") { inClass = true; continue; } + if (c === "/") { j++; closed = true; break; } + } + if (closed) { out += pad(src.slice(i, j)); i = j; continue; } + } + if (src.startsWith('"""', i)) { // Kotlin raw string + let stop = src.length; + const end = src.indexOf('"""', i + 3); + if (end !== -1) { + // A raw string may END with a quote (`"""ab""""`), so the closer is the + // LAST three of the run, not the first: stopping at the first leaves a + // stray `"` that opens a phantom string over the rest of the file. + stop = end + 3; + while (src[stop] === '"') stop++; + } + out += blankStrings ? pad(src.slice(i, stop)) : src.slice(i, stop); + i = stop; + } else if (src.startsWith("//", i)) { + const end = src.indexOf("\n", i); + const stop = end === -1 ? src.length : end; + out += pad(src.slice(i, stop)); + i = stop; + } else if (src.startsWith("/*", i)) { + const end = src.indexOf("*/", i + 2); + const stop = end === -1 ? src.length : end + 2; + out += pad(src.slice(i, stop)); + i = stop; + } else if (src[i] === '"' || src[i] === "'" || src[i] === "`") { + const quote = src[i]; + let j = i + 1; + let quoteIsCode = false; + let closed = false; + for (; j < src.length; j++) { + if (src[j] === "\\") { j++; continue; } // an escaped quote does not close + if (src[j] === quote) { j++; closed = true; break; } + // Neither language lets a `'`/`"` literal span a line, so one that does + // was never a string β€” overwhelmingly a quote inside a JS REGEX class + // (`/[.,;:!?'"]+$/`, which this repo really contains). Reading it as a + // string opens a phantom literal that swallows every comment to the next + // matching quote, un-blanking them: the P3 false positive, re-armed by a + // one-character edit to any character class. + if (src[j] === "\n" && quote !== "`") { quoteIsCode = true; break; } + } + // A literal that never closes was never a literal. That covers the + // newline rule's blind spot, the BACKTICK β€” exempt from it because a + // template literal may legitimately span lines, so a regex holding an odd + // number of them (`` /`([^`]+)`/g ``, which glasses and veiller both + // contain) would otherwise open a phantom literal that ran to EOF and + // stopped comment-blanking for the rest of the file. + if (quoteIsCode || !closed) { out += quote; i++; continue; } + const lit = src.slice(i, j); + out += blankStrings ? quote + pad(lit.slice(1, -1)) + quote : lit; + i = j; + } else { + out += src[i]; + i++; + } + } + return out; +} + +test("turma/Dockerfile copies every local module the hub requires", () => { + // The COPY list is hand-maintained, and a module missing from it is a + // container that boots to a MODULE_NOT_FOUND β€” invisible until deploy, since + // every test here requires straight off the working tree. + // + // The whole require GRAPH, not just server.js's own line: a module two hops + // out is just as absent from the image, and the crash reads the same. Both + // quote styles and a nested path count, because a pattern that only matches + // today's three requires is a test that passes on the shape it was written + // against rather than the one that breaks. + const dir = path.join(__dirname, ".."); + const dockerfile = fs.readFileSync(path.join(dir, "Dockerfile"), "utf8") + .replace(/^\s*#[^\n]*/gm, ""); // a name in a COMMENT copies nothing + // Known bounds, all accepted: a require written INSIDE a string reads as a + // real one (the walk needs string contents to read the path out, so it cannot + // tell them apart without a parser), and `../`, `require.resolve` and a + // computed path stay invisible β€” two of those are not statically analysable + // and the hub uses none of them. + const local = /require\(\s*['"`]\.\/([\w./-]+)['"`]\s*\)/g; + const seen = new Set(["server.js"]); + const queue = ["server.js"]; + const required = new Set(); + while (queue.length) { + const file = queue.pop(); + const full = path.join(dir, file); + // A module named only in PROSE is not a require, and this repo's comments + // name modules constantly β€” a stale note about one that has since been + // deleted must not redden CI. Nothing asserts the file exists: a genuinely + // missing local module takes the whole suite down at `require("../server.js")` + // long before this test runs, so an assert here could only ever fire on a + // false positive. + if (!fs.existsSync(full)) continue; + const code = blankNonCode(fs.readFileSync(full, "utf8"), { regexLiterals: true }); + for (const m of code.matchAll(local)) { + required.add(m[1]); + if (!seen.has(m[1])) { seen.add(m[1]); queue.push(m[1]); } + } + } + assert.ok(required.size >= 3, "no local requires found β€” did the pattern rot?"); + for (const f of required) { + // A module in a subdirectory is copied by copying that directory (the + // `COPY public ./public` idiom), so the first segment is what must appear. + const want = f.includes("/") ? f.split("/")[0] : f; + assert.match(dockerfile, new RegExp(`COPY[^\\n]*(^|[\\s/])${want.replace(/\./g, "\\.")}([\\s/]|$)`, "m"), + `${f} is required by the hub but never COPYed into the image`); + } +}); + +test("the shape table only uses tags the walker knows", () => { + // The walker passes an unrecognised tag THROUGH rather than throwing (a throw + // on ingest refuses the beat; on the restore path it abandons every record + // after it). That makes a typo in the table silent β€” so it is caught here + // instead. + const known = new Set(["s", "s?", "b", "b?", "i", "i?", "l", "l?", "d", "d?"]); + const seen = new Set(); + const walk = (spec, where) => { + if (typeof spec === "string") { + assert.ok(known.has(spec), `unknown spec tag ${JSON.stringify(spec)} at ${where}`); + return; + } + assert.ok(["obj", "list", "map"].includes(spec.kind), + `unknown spec kind ${JSON.stringify(spec.kind)} at ${where}`); + if (spec.kind === "obj") { + if (seen.has(spec)) return; // shared sub-shapes are reused by reference + seen.add(spec); + for (const [k, v] of Object.entries(spec.fields)) walk(v, `${where}.${k}`); + } else walk(spec.of, `${where}[]`); + }; + for (const [k, v] of Object.entries(hub.AGENT_WIRE_SHAPE)) walk(v, k); +}); + +test("every field Android types on the fleet payload is in the shape table", () => { + // The durable form of "typing a field on a client and coercing it hub-side are + // ONE change": this reads `Models.kt` and walks it, so a field typed there + // without an entry here fails the hub's own suite rather than waiting for a + // phone to stop signing in. + // String and char literals are blanked BEFORE anything reads this as syntax: + // a default of `"https://x"` hid a comment, `"/*"` opened one, `">"` broke the + // bracket depth, and `"use val x: Int"` invented a field β€” each of which + // failed a paired, CORRECT change with a message accusing the reader of + // breaking the parser. Length-preserving, so the offsets below stay valid. + const src = blankNonCode(fs.readFileSync(path.join(__dirname, "..", "..", "android", + "app", "src", "main", "java", "com", "xerktech", "turma", "model", "Models.kt"), + "utf8"), { blankStrings: true }); + // One entry per `data class X(...)`: its fields as {name, type}. + // + // Parsed by matching parens and then splitting the constructor on TOP-LEVEL + // commas, not by a per-line regex. Both departures are load-bearing, and a + // parser that quietly sees fewer fields than exist is the one failure mode + // this test cannot survive: + // - a constructor body carries its own `()`/`<>` (`= UsageBucket()`, + // `Map`), so "up to the next `\n)`" mis-parses; + // - the block classes are written on ONE line, so an `^`-anchored field + // pattern sees only the first field of each; + // - a field with NO default (`val x: String,`) is the DANGEROUS kind β€” it is + // required on the wire, so every host omitting it kills the decode β€” and a + // pattern keyed on `=` skips exactly those. + const classes = {}; + const subclassesOfBlock = []; + // `\s*` around the paren: `data class X` with its `(` on the next line is a + // legal declaration, and a class this misses is one whose sub-shape then goes + // unchecked β€” the guard's reach must not ride on brace style. + for (const m of src.matchAll(/data class\s+(\w+)\s*\(/g)) { + let depth = 1, i = m.index + m[0].length; + for (; i < src.length && depth; i++) { + if (src[i] === "(") depth++; + else if (src[i] === ")") depth--; + } + const body = src.slice(m.index + m[0].length, i - 1); // already blanked + const params = []; + for (let d = 0, start = 0, j = 0; j <= body.length; j++) { + const ch = body[j]; + if (ch === "(" || ch === "<" || ch === "[") d++; + else if (ch === ")" || ch === ">" || ch === "]") d--; + else if ((ch === "," && !d) || j === body.length) { + params.push(body.slice(start, j)); + start = j + 1; + } + } + classes[m[1]] = params + .map((p) => /\bval (\w+)\s*:\s*([\w<>, ?]+?)\s*(?:=|$)/.exec(p)) + .filter(Boolean) + .map((f) => ({ name: f[1], type: f[2].trim() })); + // EVERY declared field must have survived the split. The splitter counts + // bracket depth and does not know string literals, so a default containing a + // bare `>`/`(`/`[` (`= ">".length`) desynchronises it and silently folds the + // rest of the constructor into one param. Counting `val`s in the same + // comment-stripped body is the check that cannot itself drift: a field-count + // FLOOR would let this pass the moment the class grew past the floor, which + // is the state a growing class reaches on its own. + const declared = (body.match(/\bval\s+\w+\s*:/g) || []).length; + assert.equal(classes[m[1]].length, declared, + `parsed ${classes[m[1]].length} of ${declared} fields in ${m[1]} β€” the parser ` + + `lost some, so this test is not checking what it claims to`); + if (/^\s*:\s*Block\(\)/.test(src.slice(i))) subclassesOfBlock.push(m[1]); + } + // ...and the shapes a mis-parse drops first were seen AT ALL: AgentInfo is + // multi-line with comments between params, TextBlock is a one-liner, and + // CreateTicketRequest's first fields carry no default. Deliberately NOT their + // field counts β€” the per-class equality above already proves every field of + // every class is visible, generically, so pinning a count here would only + // fail the day someone legitimately adds one (and `CreateTicketRequest` is not + // even on the agent record). + assert.ok(classes.AgentInfo?.length && subclassesOfBlock.length >= 5 && + classes.TextBlock?.length && classes.CreateTicketRequest?.length, + "Models.kt did not parse β€” this test is only as good as the field list it built"); + // A transcript block is polymorphic: the table carries the UNION of every + // subclass's fields under one object spec. + classes.__Block = [{ name: "t", type: "String" }, + ...subclassesOfBlock.flatMap((c) => classes[c])]; + + // Nothing agent-authored is exempt. `limits` and `localModel` are rebuilt + // wholesale by their own normalize*, and are in the table anyway β€” a rebuild + // is only as good as its own gates, and `limits` shipped one that let a + // fractional `resetsAt` through. + const SKIP = new Set([ + "AgentInfo.key", "AgentInfo.online", "AgentInfo.terminalOnline", + "AgentInfo.lastSeen", // hub-authored in serializeAgent, never off the wire + "__Block.input", // typed JsonElement? β€” any shape is legal + ]); + const scalars = { String: "s", Boolean: "b", Int: "i", Long: "l", Double: "d" }; + const missing = []; + const walk = (className, spec, path) => { + for (const f of classes[className] || []) { + if (SKIP.has(`${className}.${f.name}`)) continue; + const here = `${path}.${f.name}`; + const sub = spec && spec.kind === "obj" ? spec.fields[f.name] : undefined; + if (!sub) { missing.push(`${here} (${f.type})`); continue; } + const list = /^List<(\w+)>\??$/.exec(f.type); + const map = /^Map\??$/.exec(f.type); + const bare = f.type.replace(/\?$/, ""); + if (list || map) { + const el = (list || map)[1]; + const inner = (list ? sub.kind === "list" : sub.kind === "map") ? sub.of : null; + if (!inner) { missing.push(`${here} (${f.type} is not a ${list ? "list" : "map"} here)`); continue; } + if (classes[el] || el === "Block") walk(el === "Block" ? "__Block" : el, inner, here + "[]"); + else if (inner !== scalars[el]) missing.push(`${here} element (${el} vs ${inner})`); + } else if (classes[bare]) { + if (sub.kind !== "obj") missing.push(`${here} (${f.type} is not an object here)`); + else walk(bare, sub, here); + } else if (scalars[bare]) { + const want = scalars[bare] + (f.type.endsWith("?") ? "?" : ""); + if (sub !== want) missing.push(`${here} (${f.type} wants ${want}, table has ${JSON.stringify(sub)})`); + } + } + }; + walk("AgentInfo", { kind: "obj", fields: hub.AGENT_WIRE_SHAPE }, "AgentInfo"); + assert.deepEqual(missing, [], + "Models.kt types these but the hub does not coerce them β€” add them to AGENT_WIRE_SHAPE"); +}); + +test("heartbeat: a rogue localModel is coerced at ingest, not served raw", async () => { + await request("POST", "/api/heartbeat", { + body: { device: "lm6", localModel: { available: "yes", contextTokens: 9999999999 } }, + headers: agentHeaders, + }); + const res = await request("GET", "/api/agents", { headers: userHeaders }); + const host = res.body.agents.find((a) => a.device === "lm6"); + assert.equal(host.localModel.available, false); + assert.equal(host.localModel.contextTokens, null); + // And the whole fleet is still served β€” the point of coercing at ingest. + assert.ok(res.body.agents.length > 1); +}); + diff --git a/turma/wire-shape.js b/turma/wire-shape.js new file mode 100644 index 00000000..bd5045f8 --- /dev/null +++ b/turma/wire-shape.js @@ -0,0 +1,250 @@ +// The wire shape every client TYPES, and the coercion that holds the agent +// record to it (XERK-259). +// +// Its OWN module for a reason that is not tidiness: `server.js` restores +// `state.json` at module init, near the top of the file, and that restore +// coerces what it loads. A `const` declared further down the file is in its +// temporal dead zone at that moment, so reading one throws a ReferenceError +// that the restore's own `catch {}` swallows β€” leaving records half-coerced, +// silently, on every boot. A `require` runs this file to completion first, so +// nothing here can be in that window however the table grows. +// Tests: the wire-shape cases in `turma/tests/server.test.js`. + +"use strict"; + +// `server.js`'s own `normalize*` passes coerce three blocks by hand, each with +// its own semantics (dropping a window with no percentage, forcing +// `available:false`). This one runs after them and covers the WHOLE record, +// because "which blocks happen to have a normalize* yet" was never +// the boundary that matters: a field is decode-fatal the moment a client TYPES +// it, and Android types nearly all of `AgentInfo`. `repoUsage:[null]` β€” four +// bytes from one host β€” made the app refuse to sign in at all, reporting "Could +// not reach the hub", because `/api/agents` decodes atomically and the login +// probe reads it. +// +// Two rules this table exists to enforce, both learned the hard way: +// +// - A LIST's ELEMENTS are as fatal as its type. Rewriting a non-array +// `repoUsage` to `[]` and leaving `[null]` alone fixes the shape nobody +// sends and serves the one that breaks the phone. +// - `typeof x === "object"` is not "is an object" β€” `typeof [] === "object"`, +// so an ARRAY element slips through that test. Everything here goes through +// `isPlainObject`. +// +// Coercion is IN PLACE and touches only the keys named below. That is what keeps +// it from being a whitelist: a sub-key a newer agent adds rides through +// untouched (it is not typed anywhere yet, so `ignoreUnknownKeys` skips it), +// where rebuilding the object would drop it fleet-wide until this table caught +// up. Values become the "can't tell you" one every client already handles β€” "" +// / false / 0 / null / [] β€” never a plausible-looking default. +// +// Keep it in step with `android/.../model/Models.kt`: typing a field there and +// adding it here are the same change (see CLAUDE.md's heartbeat contract). +// Scope is the agent record only β€” the payload whose decode is ATOMIC. The +// per-request endpoints (history, archive, search) decode one at a time and fail +// only themselves. +// +// Spec grammar: "s"/"b"/"i"/"l"/"d" are String/Boolean/Int/Long/Double, a +// trailing "?" marking the ones a client declares nullable (they coerce to null +// rather than to a zero that reads as a real measurement); `objOf`/`listOf`/ +// `mapOf` nest. The bounds are the CLIENT's, not JSON's: an Int past 2^31 and a +// non-integer Long are both decode-fatal on Android. +const isPlainObject = (v) => !!v && typeof v === "object" && !Array.isArray(v); +const objOf = (fields) => ({ kind: "obj", fields }); +const listOf = (of) => ({ kind: "list", of }); +const mapOf = (of) => ({ kind: "map", of }); + +const WIRE_TICKET_REF = { key: "s", siteKey: "s", url: "s", summary: "s", branch: "s?" }; +const WIRE_PR = { url: "s", number: "i", state: "s", title: "s", checks: "s", + mergeable: "s", ready: "s" }; +const WIRE_BUCKET = { input: "l", output: "l", cacheWrite: "l", cacheRead: "l" }; +const WIRE_USAGE = { + today: objOf(WIRE_BUCKET), week: objOf(WIRE_BUCKET), totals: objOf(WIRE_BUCKET), + // `days` is a Map on Android: a null or a string VALUE is + // fatal there even though the same value is fine in a defaulted field, because + // `coerceInputValues` does not reach map values. Bad entries are dropped. + days: mapOf(objOf(WIRE_BUCKET)), + lastActivity: "s", + models: listOf(objOf({ model: "s", today: objOf(WIRE_BUCKET), + week: objOf(WIRE_BUCKET), totals: objOf(WIRE_BUCKET) })), +}; +const WIRE_GITHUB_REPO = { nameWithOwner: "s", name: "s", description: "s", + isPrivate: "b", updatedAt: "s" }; +// One transcript block. Polymorphic on `t`, so this is the UNION of every +// subclass's fields β€” no name collides on a different type, and a block whose +// `t` is not a string decodes as neither a known block nor the Unknown fallback: +// it throws. Coercing `t` to "" lands it on that fallback instead. +const WIRE_BLOCK = { + t: "s", text: "s", truncated: "b", id: "s", name: "s", caption: "s", + files: listOf(objOf({ name: "s", kind: "s", src: "s", html: "s" })), + forId: "s", isError: "b", summary: "s", status: "s", result: "s", ignored: "s", + // `input` is a ToolUseBlock's raw tool arguments, typed `JsonElement?` β€” + // deliberately ANY shape, so it is the one field here left alone. +}; +const WIRE_LIVE = { + paneBusy: "b?", agents: listOf(objOf({ type: "s", label: "s" })), + transcriptAgeSec: "d?", lastRole: "s", lastHasToolUse: "b", bridgeAttached: "b", + question: "s", questionOptions: listOf("s"), + questionOptionsRich: listOf(objOf({ label: "s", description: "s", preview: "s" })), + questionHeader: "s", questionIndex: "i?", questionTotal: "i?", questionMulti: "b", + newPrUrls: listOf("s"), + tail: listOf(objOf({ id: "s", uuid: "s", role: "s", text: "s", ts: "s", + blocks: listOf(objOf(WIRE_BLOCK)) })), +}; +const WIRE_SESSION = { + id: "s", status: "s", repo: "s", worktreePath: "s", branch: "s", + git: objOf({ repoName: "s", branch: "s", dirtyFiles: "i" }), + summary: "s", label: "s", root: "b", rcName: "s", ttydPort: "i", model: "s", + permissionMode: "s", modelSource: "s", modelSourceAt: "s", + usage: objOf(WIRE_USAGE), prs: listOf(objOf(WIRE_PR)), newWorkSincePrs: "b", + session: objOf(WIRE_LIVE), ticket: objOf(WIRE_TICKET_REF), spawnCmdId: "s", + transcriptId: "s", createdAt: "s", stoppedAt: "s", errorMsg: "s", + queuedReason: "s", queuedAt: "s", restartCount: "i", + work: objOf({ baseRef: "s?", aheadOfBase: "i?", pushed: "b?", aheadOfRemote: "i?" }), +}; +const WIRE_JIRA = { + available: "b", configured: "b", site: "s", siteKey: "s", user: "s", + fetchedAt: "s", error: "s?", truncated: "b", orgName: "s", source: "s", + tickets: listOf(objOf({ + key: "s", url: "s", summary: "s", status: "s", statusCategory: "s", + priority: "s", type: "s", project: "s", projectName: "s", labels: listOf("s"), + updated: "s", created: "s", dueDate: "s?", parentKey: "s?", + repoGuess: objOf({ repo: "s?", cloned: "b", nameWithOwner: "s?", reason: "s", + at: "s", manual: "b" }), + })), + repoOptions: listOf(objOf({ name: "s", cloned: "b", nameWithOwner: "s?", + description: "s" })), +}; +const AGENT_WIRE_SHAPE = { + device: "s", claudeVersion: "s", agentVersion: "s", startedAt: "s", + codingAgent: objOf({ name: "s", version: "s" }), + claudeAuth: objOf({ present: "b", needsLogin: "b", expiringSoon: "b", + refreshExpiresAt: "l?" }), + updating: objOf({ version: "s", until: "l" }), + repos: listOf(objOf({ + name: "s", root: "b", lastActivity: "s", + resumable: listOf(objOf({ transcriptId: "s", cwd: "s", repo: "s", root: "b", + summary: "s", endedTs: "s", + ticket: objOf(WIRE_TICKET_REF), + prs: listOf(objOf(WIRE_PR)) })), + })), + sessions: listOf(objOf(WIRE_SESSION)), + models: objOf({ available: listOf("s"), defaultLabel: "s", at: "s" }), + usage: objOf(WIRE_USAGE), + repoUsage: listOf(objOf({ repo: "s", remoteKey: "s", usage: objOf(WIRE_USAGE) })), + github: objOf({ available: "b", login: "s", repos: listOf(objOf(WIRE_GITHUB_REPO)) }), + gitSources: listOf(objOf({ source: "s", label: "s", available: "b", user: "s", + repos: listOf(objOf(WIRE_GITHUB_REPO)) })), + clones: listOf(objOf({ repo: "s", name: "s", status: "s", error: "s", + startedAt: "s" })), + // Hub-authored rather than agent-authored, and listed anyway: a table with a + // hole in it is one the next reader has to re-derive to trust. + commands: listOf(objOf({ type: "s", cmdId: "s", sessionId: "s", repo: "s" })), + jira: objOf(WIRE_JIRA), + closedSessions: listOf(objOf({ + id: "s", repo: "s", branch: "s", root: "b", summary: "s", summaryManual: "b", + label: "s", createdAt: "s", closedAt: "s", ticket: objOf(WIRE_TICKET_REF), + transcriptId: "s", prs: listOf(objOf(WIRE_PR)), + })), + capacity: objOf({ maxSessions: "i", running: "i", queued: "i", free: "i", + rootRunning: "b" }), + uploadMaxBytes: "l", + // `normalizeLimits`/`normalizeLocalModel` rebuild these two wholesale, which + // is not a reason to leave them out β€” a rebuild is only as good as its own + // gates, and `limits` shipped for a release gating its two epoch fields on + // `Number.isFinite`, so `resetsAt: 1.5` (a Long cannot take a fractional + // literal) went out raw and killed the whole payload. Listed here they get the + // same backstop as everything else AND the Models.kt drift test, which is the + // part that keeps the next such gap from lasting a release. + limits: objOf({ + fiveHour: objOf({ usedPct: "d?", resetsAt: "l?" }), + sevenDay: objOf({ usedPct: "d?", resetsAt: "l?" }), + capturedAt: "l", source: "s", + }), + localModel: objOf({ available: "b", model: "s?", contextTokens: "i?" }), +}; + +// Sentinel for "this value cannot be made decodable" β€” a list element or map +// value, where the only repair is to drop it (a null there is fatal, unlike a +// null in a defaulted FIELD, which every client reads as its default). +const WIRE_DROP = Symbol("drop"); + +const WIRE_SCALAR_OK = { + s: (v) => typeof v === "string", + b: (v) => typeof v === "boolean", + // Kotlin `Int` is 32-bit and `Long` cannot take a fractional literal; either + // overflow throws for the whole payload, so both are checked here, not just + // "is a number". + i: (v) => typeof v === "number" && Number.isInteger(v) && + v >= -2147483648 && v <= 2147483647, + l: (v) => typeof v === "number" && Number.isSafeInteger(v), + d: (v) => typeof v === "number" && Number.isFinite(v), +}; +const WIRE_SCALAR_ZERO = { s: "", b: false, i: 0, l: 0, d: 0 }; + +// Coerce one value against one spec. `nullOk` is the difference between a FIELD +// (a null is that client's default β€” leave it) and a list element or map value +// (a null throws β€” drop it). +function coerceWireValue(v, spec, nullOk) { + if (v === null || v === undefined) return nullOk ? v : WIRE_DROP; + if (typeof spec === "string") { + const nullable = spec.endsWith("?"); + const tag = nullable ? spec.slice(0, -1) : spec; + // A tag the grammar doesn't define is a typo in the table, not a value to + // judge: pass the value through rather than throw, since a throw in here is + // a refused beat on ingest and an abandoned record on the restore path. + // `every spec tag in AGENT_WIRE_SHAPE is one the walker knows` is the test + // that catches the typo instead. + if (!WIRE_SCALAR_OK[tag]) return v; + if (WIRE_SCALAR_OK[tag](v)) return v; + if (!nullOk) return WIRE_DROP; + return nullable ? null : WIRE_SCALAR_ZERO[tag]; + } + if (spec.kind === "obj") { + if (!isPlainObject(v)) return nullOk ? null : WIRE_DROP; + coerceWireFields(v, spec.fields); + return v; + } + if (spec.kind === "list") { + // A non-array becomes [], the shape "this host reported none" already has. + if (!Array.isArray(v)) return nullOk ? [] : WIRE_DROP; + // Compacted IN PLACE β€” no new array, no `push(...kept)` spread that would + // blow the stack on a long list, and the record keeps holding the same + // array `commands` was queued into. + let w = 0; + for (let r = 0; r < v.length; r++) { + const c = coerceWireValue(v[r], spec.of, false); + if (c !== WIRE_DROP) v[w++] = c; + } + v.length = w; + return v; + } + // map: string keys only, and a value that cannot be coerced is deleted. + if (!isPlainObject(v)) return nullOk ? {} : WIRE_DROP; + for (const k of Object.keys(v)) { + const c = coerceWireValue(v[k], spec.of, false); + if (c === WIRE_DROP) delete v[k]; + else v[k] = c; + } + return v; +} + +// Walk one object against its spec, touching ONLY the keys the spec names and +// only when the record actually carries them β€” an older agent's payload has to +// come out byte-identical, and a key it never sent must not appear. +function coerceWireFields(target, fields) { + for (const k of Object.keys(fields)) { + // hasOwnProperty, not `in`: `in` walks the prototype chain, so a field + // sharing a name with an Object.prototype member would read as present on + // every record and be written onto one that never carried it. + if (!Object.prototype.hasOwnProperty.call(target, k)) continue; + target[k] = coerceWireValue(target[k], fields[k], true); + } +} + +function normalizeWireShapes(a) { + if (!isPlainObject(a)) return; + coerceWireFields(a, AGENT_WIRE_SHAPE); +} + +module.exports = { normalizeWireShapes, AGENT_WIRE_SHAPE };