From 646f130d0adc0f038695e6bcb198617c5a55c51e Mon Sep 17 00:00:00 2001 From: Eric Moore Date: Thu, 27 Aug 2026 21:29:00 -0500 Subject: [PATCH] =?UTF-8?q?feat(compat):=20a=20floor,=20not=20an=20equalit?= =?UTF-8?q?y=20=E2=80=94=20and=20the=20ordering=20trap=20under=20it?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CIRISServer#497 moves the server's dependency to `ciris-client>=0.5.190,<0.6` so the client can ship for the agent team without a paired server cut. CIRISClient#16 is the other half: under a range, comparing the node's version to ours for EQUALITY fires on every legitimately-mixed pair. A 0.5.192 node with a 0.5.193 client is the point of decoupling, and it would have greeted its operator with a permanent nag. That trades a release dance for a nag, which is worse. It was never a bug while the versions were locked. They were always equal, the comparison was always true, and the branch was never exercised. Widening the range is what turns a held-still assumption into a defect. THE ORDERING TRAP, WHICH IS THE PART THAT WOULD HAVE SHIPPED QUIETLY. Equality never ORDERED anything, so it could not be wrong about order. A floor is nothing but order, and string comparison is wrong at the versions this repo actually ships: "0.5.9" > "0.5.190" lexically, because '9' > '1' "0.5.191" < "0.5.9" lexically Three-digit patch numbers make that the common case here, not an edge — a floor written the obvious way decides a 0.5.9 node clears a 0.5.190 floor. `compareVersions` is numeric per component, and the test names the exact pairs. TWO DIRECTIONS, WHICH EQUALITY CONFLATED: node too old for this client MIN_NODE_VERSION, held here, because a node that predates the idea of declaring a floor cannot tell us anything about itself. client too old for this node declared BY THE NODE, because only the node knows which clients it supports. No client-held constant can learn that a newer node needs a newer client — it would call itself compatible and be wrong. Nothing declares the second today, so it is optional, and absent means "did not say" rather than "is happy". The floor this side holds still applies, which keeps the signal the nag exists for: a genuinely incompatible pair still says so. Replacing equality with "never complain" would have deleted it, and someone shipped a mismatched pair once already. MIN_NODE_VERSION IS NOT YET EARNED, AND THE CODE SAYS SO. The server mutation-tested its floor at both ends — 0.5.186 fails the id gate, 0.5.188 passes it and fails the wheel gate, so neither gate alone would have caught a floor that breaks Android. Ours is the pairing both sides are standardising on and nothing here proves this client cannot drive an older node. Until a gate installs the floor and exercises the API surface this client calls, it is what they called it: an untested bound is a guess with a version number on it. Two existing tests changed meaning, and are rewritten rather than deleted. `version_mismatch_ignores_the_v_prefix` paired 0.5.176 with itself and asserted no flag — true only under equality; a 0.5.176 node is now genuinely too old, so the prefix is demonstrated at versions where the answer is not about age. `version_mismatch_fires_on_a_real_difference` is renamed to `..._on_a_node_below_the_floor`: it still fires, for a different reason, and a mere difference no longer fires at all. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EGE52kPzjGFiPzGcs63ZC1 --- client/VENDORING.md | 2 +- .../ciris/mobile/shared/models/ClientMode.kt | 99 +++++++++++++++++-- .../mobile/shared/models/ClientModeTest.kt | 16 ++- .../shared/models/CompatibilityFloorTest.kt | 84 ++++++++++++++++ 4 files changed, 192 insertions(+), 9 deletions(-) create mode 100644 client/shared/src/commonTest/kotlin/ai/ciris/mobile/shared/models/CompatibilityFloorTest.kt diff --git a/client/VENDORING.md b/client/VENDORING.md index c746ed0..a12ad20 100644 --- a/client/VENDORING.md +++ b/client/VENDORING.md @@ -40,7 +40,7 @@ source is the pair a bisect wants: The tree's current recorded state — sha256-of-sha256s over every git-tracked file under `client/` except this one: -**state digest:** `42c4a314d998300123ba5ff488f269cba2fc4ecc6eff2bba8814f7f7ee4a09f7` +**state digest:** `34d7639263b56a316ea2da485f6ed070bbe6822b502f900228041b443e5186ed` `packaging/check_vendoring.py` asserts it on every push, and refuses any tracked file matching a §2 never-vendor class. **Any commit that touches diff --git a/client/shared/src/commonMain/kotlin/ai/ciris/mobile/shared/models/ClientMode.kt b/client/shared/src/commonMain/kotlin/ai/ciris/mobile/shared/models/ClientMode.kt index c641297..c72af40 100644 --- a/client/shared/src/commonMain/kotlin/ai/ciris/mobile/shared/models/ClientMode.kt +++ b/client/shared/src/commonMain/kotlin/ai/ciris/mobile/shared/models/ClientMode.kt @@ -219,14 +219,101 @@ fun clientModeFrom( // unaffected: same package, same name, same `const val`. /** - * Whether [nodeVersion] differs materially from [CLIENT_VERSION] — i.e. a - * non-blocking "update recommended" banner should be shown. Compares the - * leading `major.minor.patch` (ignoring any pre-release/build suffix) and only - * flags an actual mismatch (never flags when the node version is unknown/blank). + * The oldest node this client can drive. + * + * A FLOOR, not a pin. CIRISServer 0.5.192 moves its own dependency to + * `ciris-client>=0.5.190,<0.6` (CIRISServer#497) so the client can ship for the + * agent team without a paired server cut; this is the same relaxation from the + * other side. + * + * THIS NUMBER IS NOT YET EARNED THE WAY THE SERVER EARNED ITS FLOOR. The server + * mutation-tested theirs at both ends — lower it to 0.5.186 and the id gate + * fails, lower it to 0.5.188 and the wheel gate fails — so the bound is measured + * rather than asserted. Ours is the pairing both sides are standardising on and + * nothing here proves this client cannot drive an older node. Until a gate + * installs the floor and exercises the API surface this client actually calls, + * treat it as the server team put it: an untested bound is a guess with a + * version number on it. + */ +const val MIN_NODE_VERSION: String = "0.5.190" + +/** + * Compare two `major.minor.patch` versions NUMERICALLY. Negative, zero, positive. + * + * NOT string comparison, and the difference is live at the versions this repo + * actually ships. Lexically `"0.5.9" > "0.5.190"`, because '9' > '1' — so a + * floor check written the obvious way decides a 0.5.9 node satisfies a 0.5.190 + * floor and a 0.5.191 client is below a 0.5.9 one. Three-digit patch numbers + * make this the common case here, not an edge. + * + * The old equality check was immune to this because it never ORDERED anything. + * Introducing a floor introduces ordering, and ordering is the whole trap. + * + * Missing components read as 0, so `0.5` and `0.5.0` compare equal. A component + * that is not a number sinks to 0 rather than throwing: this drives a banner, + * and a malformed version should not be able to crash the surface that reports + * it. + */ +fun compareVersions(a: String, b: String): Int { + fun parts(v: String): List = + v.trim().removePrefix("v").takeWhile { it.isDigit() || it == '.' } + .split('.').map { it.toIntOrNull() ?: 0 } + val pa = parts(a) + val pb = parts(b) + for (i in 0 until maxOf(pa.size, pb.size)) { + val d = (pa.getOrNull(i) ?: 0) - (pb.getOrNull(i) ?: 0) + if (d != 0) return if (d < 0) -1 else 1 + } + return 0 +} + +/** + * Whether the node and this client are INCOMPATIBLE — i.e. a non-blocking + * "update recommended" banner should be shown. + * + * WAS EQUALITY, WHICH ONLY WORKED BECAUSE THE VERSIONS WERE LOCKED. While the + * server pinned `ciris-client==` the two were always equal, the + * comparison was always true, and the branch was never exercised. Widening to a + * range turns that held-still assumption into a defect: a 0.5.192 node with a + * 0.5.193 client is THE POINT of decoupling, and equality would greet its + * operator with a permanent nag. Trading a release dance for a permanent nag is + * the worse trade (CIRISClient#16). + * + * TWO DIRECTIONS, WHICH EQUALITY CONFLATED INTO ONE: + * + * - is the NODE too old for this client? [MIN_NODE_VERSION], held here, + * because a node that predates the whole idea of declaring a floor cannot + * tell us anything about itself. + * - is the CLIENT too old for this node? [nodeMinClientVersion], declared + * BY THE NODE, because only the node knows which clients it supports. A + * client-held answer to this cannot ever learn that a newer node needs a + * newer client — it would call itself compatible and be wrong. + * + * Nothing declares [nodeMinClientVersion] today, so it is optional and absent + * means "the node did not say", not "the node is happy". The floor this side + * holds still applies, which keeps the real signal the nag exists for: a + * genuinely incompatible pair still says so. Replacing equality with "never + * complain" would have deleted that signal, and someone shipped a mismatched + * pair once already. + * + * Never flags on an unknown or blank node version — that is "I could not ask", + * which is not "you are broken". */ -fun isVersionMismatch(nodeVersion: String?, clientVersion: String = CLIENT_VERSION): Boolean { +fun isVersionMismatch( + nodeVersion: String?, + clientVersion: String = CLIENT_VERSION, + nodeMinClientVersion: String? = null, + minNodeVersion: String = MIN_NODE_VERSION, +): Boolean { val node = nodeVersion?.trim()?.removePrefix("v")?.takeWhile { it.isDigit() || it == '.' } if (node.isNullOrBlank()) return false val client = clientVersion.trim().removePrefix("v").takeWhile { it.isDigit() || it == '.' } - return node != client + + // The node's own claim about what it needs, when it makes one. + val declared = nodeMinClientVersion?.trim()?.removePrefix("v") + ?.takeWhile { it.isDigit() || it == '.' } + if (!declared.isNullOrBlank() && compareVersions(client, declared) < 0) return true + + // And what this client needs of the node. + return compareVersions(node, minNodeVersion) < 0 } diff --git a/client/shared/src/commonTest/kotlin/ai/ciris/mobile/shared/models/ClientModeTest.kt b/client/shared/src/commonTest/kotlin/ai/ciris/mobile/shared/models/ClientModeTest.kt index 7b7f9a3..8db092c 100644 --- a/client/shared/src/commonTest/kotlin/ai/ciris/mobile/shared/models/ClientModeTest.kt +++ b/client/shared/src/commonTest/kotlin/ai/ciris/mobile/shared/models/ClientModeTest.kt @@ -330,12 +330,24 @@ class ClientModeTest { @Test fun version_mismatch_ignores_the_v_prefix() { - assertFalse(isVersionMismatch("v0.5.176", "0.5.176")) + // Intent unchanged; the EXAMPLE had to move. It used to pair 0.5.176 + // with 0.5.176 and assert no flag, which held only because the check was + // equality. Under a compatibility floor (CIRISClient#16) a 0.5.176 node + // is genuinely too old and SHOULD flag, so the prefix is now shown to be + // ignored at versions where the answer is not about age. + assertFalse(isVersionMismatch("v0.5.191", "0.5.191")) + assertTrue(isVersionMismatch("v0.5.176", "0.5.191")) } @Test - fun version_mismatch_fires_on_a_real_difference() { + fun version_mismatch_fires_on_a_node_below_the_floor() { + // RENAMED, because the reason changed. This used to be "fires on a real + // difference" — under equality, 0.5.175 vs 0.5.176 flagged because they + // differed. It still flags, but now because 0.5.175 is below + // MIN_NODE_VERSION, and a mere difference no longer flags anything: + // that is the decoupling CIRISServer#497 asked for. assertTrue(isVersionMismatch("0.5.175", "0.5.176")) + assertFalse(isVersionMismatch("0.5.192", "0.5.193")) } // ── The runtime declares what it is (CIRISAgent#1111) ────────────────── diff --git a/client/shared/src/commonTest/kotlin/ai/ciris/mobile/shared/models/CompatibilityFloorTest.kt b/client/shared/src/commonTest/kotlin/ai/ciris/mobile/shared/models/CompatibilityFloorTest.kt new file mode 100644 index 0000000..d7a7ba2 --- /dev/null +++ b/client/shared/src/commonTest/kotlin/ai/ciris/mobile/shared/models/CompatibilityFloorTest.kt @@ -0,0 +1,84 @@ +package ai.ciris.mobile.shared.models + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * The floor, and the ordering trap underneath it. + * + * The equality check this replaces never ordered anything, so it could not be + * wrong about order. A floor is nothing BUT order. + */ +class CompatibilityFloorTest { + + // ---- the trap ---------------------------------------------------- + + @Test + fun numeric_not_lexical_at_the_versions_we_actually_ship() { + // Lexically "0.5.9" > "0.5.190" because '9' > '1'. Every one of these + // is a pair this repo could really see: three-digit patch numbers make + // it the common case, not an edge. + assertTrue(compareVersions("0.5.9", "0.5.190") < 0, "0.5.9 is BELOW 0.5.190") + assertTrue(compareVersions("0.5.191", "0.5.9") > 0, "0.5.191 is ABOVE 0.5.9") + assertTrue(compareVersions("0.5.88", "0.5.190") < 0, "0.5.88 is BELOW 0.5.190") + assertEquals(0, compareVersions("0.5.191", "v0.5.191")) + assertEquals(0, compareVersions("0.5", "0.5.0")) + } + + @Test + fun a_malformed_version_cannot_crash_the_banner() { + compareVersions("", "0.5.190") + compareVersions("not-a-version", "0.5.190") + compareVersions("0.5.x", "0.5.190") + } + + // ---- the point of decoupling ------------------------------------- + + @Test + fun a_mixed_but_compatible_pair_says_nothing() { + // THE case the range exists for: client ahead of the node, both above + // the floor. Equality nagged here, permanently, on every such pair. + assertFalse(isVersionMismatch("0.5.192", clientVersion = "0.5.193")) + assertFalse(isVersionMismatch("0.5.190", clientVersion = "0.5.199")) + assertFalse(isVersionMismatch("0.5.191", clientVersion = "0.5.191")) + } + + @Test + fun a_node_below_the_floor_still_says_so() { + // The signal the nag exists for. "Never complain" would have deleted it. + assertTrue(isVersionMismatch("0.5.188", clientVersion = "0.5.191")) + assertTrue(isVersionMismatch("0.5.186", clientVersion = "0.5.191")) + // And the lexical trap must not rescue a too-old node: "0.5.9" reads as + // greater than "0.5.190" to a string compare. + assertTrue(isVersionMismatch("0.5.9", clientVersion = "0.5.191")) + } + + // ---- the direction only the node can answer ---------------------- + + @Test + fun a_node_may_declare_a_client_floor_of_its_own() { + // A newer node needing a newer client: no client-held constant can ever + // know this, which is why the node has to be able to say it. + assertTrue(isVersionMismatch("0.6.0", clientVersion = "0.5.191", nodeMinClientVersion = "0.6.0")) + assertFalse(isVersionMismatch("0.6.0", clientVersion = "0.6.1", nodeMinClientVersion = "0.6.0")) + } + + @Test + fun a_node_that_declares_nothing_is_not_taken_as_content() { + // Absent means "did not say", not "is happy". Every node today is here, + // so this is the live path, and the floor this side holds still applies. + assertFalse(isVersionMismatch("0.5.191", clientVersion = "0.5.191", nodeMinClientVersion = null)) + assertTrue(isVersionMismatch("0.5.186", clientVersion = "0.5.191", nodeMinClientVersion = null)) + } + + // ---- unchanged behaviour ----------------------------------------- + + @Test + fun an_unknown_node_version_is_not_a_verdict() { + assertFalse(isVersionMismatch(null)) + assertFalse(isVersionMismatch("")) + assertFalse(isVersionMismatch(" ")) + } +}