OverviewFilesCommitsIssuesDocsReleasesPeers

atproto-room commits / 056d177

0.2.13: the membrane projection gate must be rev-aware, not length-aware
- if (log.length <= seq.length) return silently assumed the __commit__ log only ever GROWS. After a base rebuild or resync the log RESTARTS SHORTER, so a node carrying an old membrane (83 projected frames) with a fresh log (49 entries) satisfies 49 <= 83 and NEVER PROJECTS AGAIN. It keeps announcing a PRE-REBUILD head while its own device head is current. Found live: a wj62 co-owner has been advertising rev 3mr436gzyts22 for FOUR DAYS while both nodes' devices were at 3mrhxnkbhrw22. It is still announced and frontable, so after a bridge front-store rebuild the membrane discovered it first and served that stale head as relaySource — a REV REGRESSION to the relay, which parks a forward-only cursor. It self-corrected once the fresh front was discovered (freshest-by-rev), but the exposure is real and was invisible from every surface, which is why it survived four days. DELIBERATELY NOT AUTO-REPAIRED. The membrane firehose is an append-only core that consumers replicate; appending the new log's frames after the stale ones would put a rev regression MID-STREAM — worse than freezing. A correct repair re-bases onto a NEW core (consumers already re-front on a key change — see atproto-bridge's "re-fronts a peer that REBUILDS its cores with new keys" test) or emits #sync. That is a deliberate operation, not something to do inside a sync tick. So this splits the three cases explicitly: shorter => refuse + warn ONCE naming the DID and both lengths (the two numbers that diagnose it); equal => nothing new (the anti-balloon guard, unchanged); longer => project. Tests: 3 new / 10 asserts, including that the membrane is NOT appended to, that the warning is emitted exactly once, and that the anti-balloon guard still holds. Suite 43/43, 180/180. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
056d177a2f20a6ca8605a08a72978755bbe843e5
by plankatron <fleeky@gmail.com> · 2026-07-26T23:36:52.000Z

Changed files (4) +128 -2

lib/serve-membrane.js +29 -1
……@@ -96,9 +96,37 @@ export async function serveMembrane (device, { corestore, swarm, did, handle, se
9696 // runaway storage+CPU leak (favela then re-replicates the bloated core at ~100 MB/s). projectFirehose
9797 // was already incremental; projectRepo was not. After a full projection seq.length === log.length, so
9898 // `log.length <= seq.length` means there is nothing new to project — skip both (read-only, no writes).
99 // Freshness is a REV comparison, not a length comparison.
100 //
101 // The old gate was `if (log.length <= seq.length) return`, which silently assumes the __commit__ log
102 // only ever GROWS. After a base rebuild / resync the log RESTARTS SHORTER, so a node carrying an old
103 // membrane (say 83 frames) with a fresh log (say 49) satisfies 49 <= 83 and NEVER PROJECTS AGAIN — it
104 // advertises a pre-rebuild head forever while its device head is current. Observed live: a wj62
105 // co-owner stuck at rev 3mr436gzyts22 for four days, still announcing, so the bridge could pick it as
106 // a front and serve a REV REGRESSION to the relay (which parks a forward-only cursor).
107 //
108 // Deliberately NOT auto-repaired here. The membrane firehose is an append-only core that consumers
109 // replicate; appending the new log's frames after the stale ones would put a rev REGRESSION mid-stream
110 // — worse than freezing. A correct repair re-bases onto a NEW core (consumers already re-front on a
111 // key change) or emits #sync, and that is a deliberate change, not something to do inside a sync tick.
112 // What this DOES do is make it loud, once, naming the DID and both revs — the condition was previously
113 // invisible from every surface, which is why it survived four days.
114 let staleWarned = false
99115 const sync = async () => {
100116 const log = await device.firehoseLog()
101 if (log.length <= seq.length) return
117 if (!log.length) return
118 const logHeadRev = log[log.length - 1].rev
119 if (log.length < seq.length) {
120 if (!staleWarned) {
121 staleWarned = true
122 console.error(JSON.stringify({
123 msg: 'membrane STALE — the __commit__ log restarted shorter than the projected firehose (base rebuild/resync). This node advertises a PRE-REBUILD head and must not be fronted; it needs a membrane re-base, not an append.',
124 did, logLength: log.length, membraneLength: seq.length, logHeadRev
125 }))
126 }
127 return
128 }
129 if (log.length === seq.length) return // fully projected — nothing new (the anti-balloon guard)
102130 if (repo) await projectRepo(device, repo) // skipped entirely when the repo core is off
103131 await projectFirehose(device, seq, log)
104132 }
package.json +1 -1
……@@ -1,6 +1,6 @@
11{
22 "name": "atproto-room",
3 "version": "0.2.12",
3 "version": "0.2.13",
44 "description": "Attestation-gated, encrypted multi-writer room engine — StateBase (keet-model admission) + Lease + signer + RepoBase + LabelBase/LabelRoom + serve-membrane. Extracted from hyper-proto-peer; consumed by the atproto peer (repo devices) and the co-authored labeler.",
55 "type": "module",
66 "main": "index.js",
test/all.js +1 -0
……@@ -4,6 +4,7 @@
44// run under node: `npm run test:units`. The *.repro.mjs files are manual repros, not aggregated.
55import '../lib/bare-globals.js'
66import './repo-base.test.js'
7import './membrane-stale-gate.test.mjs'
78import './lease.test.js'
89import './lease-handoff.test.js'
910import './signer.test.js'
test/membrane-stale-gate.test.mjs +97 -0
……@@ -0,0 +1,97 @@
1// serveMembrane's projection gate must be REV/­content-aware, not length-aware.
2//
3// The old gate was `if (log.length <= seq.length) return`, which silently assumes the __commit__ log
4// only ever GROWS. After a base rebuild or resync the log RESTARTS SHORTER, so a node carrying an old
5// membrane (e.g. 83 projected frames) with a fresh log (e.g. 49 entries) satisfies 49 <= 83 and NEVER
6// PROJECTS AGAIN. It keeps announcing a PRE-REBUILD head while its device head is current.
7//
8// Observed live on wj62: a co-owner stuck at rev 3mr436gzyts22 for four days, still announced, so a
9// bridge could front it and serve a REV REGRESSION to the relay — which parks a forward-only cursor.
10// The condition was invisible from every surface, which is why it survived.
11//
12// The gate must NOT "fix" this by appending the new log after the stale frames: the membrane firehose
13// is append-only and replicated, so that would put a rev regression MID-STREAM, which is worse than
14// freezing. Correct behaviour is to refuse and say so loudly.
15//
16// Run: bare test/membrane-stale-gate.test.mjs
17import '../lib/bare-globals.js'
18import test from 'brittle'
19import b4a from 'b4a'
20import { serveMembrane } from '../lib/serve-membrane.js'
21import { openCorestore } from 'atproto-store/lib/corestore.js'
22import fs from 'bare-fs'
23
24const DID = 'did:plc:membranestalegate000000'
25const tmp = () => { const d = '/tmp/ar-msg-' + Math.floor(Math.random() * 1e9); fs.mkdirSync(d, { recursive: true }); return d }
26
27// serveMembrane only touches these few swarm members.
28function fakeSwarm () {
29 return {
30 on () {}, off () {},
31 keyPair: { publicKey: b4a.alloc(32, 7) },
32 join () { return { flushed: async () => {} } }
33 }
34}
35
36// A device whose firehose log we control outright. listRecords is empty so projectRepo is a no-op.
37function fakeDevice (log = []) {
38 return {
39 _log: log,
40 repo: { listRecords: async () => [] },
41 async firehoseLog () { return this._log }
42 }
43}
44
45async function capture (t, fn) {
46 const lines = []
47 const orig = console.error
48 console.error = (s) => lines.push(String(s))
49 t.teardown(() => { console.error = orig })
50 try { await fn() } finally { console.error = orig }
51 return lines
52}
53
54test('a log SHORTER than the membrane (base rebuild) must NOT append, and must say so', async (t) => {
55 const device = fakeDevice([])
56 const serving = await serveMembrane(device, { corestore: openCorestore(tmp()), swarm: fakeSwarm(), did: DID })
57
58 // Simulate an established membrane: 3 already-projected frames.
59 for (let i = 0; i < 3; i++) await serving.seq.append(b4a.from('frame-' + i))
60 t.is(serving.seq.core.length, 3, 'membrane has 3 projected frames')
61
62 // The base was rebuilt: the __commit__ log restarts SHORTER than what the membrane already holds.
63 device._log = [{ rev: '3knewbaserev0', seq: 1 }]
64
65 const logs = await capture(t, () => serving.sync())
66
67 t.is(serving.seq.core.length, 3, 'membrane was NOT appended to — no rev regression mid-stream')
68 t.is(logs.length, 1, 'exactly one warning emitted')
69 t.ok(/membrane STALE/.test(logs[0]), 'the stale condition is named explicitly')
70 const j = JSON.parse(logs[0])
71 t.is(j.did, DID, 'names the DID')
72 t.is(j.logLength, 1, 'reports the log length')
73 t.is(j.membraneLength, 3, 'reports the membrane length — the two numbers that diagnose it')
74
75 const again = await capture(t, () => serving.sync())
76 t.is(again.length, 0, 'deduped — it does not spam every sync tick')
77})
78
79test('an EQUAL-length log is fully projected — the anti-balloon guard still holds', async (t) => {
80 const device = fakeDevice([])
81 const serving = await serveMembrane(device, { corestore: openCorestore(tmp()), swarm: fakeSwarm(), did: DID })
82 for (let i = 0; i < 2; i++) await serving.seq.append(b4a.from('f' + i))
83
84 device._log = [{ rev: 'a', seq: 1 }, { rev: 'b', seq: 2 }]
85 await serving.sync()
86
87 // The original balloon was projectRepo re-putting the WHOLE record set on every ~2s tick with zero
88 // new commits. Nothing new => nothing written, still.
89 t.is(serving.seq.core.length, 2, 'nothing appended when the log is fully projected')
90})
91
92test('an empty log is a no-op (a device that has never signed)', async (t) => {
93 const device = fakeDevice([])
94 const serving = await serveMembrane(device, { corestore: openCorestore(tmp()), swarm: fakeSwarm(), did: DID })
95 await serving.sync()
96 t.is(serving.seq.core.length, 0, 'no frames, no writes, no warning')
97})