From 1ae36710c53716768094d895cbb08f45bad32138 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 20:44:02 +0000 Subject: [PATCH 1/5] feat(core): close() flushes and is idempotent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Teardown was two calls in a documented order, and correctness silently depended on each adapter's close() happening to be flush-inclusive — none of them are. Fold the flush into close() so the ordering is guaranteed once at the invariant layer, and drop the now-redundant flush() from the quick starts. The flush runs in a try/finally: an unwritable stack must not also leak a lock file. close() is idempotent because adapters aren't independently required to tolerate a double close (node:sqlite throws on an already-closed handle, and lock release is not re-entrant). Also corrects flush()'s doc comment, which described an offline write queue the API adapter does not have. Refs #147 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019eqUFNRUhV3x5JrjkMTpxj --- README.md | 1 - docs/spec/adapters.md | 10 +++++++++ packages/core/README.md | 1 - packages/core/src/stack.ts | 27 ++++++++++++++++-------- packages/core/tests/stack.test.ts | 34 +++++++++++++++++++++++++++++++ 5 files changed, 62 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 0c9cc22..0438da7 100644 --- a/README.md +++ b/README.md @@ -96,7 +96,6 @@ const notes = await stack.query({ }); // Tear down when done (flushes pending writes and releases resources) -await stack.flush(); await stack.close(); ``` diff --git a/docs/spec/adapters.md b/docs/spec/adapters.md index bd518f8..35733a5 100644 --- a/docs/spec/adapters.md +++ b/docs/spec/adapters.md @@ -105,3 +105,13 @@ How each adapter honors the single-writer rule differs by what it actually is: - **`record-adapter-sqlite`** (Node, real files) writes through `node:sqlite` under WAL journaling — page-level writes and crash safety are properties of the storage engine itself. It still acquires a PID-stamped lock file beside the database on `open()`/`initialize()`, released on `close()`, so a second opener gets a clear, immediate error rather than discovering the trust-boundary problem the hard way. A stale lock (owning process no longer alive) is reclaimed automatically, and an explicit override is available for the rare case of PID reuse. - **`record-adapter-sqljs`** (browser, no filesystem of its own) is a purely in-memory engine — no file, no PID, no lock to speak of. Durability and multi-tab/multi-process coordination are the embedding host's concern entirely: the adapter calls an optional `persist(bytes) => Promise` callback after every write, and the host wires that to OPFS, IndexedDB, or a download, with whatever locking that storage layer provides. - **The planned whole-file `adapter-json`** reads its entire store into memory on open and rewrites it whole on every persist, so it must supply both guarantees itself: a PID lock file (to fail loudly on double-open) and an atomic temp-file-and-`rename()` persist (so a crash mid-write can't leave a torn, unreadable file). `record-adapter-sqlite` gets both from WAL and real file locking instead. + +## Lifecycle + +**`Stack.close()` flushes, then releases.** Teardown is one call: `close()` invokes `flush()` before `adapter.close?.()`, so no app has to know whether its adapter buffers writes. An adapter's own `close()` is therefore not required to be flush-inclusive — the ordering is an invariant of the `Stack` layer, guaranteed once for every backend rather than reimplemented per adapter. + +A failed flush still releases resources before the error propagates. The alternative — abandoning `close()` on a flush error — leaves a lock file or a connection behind precisely when the stack is in trouble, turning one failure into two. + +**`close()` is idempotent; calling it twice is a no-op and never reaches the adapter twice.** Adapters are not independently required to tolerate a double close (`node:sqlite` throws on an already-closed handle, and lock release is not re-entrant), so `Stack` absorbs it. + +**`flush()` alone is for a stack that stays open** — checkpointing before a backup, or forcing a buffered adapter to persist at a known point. It is not part of teardown. diff --git a/packages/core/README.md b/packages/core/README.md index d49f28f..29568fb 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -66,7 +66,6 @@ const notes = await stack.query({ }); // Tear down when done -await stack.flush(); await stack.close(); ``` diff --git a/packages/core/src/stack.ts b/packages/core/src/stack.ts index daa0688..6bb70f8 100644 --- a/packages/core/src/stack.ts +++ b/packages/core/src/stack.ts @@ -519,6 +519,9 @@ export class Stack implements StackClient { */ private readonly typeCache = new Map(); + /** Set by close(). See docs/spec/adapters.md § Lifecycle. */ + private closed = false; + private constructor( private readonly adapter: StackAdapter, private readonly idTimestampSkewMsValue: number | null, @@ -1442,23 +1445,29 @@ export class Stack implements StackClient { // ------------------------------------------------------- /** - * Flush any pending writes to the underlying storage. - * For adapters that write immediately (SQLite, JSON), this is a no-op. - * For the API adapter, this commits the offline write queue to the server. - * Safe to call at any time — always resolves, never rejects on its own. + * Flush pending writes to the underlying storage. A no-op for adapters + * that commit on every call (SQLite, the API adapter); meaningful for + * ones that buffer, and for checkpointing a stack that stays open — + * close() covers the teardown case on its own. */ async flush(): Promise { await this.adapter.flush?.(); } /** - * Release any resources held by the adapter (connections, file handles, timers). - * Call this when the stack is no longer needed — especially important for the - * API adapter, which holds an open connection and retry timers. - * Safe to call even if the adapter has no resources to release. + * Flush, then release any resources the adapter holds (connections, file + * handles, lock files). A failed flush still releases them before it + * propagates: an unwritable stack must not also leak a lock file. + * See docs/spec/adapters.md § Lifecycle. */ async close(): Promise { - await this.adapter.close?.(); + if (this.closed) return; + this.closed = true; + try { + await this.flush(); + } finally { + await this.adapter.close?.(); + } } // ------------------------------------------------------- diff --git a/packages/core/tests/stack.test.ts b/packages/core/tests/stack.test.ts index 5e6107f..975936d 100644 --- a/packages/core/tests/stack.test.ts +++ b/packages/core/tests/stack.test.ts @@ -1586,6 +1586,40 @@ describe('flush / close', () => { test('close() is a no-op when adapter does not implement close', async () => { await expect(stack.close()).resolves.toBeUndefined(); }); + + test('close() flushes before releasing resources', async () => { + const calls: string[] = []; + adapter.flush = async () => { + calls.push('flush'); + }; + adapter.close = async () => { + calls.push('close'); + }; + await stack.close(); + expect(calls).toEqual(['flush', 'close']); + }); + + test('close() releases resources even when the flush fails, then propagates', async () => { + let closed = false; + adapter.flush = async () => { + throw new Error('disk full'); + }; + adapter.close = async () => { + closed = true; + }; + await expect(stack.close()).rejects.toThrow('disk full'); + expect(closed).toBe(true); + }); + + test('close() is idempotent — the adapter is never closed twice', async () => { + let closes = 0; + adapter.close = async () => { + closes += 1; + }; + await stack.close(); + await stack.close(); + expect(closes).toBe(1); + }); }); // ------------------------------------------------------- From a1d8ec9f1abe521f2d5ad4e6038f26e6769ea740 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 20:47:37 +0000 Subject: [PATCH 2/5] feat(core): refuse work on a closed stack MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit close() left a flag that gated only itself; every other method carried on into a dangling adapter handle. The failure surfaced as whatever the engine said about it — node:sqlite's ERR_INVALID_STATE, or silence on an adapter that accepts writes it will never persist. Guard every public method on Stack, plus the one ScopedStack path that reaches the adapter without going through Stack first, so a closed stack writes no attachment bytes before refusing. StackClosedError stays outside the StackError taxonomy, alongside IdGenerationError and InvalidDidError: every StackError maps to a wire status, and no server responds with "your client is closed". flush() throws like any other operation; only close() is idempotent, since a caller cannot always know whether teardown already ran. Refs #147 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019eqUFNRUhV3x5JrjkMTpxj --- docs/spec/adapters.md | 4 +++ packages/core/src/index.ts | 1 + packages/core/src/stack.ts | 58 ++++++++++++++++++++++++++++++- packages/core/tests/stack.test.ts | 58 +++++++++++++++++++++++++++++++ 4 files changed, 120 insertions(+), 1 deletion(-) diff --git a/docs/spec/adapters.md b/docs/spec/adapters.md index 35733a5..e18f412 100644 --- a/docs/spec/adapters.md +++ b/docs/spec/adapters.md @@ -115,3 +115,7 @@ A failed flush still releases resources before the error propagates. The alterna **`close()` is idempotent; calling it twice is a no-op and never reaches the adapter twice.** Adapters are not independently required to tolerate a double close (`node:sqlite` throws on an already-closed handle, and lock release is not re-entrant), so `Stack` absorbs it. **`flush()` alone is for a stack that stays open** — checkpointing before a backup, or forcing a buffered adapter to persist at a known point. It is not part of teardown. + +**Every other method throws `StackClosedError` once closed**, on both `Stack` and `ScopedStack`. Without the guard the failure surfaces as whatever the underlying engine says about a dangling handle — `node:sqlite`'s `ERR_INVALID_STATE`, or nothing at all on an adapter that silently accepts writes it will never persist. The asymmetry with `close()` is deliberate: teardown is idempotent because a caller cannot always know whether it already ran, while doing _work_ through a closed client is unambiguously a bug, and `flush()` is work. + +`StackClosedError` sits outside the `StackError` taxonomy, alongside `IdGenerationError` and `InvalidDidError` (see [Wire format § The taxonomy root](./wire-format.md#the-taxonomy-root)). Every `StackError` maps to a wire status, and no server ever answers "your client is closed" — it is a local programming error, not a transportable failure. The stack-identity getters (`ownerEntityId`, `timezone`, `features`) keep working after close: they read values cached at open and touch no storage. diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 31d43b0..03c46e7 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -22,6 +22,7 @@ export { StackQueryError, StackSchemaDriftError, StackPayloadTooLargeError, + StackClosedError, assertQueryCapabilities, } from './stack.js'; export type { diff --git a/packages/core/src/stack.ts b/packages/core/src/stack.ts index 6bb70f8..e17c2da 100644 --- a/packages/core/src/stack.ts +++ b/packages/core/src/stack.ts @@ -343,6 +343,20 @@ export class StackSchemaDriftError extends StackError { } } +/** + * Thrown when a Stack or ScopedStack is used after close(). Deliberately + * outside the StackError taxonomy, alongside IdGenerationError and + * InvalidDidError: a caller holding a closed client is a local programming + * error with no wire representation — no server ever responds with it. + * See docs/spec/adapters.md § Lifecycle. + */ +export class StackClosedError extends Error { + constructor(message = 'This Stack has been closed.') { + super(message); + this.name = 'StackClosedError'; + } +} + // ------------------------------------------------------- // Record ID validation // ------------------------------------------------------- @@ -597,6 +611,7 @@ export class Stack implements StackClient { * entities. See docs/spec/access-control.md § Enforcement: Stack.asEntity(). */ asEntity(entityId: EntityId | null): ScopedStack { + this.assertOpen(); return new ScopedStack(this, entityId, this.idTimestampSkewMsValue, this.adapter); } @@ -617,6 +632,7 @@ export class Stack implements StackClient { schema: TypeSchema, opts: DefineTypeOptions = {}, ): Promise { + this.assertOpen(); const parsed = parseTypeId(id); if (!parsed) { throw new Error( @@ -662,11 +678,13 @@ export class Stack implements StackClient { } async getType(id: TypeId): Promise { + this.assertOpen(); return this.getTypeCached(id); } /** Refreshes typeCache wholesale — the explicit way to see a rename made by another writer. */ async listTypes(): Promise { + this.assertOpen(); const types = await this.adapter.listTypes(); for (const type of types) this.typeCache.set(type.id, type); return types; @@ -677,6 +695,7 @@ export class Stack implements StackClient { * Useful for duck-typed consumption across types. */ async typeIsCompatible(typeId: TypeId, requiredSchema: TypeSchema): Promise { + this.assertOpen(); const type = await this.getTypeCached(typeId); if (!type) return false; return isCompatible(type.schema, requiredSchema); @@ -693,6 +712,7 @@ export class Stack implements StackClient { * automatically. */ registerMigration(migration: Migration): void { + this.assertOpen(); if (this.migrations.has(migration.from)) { throw new StackMigrationError(`A migration from "${migration.from}" is already registered.`); } @@ -748,6 +768,7 @@ export class Stack implements StackClient { * validation failure. See docs/spec/data-model.md § Type migrations. */ async migrateAll(baseTypeId: string): Promise<{ migrated: number }> { + this.assertOpen(); const types = await this.adapter.listTypes(); const familyTypeIds = types.filter((t) => t.baseId === baseTypeId).map((t) => t.id); @@ -814,6 +835,7 @@ export class Stack implements StackClient { content: T, opts: CreateRecordOptions = {}, ): Promise { + this.assertOpen(); const type = await this.getTypeCached(typeId); if (!type) { throw new Error(`Unknown type: "${typeId}". Call defineType() first.`); @@ -894,6 +916,7 @@ export class Stack implements StackClient { * commits migrations to disk. */ async get(id: string, opts: GetRecordOptions = {}): Promise { + this.assertOpen(); const record = await this.adapter.getRecord(id); if (!record) return null; return opts.presentAt === 'latest' ? this.presentAtLatest(record) : record; @@ -911,6 +934,7 @@ export class Stack implements StackClient { content: Record, opts: IfVersionOptions = {}, ): Promise { + this.assertOpen(); const existing = await this.adapter.getRecord(id); if (!existing) { throw new StackNotFoundError(`Record not found: "${id}"`); @@ -962,6 +986,7 @@ export class Stack implements StackClient { association: Association, opts: IfVersionOptions = {}, ): Promise { + this.assertOpen(); const existing = await this.adapter.getRecord(id); if (!existing) { throw new StackNotFoundError(`Record not found: "${id}"`); @@ -984,6 +1009,7 @@ export class Stack implements StackClient { association: Association, opts: IfVersionOptions = {}, ): Promise { + this.assertOpen(); const existing = await this.adapter.getRecord(id); if (!existing) { throw new StackNotFoundError(`Record not found: "${id}"`); @@ -1007,6 +1033,7 @@ export class Stack implements StackClient { permissions: Permission[], opts: IfVersionOptions = {}, ): Promise { + this.assertOpen(); const existing = await this.adapter.getRecord(id); if (!existing) { throw new StackNotFoundError(`Record not found: "${id}"`); @@ -1028,6 +1055,7 @@ export class Stack implements StackClient { * § Deletion. */ async delete(id: string, opts: DeleteRecordOptions = {}): Promise { + this.assertOpen(); if (id === SYSTEM_TYPES.CONFIG) { throw new StackConflictError( "Cannot delete the _config record: it holds the stack's identity and is required for every permission check.", @@ -1057,6 +1085,7 @@ export class Stack implements StackClient { * Snapshots and bumps version, same as delete(). */ async undelete(id: string, opts: IfVersionOptions = {}): Promise { + this.assertOpen(); const existing = await this.adapter.getRecord(id); if (!existing) { throw new StackNotFoundError(`Record not found: "${id}"`); @@ -1077,6 +1106,7 @@ export class Stack implements StackClient { * docs/spec/data-model.md § Queries. */ async query(query: StackQuery = {}): Promise { + this.assertOpen(); const { presentAt, filter, limit: rawLimit, ...rest } = query; assertQueryCapabilities(filter, this.adapter.capabilities); const limit = rawLimit !== undefined ? Math.min(rawLimit, MAX_QUERY_LIMIT) : undefined; @@ -1128,10 +1158,12 @@ export class Stack implements StackClient { // ------------------------------------------------------- async getVersions(id: string): Promise { + this.assertOpen(); return this.adapter.getVersions(id); } async getVersion(id: string, version: number): Promise { + this.assertOpen(); return this.adapter.getVersion(id, version); } @@ -1147,6 +1179,7 @@ export class Stack implements StackClient { version: number, opts: IfVersionOptions = {}, ): Promise { + this.assertOpen(); const existing = await this.adapter.getRecord(id); if (!existing) { throw new StackNotFoundError(`Record not found: "${id}"`); @@ -1279,6 +1312,7 @@ export class Stack implements StackClient { * bytes-then-create(). See docs/spec/wire-format.md § Attachments. */ async putAttachment(data: Uint8Array, mimeType: string, filename?: string): Promise { + this.assertOpen(); assertAttachmentSize(data.byteLength, this.features.maxAttachmentBytes); if (this.adapter.putAttachmentWithMetadata) { const record = await this.adapter.putAttachmentWithMetadata(data, mimeType, filename); @@ -1295,6 +1329,7 @@ export class Stack implements StackClient { } async getAttachment(fileId: string): Promise { + this.assertOpen(); return this.adapter.getAttachment(fileId); } @@ -1304,6 +1339,7 @@ export class Stack implements StackClient { * Throws StackNotFoundError if neither metadata records nor bytes exist. */ async deleteAttachment(fileId: string): Promise { + this.assertOpen(); const metadataTypeId = `${SYSTEM_TYPES.ATTACHMENT}@1`; const deletedRecordIds = this.adapter.deleteUnreferencedAttachmentRecords ? await this.adapter.deleteUnreferencedAttachmentRecords(fileId, metadataTypeId) @@ -1369,6 +1405,7 @@ export class Stack implements StackClient { async collectAttachmentGarbage( opts: CollectAttachmentGarbageOptions = {}, ): Promise { + this.assertOpen(); const graceMs = opts.graceMs ?? DEFAULT_GC_GRACE_MS; const dryRun = opts.dryRun ?? false; const now = Date.now(); @@ -1451,6 +1488,7 @@ export class Stack implements StackClient { * close() covers the teardown case on its own. */ async flush(): Promise { + this.assertOpen(); await this.adapter.flush?.(); } @@ -1462,14 +1500,25 @@ export class Stack implements StackClient { */ async close(): Promise { if (this.closed) return; + // Marked closed up front so a failed flush can't leave the stack + // half-open and invite a second close() onto an already-closed adapter. + // Flushes through the adapter directly, past the now-tripped guard. this.closed = true; try { - await this.flush(); + await this.adapter.flush?.(); } finally { await this.adapter.close?.(); } } + /** + * Throws once close() has run. Public only so ScopedStack can gate the + * one path it takes to the adapter directly; not an app-facing API. + */ + assertOpen(): void { + if (this.closed) throw new StackClosedError(); + } + // ------------------------------------------------------- // Grants // ------------------------------------------------------- @@ -1485,6 +1534,7 @@ export class Stack implements StackClient { entityId: EntityId | null, grants: Array<{ actions: GrantAction[]; typeId: TypeId }>, ): Promise { + this.assertOpen(); this.checkGrantsValid(grants); const records: StackRecord[] = []; for (const g of grants) { @@ -1506,6 +1556,7 @@ export class Stack implements StackClient { * grant) — the same resolution hasGrant() uses. */ async listGrants(entityId?: EntityId | null): Promise { + this.assertOpen(); const all = await queryAllPages((q) => this.query(q), { filter: { typeId: `${SYSTEM_TYPES.GRANT}@1` }, }); @@ -1528,6 +1579,7 @@ export class Stack implements StackClient { entityId: EntityId | null, grants: Array<{ actions: GrantAction[]; typeId: TypeId }>, ): Promise { + this.assertOpen(); const all = await queryAllPages((q) => this.query(q), { filter: { typeId: `${SYSTEM_TYPES.GRANT}@1` }, }); @@ -2171,6 +2223,10 @@ export class ScopedStack implements StackClient { * create() applies. */ async putAttachment(data: Uint8Array, mimeType: string, filename?: string): Promise { + // The one ScopedStack path that reaches the adapter without going + // through Stack first — without this, a closed stack would still write + // bytes before the delegated create() refused. + this.stack.assertOpen(); const requester = this.requesterEntityId; if (!requester) { throw new StackPermissionError('Anonymous requesters cannot upload attachments'); diff --git a/packages/core/tests/stack.test.ts b/packages/core/tests/stack.test.ts index 975936d..9824d6e 100644 --- a/packages/core/tests/stack.test.ts +++ b/packages/core/tests/stack.test.ts @@ -11,6 +11,7 @@ import { StackSchemaDriftError, StackQueryError, StackPayloadTooLargeError, + StackClosedError, } from '../src/stack.js'; import { generateId, crockford32Encode, IdGenerationError } from '../src/id.js'; import { InvalidDidError } from '../src/did.js'; @@ -1622,6 +1623,63 @@ describe('flush / close', () => { }); }); +describe('use after close', () => { + beforeEach(async () => { + await stack.close(); + }); + + test('reads throw StackClosedError', async () => { + await expect(stack.get('1hk153x0a00b')).rejects.toBeInstanceOf(StackClosedError); + await expect(stack.query()).rejects.toBeInstanceOf(StackClosedError); + await expect(stack.listTypes()).rejects.toBeInstanceOf(StackClosedError); + }); + + test('writes throw StackClosedError', async () => { + await expect(stack.create(NOTE_V1, { text: 'x' })).rejects.toBeInstanceOf(StackClosedError); + await expect(stack.update('1hk153x0a00b', { text: 'x' })).rejects.toBeInstanceOf( + StackClosedError, + ); + await expect(stack.delete('1hk153x0a00b')).rejects.toBeInstanceOf(StackClosedError); + }); + + test('flush() throws, since flushing is work — only close() is idempotent', async () => { + await expect(stack.flush()).rejects.toBeInstanceOf(StackClosedError); + await expect(stack.close()).resolves.toBeUndefined(); + }); + + test('attachment uploads throw StackClosedError', async () => { + await expect(stack.putAttachment(new Uint8Array([1]), 'text/plain')).rejects.toBeInstanceOf( + StackClosedError, + ); + }); + + test('identity getters still read — they touch no storage', () => { + expect(stack.ownerEntityId).toBe('owner-123'); + expect(stack.features).toBeDefined(); + }); + + test('StackClosedError stays outside the wire taxonomy', () => { + expect(new StackClosedError()).not.toBeInstanceOf(StackError); + }); +}); + +describe('use after close — scoped views', () => { + test('a view taken before close writes no attachment bytes after it', async () => { + const scoped = stack.asEntity('owner-123'); + await stack.close(); + + await expect(scoped.putAttachment(new Uint8Array([1]), 'text/plain')).rejects.toBeInstanceOf( + StackClosedError, + ); + expect(await adapter.listFiles!()).toHaveLength(0); + }); + + test('asEntity() itself refuses once closed', async () => { + await stack.close(); + expect(() => stack.asEntity('owner-123')).toThrow(StackClosedError); + }); +}); + // ------------------------------------------------------- // grant // ------------------------------------------------------- From 52433e5f99a807aa7858d1bcfeffca9269af8252 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 20:51:34 +0000 Subject: [PATCH 3/5] feat(wire-types): define discovery version negotiation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Discovery has always carried a "version" field that nothing read and no rule governed, so a server could ship one meaning anything and a client could do nothing with it. Give it semantics: MAJOR.MINOR of the wire protocol itself, majors must match, minors never have to. A major bump is defined as a change that would make an older client read a response wrongly, which is precisely what makes refusal the only safe response; a minor is additive either way, and neither direction can misread. Missing or unparseable is refused too — the field is mandatory, so its absence is a server not implementing this spec. APIAdapter.open() applies the rule before any other request, so a caller never has to wonder which writes landed against a server it cannot speak to. DiscoveryResponse moves to wire-types alongside the constant and the comparison, so a server implementation shares them rather than reimplementing the rule from prose. Refs #147 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019eqUFNRUhV3x5JrjkMTpxj --- docs/spec/wire-format.md | 12 ++++ packages/adapter-api/src/index.ts | 47 +++++++++++---- packages/adapter-api/tests/api.test.ts | 59 +++++++++++++++++++ .../adapter-api/tests/conformance.test.ts | 16 +++++ packages/conformance-fixtures/src/index.ts | 53 ++++++++++++++++- packages/wire-types/src/index.ts | 39 ++++++++++++ packages/wire-types/tests/discovery.test.ts | 40 +++++++++++++ 7 files changed, 253 insertions(+), 13 deletions(-) create mode 100644 packages/wire-types/tests/discovery.test.ts diff --git a/docs/spec/wire-format.md b/docs/spec/wire-format.md index 5b27cb3..42bb64d 100644 --- a/docs/spec/wire-format.md +++ b/docs/spec/wire-format.md @@ -24,6 +24,18 @@ GET /.well-known/stack } ``` +### Version negotiation + +`version` is the wire protocol's own version, `MAJOR.MINOR`, and it is required — not the server's software version, which is the server's business and appears nowhere in this spec. + +**A client refuses a server whose major differs from its own**, at `open()`, before any other request. A major bump is defined as a change that would make a client of an earlier major read a response wrongly — a field whose meaning changed, a shape that no longer parses the same way. There is no way to use such a server safely, and discovering it mid-session is worse than refusing: the caller is left unsure which writes landed. + +**A minor difference is never a refusal, in either direction.** A higher server minor is additive fields an older client ignores; a higher client minor is optional fields the server may omit. Neither can make a response read wrongly — that is what makes them minor. + +A response with no `version`, or one that isn't `MAJOR.MINOR`, is refused the same way a major mismatch is. The field is mandatory here, so its absence is a server that isn't implementing this spec, and guessing on its behalf would defeat the check. + +`@haverstack/wire-types` exports the current `WIRE_PROTOCOL_VERSION` along with `parseProtocolVersion()` and `isProtocolCompatible()`, so a server implementation applies the same rule as `adapter-api` rather than reimplementing it. + ## Authentication Bearer token in the `Authorization` header. Token issuance itself is out of scope for this spec — that is the server's concern — but _how a token is earned_ has a shape worth stating: see [Authentication: challenge–response](./identity.md#authentication-challengeresponse) for the nonce/signature handshake a server implements before calling `createToken()`. The adapter sends the token if configured; the server returns `401` if missing or invalid, `403` if the requester verified but lacks a grant (see [Error responses](#error-responses)). diff --git a/packages/adapter-api/src/index.ts b/packages/adapter-api/src/index.ts index 27943c1..14a4ed1 100644 --- a/packages/adapter-api/src/index.ts +++ b/packages/adapter-api/src/index.ts @@ -30,8 +30,14 @@ import type { RecordId, FileId, } from '@haverstack/core'; -import type { WireRecord, WireType, WireVersion } from '@haverstack/wire-types'; -import { isWireError, deserializeError, errorForStatus } from '@haverstack/wire-types'; +import type { WireRecord, WireType, WireVersion, DiscoveryResponse } from '@haverstack/wire-types'; +import { + isWireError, + deserializeError, + errorForStatus, + isProtocolCompatible, + WIRE_PROTOCOL_VERSION, +} from '@haverstack/wire-types'; // ------------------------------------------------------- // Public option types @@ -89,16 +95,22 @@ export class APIAdapterCapabilityError extends APIAdapterError { } } -// ------------------------------------------------------- -// Discovery response shape -// ------------------------------------------------------- - -type DiscoveryResponse = { - version: string; - entityId: string; - timezone?: string; - capabilities: AdapterCapabilities; -}; +/** + * Thrown by open() when the server's protocol major differs from this + * client's, or when discovery reports no parseable version at all. Refusing + * at the door beats the alternative: a major difference means some response + * reads wrongly, and finding out mid-session leaves the caller unsure which + * writes landed. See docs/spec/wire-format.md § Version negotiation. + */ +export class APIAdapterVersionError extends APIAdapterError { + constructor( + public readonly serverVersion: string | undefined, + message: string, + ) { + super(message); + this.name = 'APIAdapterVersionError'; + } +} // ------------------------------------------------------- // Domain object parsers (wire JSON → typed domain objects) @@ -239,6 +251,17 @@ export class APIAdapter implements StackAdapter { const discovery = (await res.json()) as DiscoveryResponse; + if (!isProtocolCompatible(discovery.version ?? '')) { + throw new APIAdapterVersionError( + discovery.version, + discovery.version + ? `Server at "${baseUrl}" speaks wire protocol "${discovery.version}"; this client ` + + `speaks "${WIRE_PROTOCOL_VERSION}".` + : `Server at "${baseUrl}" reported no wire protocol version in discovery; ` + + `"${WIRE_PROTOCOL_VERSION}" is required.`, + ); + } + return new APIAdapter( baseUrl, opts.token, diff --git a/packages/adapter-api/tests/api.test.ts b/packages/adapter-api/tests/api.test.ts index 98891c0..3fd4766 100644 --- a/packages/adapter-api/tests/api.test.ts +++ b/packages/adapter-api/tests/api.test.ts @@ -5,7 +5,9 @@ import { APIAdapterConnectionError, APIAdapterError, APIAdapterCapabilityError, + APIAdapterVersionError, } from '../src/index.js'; +import { WIRE_PROTOCOL_VERSION } from '@haverstack/wire-types'; import type { StackRecord, StackType, RecordVersion, Association } from '@haverstack/core'; import { StackPermissionError, @@ -167,6 +169,63 @@ describe('open', () => { }); }); +// ------------------------------------------------------- +// open() — wire protocol version negotiation +// ------------------------------------------------------- + +describe('open — version negotiation', () => { + test('opens against a server declaring this client’s protocol version', async () => { + const adapter = await openAdapter({ ...DISCOVERY, version: WIRE_PROTOCOL_VERSION }); + expect(adapter.ownerEntityId).toBe('entity-owner-123'); + }); + + test('refuses a server whose protocol major differs', async () => { + mockFetch.mockResolvedValueOnce(jsonResponse({ ...DISCOVERY, version: '2.0' })); + await expect(APIAdapter.open({ url: BASE_URL, token: TOKEN })).rejects.toThrow( + APIAdapterVersionError, + ); + }); + + test('a higher server minor opens — added fields an older client ignores', async () => { + const adapter = await openAdapter({ ...DISCOVERY, version: '1.7' }); + expect(adapter.ownerEntityId).toBe('entity-owner-123'); + }); + + test('a lower server minor opens — omitted fields an older server never had', async () => { + const adapter = await openAdapter({ ...DISCOVERY, version: '1.0' }); + expect(adapter.ownerEntityId).toBe('entity-owner-123'); + }); + + test('refuses discovery with no version at all', async () => { + mockFetch.mockResolvedValueOnce(jsonResponse({ ...DISCOVERY, version: undefined })); + await expect(APIAdapter.open({ url: BASE_URL, token: TOKEN })).rejects.toThrow( + APIAdapterVersionError, + ); + }); + + test('refuses a version that is not MAJOR.MINOR', async () => { + mockFetch.mockResolvedValueOnce(jsonResponse({ ...DISCOVERY, version: 'v1' })); + await expect(APIAdapter.open({ url: BASE_URL, token: TOKEN })).rejects.toThrow( + APIAdapterVersionError, + ); + }); + + test('carries the offending version for a caller that wants to report it', async () => { + mockFetch.mockResolvedValueOnce(jsonResponse({ ...DISCOVERY, version: '2.0' })); + const err = await APIAdapter.open({ url: BASE_URL, token: TOKEN }).catch((e: unknown) => e); + expect(err).toBeInstanceOf(APIAdapterVersionError); + expect((err as APIAdapterVersionError).serverVersion).toBe('2.0'); + }); + + test('refuses before sending any other request', async () => { + mockFetch.mockResolvedValueOnce(jsonResponse({ ...DISCOVERY, version: '2.0' })); + await expect(APIAdapter.open({ url: BASE_URL, token: TOKEN })).rejects.toThrow( + APIAdapterVersionError, + ); + expect(mockFetch).toHaveBeenCalledTimes(1); + }); +}); + // ------------------------------------------------------- // createRecord // ------------------------------------------------------- diff --git a/packages/adapter-api/tests/conformance.test.ts b/packages/adapter-api/tests/conformance.test.ts index d10099d..d8f409f 100644 --- a/packages/adapter-api/tests/conformance.test.ts +++ b/packages/adapter-api/tests/conformance.test.ts @@ -21,6 +21,7 @@ import { getVersionsAfterMutateFixtures, restoreVersionFixtures, commitMigrationFixtures, + discoveryFixtures, errorResponseFixtures, attachmentUploadFixtures, } from '@haverstack/conformance-fixtures'; @@ -75,6 +76,21 @@ const openAdapter = async (): Promise => { /** Record id embedded in a fixture path like "/records/rec-1" or ".../rec-1/permissions". */ const idFromPath = (path: string): string => path.split('/')[2].split('?')[0]; +describe('discovery fixtures', () => { + for (const fixture of discoveryFixtures) { + test(fixture.name, async () => { + mockFetch.mockResolvedValueOnce(jsonResponse(fixture.responseBody, fixture.responseStatus)); + const adapter = await APIAdapter.open({ url: BASE_URL }); + + const [url] = mockFetch.mock.lastCall as [string, RequestInit]; + expect(url).toBe(`${BASE_URL}${fixture.path}`); + expect(adapter.ownerEntityId).toBe(fixture.responseBody!.entityId); + expect(adapter.timezone).toBe(fixture.responseBody!.timezone); + expect(adapter.capabilities).toEqual(fixture.responseBody!.capabilities); + }); + } +}); + describe('createRecord fixtures', () => { for (const fixture of createRecordFixtures) { test(fixture.name, async () => { diff --git a/packages/conformance-fixtures/src/index.ts b/packages/conformance-fixtures/src/index.ts index 06ec564..b31e24a 100644 --- a/packages/conformance-fixtures/src/index.ts +++ b/packages/conformance-fixtures/src/index.ts @@ -23,7 +23,8 @@ * prior state — that's the consumer's test setup. */ -import type { WireRecord, WireError, WireVersion } from '@haverstack/wire-types'; +import type { WireRecord, WireError, WireVersion, DiscoveryResponse } from '@haverstack/wire-types'; +import { WIRE_PROTOCOL_VERSION } from '@haverstack/wire-types'; export type WireMethod = 'GET' | 'POST' | 'PATCH' | 'PUT' | 'DELETE'; @@ -45,6 +46,55 @@ export type ConformanceFixture = { responseBody?: Res; }; +// ------------------------------------------------------- +// Discovery +// ------------------------------------------------------- + +export const discoveryFixtures: ConformanceFixture[] = [ + { + name: 'discovery-declares-protocol-version-and-capabilities', + description: + 'GET /.well-known/stack declares the wire protocol version, the owner DID, and the ' + + "capability set a client uses to gate queries. `version` is the protocol's version, not " + + "the server's software version, and a client refuses a server whose major differs from " + + 'its own — see docs/spec/wire-format.md § Version negotiation.', + method: 'GET', + path: '/.well-known/stack', + responseStatus: 200, + responseBody: { + version: WIRE_PROTOCOL_VERSION, + entityId: 'did:key:z6MkhaXgBZDvotDkL5257faiztiGiC2QtKLGpbnnEGta2doK', + timezone: 'America/New_York', + capabilities: { + fullTextSearch: true, + contentFieldQuery: true, + sortableFields: ['createdAt', 'updatedAt', 'version'], + maxAttachmentBytes: 52428800, + }, + }, + }, + { + name: 'discovery-omits-absent-timezone', + description: + 'A stack with no timezone omits the field rather than defaulting it. An absent timezone ' + + 'stays undefined end to end — a default would assert knowledge the stack was never ' + + 'given (docs/spec.md § Stack identity).', + method: 'GET', + path: '/.well-known/stack', + responseStatus: 200, + responseBody: { + version: WIRE_PROTOCOL_VERSION, + entityId: 'did:key:z6MkhaXgBZDvotDkL5257faiztiGiC2QtKLGpbnnEGta2doK', + capabilities: { + fullTextSearch: false, + contentFieldQuery: false, + sortableFields: ['createdAt'], + maxAttachmentBytes: null, + }, + }, + }, +]; + // ------------------------------------------------------- // Records: create // ------------------------------------------------------- @@ -1186,6 +1236,7 @@ export const attachmentUploadFixtures: AttachmentUploadFixture[] = [ * JSON request/response pair), imported separately. */ export const allConformanceFixtures: ConformanceFixture[] = [ + ...discoveryFixtures, ...createRecordFixtures, ...patchContentFixtures, ...deleteRecordFixtures, diff --git a/packages/wire-types/src/index.ts b/packages/wire-types/src/index.ts index 82eeb15..900179c 100644 --- a/packages/wire-types/src/index.ts +++ b/packages/wire-types/src/index.ts @@ -7,6 +7,7 @@ import type { ValidationError, SchemaDriftViolation, StackErrorCode, + AdapterCapabilities, } from '@haverstack/core'; import { StackError, @@ -272,3 +273,41 @@ export function errorForStatus(status: number, message: string): Error | null { const code = STATUS_TO_CODE[status]; return code ? deserializeError({ error: { code, message } }) : null; } + +// ------------------------------------------------------- +// Discovery +// ------------------------------------------------------- + +/** + * The wire protocol this package describes. Bump the major when a change + * would make an older client read a response wrongly; bump the minor for + * additions an older client can ignore. See docs/spec/wire-format.md + * § Version negotiation. + */ +export const WIRE_PROTOCOL_VERSION = '1.0'; + +/** GET /.well-known/stack. See docs/spec/wire-format.md § Discovery. */ +export type DiscoveryResponse = { + version: string; + entityId: string; + timezone?: string; + capabilities: AdapterCapabilities; +}; + +/** Splits a MAJOR.MINOR protocol version. Returns null if it isn't one. */ +export function parseProtocolVersion(version: string): { major: number; minor: number } | null { + const match = /^(\d+)\.(\d+)$/.exec(version); + return match ? { major: Number(match[1]), minor: Number(match[2]) } : null; +} + +/** + * Majors must match; minors never have to. A higher server minor is additive + * fields this client ignores, and a higher client minor is optional fields + * the server may omit — neither can make a response read wrongly, which is + * the only thing a major bump signals. + */ +export function isProtocolCompatible(version: string, against = WIRE_PROTOCOL_VERSION): boolean { + const server = parseProtocolVersion(version); + const client = parseProtocolVersion(against); + return server !== null && client !== null && server.major === client.major; +} diff --git a/packages/wire-types/tests/discovery.test.ts b/packages/wire-types/tests/discovery.test.ts new file mode 100644 index 0000000..1158295 --- /dev/null +++ b/packages/wire-types/tests/discovery.test.ts @@ -0,0 +1,40 @@ +import { describe, it, expect } from 'vitest'; +import { WIRE_PROTOCOL_VERSION, parseProtocolVersion, isProtocolCompatible } from '../src/index.js'; + +describe('parseProtocolVersion', () => { + it('splits a MAJOR.MINOR version', () => { + expect(parseProtocolVersion('1.0')).toEqual({ major: 1, minor: 0 }); + expect(parseProtocolVersion('12.34')).toEqual({ major: 12, minor: 34 }); + }); + + it('returns null for anything that is not MAJOR.MINOR', () => { + for (const bad of ['1', '1.0.0', 'v1.0', '1.x', '', ' 1.0']) { + expect(parseProtocolVersion(bad)).toBeNull(); + } + }); +}); + +describe('isProtocolCompatible', () => { + it('accepts a matching major', () => { + expect(isProtocolCompatible('1.0', '1.0')).toBe(true); + }); + + it('accepts either side having the higher minor', () => { + expect(isProtocolCompatible('1.9', '1.0')).toBe(true); + expect(isProtocolCompatible('1.0', '1.9')).toBe(true); + }); + + it('rejects a differing major in either direction', () => { + expect(isProtocolCompatible('2.0', '1.0')).toBe(false); + expect(isProtocolCompatible('1.0', '2.0')).toBe(false); + }); + + it('rejects an unparseable version rather than guessing', () => { + expect(isProtocolCompatible('', '1.0')).toBe(false); + expect(isProtocolCompatible('v1', '1.0')).toBe(false); + }); + + it('compares against this package’s own version by default', () => { + expect(isProtocolCompatible(WIRE_PROTOCOL_VERSION)).toBe(true); + }); +}); From 776a0a9856f17717596d9017ad4167d87a9ba471 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 20:52:32 +0000 Subject: [PATCH 4/5] docs: handle is a label, not a key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both handle fields opened with "Short unique identifier", so a reader who stopped at the first three words got a guarantee that does not exist. State what the field is instead of qualifying a misleading phrase. Uniqueness is unenforced deliberately, not pending: the petname model makes global uniqueness incoherent, per-stack uniqueness adds nothing over the DID that already identifies the profile, and nothing in the library resolves an entity or group by handle at all. The reasoning goes in the spec, with the field comments pointing at it. Also records that handle lookup, for an app that wants it anyway, rests on contentFieldQuery — a capability a server may decline. Refs #147 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019eqUFNRUhV3x5JrjkMTpxj --- docs/spec/identity.md | 10 ++++++++-- packages/core/src/types.ts | 13 +++++++++++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/docs/spec/identity.md b/docs/spec/identity.md index f1973ab..f59ffbb 100644 --- a/docs/spec/identity.md +++ b/docs/spec/identity.md @@ -26,12 +26,16 @@ Crucially, an `_entity` record is a **stack-local profile card about a DID** — type EntityContent = { did: string; // The identity this profile is about, e.g. "did:key:z6Mk..." name: string; // Display name — human-friendly, not necessarily unique. May contain spaces and punctuation. e.g. "Jane Smith" - handle?: string; // Short unique identifier — URL-safe, no spaces. e.g. "janesmith". Like a username. Optional for private entities. + handle?: string; // Short, conventionally URL-safe label. e.g. "janesmith". Optional. }; ``` `name`/`handle` are _this stack owner's_ labels for that DID — the petname pattern (Zooko's triangle: global, human-readable, decentralized — pick two; the escape is names local to the observer). Two stacks holding `_entity` cards with different display names for the same `did:key:...` is correct behavior: each is its owner's own contact card for that identity. Cross-stack ID collisions are a non-issue mechanically — DIDs are globally unique by construction, unlike a `RecordId` (unique within a stack only; see [Record IDs](./data-model.md#record-ids)). +**`handle` is a label, not a key, and nothing enforces its uniqueness — deliberately.** The petname model makes global uniqueness incoherent: a handle is one observer's name for a DID, so two stacks are free to use the same handle for different people, or different handles for the same person. Uniqueness _within_ a stack is coherent but pointless, since `did` already identifies the profile and nothing in the library ever resolves an entity by handle. Two cards labeled `janesmith` are a display problem for the app that allowed it, not a data-integrity violation — and forbidding them would reject ordinary states like a half-finished rename or a contact import. The URL-safe, no-spaces shape is likewise conventional: `_entity@1` declares `handle` as a plain string and validates nothing beyond that. + +An app that wants handle lookup anyway builds it on `query({ filter: { content: { handle } } })` and must handle duplicates itself. Note that filtering on `content` requires the `contentFieldQuery` capability, which a server behind `adapter-api` may decline (see [Adapters](./adapters.md#adapter-capabilities)) — another reason not to design a lookup around it. + The Stack has a designated owner, identified by `_config.entityId` (a DID) — not by pointing at any particular `_entity` record's `RecordId`. The owner's own `_entity` record (`content.did === ownerEntityId`) is created automatically by `Stack.create(adapter, { ownerProfile })` if one doesn't exist yet — idempotent, safe to pass on every open. An Entity record's `entityId` (author) may point to itself but doesn't have to; `Stack.create()`'s bootstrap leaves it unset, matching the owner-attributed, no-`entityId` convention used elsewhere. ## App @@ -59,11 +63,13 @@ A permission group can be promoted to a collaborative group at any time by addin ```ts type GroupContent = { name: string; // Display name — human-friendly, not necessarily unique. e.g. "Jane's Book Club" - handle?: string; // Short unique identifier — URL-safe, no spaces. e.g. "janes-book-club". Optional for private groups. + handle?: string; // Short, conventionally URL-safe label. e.g. "janes-book-club". Optional. stackUrl?: string; // If present, this group owns a shared collaborative stack at this URL. Absent = permission-only group. }; ``` +A group's `handle` is a label on the same terms as an entity's — unenforced, not a key, and not what addresses the group. A collaborative group is reached at its `stackUrl`; a permission group is referenced by its record id. + **Group identity.** "A group with cohesive identity" is anything that controls a key: a group can be given its own keypair (held by its admins), so it can be granted access, own a collaborative stack (`stackUrl`), and sign as itself. Membership associations list member DIDs, same as any other entity reference — no new machinery. Group key generation/custody is deferred; nothing here blocks it. **Membership** is expressed via associations on the `_group` Record, using the existing Association model: diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 1eff6ac..a739af3 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -180,7 +180,11 @@ export type EntityContent = { did: string; /** Display name — human-friendly, not necessarily unique. May contain spaces and punctuation. e.g. "Jane Smith" */ name: string; - /** Short unique identifier within a namespace — URL-safe, no spaces. e.g. "janesmith". Like a username. Optional for private entities. */ + /** + * Short, conventionally URL-safe label. e.g. "janesmith". A label, not a + * key: duplicates are legitimate and `did` is what identifies this + * profile. See docs/spec/identity.md § Entity. + */ handle?: string; }; @@ -200,7 +204,12 @@ export type AppContent = { export type GroupContent = { /** Display name — human-friendly, not necessarily unique. May contain spaces and punctuation. e.g. "Jane's Book Club" */ name: string; - /** Short unique identifier — URL-safe, no spaces. e.g. "janes-book-club". Useful for groups other people need to reference. Optional for private groups. */ + /** + * Short, conventionally URL-safe label. e.g. "janes-book-club". A label, + * not a key: duplicates are legitimate, and a group is addressed by + * `stackUrl` when it has one and by its record id otherwise. See + * docs/spec/identity.md § Group. + */ handle?: string; /** If present, this group owns a shared collaborative stack at this URL. Absent = permission-only group. */ stackUrl?: string; From b6bbd26a18e06603511833dd6f269a01297d25dd Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 00:10:26 +0000 Subject: [PATCH 5/5] feat(core)!: putAttachment returns the _attachment@1 record MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The uploader got back only a fileId, so the record they had just created was the one thing they could not address — and filename, its only mutable field, needs an id to set. Getting one meant querying by fileId and disambiguating among the several records a shared fileId can have. Return the record instead, matching what POST /attachments returns on the wire and what create() already returns. Every path had it in hand and was discarding it: the atomic path takes the server's response, and both fallback paths take what their own create() produced. BREAKING CHANGE: Stack.putAttachment(), ScopedStack.putAttachment(), and StackClient.putAttachment() return StackRecord & { content: AttachmentContent } instead of the fileId string. Callers wanting the id read content.fileId. The adapter-level StackBlobAdapter.putAttachment() is unchanged — it remains the bytes-only primitive returning a FileId. Refs #147 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019eqUFNRUhV3x5JrjkMTpxj --- docs/spec/attachments.md | 12 +- docs/spec/wire-format.md | 2 + packages/core/src/stack.ts | 43 ++++--- packages/core/tests/scoped-stack.test.ts | 71 ++++++++--- packages/core/tests/stack.test.ts | 149 +++++++++++++++++++---- 5 files changed, 217 insertions(+), 60 deletions(-) diff --git a/docs/spec/attachments.md b/docs/spec/attachments.md index 5e63645..e9d03e8 100644 --- a/docs/spec/attachments.md +++ b/docs/spec/attachments.md @@ -3,11 +3,13 @@ Binary files are stored and retrieved through the library using **content-addressed storage**. A file's ID is the SHA-256 hash of its bytes, so uploading identical bytes twice returns the same `fileId` without writing a second binary copy. Each upload creates a new `_attachment@1` metadata record regardless of deduplication, so metadata (mimeType, size, filename) is tracked per upload. ```ts -// Upload a file — returns a stable SHA-256 hex ID and creates an _attachment@1 record -const fileId = await stack.putAttachment(data: Uint8Array, mimeType: string, filename?: string): Promise +// Upload a file — stores the bytes and creates the _attachment@1 record, +// returning that record. content.fileId is a stable SHA-256 hex ID. +const record = await stack.putAttachment(data: Uint8Array, mimeType: string, filename?: string) + : Promise // Fetch the binary -const data: Uint8Array = await stack.getAttachment(fileId) +const data: Uint8Array = await stack.getAttachment(record.content.fileId) // Delete the binary and its _attachment@1 metadata record(s) // Throws StackConflictError if any record still references the file @@ -29,7 +31,9 @@ type AttachmentContent = { An `_attachment@1` record is created on every `putAttachment()` call — even if the same bytes were previously uploaded. Multiple `_attachment@1` records may therefore exist for the same `fileId`, each with its own `filename`; the binary is stored only once. -To read metadata for a given `fileId`, query `_attachment@1` records: +**`putAttachment()` returns the record it created**, matching what `POST /attachments` returns on the wire — the uploader's own metadata record is never something they have to go looking for. The `id` is the part that matters: `filename` is the only mutable field on an `_attachment@1` record, and setting it later needs an id. Without this, every caller wanting one would have to query by `fileId` and disambiguate among the several records a shared `fileId` can have. + +To read metadata for a `fileId` uploaded by _someone else_, query `_attachment@1` records — note that a `fileId` may have several, one per upload: ```ts const results = await stack.query({ diff --git a/docs/spec/wire-format.md b/docs/spec/wire-format.md index 42bb64d..8c0fc34 100644 --- a/docs/spec/wire-format.md +++ b/docs/spec/wire-format.md @@ -259,6 +259,8 @@ Returns `413 Request Entity Too Large` (code `payload_too_large`, reconstructed **SDK usage.** `Stack.putAttachment()`, when backed by `APIAdapter`, calls this endpoint directly — one request, carrying the real `mimeType`/`filename` — via the optional [`putAttachmentWithMetadata()`](./adapters.md#interface-split) capability. Local storage adapters don't implement that capability — bytes and records are different backends there, with no shared transaction — so `Stack.putAttachment()` falls back to its own `create()` call. +Either way the caller gets the created `_attachment@1` record back, the same thing this endpoint returns — the atomic path passes the server's response through, and the fallback path returns what its own `create()` produced. See [Attachments](./attachments.md#the-_attachment-record-type). + One consequence: there is no bytes-only upload anywhere on the wire — this endpoint always creates a record. Accordingly, **bytes-only upload has no public SDK surface either**: `putAttachment(data, mimeType, filename?)` is the upload operation, everywhere, for everyone. `StackBlobAdapter.putAttachment()` remains the required adapter-level primitive local storage needs (it's what `Stack.putAttachment()`'s fallback writes bytes through), but on `APIAdapter` it is **unsupported and throws** rather than mapping to this endpoint — implementing it anyway would silently create a record with a default `mimeType`, a bytes-only upload that isn't. `Stack.putAttachment()` never reaches it there (the atomic capability takes precedence), so the throw guards direct adapter-level callers only. ### Download diff --git a/packages/core/src/stack.ts b/packages/core/src/stack.ts index e17c2da..4eb0b7b 100644 --- a/packages/core/src/stack.ts +++ b/packages/core/src/stack.ts @@ -443,7 +443,11 @@ export interface StackClient { getVersion(id: string, version: number): Promise; restoreVersion(id: string, version: number, opts?: IfVersionOptions): Promise; getAttachment(fileId: string): Promise; - putAttachment(data: Uint8Array, mimeType: string, filename?: string): Promise; + putAttachment( + data: Uint8Array, + mimeType: string, + filename?: string, + ): Promise; deleteAttachment(fileId: string): Promise; collectAttachmentGarbage( opts?: CollectAttachmentGarbageOptions, @@ -1306,26 +1310,30 @@ export class Stack implements StackClient { /** * Store bytes and create an _attachment@1 metadata record (owner- - * attributed, no entityId). Delegates to the adapter's atomic - * putAttachmentWithMetadata() when implemented, trusting the returned - * record as backend-authoritative; otherwise falls back to + * attributed, no entityId), returning that record — `content.fileId` + * addresses the bytes, `id` addresses the metadata. Delegates to the + * adapter's atomic putAttachmentWithMetadata() when implemented, trusting + * the returned record as backend-authoritative; otherwise falls back to * bytes-then-create(). See docs/spec/wire-format.md § Attachments. */ - async putAttachment(data: Uint8Array, mimeType: string, filename?: string): Promise { + async putAttachment( + data: Uint8Array, + mimeType: string, + filename?: string, + ): Promise { this.assertOpen(); assertAttachmentSize(data.byteLength, this.features.maxAttachmentBytes); if (this.adapter.putAttachmentWithMetadata) { const record = await this.adapter.putAttachmentWithMetadata(data, mimeType, filename); - return (record.content as AttachmentContent).fileId; + return record as StackRecord & { content: AttachmentContent }; } const fileId = await this.adapter.putAttachment(data); - await this.create(`${SYSTEM_TYPES.ATTACHMENT}@1`, { + return this.create(`${SYSTEM_TYPES.ATTACHMENT}@1`, { fileId, mimeType, size: data.byteLength, ...(filename && { filename }), - } satisfies AttachmentContent); - return fileId; + }); } async getAttachment(fileId: string): Promise { @@ -2218,11 +2226,15 @@ export class ScopedStack implements StackClient { /** * Store bytes and create an _attachment@1 metadata record (create grant - * on `_attachment@1` required; anonymous denied). entityId is the - * requester, omitted when that's the owner — the same normalization - * create() applies. + * on `_attachment@1` required; anonymous denied), returning that record. + * entityId is the requester, omitted when that's the owner — the same + * normalization create() applies. */ - async putAttachment(data: Uint8Array, mimeType: string, filename?: string): Promise { + async putAttachment( + data: Uint8Array, + mimeType: string, + filename?: string, + ): Promise { // The one ScopedStack path that reaches the adapter without going // through Stack first — without this, a closed stack would still write // bytes before the delegated create() refused. @@ -2237,17 +2249,16 @@ export class ScopedStack implements StackClient { assertAttachmentSize(data.byteLength, this.features.maxAttachmentBytes); const fileId = await this.adapter.putAttachment(data); const isOwner = requester === this.stack.ownerEntityId; - await this.stack.create( + return this.stack.create( `${SYSTEM_TYPES.ATTACHMENT}@1`, { fileId, mimeType, size: data.byteLength, ...(filename && { filename }), - } satisfies AttachmentContent, + }, { entityId: isOwner ? undefined : requester }, ); - return fileId; } /** diff --git a/packages/core/tests/scoped-stack.test.ts b/packages/core/tests/scoped-stack.test.ts index c2cbe14..a30b531 100644 --- a/packages/core/tests/scoped-stack.test.ts +++ b/packages/core/tests/scoped-stack.test.ts @@ -418,7 +418,9 @@ describe('ScopedStack — versions', () => { test('allows restoring an attachment association to a file the requester can currently access', async () => { await stack.grant(MEMBER, [{ actions: ['create'], typeId: '_attachment@1' }]); - const fileId = await stack.asEntity(MEMBER).putAttachment(new Uint8Array([1]), 'image/png'); + const { + content: { fileId }, + } = await stack.asEntity(MEMBER).putAttachment(new Uint8Array([1]), 'image/png'); const record = await adapter.createRecord( makeRecord({ version: 2, @@ -903,7 +905,9 @@ describe('ScopedStack.putAttachment', () => { const data = new Uint8Array([1, 2, 3]); test('owner can always upload without a grant', async () => { - const fileId = await stack.asEntity(OWNER).putAttachment(data, 'image/png'); + const { + content: { fileId }, + } = await stack.asEntity(OWNER).putAttachment(data, 'image/png'); expect(typeof fileId).toBe('string'); }); @@ -921,7 +925,9 @@ describe('ScopedStack.putAttachment', () => { test('entity with create grant on _attachment@1 can upload', async () => { await stack.grant(MEMBER, [{ actions: ['create'], typeId: '_attachment@1' }]); - const fileId = await stack.asEntity(MEMBER).putAttachment(data, 'image/png'); + const { + content: { fileId }, + } = await stack.asEntity(MEMBER).putAttachment(data, 'image/png'); expect(typeof fileId).toBe('string'); }); @@ -937,6 +943,17 @@ describe('ScopedStack.putAttachment', () => { expect(content.filename).toBe('photo.png'); }); + test('returns the attributed record, so the uploader needs no follow-up query', async () => { + await stack.grant(MEMBER, [{ actions: ['create'], typeId: '_attachment@1' }]); + + const record = await stack.asEntity(MEMBER).putAttachment(data, 'image/png', 'photo.png'); + + expect(record.typeId).toBe('_attachment@1'); + expect(record.entityId).toBe(MEMBER); + expect(record.content.filename).toBe('photo.png'); + expect(await stack.get(record.id)).toMatchObject({ id: record.id, entityId: MEMBER }); + }); + test('upload without filename omits filename from record content', async () => { await stack.grant(MEMBER, [{ actions: ['create'], typeId: '_attachment@1' }]); await stack.asEntity(MEMBER).putAttachment(data, 'image/png'); @@ -946,7 +963,9 @@ describe('ScopedStack.putAttachment', () => { test('default grant allows any authenticated entity to upload', async () => { await stack.grant(null, [{ actions: ['create'], typeId: '_attachment@1' }]); - const fileId = await stack.asEntity(STRANGER).putAttachment(data, 'image/png'); + const { + content: { fileId }, + } = await stack.asEntity(STRANGER).putAttachment(data, 'image/png'); expect(typeof fileId).toBe('string'); }); @@ -1165,7 +1184,9 @@ describe('ScopedStack.getAttachment — file-ref content fields', () => { describe('ScopedStack.collectAttachmentGarbage', () => { test('owner can run the sweep', async () => { - const fileId = await stack.putAttachment(new Uint8Array([1]), 'image/png'); + const { + content: { fileId }, + } = await stack.putAttachment(new Uint8Array([1]), 'image/png'); const result = await stack.asEntity(OWNER).collectAttachmentGarbage({ graceMs: 0 }); @@ -1409,7 +1430,9 @@ describe('ScopedStack.create — attachment association gating', () => { test('attachment association referencing a file the requester uploaded is allowed', async () => { await stack.grant(MEMBER, [{ actions: ['create'], typeId: '_attachment@1' }]); - const fileId = await stack.asEntity(MEMBER).putAttachment(new Uint8Array([1]), 'image/png'); + const { + content: { fileId }, + } = await stack.asEntity(MEMBER).putAttachment(new Uint8Array([1]), 'image/png'); const record = await stack.asEntity(MEMBER).create( COMMENT, { text: 'hi' }, @@ -1425,7 +1448,9 @@ describe('ScopedStack.create — attachment association gating', () => { }); test('attachment association referencing a file readable via another record is allowed', async () => { - const fileId = await stack.putAttachment(new Uint8Array([1]), 'image/png'); + const { + content: { fileId }, + } = await stack.putAttachment(new Uint8Array([1]), 'image/png'); const owned = await stack.create(NOTE, { text: 'owner note' }); await stack.associate(owned.id, { kind: 'attachment', @@ -1449,7 +1474,9 @@ describe('ScopedStack.create — attachment association gating', () => { }); test('nonexistent and existing-but-forbidden fileIds produce indistinguishable errors', async () => { - const forbiddenFileId = await stack.putAttachment(new Uint8Array([9]), 'image/png'); + const { + content: { fileId: forbiddenFileId }, + } = await stack.putAttachment(new Uint8Array([9]), 'image/png'); let nonexistentError: Error | undefined; let forbiddenError: Error | undefined; try { @@ -1564,7 +1591,9 @@ describe('ScopedStack.create — non-owner _attachment@1 refusal', () => { test('non-owner putAttachment(bytes, mime, filename) still works end-to-end', async () => { const data = new Uint8Array([1, 2, 3]); - const fileId = await stack.asEntity(MEMBER).putAttachment(data, 'image/png', 'photo.png'); + const { + content: { fileId }, + } = await stack.asEntity(MEMBER).putAttachment(data, 'image/png', 'photo.png'); // The upload is now accessible to them... const bytes = await stack.asEntity(MEMBER).getAttachment(fileId); @@ -1592,7 +1621,9 @@ describe('ScopedStack.create — non-owner _attachment@1 refusal', () => { // without re-uploading bytes — this conveys no access they didn't already // have via the readable record. test('carve-out: a non-owner with a readable referencing record can add a second metadata record', async () => { - const fileId = await stack.putAttachment(new Uint8Array([1]), 'image/png', 'owner.png'); + const { + content: { fileId }, + } = await stack.putAttachment(new Uint8Array([1]), 'image/png', 'owner.png'); const owned = await stack.create(NOTE, { text: 'owner note' }); await stack.associate(owned.id, { kind: 'attachment', @@ -1633,7 +1664,9 @@ describe('ScopedStack.create — non-owner _attachment@1 refusal', () => { }); test('a non-owner without a readable referencing record is refused even for a real, existing fileId', async () => { - const fileId = await stack.putAttachment(new Uint8Array([9]), 'image/png'); + const { + content: { fileId }, + } = await stack.putAttachment(new Uint8Array([9]), 'image/png'); // fileId is real and exists, but MEMBER has no readable record referencing it. await expect( stack.asEntity(MEMBER).create('_attachment@1', { @@ -1775,7 +1808,9 @@ describe('ScopedStack.associate — reference-creation gating', () => { test('associate() allows an attachment association to a file the requester uploaded', async () => { await stack.grant(MEMBER, [{ actions: ['create'], typeId: '_attachment@1' }]); - const fileId = await stack.asEntity(MEMBER).putAttachment(new Uint8Array([1]), 'image/png'); + const { + content: { fileId }, + } = await stack.asEntity(MEMBER).putAttachment(new Uint8Array([1]), 'image/png'); await stack .asEntity(MEMBER) .associate(ownedRecord.id, { kind: 'attachment', label: 'x', fileId }); @@ -1826,14 +1861,18 @@ describe('ScopedStack — file-ref content field gating', () => { test('create() allows a file-ref field pointing at a file the requester uploaded', async () => { await stack.grant(MEMBER, [{ actions: ['create'], typeId: '_attachment@1' }]); - const fileId = await stack.asEntity(MEMBER).putAttachment(new Uint8Array([1]), 'image/png'); + const { + content: { fileId }, + } = await stack.asEntity(MEMBER).putAttachment(new Uint8Array([1]), 'image/png'); const record = await stack.asEntity(MEMBER).create(PHOTO_NOTE, { coverFileId: fileId }); expect(record.content.coverFileId).toBe(fileId); }); test('update() rejects changing a file-ref field to an inaccessible file', async () => { await stack.grant(MEMBER, [{ actions: ['create'], typeId: '_attachment@1' }]); - const fileId = await stack.asEntity(MEMBER).putAttachment(new Uint8Array([1]), 'image/png'); + const { + content: { fileId }, + } = await stack.asEntity(MEMBER).putAttachment(new Uint8Array([1]), 'image/png'); const record = await stack.asEntity(MEMBER).create(PHOTO_NOTE, { coverFileId: fileId }); await expect( @@ -1844,7 +1883,9 @@ describe('ScopedStack — file-ref content field gating', () => { test('update() leaving the file-ref field untouched is unaffected by its accessibility', async () => { // Owner-created record with a file-ref the MEMBER updater can't independently access; // a patch that never mentions coverFileId carries no new reference and isn't gated. - const fileId = await stack.putAttachment(new Uint8Array([1]), 'image/png'); + const { + content: { fileId }, + } = await stack.putAttachment(new Uint8Array([1]), 'image/png'); const record = await stack.create( PHOTO_NOTE, { coverFileId: fileId }, diff --git a/packages/core/tests/stack.test.ts b/packages/core/tests/stack.test.ts index 9824d6e..f216dcf 100644 --- a/packages/core/tests/stack.test.ts +++ b/packages/core/tests/stack.test.ts @@ -16,7 +16,7 @@ import { import { generateId, crockford32Encode, IdGenerationError } from '../src/id.js'; import { InvalidDidError } from '../src/did.js'; import { MemoryAdapter, IncapableMemoryAdapter } from '../src/testing.js'; -import type { BlobFileInfo, StackAdapter, StackRecord } from '../src/types.js'; +import type { AttachmentContent, BlobFileInfo, StackAdapter, StackRecord } from '../src/types.js'; // ------------------------------------------------------- // Test setup @@ -2054,7 +2054,9 @@ describe('setPermissions', () => { describe('putAttachment', () => { test('stores bytes and returns fileId', async () => { const data = new Uint8Array([1, 2, 3]); - const fileId = await stack.putAttachment(data, 'image/png'); + const { + content: { fileId }, + } = await stack.putAttachment(data, 'image/png'); expect(typeof fileId).toBe('string'); }); @@ -2111,12 +2113,14 @@ describe('putAttachment — maxAttachmentBytes pre-check', () => { await expect( limitedStack.putAttachment(new Uint8Array([1, 2, 3]), 'image/png'), - ).resolves.toEqual(expect.any(String)); + ).resolves.toMatchObject({ typeId: '_attachment@1' }); }); test('null maxAttachmentBytes never throws, regardless of size', async () => { const data = new Uint8Array(1000); - await expect(stack.putAttachment(data, 'image/png')).resolves.toEqual(expect.any(String)); + await expect(stack.putAttachment(data, 'image/png')).resolves.toMatchObject({ + typeId: '_attachment@1', + }); }); }); @@ -2145,7 +2149,9 @@ describe('putAttachment — atomic adapter path', () => { const atomicStack = await Stack.create(atomicAdapter); const createSpy = vi.spyOn(atomicStack, 'create'); - const fileId = await atomicStack.putAttachment(data, 'image/png', 'photo.png'); + const { + content: { fileId }, + } = await atomicStack.putAttachment(data, 'image/png', 'photo.png'); expect(fileId).toBe('atomic-file-id'); expect(atomicAdapter.putAttachmentWithMetadata).toHaveBeenCalledWith( @@ -2164,6 +2170,59 @@ describe('putAttachment — atomic adapter path', () => { expect(createSpy).toHaveBeenCalledTimes(1); }); + + test('the returned record is the one in storage, on the atomic path', async () => { + const fabricatedRecord: StackRecord = { + id: generateId(), + typeId: '_attachment@1', + createdAt: new Date(), + updatedAt: new Date(), + content: { fileId: 'atomic-file-id', mimeType: 'image/png', size: 3 }, + version: 1, + }; + const atomicAdapter: StackAdapter = Object.assign( + new MemoryAdapter({ ownerEntityId: 'owner-123', timezone: 'UTC' }), + { putAttachmentWithMetadata: vi.fn().mockResolvedValue(fabricatedRecord) }, + ); + const atomicStack = await Stack.create(atomicAdapter); + + const record = await atomicStack.putAttachment(new Uint8Array([1, 2, 3]), 'image/png'); + + expect(record.id).toBe(fabricatedRecord.id); + expect(record.content.fileId).toBe('atomic-file-id'); + }); +}); + +// ------------------------------------------------------- +// putAttachment returns the _attachment@1 record, matching what +// POST /attachments returns on the wire. The metadata record's id is the +// point: filename is the one mutable field, and setting it later needs an +// id the caller would otherwise have to go query for. +// ------------------------------------------------------- + +describe('putAttachment — returned record', () => { + test('returns the metadata record, not just the fileId', async () => { + const data = new Uint8Array([1, 2, 3]); + + const record = await stack.putAttachment(data, 'image/png', 'photo.png'); + + expect(record.typeId).toBe('_attachment@1'); + expect(record.content).toEqual({ + fileId: expect.any(String), + mimeType: 'image/png', + size: 3, + filename: 'photo.png', + }); + expect(await stack.get(record.id)).toMatchObject({ id: record.id }); + }); + + test('the returned id sets filename later without a lookup', async () => { + const record = await stack.putAttachment(new Uint8Array([1, 2, 3]), 'image/png'); + + const renamed = await stack.update(record.id, { filename: 'renamed.png' }); + + expect((renamed.content as AttachmentContent).filename).toBe('renamed.png'); + }); }); // ------------------------------------------------------- @@ -2174,8 +2233,12 @@ describe('putAttachment — atomic adapter path', () => { describe('_attachment@1 mimeType conflict on create', () => { test('second upload of identical bytes with a matching mimeType succeeds', async () => { const data = new Uint8Array([1, 2, 3]); - const fileId1 = await stack.putAttachment(data, 'image/png', 'first.png'); - const fileId2 = await stack.putAttachment(data, 'image/png', 'second.png'); + const { + content: { fileId: fileId1 }, + } = await stack.putAttachment(data, 'image/png', 'first.png'); + const { + content: { fileId: fileId2 }, + } = await stack.putAttachment(data, 'image/png', 'second.png'); expect(fileId2).toBe(fileId1); const result = await stack.query({ filter: { typeId: '_attachment@1' } }); @@ -2336,7 +2399,9 @@ describe('_attachment@1 immutable fields on update', () => { describe('deleteAttachment', () => { test('throws StackConflictError when a record still references the file (fallback path)', async () => { const data = new Uint8Array([1, 2, 3]); - const fileId = await stack.putAttachment(data, 'image/png'); + const { + content: { fileId }, + } = await stack.putAttachment(data, 'image/png'); const note = await stack.create(NOTE_V1, { text: 'hi' }); await stack.associate(note.id, { kind: 'attachment', @@ -2352,7 +2417,9 @@ describe('deleteAttachment', () => { // moment the record comes back. test('throws StackConflictError when only a soft-deleted record still references the file', async () => { const data = new Uint8Array([1, 2, 3]); - const fileId = await stack.putAttachment(data, 'image/png'); + const { + content: { fileId }, + } = await stack.putAttachment(data, 'image/png'); const note = await stack.create(NOTE_V1, { text: 'hi' }); await stack.associate(note.id, { kind: 'attachment', @@ -2366,7 +2433,9 @@ describe('deleteAttachment', () => { test('hard-deletes a soft-deleted _attachment@1 metadata record too (fallback path)', async () => { const data = new Uint8Array([1, 2, 3]); - const fileId = await stack.putAttachment(data, 'image/png'); + const { + content: { fileId }, + } = await stack.putAttachment(data, 'image/png'); const [metaRecord] = (await stack.query({ filter: { typeId: '_attachment@1' } })).records; await stack.delete(metaRecord.id); @@ -2378,7 +2447,9 @@ describe('deleteAttachment', () => { test('deletes the _attachment@1 metadata record when unreferenced (fallback path)', async () => { const data = new Uint8Array([1, 2, 3]); - const fileId = await stack.putAttachment(data, 'image/png'); + const { + content: { fileId }, + } = await stack.putAttachment(data, 'image/png'); await stack.deleteAttachment(fileId); @@ -2443,7 +2514,9 @@ describe('deleteAttachment', () => { }); const data = new Uint8Array([1, 2, 3]); - const fileId = await stack.putAttachment(data, 'image/png'); + const { + content: { fileId }, + } = await stack.putAttachment(data, 'image/png'); await stack.create(attachmentTypeId, { coverFileId: fileId }); await expect(stack.deleteAttachment(fileId)).rejects.toThrow(StackConflictError); @@ -2456,7 +2529,9 @@ describe('deleteAttachment', () => { }); const data = new Uint8Array([1, 2, 3]); - const fileId = await stack.putAttachment(data, 'image/png'); + const { + content: { fileId }, + } = await stack.putAttachment(data, 'image/png'); await stack.create(attachmentTypeId, { coverFileId: fileId }); await expect(stack.deleteAttachment(fileId)).resolves.toBeUndefined(); @@ -2486,7 +2561,9 @@ describe('deleteAttachment', () => { ); await atomicStack.defineType(NOTE_V1, 'Note', { text: { kind: 'text', required: true } }); - const fileId = await atomicStack.putAttachment(new Uint8Array([9]), 'image/png'); + const { + content: { fileId }, + } = await atomicStack.putAttachment(new Uint8Array([9]), 'image/png'); await atomicStack.deleteAttachment(fileId); expect(calls).toEqual(['atomic']); @@ -2499,7 +2576,9 @@ describe('deleteAttachment', () => { describe('collectAttachmentGarbage', () => { test('collects a file whose only referencing record was hard-deleted', async () => { - const fileId = await stack.putAttachment(new Uint8Array([1]), 'image/png'); + const { + content: { fileId }, + } = await stack.putAttachment(new Uint8Array([1]), 'image/png'); const note = await stack.create(NOTE_V1, { text: 'hi' }); await stack.associate(note.id, { kind: 'attachment', @@ -2516,7 +2595,9 @@ describe('collectAttachmentGarbage', () => { }); test('does not collect a file referenced by a live record', async () => { - const fileId = await stack.putAttachment(new Uint8Array([1]), 'image/png'); + const { + content: { fileId }, + } = await stack.putAttachment(new Uint8Array([1]), 'image/png'); const note = await stack.create(NOTE_V1, { text: 'hi' }); await stack.associate(note.id, { kind: 'attachment', @@ -2532,7 +2613,9 @@ describe('collectAttachmentGarbage', () => { // Soft-deleted records are recoverable via undelete() // and must find their attachments intact — so they still count as references. test('does not collect a file referenced only by a soft-deleted record', async () => { - const fileId = await stack.putAttachment(new Uint8Array([1]), 'image/png'); + const { + content: { fileId }, + } = await stack.putAttachment(new Uint8Array([1]), 'image/png'); const note = await stack.create(NOTE_V1, { text: 'hi' }); await stack.associate(note.id, { kind: 'attachment', @@ -2553,7 +2636,9 @@ describe('collectAttachmentGarbage', () => { await stack.defineType(photoType, 'Photo note', { coverFileId: { kind: 'file-ref', required: true }, }); - const fileId = await stack.putAttachment(new Uint8Array([1]), 'image/png'); + const { + content: { fileId }, + } = await stack.putAttachment(new Uint8Array([1]), 'image/png'); await stack.create(photoType, { coverFileId: fileId }); const result = await stack.collectAttachmentGarbage({ graceMs: 0 }); @@ -2572,7 +2657,9 @@ describe('collectAttachmentGarbage', () => { }); test('graceMs: 0 collects an unreferenced upload immediately', async () => { - const fileId = await stack.putAttachment(new Uint8Array([1]), 'image/png'); + const { + content: { fileId }, + } = await stack.putAttachment(new Uint8Array([1]), 'image/png'); const result = await stack.collectAttachmentGarbage({ graceMs: 0 }); @@ -2580,8 +2667,12 @@ describe('collectAttachmentGarbage', () => { }); test('reports reclaimedBytes summed across deleted files', async () => { - const fileId1 = await stack.putAttachment(new Uint8Array([1, 2, 3]), 'image/png'); - const fileId2 = await stack.putAttachment(new Uint8Array([1, 2, 3, 4, 5]), 'image/png'); + const { + content: { fileId: fileId1 }, + } = await stack.putAttachment(new Uint8Array([1, 2, 3]), 'image/png'); + const { + content: { fileId: fileId2 }, + } = await stack.putAttachment(new Uint8Array([1, 2, 3, 4, 5]), 'image/png'); const result = await stack.collectAttachmentGarbage({ graceMs: 0 }); @@ -2590,7 +2681,9 @@ describe('collectAttachmentGarbage', () => { }); test('dryRun reports what would be deleted without deleting anything', async () => { - const fileId = await stack.putAttachment(new Uint8Array([1, 2, 3]), 'image/png'); + const { + content: { fileId }, + } = await stack.putAttachment(new Uint8Array([1, 2, 3]), 'image/png'); const result = await stack.collectAttachmentGarbage({ graceMs: 0, dryRun: true }); @@ -2620,7 +2713,9 @@ describe('collectAttachmentGarbage', () => { const noListFilesStack = await Stack.create( new NoListFilesAdapter({ ownerEntityId: 'owner-123', timezone: 'UTC' }), ); - const fileId = await noListFilesStack.putAttachment(new Uint8Array([1]), 'image/png'); + const { + content: { fileId }, + } = await noListFilesStack.putAttachment(new Uint8Array([1]), 'image/png'); const result = await noListFilesStack.collectAttachmentGarbage({ graceMs: 0 }); @@ -2648,8 +2743,12 @@ describe('collectAttachmentGarbage', () => { // StackConflictError — the sweep must skip that one file, not abort, and // must keep collecting everything else it already found. test('a file whose delete call races is skipped, not thrown, and the rest of the sweep still completes', async () => { - const racedFileId = await stack.putAttachment(new Uint8Array([1]), 'image/png'); - const okFileId = await stack.putAttachment(new Uint8Array([2, 2]), 'image/png'); + const { + content: { fileId: racedFileId }, + } = await stack.putAttachment(new Uint8Array([1]), 'image/png'); + const { + content: { fileId: okFileId }, + } = await stack.putAttachment(new Uint8Array([2, 2]), 'image/png'); const realDeleteAttachment = stack.deleteAttachment.bind(stack); stack.deleteAttachment = async (fileId: string) => {