OverviewFilesCommitsIssuesDocsReleasesPeers

atproto-store commits / bf041d5

0.0.25: identify a Corestore by behaviour, not by module identity
`instanceof Corestore` compares against ONE module instance's class object. A nested node_modules tree routinely produces two copies, and that is exactly what bare-pack ships to the phone — the daemon's own corestore, plus …/hyper-proto-peer/node_modules/atproto-room/node_modules/atproto-store/node_modules/corestore. A store built by one copy fails `instanceof` against the other, and all three discriminators here read that as "not a store, therefore a PATH". Corestore then path.join()s an object: TypeError: Cannot read properties of undefined (reading 'length') at Object.join (bare-path/lib/posix.js:67) at new CorestoreStorage (hypercore-storage/index.js:496) at new Corestore (corestore/index.js:242) at new RepoStore (atproto-store/lib/repo-store.js) Measured on a moto g power (hyper-social #40): serve-membrane passes `shared.store.namespace('membrane:' + did)` into RepoStore, and the DID died at that line after opening its record store, taking the joiner posture, opening blobs and serving two control connections. The message named a JS idiom and no cause. It cannot reproduce on a workstation, where the tree is deduped to one copy — the two environments differ in their MODULE GRAPH, not their code. So identity is the wrong question: a store is what it does. isCorestore now accepts anything with get + namespace + replicate, keeping instanceof as the fast path, and repo-store / blob-store / overlay-blob-store all route through it. test/corestore-cross-copy.test.js reproduces the phone's stack on a workstation by passing a delegating stand-in for a second copy — red with the identical path.join(undefined) trace, green after. Suite 86/86, 256 asserts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
bf041d5aafe09c63149a6d84732389dc472bf96f
by plankatron <fleeky@gmail.com> · 2026-08-10T16:07:44.000Z

Changed files (6) +142 -6

lib/blob-store.js +4 -1
……@@ -11,13 +11,16 @@
1111// for avatars/images and adds no dependency. Revisit if blob sizes grow.)
1212
1313import Corestore from 'corestore'
14import { isCorestore } from './corestore.js'
1415import Hyperbee from 'hyperbee'
1516import b4a from 'b4a'
1617import { cidForBlobSync } from 'atproto-repo/lib/cid.js'
1718
1819export class BlobStore {
1920 constructor (storage, opts = {}) {
20 this._ownStore = !(storage instanceof Corestore)
21 // Behavioural check, NOT `instanceof` — a second copy of corestore in a bundled tree makes a real
22 // store look like a path and Corestore then path.join()s an object. See lib/corestore.js.
23 this._ownStore = !isCorestore(storage)
2124 this._store = this._ownStore ? new Corestore(storage) : storage
2225 this._name = opts.name || 'pds-blobs'
2326 this.bee = null
lib/corestore.js +18 -2
……@@ -6,9 +6,25 @@ import Corestore from 'corestore'
66
77// A dir path (or an existing Corestore) -> a Corestore. Idempotent on an instance.
88export function openCorestore (storage) {
9 return storage instanceof Corestore ? storage : new Corestore(storage)
9 return isCorestore(storage) ? storage : new Corestore(storage)
1010}
1111
12export function isCorestore (x) { return x instanceof Corestore }
12// ⛔ NOT `instanceof`. That compares against ONE module instance's class object, and a nested
13// node_modules tree routinely yields two copies of corestore — which is what bare-pack ships to the
14// phone (…/hyper-proto-peer/node_modules/atproto-room/node_modules/atproto-store/node_modules/corestore
15// alongside the daemon's own). A store built by copy A then fails `instanceof` against copy B, and every
16// discriminator here reads that as "not a store, therefore a PATH", so Corestore tried to path.join an
17// OBJECT and the node died with `Cannot read properties of undefined (reading 'length')` — a message
18// that names a JS idiom and no cause (hyper-social #40).
19//
20// It cannot reproduce on a workstation, where the tree is deduped to one copy. The environments differ
21// in their module graph, not their code, so identity is the wrong question: a store is what it DOES.
22// `get` + `namespace` + `replicate` are the surface every caller here uses and nothing else implements.
23// The instanceof check stays as a fast path for the common single-copy case.
24export function isCorestore (x) {
25 if (!x || typeof x !== 'object') return false
26 if (x instanceof Corestore) return true
27 return typeof x.get === 'function' && typeof x.namespace === 'function' && typeof x.replicate === 'function'
28}
1329
1430export { Corestore }
lib/overlay-blob-store.js +4 -1
……@@ -15,6 +15,7 @@
1515// Corestore for the tiny metadata bee.
1616
1717import Corestore from 'corestore'
18import { isCorestore } from './corestore.js'
1819import Hyperbee from 'hyperbee'
1920import b4a from 'b4a'
2021import fs from '#node:fs' // dual-runtime: bare-fs under Bare (the daemon), builtin fs under Node (tests)
……@@ -27,7 +28,9 @@ export class OverlayBlobStore {
2728 if (!indexStore) throw new Error('OverlayBlobStore: indexStore (Corestore) required')
2829 this._blobRoot = blobRoot
2930 this._overlay = overlay
30 this._ownStore = !(indexStore instanceof Corestore)
31 // Behavioural check, NOT `instanceof` — a second copy of corestore in a bundled tree makes a real
32 // store look like a path and Corestore then path.join()s an object. See lib/corestore.js.
33 this._ownStore = !isCorestore(indexStore)
3134 this._store = this._ownStore ? new Corestore(indexStore) : indexStore
3235 this._name = name
3336 this.bee = null
lib/repo-store.js +4 -1
……@@ -8,6 +8,7 @@
88// this layer stores nothing atproto-specific beyond the records themselves.
99
1010import Corestore from 'corestore'
11import { isCorestore } from './corestore.js'
1112import Hyperbee from 'hyperbee'
1213import * as dagCbor from '@ipld/dag-cbor'
1314import b4a from 'b4a'
……@@ -20,7 +21,9 @@ function prefixUpper (prefix) {
2021
2122export class RepoStore {
2223 constructor (storage, opts = {}) {
23 this._ownStore = !(storage instanceof Corestore)
24 // Behavioural check, NOT `instanceof` — a second copy of corestore in a bundled tree makes a real
25 // store look like a path and Corestore then path.join()s an object. See lib/corestore.js.
26 this._ownStore = !isCorestore(storage)
2427 this._store = this._ownStore ? new Corestore(storage) : storage
2528 this._name = opts.name || 'atproto-repo'
2629 this._key = opts.key || null // open a specific core by KEY (a replica of another instance's repo)
package.json +1 -1
……@@ -1,6 +1,6 @@
11{
22 "name": "atproto-store",
3 "version": "0.0.24",
3 "version": "0.0.25",
44 "description": "Holepunch-native atproto repo store — records in a classic Hyperbee (collection/rkey -> DAG-CBOR). The P2P home; the atproto-repo module is the bridge serializer over it.",
55 "type": "module",
66 "main": "lib/repo-store.js",
test/corestore-cross-copy.test.js +111 -0
……@@ -0,0 +1,111 @@
1// A Corestore FROM A DIFFERENT COPY OF THE MODULE is still a Corestore.
2//
3// `instanceof` compares against one module instance's class object. Bundle two copies of corestore —
4// which is the normal outcome of a nested node_modules tree, and exactly what bare-pack ships to the
5// phone — and a store built by copy A fails `instanceof` against copy B. Every discriminator in this
6// package was written as `!(storage instanceof Corestore)`, i.e. "not a store, so it must be a PATH":
7//
8// TypeError: Cannot read properties of undefined (reading 'length')
9// at Object.join (bare-path/lib/posix.js:67)
10// at new CorestoreStorage (hypercore-storage/index.js:496)
11// at new Corestore (corestore/index.js:242)
12// at new RepoStore (atproto-store/lib/repo-store.js)
13//
14// Measured on a real device (hyper-social #40): the daemon passed
15// `shared.store.namespace('membrane:' + did)` into serve-membrane's RepoStore, the check said "path",
16// and Corestore tried to path.join an OBJECT. The node had already opened its record store, taken the
17// joiner posture, opened blobs and served two control connections — it died at the very last step, and
18// the message named a JS idiom rather than a cause.
19//
20// It never reproduced on a workstation because a deduped/symlinked tree has exactly one copy. The
21// difference between the two environments is the module graph, not the code — so the discriminator
22// must not depend on module identity. A store is identified by what it DOES.
23//
24// Run: brittle-node test/corestore-cross-copy.test.js
25import test from 'brittle'
26import fs from 'fs'
27import os from 'os'
28import path from 'path'
29import Corestore from 'corestore'
30import { isCorestore, openCorestore } from '../lib/corestore.js'
31import { RepoStore } from '../lib/repo-store.js'
32import { BlobStore } from '../lib/blob-store.js'
33
34const tmp = (t) => {
35 const d = fs.mkdtempSync(path.join(os.tmpdir(), 'atproto-store-crosscopy-'))
36 t.teardown(() => { try { fs.rmSync(d, { recursive: true, force: true }) } catch {} })
37 return d
38}
39
40// A store from ANOTHER copy of corestore. Same shape, same behaviour, different class object — which
41// is all a second copy in the bundle amounts to. Delegating to a real store keeps the test honest:
42// whatever the code does with it has to actually work, not merely type-check.
43function foreignCopy (store) {
44 const surface = {}
45 for (const m of ['get', 'namespace', 'replicate', 'ready', 'close', 'session']) {
46 if (typeof store[m] === 'function') surface[m] = store[m].bind(store)
47 }
48 return surface
49}
50
51test('the premise: a foreign-copy store fails instanceof', async (t) => {
52 const store = new Corestore(tmp(t))
53 await store.ready()
54 const foreign = foreignCopy(store)
55 t.absent(foreign instanceof Corestore, 'it is NOT instanceof the local copy — this is the whole bug')
56 t.ok(typeof foreign.get === 'function' && typeof foreign.namespace === 'function', 'but it is a store by every behaviour that matters')
57 await store.close()
58})
59
60test('isCorestore recognises a store by behaviour, not module identity', async (t) => {
61 const store = new Corestore(tmp(t))
62 await store.ready()
63 t.ok(isCorestore(store), 'the local copy')
64 t.ok(isCorestore(foreignCopy(store)), 'and one from another copy of the module')
65 t.absent(isCorestore('/some/path'), 'a path is not a store')
66 t.absent(isCorestore(null), 'and neither is null')
67 t.absent(isCorestore({ get: 1, namespace: 2 }), 'nor an object that merely has the names')
68 await store.close()
69})
70
71test('RepoStore accepts a foreign-copy store instead of path.join-ing it', async (t) => {
72 const store = new Corestore(tmp(t))
73 await store.ready()
74 const repo = await new RepoStore(foreignCopy(store), { name: 'membrane-repo:did:plc:test' }).ready()
75
76 // Before the fix this threw inside the constructor. Assert it is USABLE, not merely constructed.
77 await repo.put('app.bsky.feed.post', 'abc', { $type: 'app.bsky.feed.post', text: 'hello', createdAt: '2026-08-10T00:00:00.000Z' })
78 const got = await repo.get('app.bsky.feed.post', 'abc')
79 t.is(got && got.text, 'hello', 'a record round-trips through the foreign-copy store')
80
81 await repo.close()
82 await store.close()
83})
84
85test('BlobStore too — the same discriminator, the same trap', async (t) => {
86 const store = new Corestore(tmp(t))
87 await store.ready()
88 const blobs = await new BlobStore(foreignCopy(store), { name: 'membrane-blobs:did:plc:test' }).ready()
89 const put = await blobs.put(Buffer.from('some bytes'), 'application/octet-stream')
90 const back = await blobs.get(put.cid.toString())
91 t.is(back && back.size, 10, 'a blob round-trips through the foreign-copy store')
92 t.is(back && Buffer.from(back.bytes).toString(), 'some bytes', 'and its bytes are intact')
93 await store.close()
94})
95
96test('a PATH still gets its own store — the other half of the discriminator', async (t) => {
97 const dir = tmp(t)
98 const repo = await new RepoStore(dir).ready()
99 await repo.put('app.bsky.feed.post', 'p1', { $type: 'app.bsky.feed.post', text: 'from a path', createdAt: '2026-08-10T00:00:00.000Z' })
100 t.is((await repo.get('app.bsky.feed.post', 'p1')).text, 'from a path', 'a string path is still opened as a directory')
101 await repo.close()
102 t.ok(fs.existsSync(dir), 'and it wrote there')
103})
104
105test('openCorestore is idempotent on a foreign-copy store', async (t) => {
106 const store = new Corestore(tmp(t))
107 await store.ready()
108 const foreign = foreignCopy(store)
109 t.is(openCorestore(foreign), foreign, 'it is handed back untouched, not wrapped in a second Corestore')
110 await store.close()
111})