A story is identified by its key, never by the other device's row number

The two engines both had a notion of "this story, wherever it is read",
and kept it in different fields. This app put it in `id`. The iPhone
numbers its rows locally in `id` and kept the shared identity in `key`.
One name, two meanings, so a story travelling between the two phones came
back unrecognisable and the import filed it as a second copy of itself.

`key` is the identity now and `id` is written by neither engine. A local
handle has no business in a shared shape.

`id` is still read as a fallback for files written before this was
settled, and only when it is a STRING. A number there is the writing
device's row handle: stringifying it produced a confident, meaningless
identity, which is the original bug reached by another path. A story with
neither falls back to its post time, the same value the iPhone falls back
to, so the two still land together.

Two tests were pinning the old contract rather than testing it. One
asserted the internal id came from the shared `id` when both were present;
the other was called `a numeric story id from the iphone becomes a usable
name`, which is the bug stated as a requirement. Both rewritten, and the
new sibling covering the post-time fallback is what caught the numeric
case above.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Paul Hitt
2026-09-16 14:47:35 -04:00
co-authored by Claude Opus 5
parent 1049c1d118
commit 88397a8bda
2 changed files with 78 additions and 16 deletions
@@ -240,10 +240,19 @@ object BackupPayload {
return out
}
/**
* A story in the shared shape.
*
* `key` is the identity and `id` is NOT written. The two engines had the
* same idea and put it in different places: this app keeps the shared
* identity straight in `id`, while the iPhone numbers a story locally and
* keeps the identity in `key`. One name, two meanings, so a story
* travelling between the two came back under a different id and imported
* as a second copy of itself.
*/
private fun storyToShared(s: JSONObject): JSONObject {
val out =
JSONObject()
.put("id", s.opt("id"))
.put("author", s.optLong("author"))
.put("mine", s.optBoolean("mine"))
.put("caption", s.optString("caption"))
@@ -252,7 +261,14 @@ object BackupPayload {
s.optJSONObject("ref")?.let { out.put("att", attachmentToShared(it, Attachment.STORY)) }
s.optString("gid").takeIf { it.isNotEmpty() }?.let { out.put("gid", it) }
s.optString("stream").takeIf { it.isNotEmpty() }?.let { out.put("stream", it) }
s.optString("key").takeIf { it.isNotEmpty() }?.let { out.put("key", it) }
// Always written. This app's `id` IS the identity, so it is the
// fallback when an older record carried no separate key, and the post
// time is the last resort rather than letting a story travel with no
// way to recognise it.
out.put(
"key",
s.optString("key").ifEmpty { s.optString("id") }.ifEmpty { s.optLong("at").toString() },
)
s.optJSONObject("set")?.let { out.put("set", it) }
// The iPhone has no unread mark on a story, so this is ours alone. It
// travels anyway: coming back here it is the difference between a strip
@@ -429,11 +445,24 @@ object BackupPayload {
): JSONObject {
val out =
JSONObject()
// The iPhone numbers its stories and this app names them, so an
// id that arrives as a number becomes its own digits rather
// than being dropped. `mergeImportedPayload` de-duplicates on
// this string, so it only has to be stable, not numeric.
.put("id", s.opt("id")?.toString().orEmpty())
// The identity, which this app stores as its `id`. `key` is
// where it lives in the shared shape; a file written before
// that was settled carries it in `id`, and the post time is the
// last resort. `mergeImportedPayload` de-duplicates on this
// string, so it only has to be stable, not numeric.
.put(
"id",
s
.optString("key")
// `id` only when it is a STRING. A number there is the
// writing device's own row handle and names nothing
// here; adopting it is how a story imported as a second
// copy of itself. The post time is the last resort, and
// it is what the iPhone falls back to as well, so the
// two still land on the same value.
.ifEmpty { (s.opt("id") as? String).orEmpty() }
.ifEmpty { s.optLong("at").toString() },
)
.put("author", if (s.has("author")) s.optLong("author") else author)
.put("mine", s.optBoolean("mine"))
.put("caption", s.optString("caption"))
@@ -445,7 +474,6 @@ object BackupPayload {
s.optJSONObject("att")?.let { out.put("ref", attachmentFromShared(it)) }
s.optString("gid").takeIf { it.isNotEmpty() }?.let { out.put("gid", it) }
s.optString("stream").takeIf { it.isNotEmpty() }?.let { out.put("stream", it) }
s.optString("key").takeIf { it.isNotEmpty() }?.let { out.put("key", it) }
s.optJSONObject("set")?.let { out.put("set", it) }
return out
}
@@ -274,7 +274,12 @@ class BackupPayloadTest {
@Test
fun `a story comes back in seconds, where the strip reads it`() {
val s = roundTrip().getJSONObject("stories").getJSONArray("items").getJSONObject(0)
assertEquals("story-1", s.getString("id"))
// `key`, not `id`. The two engines both had a notion of "this story,
// wherever it is read", and kept it in different fields: this app used
// `id` and the iPhone used `key` while numbering rows locally in `id`.
// Reading the other side's `id` turned a story into a different story,
// and the import filed it as a second copy of itself.
assertEquals("story-key", s.getString("id"))
assertEquals(9L, s.getLong("author"))
// Seconds again. `isActive` compares against unix seconds, and a value
// a thousand times too large means an expired story never drops.
@@ -292,14 +297,20 @@ class BackupPayloadTest {
}
@Test
fun `a numeric story id from the iphone becomes a usable name`() {
// The iPhone numbers its stories; this app names them. Dropping the id
// would make every imported story look new on every import, because
// the merge de-duplicates on exactly this.
fun `a numeric story id from an old iphone backup is not the identity`() {
// A number under `id` is the WRITER's local row handle and names
// nothing here. Adopting it is how a story imported as a second copy
// of itself. The identity is `key`; a file old enough to carry neither
// falls back to the post time, which is at least stable.
val shared =
JSONObject().put("v", 1).put(
"stories",
JSONObject().put("9", JSONArray().put(JSONObject().put("id", 1_700_000_000).put("at", 1L))),
JSONObject().put(
"9",
JSONArray().put(
JSONObject().put("id", 1_700_000_000).put("at", 1L).put("key", "story-key"),
),
),
)
val s =
BackupPayload
@@ -307,10 +318,31 @@ class BackupPayloadTest {
.getJSONObject("stories")
.getJSONArray("items")
.getJSONObject(0)
assertEquals("1700000000", s.getString("id"))
assertEquals("story-key", s.getString("id"))
assertEquals(9L, s.getLong("author"))
}
@Test
fun `a story with no key at all falls back to its post time`() {
val shared =
JSONObject().put("v", 1).put(
"stories",
JSONObject().put(
"9",
JSONArray().put(JSONObject().put("id", 1_700_000_000).put("at", 1_700_000_000_000L)),
),
)
val s =
BackupPayload
.fromShared(shared)
.getJSONObject("stories")
.getJSONArray("items")
.getJSONObject(0)
// The post time, not the writer's row number. Stable, and the same
// value the iPhone falls back to, so the two still agree.
assertEquals("1700000000000", s.getString("id"))
}
@Test
fun `an empty history makes an empty payload rather than nothing`() {
val shared = BackupPayload.toShared(JSONObject().put("threads", JSONObject()))
@@ -442,7 +474,9 @@ class BackupPayloadTest {
assertEquals(9L, g.getLong("author"))
val s = out.getJSONObject("stories").getJSONArray("items").getJSONObject(0)
assertEquals("1700000000", s.getString("id"))
// The iPhone's payload carries both; `key` is the one that means the
// same thing on both phones.
assertEquals("story-key", s.getString("id"))
assertEquals(1_700_600_000.0, s.getDouble("expires"), "milliseconds there, seconds here")
assertTrue(out.getJSONObject("roster").getBoolean("offServer"))