From a7a6f3b6cbadcf5a307f3862fe0d8c6385daf5f4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 14:06:08 +0000 Subject: [PATCH 01/12] Add readSource, the stored-bytes read, as a base operation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A realm serves three reads and the operation core modeled one: the card+json document, assembled from the search index. The other two — the `card+source` GET/HEAD that returns a resource's stored text and the raw byte serve that returns a file's bytes — had no operation to dispatch through, so a policy layered on operations would have gated neither. `readSource` is that operation. It resolves without a definition, which is the point rather than an optimization: a module has no `adoptsFrom` and no definition-cache entry, and a `.gts` path takes the file-def branch of `definitionFor`, so gating its source on a cache lookup would refuse bytes that are plainly on disk. Dispatch answers the name before it would reach one, taking the target's kind from the URL — which is where an instance target's kind comes from anyway. A type target has no stored bytes, so it refuses as `operation-not-allowed` without a lookup either, and a FieldDef type refuses for that reason rather than because a field def carries nothing. Nothing may declare one. The authoring decorator refuses `base: 'readSource'` and dispatch refuses a stored definition that carries it, which is what makes skipping the definition safe: no declaration can take the name. The two rules are the same rule read from opposite ends, so they are worth keeping together. The executor answers `{ contentType, lastModified, created, version, body }` and, in headers-only mode, everything but `body` — leaving the adapter's `content` untouched, since it is a lazy getter that opens a real stream on first touch. `version` is the content hash, resolved by the realm from its own file-meta row so both modes report the same one by construction. The redirects, the ETag, the 304 and the source cache stay at the facade, which takes the resolved path. No route dispatches here; every existing handler and suite is untouched. `OperationCore.readSource` is renamed `readFileAsText`, after the realm method it is bound to, so the one name does not mean both the text read and the operation. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt --- packages/base/operations.ts | 31 +- .../host/tests/integration/operations-test.ts | 72 ++++- .../tests/card-operations-core-test.ts | 247 ++++++++++++++- .../tests/card-operations-dispatch-test.ts | 28 ++ .../card-operations/dispatch.ts | 169 ++++++++-- .../runtime-common/card-operations/index.ts | 6 + .../card-operations/read-source.ts | 106 +++++++ .../runtime-common/card-operations/read.ts | 2 +- .../runtime-common/card-operations/types.ts | 54 +++- packages/runtime-common/realm.ts | 59 +++- .../tests/card-operations-dispatch-test.ts | 293 +++++++++++++++++- 11 files changed, 1018 insertions(+), 49 deletions(-) create mode 100644 packages/runtime-common/card-operations/read-source.ts diff --git a/packages/base/operations.ts b/packages/base/operations.ts index cb071ef46fe..681c826f4bf 100644 --- a/packages/base/operations.ts +++ b/packages/base/operations.ts @@ -62,7 +62,7 @@ import { // The behaviors every declaration builds on. Which of them a def carries is // implied by the def type rather than written in author code — a `CardDef` -// has all six, a `FileDef` only `read`, a `FieldDef` none — so +// has all of them, a `FileDef` only the two reads, a `FieldDef` none — so // `getOperations` synthesizes them. A declaration *named* after a base op // takes its place: it specializes that behavior when it names the same // `base`, and rebinds the verb when it names another — a `delete` declared @@ -72,6 +72,7 @@ import { // and neither is inferred from the other. export const BASE_OPERATIONS = [ 'read', + 'readSource', 'create', 'update', 'delete', @@ -81,6 +82,14 @@ export const BASE_OPERATIONS = [ export type BaseOperationName = (typeof BASE_OPERATIONS)[number]; +// The base operations no declaration may name as its `base`. A stored-bytes +// read serves what is on disk: there is no payload to reshape, no program +// stage to run, and no result to project, so a declaration built on it would +// describe work nothing carries out. Refusing at the decorator is what keeps +// the realm's dispatch free to answer it without consulting a definition — +// the two rules are the same rule, read from opposite ends. +const NOT_DECLARABLE: readonly BaseOperationName[] = ['readSource']; + // ============================================================================ // Typed references // @@ -420,6 +429,10 @@ const CLAUSE_KEYS: Record = { update: [], delete: [], read: [], + // A stored-bytes read takes no clauses because it takes no declaration at + // all; the entry is here because this table is exhaustive over the base + // operations, so a new one has to say what it accepts. + readSource: [], query: ['query'], }; @@ -547,14 +560,19 @@ function impliedOperations( } if (isSubclassOf(owner, FileDef)) { // A file's metadata is content-derived and read-only: there is no - // JSON:API mutation surface for anything else to reach. + // JSON:API mutation surface for anything else to reach. Its bytes are the + // representation that matters, so it carries the stored-bytes read too. return READ_ONLY; } - // The one operation every addressable def shares. + // The operations every addressable def shares. return READ_ONLY; } -const READ_ONLY = ['read'] as const; +// The two reads, neither of which writes. A `read` serves the def's indexed +// document; a `readSource` serves the bytes stored at the instance's URL, a +// representation every addressable def has whether or not its document is the +// interesting one — for a file it is the bytes that are the point. +const READ_ONLY = ['read', 'readSource'] as const; function declaredOperations( owner: typeof BaseDef, @@ -694,6 +712,11 @@ function assertValidDeclaration( `${label}: \`base\` must name the built-in behavior this operation builds on — one of ${quoteList(BASE_OPERATIONS)}`, ); } + if (NOT_DECLARABLE.includes(base)) { + throw new Error( + `${label}: a "${base}" operation serves the bytes stored at the def's URL, so there is nothing for a declaration to specialize or rebind`, + ); + } // An author may only specialize a base operation the def type actually // carries. Read from the same list `getOperations` synthesizes: only a card // has a mutation surface, and a file's metadata is content-derived and diff --git a/packages/host/tests/integration/operations-test.ts b/packages/host/tests/integration/operations-test.ts index f2c17cb1ef7..eae8cbe7743 100644 --- a/packages/host/tests/integration/operations-test.ts +++ b/packages/host/tests/integration/operations-test.ts @@ -173,13 +173,14 @@ module('Integration | operations', function (hooks) { getOperations(CardDef), { read: { base: 'read' }, + readSource: { base: 'readSource' }, create: { base: 'create' }, update: { base: 'update' }, delete: { base: 'delete' }, query: { base: 'query' }, transform: { base: 'transform' }, }, - 'a card def carries all six, implied by the def type', + 'a card def carries every base operation, implied by the def type', ); assert.deepEqual( Object.keys(getDeclaredOperations(CardDef)), @@ -188,8 +189,8 @@ module('Integration | operations', function (hooks) { ); assert.deepEqual( getOperations(FileDef), - { read: { base: 'read' } }, - "a file's metadata is read-only, so a file def carries only read", + { read: { base: 'read' }, readSource: { base: 'readSource' } }, + "a file's metadata is read-only, so a file def carries only its two reads", ); assert.deepEqual( getOperations(FieldDef), @@ -211,7 +212,15 @@ module('Integration | operations', function (hooks) { ); assert.deepEqual( Object.keys(getOperations(Report)).sort(), - ['create', 'delete', 'query', 'read', 'transform', 'update'], + [ + 'create', + 'delete', + 'query', + 'read', + 'readSource', + 'transform', + 'update', + ], 'and adds no name, because it is that base operation', ); @@ -230,7 +239,15 @@ module('Integration | operations', function (hooks) { ); assert.deepEqual( Object.keys(getOperations(Archivable)).sort(), - ['create', 'delete', 'query', 'read', 'transform', 'update'], + [ + 'create', + 'delete', + 'query', + 'read', + 'readSource', + 'transform', + 'update', + ], 'which stands in for the removal rather than beside it', ); }); @@ -390,7 +407,7 @@ module('Integration | operations', function (hooks) { ); }); - test('a file definition can only declare read operations', function (assert) { + test('a file definition can only declare document reads', function (assert) { class Attachment extends FileDef { @operation static readRedacted = { base: 'read', output: { name: true } }; } @@ -409,11 +426,44 @@ module('Integration | operations', function (hooks) { } return Mutable; }, - /carries only "read"/, + /carries only "read", "readSource"/, 'file metadata is content-derived, so it has no mutation surface', ); }); + test('a stored-bytes read takes no declaration at all', function (assert) { + // The other half of the realm's definition-free dispatch: it answers a + // `readSource` without consulting a definition, which is only safe while + // no declaration can take that name. Refusing here is what makes it so. + for (let Def of [CardDef, FileDef]) { + assert.throws( + () => { + class Exported extends (Def as typeof CardDef) { + @operation static exportBytes = { base: 'readSource' }; + } + return Exported; + }, + /serves the bytes stored at the def's URL/, + `a ${Def.name} cannot build an operation on a stored-bytes read`, + ); + } + assert.throws( + () => { + class Redacted extends CardDef { + // Not even under its own name: specializing it is the same ask as + // rebinding a verb onto it, since there is no stage to specialize. + @operation static readSource = { + base: 'readSource', + output: { redacted: true }, + }; + } + return Redacted; + }, + /serves the bytes stored at the def's URL/, + 'and it cannot be specialized under its own name either', + ); + }); + test('the decorator rejects an operation name that is already a static', function (assert) { assert.throws( () => { @@ -1378,12 +1428,12 @@ module('Integration | operations', function (hooks) { } }); - test('a def with no mutation surface carries only read', function (assert) { + test('a def with no mutation surface carries only its reads', function (assert) { class Bare extends cardAPI.BaseDef {} assert.deepEqual( getOperations(Bare), - { read: { base: 'read' } }, - 'read is the one operation every addressable def shares', + { read: { base: 'read' }, readSource: { base: 'readSource' } }, + 'the two reads are what every addressable def shares', ); assert.throws( () => { @@ -1395,7 +1445,7 @@ module('Integration | operations', function (hooks) { } return Mutable; }, - /carries only "read"/, + /carries only "read", "readSource"/, 'and a def that carries no mutation base cannot declare one', ); }); diff --git a/packages/realm-server/tests/card-operations-core-test.ts b/packages/realm-server/tests/card-operations-core-test.ts index 25d5ea0cd30..6b77b695157 100644 --- a/packages/realm-server/tests/card-operations-core-test.ts +++ b/packages/realm-server/tests/card-operations-core-test.ts @@ -4,12 +4,18 @@ import { basename } from 'path'; import type { Test, SuperTest } from 'supertest'; import type { RealmHttpServer as Server } from '../server.ts'; import type { DirResult } from 'tmp'; -import type { Realm } from '@cardstack/runtime-common'; -import { SupportedMimeType, baseRealm, rri } from '@cardstack/runtime-common'; +import type { CodeRef, Realm } from '@cardstack/runtime-common'; +import { + SupportedMimeType, + baseRealm, + computeContentHash, + rri, +} from '@cardstack/runtime-common'; import { isDocumentResult, isHeadResult, isOperationFailure, + isSourceResult, lowerQueryOperation, runOperation, type OperationCore, @@ -17,6 +23,8 @@ import { type OperationHeadResult, type OperationRequest, type OperationResult, + type OperationSourceBody, + type OperationSourceResult, type OperationTarget, } from '@cardstack/runtime-common/card-operations'; import { @@ -31,6 +39,12 @@ import type { PgAdapter } from '@cardstack/postgres'; // The operation core, exercised the way the HTTP surfaces will once they route // to it: obtain the core from a realm and call `runOperation` directly. Nothing // is routed here yet, so these are the only callers. +// +// A `read` is held against what the card+json GET serves and a `readSource` +// against what the `card+source` GET serves, because those are the surfaces +// each will delegate to. Where a result is checked against a handler's +// response rather than against a literal, that is deliberate: the parity is +// the requirement, and a literal would let the two drift together. function request( target: OperationTarget, @@ -64,6 +78,49 @@ function headersOf(result: OperationResult): OperationHeadResult { return result; } +function sourceOf(result: OperationResult): OperationSourceResult { + if (!isSourceResult(result)) { + throw new Error(`expected a source result, got ${JSON.stringify(result)}`); + } + return result; +} + +// A stored-bytes read hands back whatever form the realm's file adapter +// produced — under Node that is an unread stream — so a test comparing content +// has to materialize it. Doing so here rather than in the executor is the +// point: a facade puts the body on a response without ever reading it. +async function bytesOf( + body: OperationSourceBody | undefined, +): Promise { + if (body === undefined) { + throw new Error('expected a body'); + } + if (typeof body === 'string') { + return new TextEncoder().encode(body); + } + if (body instanceof Uint8Array) { + return body; + } + let chunks: Uint8Array[] = []; + for await (let chunk of body as AsyncIterable) { + chunks.push( + typeof chunk === 'string' ? new TextEncoder().encode(chunk) : chunk, + ); + } + let total = chunks.reduce((sum, chunk) => sum + chunk.byteLength, 0); + let bytes = new Uint8Array(total); + let offset = 0; + for (let chunk of chunks) { + bytes.set(chunk, offset); + offset += chunk.byteLength; + } + return bytes; +} + +async function textOf(body: OperationSourceBody | undefined): Promise { + return new TextDecoder().decode(await bytesOf(body)); +} + // The failure a call is expected to produce. Rethrows anything that is not an // operation failure so a genuine bug reports itself rather than being read as // the refusal under test. @@ -288,6 +345,192 @@ module(basename(import.meta.filename), function () { assert.strictEqual(error.status, 404); }); + test('a stored-bytes read serves a card instance source verbatim', async function (assert) { + // The `.json` spelling names the card's stored bytes, which is why + // dispatch leaves it as written rather than resolving it to the card. + let result = await runOperation( + testRealm.operationCore, + request( + { kind: 'instance', url: `${testRealmHref}person-1.json` }, + 'readSource', + ), + ); + let source = sourceOf(result); + assert.strictEqual( + source.contentType, + 'application/json', + 'the content type is inferred from the path, as both byte routes infer it', + ); + + // Against the `card+source` GET rather than the card+json one: that is + // the surface this read will serve, so it is the one it has to + // reproduce. + let response = await realmRequest + .get('/person-1.json') + .set('Accept', SupportedMimeType.CardSource); + assert.strictEqual( + response.status, + 200, + `HTTP 200 status: ${response.text}`, + ); + assert.strictEqual( + await textOf(source.body), + response.text, + 'the bytes are the ones the source route serves', + ); + assert.true( + response.headers['etag'].startsWith(source.version!), + `the source ETag is built from the version the read reports: ${response.headers['etag']} / ${source.version}`, + ); + }); + + test('a stored-bytes read of a module serves its text', async function (assert) { + // A module has no `adoptsFrom` and no definition-cache entry, so this is + // the target the definition-free dispatch exists for. Its extension is + // a registered one, which is what would otherwise send it through a + // file def's type and a cache lookup on it. + let result = await runOperation( + testRealm.operationCore, + request( + { kind: 'instance', url: `${testRealmHref}person.gts` }, + 'readSource', + ), + ); + let source = sourceOf(result); + assert.strictEqual(source.contentType, 'text/typescript+glimmer'); + + let response = await realmRequest + .get('/person.gts') + .set('Accept', SupportedMimeType.CardSource); + assert.strictEqual( + response.status, + 200, + `HTTP 200 status: ${response.text}`, + ); + assert.strictEqual(await textOf(source.body), response.text); + }); + + test('a stored-bytes read of an image serves its bytes and its content type', async function (assert) { + // Written through the realm rather than taken from the fixture, for two + // reasons: the fixture holds no binary file, and a realm write is what + // persists the content-meta row, so this is also the case where + // `version` is the hash the realm recorded rather than one computed from + // the bytes on read. + let bytes = new Uint8Array([ + 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0xff, + ]); + await testRealm.write('logo.png', bytes); + + let result = await runOperation( + testRealm.operationCore, + request( + { kind: 'instance', url: `${testRealmHref}logo.png` }, + 'readSource', + ), + ); + let source = sourceOf(result); + assert.strictEqual(source.contentType, 'image/png'); + assert.deepEqual( + Array.from(await bytesOf(source.body)), + Array.from(bytes), + 'the bytes come back undecoded', + ); + assert.strictEqual( + source.version, + computeContentHash(bytes), + 'and `version` is the content hash of the bytes the realm stored', + ); + assert.strictEqual( + typeof source.created, + 'number', + 'a realm write records when the realm first saw the path', + ); + }); + + test('a headers-only stored-bytes read reports the same metadata with no body', async function (assert) { + let target: OperationTarget = { + kind: 'instance', + url: `${testRealmHref}sample.md`, + }; + let headers = sourceOf( + await runOperation( + testRealm.operationCore, + request(target, 'readSource'), + { headersOnly: true }, + ), + ); + assert.strictEqual(headers.body, undefined, 'no bytes in this mode'); + assert.strictEqual(headers.contentType, 'text/markdown'); + assert.strictEqual(typeof headers.version, 'string'); + + let withBody = sourceOf( + await runOperation( + testRealm.operationCore, + request(target, 'readSource'), + ), + ); + let { body: _body, ...metadata } = withBody; + assert.deepEqual( + metadata, + headers, + 'the two modes agree on every value a response header is computed from', + ); + }); + + test('a stored-bytes read of a path with no bytes is not found', async function (assert) { + for (let path of ['does-not-exist', 'dir', '_search']) { + let error = await refusalFrom(() => + runOperation( + testRealm.operationCore, + request( + { kind: 'instance', url: `${testRealmHref}${path}` }, + 'readSource', + ), + ), + ); + assert.strictEqual( + error.code, + 'target-not-found', + `${path} has no bytes to read: ${error.detail}`, + ); + assert.strictEqual(error.status, 404); + } + }); + + test('a type target carries no stored-bytes read', async function (assert) { + // A type has no stored bytes, and the refusal does not depend on which + // kind of def the type turns out to be: a `FieldDef` refuses for the + // same reason a `CardDef` does, rather than because a field def carries + // nothing. + let types: { label: string; codeRef: CodeRef }[] = [ + { + label: 'a card def', + codeRef: { module: rri(`${testRealmHref}person`), name: 'Person' }, + }, + { + label: 'a field def', + codeRef: { module: rri(`${baseRealm.url}string`), name: 'default' }, + }, + ]; + for (let { label, codeRef } of types) { + let error = await refusalFrom(() => + runOperation( + testRealm.operationCore, + request( + { kind: 'type', codeRef, realm: testRealmHref }, + 'readSource', + ), + ), + ); + assert.strictEqual( + error.code, + 'operation-not-allowed', + `${label} type target is refused: ${error.detail}`, + ); + assert.strictEqual(error.status, 405); + } + }); + test('a target outside the realm is not this core to serve', async function (assert) { let error = await refusalFrom(() => runOperation( diff --git a/packages/realm-server/tests/card-operations-dispatch-test.ts b/packages/realm-server/tests/card-operations-dispatch-test.ts index 37f9d8430a1..b589013d01e 100644 --- a/packages/realm-server/tests/card-operations-dispatch-test.ts +++ b/packages/realm-server/tests/card-operations-dispatch-test.ts @@ -102,6 +102,34 @@ module(basename(import.meta.filename), function () { await runSharedTest(cardOperationsDispatchTests, assert, {}); }); + test('a stored-bytes read serves the bytes and infers their content type', async function (assert) { + await runSharedTest(cardOperationsDispatchTests, assert, {}); + }); + + test('a stored-bytes read consults neither a definition nor the index', async function (assert) { + await runSharedTest(cardOperationsDispatchTests, assert, {}); + }); + + test('the headers-only mode reports the metadata without touching the bytes', async function (assert) { + await runSharedTest(cardOperationsDispatchTests, assert, {}); + }); + + test('a stored-bytes read reports a version the realm never recorded as absent', async function (assert) { + await runSharedTest(cardOperationsDispatchTests, assert, {}); + }); + + test('a path with no stored bytes is not found, never not-indexed', async function (assert) { + await runSharedTest(cardOperationsDispatchTests, assert, {}); + }); + + test('a type has no stored bytes to read', async function (assert) { + await runSharedTest(cardOperationsDispatchTests, assert, {}); + }); + + test('a declaration may not build on a stored-bytes read', async function (assert) { + await runSharedTest(cardOperationsDispatchTests, assert, {}); + }); + test('the row peek is memoized for one invocation and no longer', async function (assert) { await runSharedTest(cardOperationsDispatchTests, assert, {}); }); diff --git a/packages/runtime-common/card-operations/dispatch.ts b/packages/runtime-common/card-operations/dispatch.ts index 074bb23d117..f34d80cc3f1 100644 --- a/packages/runtime-common/card-operations/dispatch.ts +++ b/packages/runtime-common/card-operations/dispatch.ts @@ -1,12 +1,14 @@ import { RealmPaths, type LocalPath } from '../paths.ts'; import { urlNamesFile } from '../file-def-code-ref.ts'; import { readOperation } from './read.ts'; +import { readSourceOperation } from './read-source.ts'; import { OperationFailure, type BaseOperation, type OperationDefinition, type OperationResult, type OperationRequest, + type OperationSourceBody, type OperationTarget, } from './types.ts'; import type { CodeRef, ResolvedCodeRef } from '../code-ref.ts'; @@ -46,10 +48,34 @@ export interface OperationCore { realmURL: string; definitionLookup: OperationDefinitionLookup; indexQueryEngine: OperationIndexQueryEngine; - // The target's source file as stored on disk, by local path. The index is - // what a read serves, so this is how a behavior distinguishes "no such card" - // from "the write landed and the index row is still coming". - readSource(localPath: LocalPath): Promise; + // The target's source file as stored on disk, decoded as text, by local + // path. The index is what a read serves, so this is how a behavior + // distinguishes "no such card" from "the write landed and the index row is + // still coming". + readFileAsText(localPath: LocalPath): Promise; + // The stored bytes at a local path, opened the way the realm's own byte + // serve opens them, with the realm's own refusals applied: a path it will + // not serve — an empty one, an `_`-prefixed realm endpoint, a directory — + // answers undefined, and so does one that is not there. Unlike + // `fileMetaDocument` this does not decline a card's `.json`: that path names + // the card's stored source, which is exactly what a stored-bytes read is + // for. + // + // `content` is unread until it is touched. The adapter opens a real stream + // on first touch, so a caller that wants only the metadata must leave it + // alone rather than open and strand one. + openStoredFile( + localPath: LocalPath, + ): Promise; + // The write-time record behind a path's bytes: the content hash a + // stored-bytes read reports as `version`, and when the realm first saw the + // path. Both come from the realm's own file-meta row, and resolving the hash + // is the realm's job rather than the executor's for two reasons — the row is + // authoritative for it (it is written in the same critical section as the + // bytes), and where the row carries none the fallback is to hash the bytes, + // which only the side that can open a second handle on them can do without + // consuming the one a read is about to serve. + storedFileMeta(localPath: LocalPath): Promise; // Whether the realm's ignore rules exclude this URL. An ignored path is // never visited, so no amount of waiting produces an index row for it. isIgnored(url: URL): Promise; @@ -84,6 +110,29 @@ export interface OperationCore { }): void; } +// The realm's own `FileRef`, narrowed to what a stored-bytes read uses. Stated +// here rather than imported so the core does not depend on the realm module it +// is a collaborator of. +export interface OperationStoredFile { + path: LocalPath; + // A lazy getter on every adapter that can stream: reading it opens the + // stream, so the headers-only mode leaves it untouched. Typed as the body a + // stored-bytes read answers with, since it is handed through unchanged. + content: OperationSourceBody; + lastModified: number; + // The byte size where the adapter knows it from the stat it already + // performed, and absent where knowing it would cost reading the bytes. + size?: number; +} + +export interface OperationStoredFileMeta { + // The content hash of the stored bytes, absent where the realm can neither + // recall nor compute one. + version?: string; + // Epoch seconds, absent where the realm holds no record of this path. + createdAt?: number; +} + // `CachingDefinitionLookup`, narrowed to the one read an operation makes. export interface OperationDefinitionLookup { lookupDefinition(codeRef: ResolvedCodeRef): Promise; @@ -103,8 +152,10 @@ export interface OperationIndexQueryEngine { } export interface RunOperationOptions { - // A `read` that needs only the values the card+json response headers are - // computed from, and no document. Ignored by every other behavior. + // The metadata a response's headers are computed from, and no body: for a + // `read` the four values the card+json headers rest on and no document, for + // a `readSource` everything but the bytes. What a `HEAD`, or a conditional + // `GET` deciding on a validator, asks for. Ignored by every other behavior. headersOnly?: true; // Leave a query-backed field unexpanded. A render inside a prerender request // must not recurse back into the search that would resolve one, so a read @@ -155,18 +206,20 @@ export function newOperationScope(core: OperationCore): OperationScope { // from a UUID — which is the same assumption the index query engine's own // file/instance split rests on. The one path that does carry one is a card's // `.json` source spelling, and that is classified as a file deliberately: it -// names the card's stored bytes rather than the card. +// names the card's stored bytes rather than the card, and a `readSource` of it +// is what serves them. // // A `type` target has only its ref, so its kind comes from the entry, and a // file def named that way therefore carries nothing. That is correct rather -// than a gap: operations belong to cards, and the one operation a file has is -// `read`, which needs an instance to read. +// than a gap: operations belong to cards, and what a file has are its two +// reads, both of which need an instance to read. type DefKind = 'card-def' | 'file-def' | 'field-def'; -// Exhaustive over `BaseOperation` on purpose: a seventh built-in behavior has +// Exhaustive over `BaseOperation` on purpose: a further built-in behavior has // to say here whether a card carries it, rather than defaulting to "no". const CARD_DEF_OPERATIONS: Readonly> = { read: true, + readSource: true, create: true, update: true, delete: true, @@ -179,17 +232,44 @@ const ALLOWED_BASE_OPERATIONS: Readonly< > = { 'card-def': CARD_DEF_OPERATIONS, // A file's metadata is derived from its bytes and read-only: there is no - // JSON:API mutation surface for anything else to reach. - 'file-def': { read: true }, + // JSON:API mutation surface for anything else to reach. Its bytes are the + // representation that matters for a file, though, so it carries the + // stored-bytes read alongside the document one. + 'file-def': { read: true, readSource: true }, // A field's instances have no URL, so nothing is invocable on one. Field // data is reached through the operations of the card that contains it. 'field-def': {}, }; +// The base operations that resolve without consulting a definition. +// +// A definition is consulted for two reasons — to find a declaration of the +// requested name, and to learn a type target's def kind — and a stored-bytes +// read needs neither. Nothing may declare one (the authoring decorator refuses +// it, and so does the declared branch below), so no declaration can take the +// name; and a type has no stored bytes, so there is no type-target form whose +// kind would have to be resolved. +// +// That is what makes skipping the lookup correct rather than merely cheaper. A +// module path is the case that needs it: `.gts` is a registered extension, so +// resolving a definition for one means a cache lookup on the file def its +// extension names, and a path whose type nothing can resolve would then refuse +// a read of bytes that are plainly on disk. Definition-free means +// definition-free. +const DEFINITION_FREE_OPERATIONS: Readonly< + Partial> +> = { + readSource: true, +}; + function isBaseOperation(name: string): name is BaseOperation { return own(CARD_DEF_OPERATIONS, name) !== undefined; } +function isDefinitionFreeOperation(name: string): name is BaseOperation { + return own(DEFINITION_FREE_OPERATIONS, name) !== undefined; +} + // Read a record by a key that arrived over the wire. Every name-keyed lookup // on this path goes through here: a plain object answers `toString` and // `constructor` with something that is not an operation, and reading one of @@ -223,6 +303,18 @@ export async function resolveOperation( scope: OperationScope = newOperationScope(core), ): Promise { assertInRealm(core, target); + if (isDefinitionFreeOperation(name)) { + // Before the lookup, not merely without it: for a module path the lookup + // cannot succeed, and reaching it at all would gate a read of bytes on a + // definition. The kind still decides whether the target carries the + // operation, so it comes from the target alone — which is where an + // instance target's kind comes from anyway. + let kind = definitionFreeKind(target); + if (!kind || !own(ALLOWED_BASE_OPERATIONS[kind], name)) { + throw notAllowed(target, name, kind, name); + } + return { base: name, deterministic: true }; + } let definition = await definitionFor(core, target, scope); if (target.kind === 'type' && !definition) { // Nothing else can be said about a type nobody can resolve: whether it @@ -250,6 +342,26 @@ export async function resolveOperation( meta: { issues: declared.issues ?? [] }, }); } + if (own(DEFINITION_FREE_OPERATIONS, declared.base)) { + // A declaration wins over the built-in of the same name, which is what + // lets an author specialize `read` or rebind `delete` onto `transform`. + // A definition-free behavior is the one thing that cannot be won that + // way: it serves the bytes on disk, so there is no payload to reshape + // and no stage to run, and dispatching a declared name to it would + // answer under the author's name without doing what the author wrote. + // The authoring decorator refuses such a declaration; this refuses one + // that reached a stored definition regardless. + throw new OperationFailure({ + id: targetId(target), + status: 405, + code: 'operation-not-allowed', + title: 'Operation not allowed', + detail: + `operation "${name}" is declared on "${declared.base}", which is ` + + `not a behavior a declaration may build on: a "${declared.base}" ` + + `serves the bytes stored at the target's URL`, + }); + } if (!own(ALLOWED_BASE_OPERATIONS[kind], declared.base)) { throw notAllowed(target, name, kind, declared.base); } @@ -287,6 +399,11 @@ export async function runOperation( switch (definition.base) { case 'read': return await readOperation(core, canonical, definition, opts, scope); + case 'readSource': + // No definition and no scope: the operation is resolved without either, + // and an executor that peeked a row would put back the index read + // resolving it definition-free just took out. + return await readSourceOperation(core, canonical, opts); case 'query': // A declared query is a saved search, not work the realm carries out // here: an invocation resolves its markers with `lowerQueryOperation` @@ -403,11 +520,10 @@ export function canonicalizeTarget( } // A `.json` path is deliberately left alone. It names a card's stored source // rather than the card, and the stored bytes of an instance are a different - // read from the instance itself — one this core does not serve yet. So the - // extension stands and the target routes as a file, which is where that read - // will live. Resolving it to the card instead would answer a question nobody - // asked, and would be the one spelling where the extension test and the rest - // of the core disagreed. + // read from the instance itself — the `readSource` this spelling routes to. + // Resolving it to the card instead would answer a question nobody asked, and + // would be the one spelling where the extension test and the rest of the + // core disagreed. let canonical = paths.fileURL(localPath).href; return canonical === target.url ? target @@ -535,6 +651,18 @@ async function adoptsFromOf( return row.instance.meta?.adoptsFrom; } +// The target's kind as far as the target itself can say, for a behavior that +// will not read a definition to find out. An instance target's kind never +// needed one — it comes from the URL — so the only answer lost is a type +// target's, which is `undefined` here rather than guessed. That is the whole +// answer for a stored-bytes read: a type has no stored bytes, so it carries no +// operation that serves them, and saying so does not depend on which kind of +// def the type turns out to be. A `FieldDef` type refuses for that reason and +// not because a field def carries nothing. +function definitionFreeKind(target: OperationTarget): DefKind | undefined { + return target.kind === 'instance' ? defKindFor(target, undefined) : undefined; +} + function defKindFor( target: OperationTarget, definition: Definition | undefined, @@ -564,14 +692,17 @@ function parseTargetURL(url: string): URL | undefined { function notAllowed( target: OperationTarget, name: string, - kind: DefKind, + // Absent only for a type target reached by a behavior that reads no + // definition, and unread in that case: the message below takes a type + // target's own terms instead. + kind: DefKind | undefined, base: BaseOperation, ): OperationFailure { // A type target's kind is inferred from an entry that cannot say `file-def`, // so naming the kind there would report a file def as a field def. The // target's own terms are accurate either way. let because = - target.kind === 'instance' + target.kind === 'instance' && kind ? `a ${kind} allows ${describeAllowed(kind)}` : `a type carries only what its definition declares, and a "${base}" ` + `runs against an instance`; diff --git a/packages/runtime-common/card-operations/index.ts b/packages/runtime-common/card-operations/index.ts index 703ac48599b..e1ccbded150 100644 --- a/packages/runtime-common/card-operations/index.ts +++ b/packages/runtime-common/card-operations/index.ts @@ -14,9 +14,12 @@ export type { OperationDefinitionLookup, OperationIndexQueryEngine, OperationScope, + OperationStoredFile, + OperationStoredFileMeta, RunOperationOptions, } from './dispatch.ts'; export { readOperation } from './read.ts'; +export { readSourceOperation } from './read-source.ts'; export { lowerQueryOperation } from './query.ts'; export type { QueryInvocation } from './query.ts'; export { @@ -25,6 +28,7 @@ export { isHeadResult, isIdentityResult, isOperationFailure, + isSourceResult, } from './types.ts'; export type { BaseOperation, @@ -41,6 +45,8 @@ export type { OperationProgram, OperationRequest, OperationResult, + OperationSourceBody, + OperationSourceResult, OperationTarget, OperationTemplate, } from './types.ts'; diff --git a/packages/runtime-common/card-operations/read-source.ts b/packages/runtime-common/card-operations/read-source.ts new file mode 100644 index 00000000000..6d48bad8b23 --- /dev/null +++ b/packages/runtime-common/card-operations/read-source.ts @@ -0,0 +1,106 @@ +import { inferContentType } from '../infer-content-type.ts'; +import { + canonicalizeTarget, + instanceTargetURL, + localPathFor, +} from './dispatch.ts'; +import { + OperationFailure, + type OperationRequest, + type OperationSourceResult, +} from './types.ts'; +import type { OperationCore, RunOperationOptions } from './dispatch.ts'; + +// ============================================================================ +// The `readSource` executor. +// +// A stored-bytes read serves a resource exactly as it sits on disk: a card +// instance's `.json`, a `.gts` or `.ts` module's text, an image's or a PDF's +// bytes. It is the third of the realm's three reads — the document one is +// `read`, assembled from the search index; this one is the representation the +// realm's two byte routes serve, the `card+source` `GET`/`HEAD` and the raw +// byte serve. +// +// Two things separate it from `read`, and both come from what it serves: +// +// * It consults no definition. `read` needs a type's definition to assemble +// a document; bytes need only a path. A module has no `adoptsFrom` and no +// definition-cache entry, so gating its source on a lookup would refuse a +// read of bytes that are plainly there. Dispatch resolves the name before +// it would reach one, and this executor asks for none. +// +// * It touches the index not at all. There is no row to peek and no +// generation to join on, so a `readSource` costs one file open and one +// file-meta row — never a search read. That is also why it can answer for +// a path the index has no row for, and for one it never will. +// +// Two modes, as `read` has: +// +// * bytes — the metadata plus `body`. +// * headers — the same metadata with no `body`, for a `HEAD` or for a +// conditional `GET` deciding on a validator. It leaves the +// adapter's `content` untouched rather than reading and +// discarding it, which matters concretely: `content` is a lazy +// getter that opens a real stream on first touch, so touching it +// to throw it away would strand one. +// +// What stays outside, and what a facade routing here has to keep: +// +// * The redirects. An extension-less URL naming `foo.gts`, or a card id +// naming its `.json`, is resolved by the facade — this executor takes the +// resolved path and reads it. A read has no redirect to give. +// * The response around the bytes. The `ETag` built from `version` and its +// source variant, `Last-Modified`, `x-created`, the 304, `Range` and the +// source cache are all the facade's, computed from what comes back here. +// * A batching envelope over operations does not carry this one at all: +// bytes do not belong in a JSON batch, and a stream cannot be one member +// of one. +// ============================================================================ + +export async function readSourceOperation( + core: OperationCore, + request: OperationRequest, + opts: RunOperationOptions = {}, +): Promise { + // `runOperation` canonicalized the target already; doing it again is a no-op + // and keeps a direct caller of this executor addressing the same path + // dispatch would have. + let target = canonicalizeTarget(core, request.target); + let url = instanceTargetURL({ ...request, target }); + let localPath = localPathFor(core, url); + let file = await core.openStoredFile(localPath); + if (!file) { + // One refusal for every way there is nothing to read: no such path, a + // directory, or a path the realm declines to serve at all. The realm + // applies its own refusals inside `openStoredFile`, so a `_`-prefixed + // realm endpoint lands here the same way a missing file does, which is + // what the byte routes answer for one. + // + // Never `target-not-indexed`. That code says waiting will resolve the + // absence, and it is the index it is waiting for; the bytes either exist + // or they do not, and a read of them has nothing to wait for. + throw new OperationFailure({ + id: url.href, + status: 404, + code: 'target-not-found', + title: 'Not found', + detail: `${url.href} does not exist in realm ${core.realmURL}`, + }); + } + // `file.path` rather than the requested path: they are the same here, since + // the facade resolved any fallback before dispatching, but the content type + // has to describe the bytes that actually arrived rather than the name they + // were asked for. + let contentType = inferContentType(file.path); + let meta = await core.storedFileMeta(localPath); + let result: OperationSourceResult = { + contentType, + lastModified: file.lastModified, + created: meta.createdAt ?? null, + version: meta.version ?? null, + }; + if (opts.headersOnly) { + return result; + } + return { ...result, body: file.content }; +} diff --git a/packages/runtime-common/card-operations/read.ts b/packages/runtime-common/card-operations/read.ts index 22684ecaf70..0596fb2eecb 100644 --- a/packages/runtime-common/card-operations/read.ts +++ b/packages/runtime-common/card-operations/read.ts @@ -309,7 +309,7 @@ async function missingTarget( if (await core.isIgnored(pathsFor(core).fileURL(sourcePath))) { return notFound; } - let source = await core.readSource(sourcePath); + let source = await core.readFileAsText(sourcePath); if (source === undefined) { return notFound; } diff --git a/packages/runtime-common/card-operations/types.ts b/packages/runtime-common/card-operations/types.ts index a004319463b..eea93e40bce 100644 --- a/packages/runtime-common/card-operations/types.ts +++ b/packages/runtime-common/card-operations/types.ts @@ -1,3 +1,4 @@ +import type { Readable } from 'stream'; import type { CodeRef } from '../code-ref.ts'; import type { ScreenshotManifest } from '../capture-spec.ts'; import type { @@ -201,7 +202,7 @@ export interface LowerOperationDeclarationsResult { // the payload, which are known only at invocation. // ============================================================================ -// The six built-in behaviors, under the operation runtime's own name. The +// The built-in behaviors, under the operation runtime's own name. The // authoring API owns the list because that is where a declaration names one; // re-stating it here lets a consumer of the runtime types stay clear of the // card authoring surface, which only loads inside a card module. @@ -224,9 +225,9 @@ export type OperationTarget = export interface OperationRequest { target: OperationTarget; - // The name the operation is invoked under — a declared name, or one of the - // six base names for the built-in behavior. Never `base`: a `delete` built - // on `transform` is invoked as `delete`. + // The name the operation is invoked under — a declared name, or a base name + // for the built-in behavior. Never `base`: a `delete` built on `transform` + // is invoked as `delete`. name: string; // The payload, keyed the way the definition's `params` schema declares it. params?: Record; @@ -266,6 +267,41 @@ export interface OperationHeadResult { deps: string[] | null; } +// The stored bytes of a resource, and what the byte-serve headers are computed +// from. This is what the source and byte-serve routes answer with: a card +// instance's `.json`, a module's text, an image's bytes — the resource exactly +// as it sits on disk, with no assembly and no index read behind it. +export interface OperationSourceResult { + // Inferred from the path's extension by `inferContentType`, which is what + // both byte routes infer theirs with. A path with no extension the platform + // knows resolves to `application/octet-stream`, the byte-preserving + // default. + contentType: string; + lastModified: number; + // When the realm first saw this path, in epoch seconds, and null where it + // holds no record of it — a file written outside the realm's own write path, + // say. The byte serve omits `x-created` in that case rather than + // substituting the modification time, so this reports the absence rather + // than filling it in. + created: number | null; + // The content hash of the stored bytes — the same identity the rest of the + // project calls `version`, and what the source route's `ETag` is built + // from. Null only where the realm can neither recall nor compute one. + version: string | null; + // The bytes. Absent in the headers-only mode, which is the whole difference + // between the two: a `HEAD` reports the metadata above and would discard + // this. Whatever form the realm's file adapter produced — a string, a byte + // array, or an unread stream — so a caller hands it to a response body + // rather than materializing it. + body?: OperationSourceBody; +} + +export type OperationSourceBody = + | string + | Uint8Array + | ReadableStream + | Readable; + // A write's answer: the identity of what was written and the version it now // holds, without reprinting the document. A caller that wants the new state // reads it; a caller that wrote it already has it, and the common case is a @@ -290,6 +326,7 @@ export type OperationResult = | OperationDocumentResult | OperationHeadResult | OperationIdentityResult + | OperationSourceResult | null; export function isDocumentResult( @@ -310,6 +347,15 @@ export function isIdentityResult( return result != null && 'id' in result; } +// Keyed on `contentType` rather than on `body`, which the headers-only mode +// leaves out: a guard that read the body would report false for exactly the +// result a `HEAD` asks for. +export function isSourceResult( + result: OperationResult, +): result is OperationSourceResult { + return result != null && 'contentType' in result; +} + export type OperationErrorCode = // No operation of that name, and the name is not a base operation the // target's def type carries. diff --git a/packages/runtime-common/realm.ts b/packages/runtime-common/realm.ts index 9c2b0792721..483bd3e9bf7 100644 --- a/packages/runtime-common/realm.ts +++ b/packages/runtime-common/realm.ts @@ -143,7 +143,10 @@ import { type LooseCardResource, type FileMetaResource, } from './index.ts'; -import type { OperationCore } from './card-operations/dispatch.ts'; +import type { + OperationCore, + OperationStoredFileMeta, +} from './card-operations/dispatch.ts'; import type { FromScratchResult } from './tasks/indexer.ts'; import { isCodeRef, visitModuleDeps } from './code-ref.ts'; import { merge } from 'lodash-es'; @@ -3172,8 +3175,10 @@ export class Realm { realmURL: this.url, definitionLookup: this.#definitionLookup, indexQueryEngine: this.#realmIndexQueryEngine, - readSource: async (localPath) => + readFileAsText: async (localPath) => (await this.readFileAsText(localPath))?.content, + openStoredFile: (localPath) => this.#operationStoredFile(localPath), + storedFileMeta: (localPath) => this.#operationStoredFileMeta(localPath), isIgnored: (url) => this.isIgnored(url), fileMetaDocument: (localPath) => this.#operationFileMetaDocument(localPath), @@ -5522,6 +5527,25 @@ export class Realm { return this.#adapter.openFile(localPath); } + // The stored bytes at a local path, as the operation core opens them: + // `openFileForMetadata`'s guard above minus its `.json` refusal. That + // refusal is right for metadata — a `.json` is a card's source, not a file + // with metadata of its own — and wrong here, because a card's stored source + // is exactly what a stored-bytes read reads. The two it keeps are the byte + // routes' own: an empty path names the realm root, which is a listing rather + // than a file, and an `_`-prefixed path names a realm endpoint the realm + // serves no bytes from. `#adapter.openFile` answers undefined for a + // directory and for a path that is not there, so every way there is nothing + // to read arrives at the core the same way. + async #operationStoredFile( + localPath: LocalPath, + ): Promise { + if (!localPath || localPath.startsWith('_')) { + return undefined; + } + return await this.#adapter.openFile(localPath); + } + private async nonJsonFileExists(localPath: LocalPath): Promise { if (localPath?.endsWith('.json')) { localPath = localPath.slice(0, -5); @@ -5790,6 +5814,37 @@ export class Realm { return await this.#fileMetaDocumentFromDisk(localPath); } + // The write-time record behind a path's bytes, resolved exactly as + // `#fileMetaDocumentFromDisk` resolves the same two values: the persisted + // row is authoritative for the content hash because it is written in the + // same critical section as the bytes, and a path with no hash recorded falls + // back to hashing them. The fallback opens its own handle on the file, so it + // never consumes the one a read is serving from. + // + // Two things about the cost, for whoever routes a byte response through + // here. Every realm write records a hash, so the fallback is reached only by + // a path that arrived on disk outside the realm's write path — a copied + // fixture, an rsync — and it costs a full read of the bytes when it is. And + // this is one row lookup more than `getSourceOrRedirect` pays, since that + // computes the hash from bytes it has already materialized; both lookups + // here are single-row reads on `realm_file_meta`'s primary key. + async #operationStoredFileMeta( + localPath: LocalPath, + ): Promise { + let persisted = this.#dbAdapter + ? await getContentMeta(this.#dbAdapter, this.url, localPath) + : { contentHash: undefined }; + let version = persisted.contentHash; + if (version === undefined) { + let fileRef = await this.#operationStoredFile(localPath); + version = fileRef ? await computeContentHashFromRef(fileRef) : undefined; + } + return { + version, + createdAt: await this.getCreatedTime(localPath), + }; + } + private async getFileMeta( request: Request, requestContext: RequestContext, diff --git a/packages/runtime-common/tests/card-operations-dispatch-test.ts b/packages/runtime-common/tests/card-operations-dispatch-test.ts index 947e0f8b93a..83c02042f0e 100644 --- a/packages/runtime-common/tests/card-operations-dispatch-test.ts +++ b/packages/runtime-common/tests/card-operations-dispatch-test.ts @@ -2,6 +2,7 @@ import { isDocumentResult, isHeadResult, isOperationFailure, + isSourceResult, newOperationScope, resolveOperation, runOperation, @@ -25,7 +26,10 @@ import type { SharedTests } from '../helpers/index.ts'; // // The `read` executor rides along, since its refusals come from the same // taxonomy and its status mapping is the one thing a caller can observe about -// a card that will not read cleanly. +// a card that will not read cleanly. So does `readSource`, whose whole +// contract is about what dispatch does *not* consult: the stub records every +// collaborator call, which is what makes "definition-free" checkable rather +// than merely asserted. // ============================================================================ const REALM = 'http://example.com/test/'; @@ -48,6 +52,17 @@ interface StubOptions { fileMeta?: boolean; // Whether the file target has an index row. fileRow?: boolean; + // The bytes the realm holds, by local path. A path absent from this is one + // the realm has nothing to open at: a missing file, a directory, an + // `_`-prefixed endpoint. Those refusals live on the realm's side of + // `openStoredFile`, so the stub expresses all of them the one way the core + // can observe. + stored?: Record; + // The content hash the realm recorded for a path. Absent means it recorded + // none, which is a case the executor has to report rather than invent a + // value for. + storedVersions?: Record; + storedCreatedAt?: Record; } interface Stub { @@ -79,6 +94,9 @@ function stub(opts: StubOptions = {}): Stub { source, fileMeta = true, fileRow = false, + stored = {}, + storedVersions = {}, + storedCreatedAt = {}, } = opts; let core: OperationCore = { @@ -168,10 +186,38 @@ function stub(opts: StubOptions = {}): Stub { : undefined; }, }, - async readSource() { - calls.push('readSource'); + async readFileAsText() { + calls.push('readFileAsText'); return source; }, + async openStoredFile(localPath) { + calls.push('openStoredFile'); + let content = Object.prototype.hasOwnProperty.call(stored, localPath) + ? stored[localPath] + : undefined; + if (content === undefined) { + return undefined; + } + return { + path: localPath, + // A getter, as every streaming adapter's is, and it records the touch: + // the headers-only mode has to leave the bytes alone rather than open + // a stream it discards, and only reading the read tells us it did. + get content() { + calls.push('storedContent'); + return content; + }, + lastModified: 1699, + size: typeof content === 'string' ? content.length : content.byteLength, + }; + }, + async storedFileMeta(localPath) { + calls.push('storedFileMeta'); + return { + version: storedVersions[localPath], + createdAt: storedCreatedAt[localPath], + }; + }, async isIgnored() { return false; }, @@ -676,7 +722,8 @@ const tests = Object.freeze({ 'a card source spelling names the source, not the card': async (assert) => { // `.json` names the card's stored bytes. That is a different read // from the card, so the target routes as a file rather than resolving to - // the instance — and a file carries only `read`. + // the instance — and a file carries neither `update` nor anything else + // that writes. let { core, calls } = stub(); let error = await refusalFrom(() => runOperation( @@ -710,8 +757,8 @@ const tests = Object.freeze({ 'the file def type entry is consulted', ); - // The allowance still holds: a file carries `read` and nothing else, - // whatever it declares. + // The allowance still holds: a file carries its two reads and nothing + // else, whatever it declares. let mutating = stub({ operations: { touch: { base: 'transform' as const, deterministic: true }, @@ -818,6 +865,240 @@ const tests = Object.freeze({ } }, + 'a stored-bytes read serves the bytes and infers their content type': async ( + assert, + ) => { + let cases = [ + // A card's stored source. The `.json` spelling names the bytes rather + // than the card, which is why dispatch leaves it as written. + { + path: 'person-1.json', + content: '{"data":{"type":"card"}}', + contentType: 'application/json', + }, + // A module, which has no `adoptsFrom` and no definition entry — the + // case the whole definition-free path exists for. + { + path: 'person.gts', + content: 'export class Person {}', + contentType: 'text/typescript+glimmer', + }, + // Bytes rather than text, handed back as the adapter produced them: a + // read of an image must not decode them. + { + path: 'logo.png', + content: new Uint8Array([137, 80, 78, 71]), + contentType: 'image/png', + }, + ]; + for (let { path, content, contentType } of cases) { + let { core } = stub({ stored: { [path]: content } }); + let result = await runOperation( + core, + invoke({ kind: 'instance', url: `${REALM}${path}` }, 'readSource'), + ); + assert.true(isSourceResult(result), `${path} reads as stored bytes`); + if (isSourceResult(result)) { + assert.strictEqual( + result.contentType, + contentType, + `${path} carries the content type its extension names`, + ); + assert.strictEqual( + result.body, + content, + `${path} hands back exactly what the adapter produced`, + ); + assert.strictEqual(result.lastModified, 1699); + } + } + }, + + 'a stored-bytes read consults neither a definition nor the index': async ( + assert, + ) => { + // The hard constraint, as the only thing that can check it: a module has + // no definition entry, so a read that reached the cache for one would + // refuse bytes that are on disk. `.gts` is a registered extension, so this + // is the path that would otherwise resolve a file def's type and look it + // up. + let { core, calls } = stub({ + stored: { 'person.gts': 'export class Person {}' }, + }); + await runOperation( + core, + invoke({ kind: 'instance', url: `${REALM}person.gts` }, 'readSource'), + ); + assert.deepEqual( + calls, + ['openStoredFile', 'storedFileMeta', 'storedContent'], + 'one file open, one file-meta row, one touch of the bytes — and no ' + + 'definition lookup and no index read at all', + ); + }, + + 'the headers-only mode reports the metadata without touching the bytes': + async (assert) => { + let { core, calls } = stub({ + stored: { 'sample.md': '# hi' }, + storedVersions: { 'sample.md': 'abc123' }, + storedCreatedAt: { 'sample.md': 1600 }, + }); + let target: OperationTarget = { + kind: 'instance', + url: `${REALM}sample.md`, + }; + let headers = await runOperation(core, invoke(target, 'readSource'), { + headersOnly: true, + }); + assert.true(isSourceResult(headers), 'the metadata comes back'); + if (isSourceResult(headers)) { + assert.strictEqual(headers.body, undefined, 'and no bytes with it'); + assert.deepEqual( + { + contentType: headers.contentType, + lastModified: headers.lastModified, + created: headers.created, + version: headers.version, + }, + { + contentType: 'text/markdown', + lastModified: 1699, + created: 1600, + version: 'abc123', + }, + ); + } + assert.false( + calls.includes('storedContent'), + 'the adapter opens a stream on first touch, so a HEAD must not touch it', + ); + + // Identical metadata either way, which is what lets a `HEAD` and a + // conditional `GET` agree on a validator for the same file. + let bytes = await runOperation(core, invoke(target, 'readSource')); + assert.true(isSourceResult(bytes)); + if (isSourceResult(bytes) && isSourceResult(headers)) { + let { body: _body, ...metadata } = bytes; + assert.deepEqual(metadata, headers); + } + }, + + 'a stored-bytes read reports a version the realm never recorded as absent': + async (assert) => { + // The realm resolves the hash — from its own row, or by hashing the + // bytes when the row carries none — so a null here means it could do + // neither. Reporting the absence is what lets the facade omit an `ETag` + // rather than emit one that identifies nothing. + let { core } = stub({ stored: { 'sample.md': '# hi' } }); + let result = await runOperation( + core, + invoke({ kind: 'instance', url: `${REALM}sample.md` }, 'readSource'), + ); + assert.true(isSourceResult(result)); + if (isSourceResult(result)) { + assert.strictEqual(result.version, null); + assert.strictEqual( + result.created, + null, + 'and the same for a path the realm has no record of', + ); + } + }, + + 'a path with no stored bytes is not found, never not-indexed': async ( + assert, + ) => { + // Every way there is nothing to read answers alike, because the realm + // applies its own refusals inside `openStoredFile` and the core cannot + // tell them apart: a missing file, a directory, an `_`-prefixed endpoint. + for (let path of ['does-not-exist', 'dir', '_search']) { + let { core } = stub({ stored: { 'sample.md': '# hi' } }); + let error = await refusalFrom(() => + runOperation( + core, + invoke({ kind: 'instance', url: `${REALM}${path}` }, 'readSource'), + ), + ); + assert.strictEqual(error.code, 'target-not-found', `${path} is 404`); + assert.strictEqual(error.status, 404); + } + + // `target-not-indexed` says waiting will resolve the absence, and what it + // waits for is the index. A read of bytes has nothing to wait for, so a + // source file already on disk does not change the answer for a path whose + // own bytes are missing. + let { core } = stub({ source: '{"data":{"type":"card"}}' }); + let error = await refusalFrom(() => + runOperation( + core, + invoke( + { kind: 'instance', url: `${REALM}person-1.json` }, + 'readSource', + ), + ), + ); + assert.strictEqual(error.code, 'target-not-found'); + }, + + 'a type has no stored bytes to read': async (assert) => { + let { core, calls } = stub(); + let error = await refusalFrom(() => + resolveOperation( + core, + { kind: 'type', codeRef: PERSON, realm: REALM }, + 'readSource', + ), + ); + assert.strictEqual(error.code, 'operation-not-allowed'); + assert.strictEqual(error.status, 405); + assert.deepEqual( + calls, + [], + 'and the refusal costs no definition lookup: the answer does not ' + + 'depend on which kind of def the type turns out to be', + ); + + // A field def type refuses for the same reason rather than because a + // field carries nothing — the stub is set up to say `field-def`, and the + // refusal still arrives without the entry being read. + let field = stub({ definitionType: 'field-def' }); + let onField = await refusalFrom(() => + resolveOperation( + field.core, + { kind: 'type', codeRef: PERSON, realm: REALM }, + 'readSource', + ), + ); + assert.strictEqual(onField.code, 'operation-not-allowed'); + assert.deepEqual(field.calls, []); + }, + + 'a declaration may not build on a stored-bytes read': async (assert) => { + // A declaration wins over the built-in of the same name, which is how a + // `read` is specialized and how a `delete` is rebound onto `transform`. + // A stored-bytes read is the one behavior that cannot be won that way: + // there is no payload to reshape and no stage to run, so dispatching a + // declared name to it would answer under the author's name without doing + // what the author wrote. The authoring decorator refuses such a + // declaration; this is the realm refusing one that reached a stored + // definition regardless. + let { core } = stub({ + operations: { + exportBytes: { base: 'readSource' as const, deterministic: true }, + }, + }); + let error = await refusalFrom(() => + runOperation(core, invoke(CARD, 'exportBytes')), + ); + assert.strictEqual(error.code, 'operation-not-allowed'); + assert.strictEqual(error.status, 405); + assert.true( + error.detail.includes('readSource'), + `the refusal names the base it was built on: ${error.detail}`, + ); + }, + 'the row peek is memoized for one invocation and no longer': async ( assert, ) => { From 27193c0b8c6ed483240f6481c00d94d632a50b87 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 15:13:25 +0000 Subject: [PATCH 02/12] Derive the source version from the bytes, and stop refusing underscore names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two corrections to the stored-bytes read, both about parity with the byte routes the facade will hand off to. `version` has one job — identifying the bytes it is returned with — and the persisted file-meta row only does that job while it describes the file on disk. `persistFileMeta` is reached from the realm's own write path and nowhere else, so a file overwritten out of band keeps a row describing bytes that are gone; handing that hash back would let a conditional GET answer 304 for content that changed, where `getSourceOrRedirect` hashes the bytes it materialized and does not. The row is now trusted only where the length it recorded matches the handle being read, and the bytes are hashed otherwise. The size travels from the executor with the request for the version, so the check is against the handle those bytes come from rather than a later stat. An out-of-band overwrite preserving the exact byte length is the residual case; closing it needs an unconditional hash per read or an mtime on the row, which is the facade's call. The `_`-prefix refusal described the wrong routes. It is `openFileForMetadata`'s, and the byte routes have no equivalent: the `card+source` GET/HEAD and the raw byte serve are registered on `/.*` and refuse no name, and `upsertCardSource` writes whatever path it is given, so a caller can store `_notes.md` and read it back over HTTP. Only the specific registered `_` endpoints are routed away from the file handlers. Refusing the prefix made this the one read that could not reach such a file, so what is left is the path that names no file at all. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt --- .../tests/card-operations-core-test.ts | 89 ++++++++++++++++++- .../card-operations/dispatch.ts | 21 +++-- .../card-operations/read-source.ts | 10 ++- packages/runtime-common/realm.ts | 80 +++++++++++------ .../tests/card-operations-dispatch-test.ts | 45 ++++++++-- 5 files changed, 198 insertions(+), 47 deletions(-) diff --git a/packages/realm-server/tests/card-operations-core-test.ts b/packages/realm-server/tests/card-operations-core-test.ts index 6b77b695157..c65145ffc46 100644 --- a/packages/realm-server/tests/card-operations-core-test.ts +++ b/packages/realm-server/tests/card-operations-core-test.ts @@ -1,6 +1,7 @@ import QUnit from 'qunit'; const { module, test } = QUnit; -import { basename } from 'path'; +import { writeFileSync } from 'fs'; +import { basename, join } from 'path'; import type { Test, SuperTest } from 'supertest'; import type { RealmHttpServer as Server } from '../server.ts'; import type { DirResult } from 'tmp'; @@ -144,15 +145,22 @@ module(basename(import.meta.filename), function () { let testRealmHttpServer: Server; let realmRequest: RealmRequest; + // The realm's directory on disk, for the one case that has to reach past + // the realm to set itself up: a file changed without the realm's write + // path seeing it. + let testRealmPath: string; + function onRealmSetup(args: { testRealm: Realm; testRealmHttpServer: Server; + testRealmPath: string; request: SuperTest; dir: DirResult; dbAdapter: PgAdapter; }) { testRealm = args.testRealm; testRealmHttpServer = args.testRealmHttpServer; + testRealmPath = args.testRealmPath; realmRequest = withRealmPath(args.request, realmURL); } @@ -447,6 +455,79 @@ module(basename(import.meta.filename), function () { ); }); + test('a stored-bytes read of an underscore-prefixed file reaches it', async function (assert) { + // Only the specific registered `_` endpoints are realm endpoints. A file + // stored under such a name is one `upsertCardSource` writes and the byte + // routes serve, so a read of its bytes must not report it missing. + await testRealm.write('_notes.md', '# notes'); + + let result = await runOperation( + testRealm.operationCore, + request( + { kind: 'instance', url: `${testRealmHref}_notes.md` }, + 'readSource', + ), + ); + let source = sourceOf(result); + assert.strictEqual(source.contentType, 'text/markdown'); + assert.strictEqual(await textOf(source.body), '# notes'); + + let response = await realmRequest + .get('/_notes.md') + .set('Accept', SupportedMimeType.CardSource); + assert.strictEqual( + response.status, + 200, + `the source route serves it too: ${response.text}`, + ); + assert.strictEqual(await textOf(source.body), response.text); + }); + + test('a stored-bytes read never reports a version that describes other bytes', async function (assert) { + // `version` identifies the body it comes back with, and the realm's + // file-meta row is refreshed only by the realm's own write path. A file + // overwritten out of band therefore has a row describing bytes that are + // gone, and reporting that hash would let a conditional GET answer 304 + // for content that changed. + await testRealm.write('notes.md', '# first'); + let recorded = sourceOf( + await runOperation( + testRealm.operationCore, + request( + { kind: 'instance', url: `${testRealmHref}notes.md` }, + 'readSource', + ), + ), + ); + assert.strictEqual( + recorded.version, + computeContentHash('# first'), + 'a realm write records the hash of what it wrote', + ); + + // Straight to disk, so nothing refreshes the row. + writeFileSync(join(testRealmPath, 'notes.md'), '# second, and longer'); + let overwritten = sourceOf( + await runOperation( + testRealm.operationCore, + request( + { kind: 'instance', url: `${testRealmHref}notes.md` }, + 'readSource', + ), + ), + ); + assert.strictEqual( + await textOf(overwritten.body), + '# second, and longer', + 'the read serves the bytes that are on disk', + ); + assert.strictEqual( + overwritten.version, + computeContentHash('# second, and longer'), + 'and `version` identifies those bytes rather than the recorded ones', + ); + }); + test('a headers-only stored-bytes read reports the same metadata with no body', async function (assert) { let target: OperationTarget = { kind: 'instance', @@ -478,7 +559,11 @@ module(basename(import.meta.filename), function () { }); test('a stored-bytes read of a path with no bytes is not found', async function (assert) { - for (let path of ['does-not-exist', 'dir', '_search']) { + // A name nothing is stored under, and a name that is a directory. Both + // are "no bytes here" and answer alike; `_search` is deliberately not + // among them, since what makes a path unreadable is having no file, not + // its name. + for (let path of ['does-not-exist', 'dir']) { let error = await refusalFrom(() => runOperation( testRealm.operationCore, diff --git a/packages/runtime-common/card-operations/dispatch.ts b/packages/runtime-common/card-operations/dispatch.ts index f34d80cc3f1..0642d44dccd 100644 --- a/packages/runtime-common/card-operations/dispatch.ts +++ b/packages/runtime-common/card-operations/dispatch.ts @@ -69,13 +69,20 @@ export interface OperationCore { ): Promise; // The write-time record behind a path's bytes: the content hash a // stored-bytes read reports as `version`, and when the realm first saw the - // path. Both come from the realm's own file-meta row, and resolving the hash - // is the realm's job rather than the executor's for two reasons — the row is - // authoritative for it (it is written in the same critical section as the - // bytes), and where the row carries none the fallback is to hash the bytes, - // which only the side that can open a second handle on them can do without - // consuming the one a read is about to serve. - storedFileMeta(localPath: LocalPath): Promise; + // path. Resolving the hash is the realm's job rather than the executor's, + // because the realm is the side that can both read its own file-meta row and + // open a second handle on the bytes to hash them — which is what it takes to + // answer without consuming the handle a read is about to serve. + // + // `observedSize` is the byte size of the handle the caller is reading from, + // where its adapter knew it from a stat rather than from the bytes. The + // realm needs it to tell whether its recorded hash still describes the file: + // a `version` that identifies bytes other than the ones it is returned with + // is what a conditional GET would build a wrong validator from. + storedFileMeta( + localPath: LocalPath, + observedSize?: number, + ): Promise; // Whether the realm's ignore rules exclude this URL. An ignored path is // never visited, so no amount of waiting produces an index row for it. isIgnored(url: URL): Promise; diff --git a/packages/runtime-common/card-operations/read-source.ts b/packages/runtime-common/card-operations/read-source.ts index 6d48bad8b23..21395bac1ae 100644 --- a/packages/runtime-common/card-operations/read-source.ts +++ b/packages/runtime-common/card-operations/read-source.ts @@ -72,9 +72,8 @@ export async function readSourceOperation( if (!file) { // One refusal for every way there is nothing to read: no such path, a // directory, or a path the realm declines to serve at all. The realm - // applies its own refusals inside `openStoredFile`, so a `_`-prefixed - // realm endpoint lands here the same way a missing file does, which is - // what the byte routes answer for one. + // applies its own refusals inside `openStoredFile`, so whichever of those + // it was arrives here the same way. // // Never `target-not-indexed`. That code says waiting will resolve the // absence, and it is the index it is waiting for; the bytes either exist @@ -92,7 +91,10 @@ export async function readSourceOperation( // has to describe the bytes that actually arrived rather than the name they // were asked for. let contentType = inferContentType(file.path); - let meta = await core.storedFileMeta(localPath); + // The size travels with the request for the version so the realm can check + // its recorded hash against the very handle these bytes come from, rather + // than against whatever a later stat would see. + let meta = await core.storedFileMeta(localPath, file.size); let result: OperationSourceResult = { contentType, lastModified: file.lastModified, diff --git a/packages/runtime-common/realm.ts b/packages/runtime-common/realm.ts index 483bd3e9bf7..6dcce4a1bd0 100644 --- a/packages/runtime-common/realm.ts +++ b/packages/runtime-common/realm.ts @@ -3178,7 +3178,8 @@ export class Realm { readFileAsText: async (localPath) => (await this.readFileAsText(localPath))?.content, openStoredFile: (localPath) => this.#operationStoredFile(localPath), - storedFileMeta: (localPath) => this.#operationStoredFileMeta(localPath), + storedFileMeta: (localPath, observedSize) => + this.#operationStoredFileMeta(localPath, observedSize), isIgnored: (url) => this.isIgnored(url), fileMetaDocument: (localPath) => this.#operationFileMetaDocument(localPath), @@ -5527,20 +5528,26 @@ export class Realm { return this.#adapter.openFile(localPath); } - // The stored bytes at a local path, as the operation core opens them: - // `openFileForMetadata`'s guard above minus its `.json` refusal. That - // refusal is right for metadata — a `.json` is a card's source, not a file - // with metadata of its own — and wrong here, because a card's stored source - // is exactly what a stored-bytes read reads. The two it keeps are the byte - // routes' own: an empty path names the realm root, which is a listing rather - // than a file, and an `_`-prefixed path names a realm endpoint the realm - // serves no bytes from. `#adapter.openFile` answers undefined for a - // directory and for a path that is not there, so every way there is nothing - // to read arrives at the core the same way. + // The stored bytes at a local path, as the operation core opens them. + // + // Neither of `openFileForMetadata`'s two name-based refusals applies here. + // Its `.json` refusal is right for metadata — a `.json` is a card's source, + // not a file with metadata of its own — and wrong for this, because a card's + // stored source is exactly what a stored-bytes read reads. Its `_`-prefix + // refusal does not describe the byte routes at all: the `card+source` + // GET/HEAD and the raw byte serve are both registered on `/.*` and refuse no + // name, `upsertCardSource` writes whatever path it is given, and only the + // specific registered `_` endpoints are routed away from the file handlers. + // So an `_`-prefixed file a caller stored is a file the realm serves, and + // refusing it here would make this the one read that could not reach it. + // + // What is left is the path that names no file at all. `#adapter.openFile` + // answers undefined for a directory and for a path that is not there, so + // every way there is nothing to read arrives at the core the same way. async #operationStoredFile( localPath: LocalPath, ): Promise { - if (!localPath || localPath.startsWith('_')) { + if (!localPath) { return undefined; } return await this.#adapter.openFile(localPath); @@ -5814,27 +5821,46 @@ export class Realm { return await this.#fileMetaDocumentFromDisk(localPath); } - // The write-time record behind a path's bytes, resolved exactly as - // `#fileMetaDocumentFromDisk` resolves the same two values: the persisted - // row is authoritative for the content hash because it is written in the - // same critical section as the bytes, and a path with no hash recorded falls - // back to hashing them. The fallback opens its own handle on the file, so it - // never consumes the one a read is serving from. + // The write-time record behind a path's bytes: the content hash a + // stored-bytes read reports as `version`, and when the realm first saw the + // path. // - // Two things about the cost, for whoever routes a byte response through - // here. Every realm write records a hash, so the fallback is reached only by - // a path that arrived on disk outside the realm's write path — a copied - // fixture, an rsync — and it costs a full read of the bytes when it is. And - // this is one row lookup more than `getSourceOrRedirect` pays, since that - // computes the hash from bytes it has already materialized; both lookups - // here are single-row reads on `realm_file_meta`'s primary key. + // `version` has one job — to identify the bytes it is returned alongside — + // and the persisted row only does that job while it describes the file on + // disk. `persistFileMeta` is reached from the realm's own write path and + // nowhere else, so a file overwritten out of band (a deploy rsync, an + // operator editing the volume) keeps a row describing bytes that are gone. + // Handing that hash back would be worse than computing one slowly: it is + // what a conditional GET builds its validator from, so a stale one answers + // 304 for content that changed. + // + // So the row is trusted only where the length it recorded matches the handle + // the caller is reading from — `observedSize`, taken from the adapter's stat + // rather than from the bytes — and the bytes are hashed otherwise. An + // out-of-band overwrite that preserves the exact byte length is the one case + // this does not catch; closing it needs either an unconditional hash on + // every read or an mtime on the row, and which of those the byte facade + // wants is its call to make. + // + // The hash is computed from its own handle, so it never consumes the one a + // read is serving from. Two notes on cost for whoever routes a byte response + // through here: hashing reads the whole file (`computeContentHash` samples + // large content, but only after materializing it), and this is one row + // lookup more than `getSourceOrRedirect` pays, since that hashes bytes it + // has already materialized. Both lookups here are single-row reads on + // `realm_file_meta`'s primary key. async #operationStoredFileMeta( localPath: LocalPath, + observedSize?: number, ): Promise { let persisted = this.#dbAdapter ? await getContentMeta(this.#dbAdapter, this.url, localPath) - : { contentHash: undefined }; - let version = persisted.contentHash; + : { contentHash: undefined, contentSize: undefined }; + let describesThisFile = + persisted.contentHash !== undefined && + observedSize !== undefined && + persisted.contentSize === observedSize; + let version = describesThisFile ? persisted.contentHash : undefined; if (version === undefined) { let fileRef = await this.#operationStoredFile(localPath); version = fileRef ? await computeContentHashFromRef(fileRef) : undefined; diff --git a/packages/runtime-common/tests/card-operations-dispatch-test.ts b/packages/runtime-common/tests/card-operations-dispatch-test.ts index 83c02042f0e..983adc72584 100644 --- a/packages/runtime-common/tests/card-operations-dispatch-test.ts +++ b/packages/runtime-common/tests/card-operations-dispatch-test.ts @@ -53,8 +53,8 @@ interface StubOptions { // Whether the file target has an index row. fileRow?: boolean; // The bytes the realm holds, by local path. A path absent from this is one - // the realm has nothing to open at: a missing file, a directory, an - // `_`-prefixed endpoint. Those refusals live on the realm's side of + // the realm has nothing to open at: a missing file, a directory, a path it + // serves no bytes from. Those refusals live on the realm's side of // `openStoredFile`, so the stub expresses all of them the one way the core // can observe. stored?: Record; @@ -68,6 +68,10 @@ interface StubOptions { interface Stub { core: OperationCore; calls: string[]; + // What each `storedFileMeta` call was asked about, for the one part of the + // version contract that is the core's: handing the realm the size of the + // handle whose bytes it is returning. + metaCalls: { localPath: string; observedSize: number | undefined }[]; } // `getInstance` matches `i.url` / `i.file_alias`, so a row answers to the @@ -86,6 +90,7 @@ function isCanonicalKey(url: URL): boolean { function stub(opts: StubOptions = {}): Stub { let calls: string[] = []; + let metaCalls: Stub['metaCalls'] = []; let { document = 'ok', row = 'ok', @@ -211,8 +216,13 @@ function stub(opts: StubOptions = {}): Stub { size: typeof content === 'string' ? content.length : content.byteLength, }; }, - async storedFileMeta(localPath) { + async storedFileMeta(localPath, observedSize) { calls.push('storedFileMeta'); + // Recorded rather than acted on: deciding whether a recorded hash still + // describes the file is the realm's, so what the core owes is passing + // the size of the handle it is reading from. The realm-server suite + // holds the realm to the decision. + metaCalls.push({ localPath, observedSize }); return { version: storedVersions[localPath], createdAt: storedCreatedAt[localPath], @@ -241,7 +251,7 @@ function stub(opts: StubOptions = {}): Stub { calls.push('unresolveInstanceIds'); }, }; - return { core, calls }; + return { core, calls, metaCalls }; } const MARKDOWN: CodeRef = { @@ -890,13 +900,33 @@ const tests = Object.freeze({ content: new Uint8Array([137, 80, 78, 71]), contentType: 'image/png', }, + // An `_`-prefixed name. Only the specific registered `_` endpoints are + // realm endpoints; a file stored under such a name is one the byte + // routes serve, so a read of its bytes has to reach it rather than + // reporting it missing. + { + path: '_notes.md', + content: '# notes', + contentType: 'text/markdown', + }, ]; for (let { path, content, contentType } of cases) { - let { core } = stub({ stored: { [path]: content } }); + let { core, metaCalls } = stub({ stored: { [path]: content } }); let result = await runOperation( core, invoke({ kind: 'instance', url: `${REALM}${path}` }, 'readSource'), ); + assert.deepEqual( + metaCalls, + [ + { + localPath: path, + observedSize: + typeof content === 'string' ? content.length : content.byteLength, + }, + ], + `${path} asks for its version against the handle being read`, + ); assert.true(isSourceResult(result), `${path} reads as stored bytes`); if (isSourceResult(result)) { assert.strictEqual( @@ -1011,8 +1041,9 @@ const tests = Object.freeze({ ) => { // Every way there is nothing to read answers alike, because the realm // applies its own refusals inside `openStoredFile` and the core cannot - // tell them apart: a missing file, a directory, an `_`-prefixed endpoint. - for (let path of ['does-not-exist', 'dir', '_search']) { + // tell them apart: a missing file, a directory, a path it serves no bytes + // from. + for (let path of ['does-not-exist', 'dir']) { let { core } = stub({ stored: { 'sample.md': '# hi' } }); let error = await refusalFrom(() => runOperation( From 699f46df4220f4f77575f308081908018469166a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 15:34:14 +0000 Subject: [PATCH 03/12] Reserve the readSource name, address paths from the realm root, one meta query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three corrections, the first a silent wrong answer. The name `readSource` is now reserved at the decorator, not just its base. A name and a base are independent, so `@operation static readSource = { base: 'read' }` passed validation, replaced the synthesized entry `getOperations` returns, and was then dispatched straight past: the realm answers that name without reading a definition, so the built-in ran and the author's operation was never reached. Refusing the name is what makes answering it definition-free correct; refusing the base is what stops the behavior being reached under another name. Both halves now exist. A stored-bytes read addresses a path, so the realm root stays the realm root. `canonicalizeTarget` resolves the root to the realm's index card, which is what makes it readable as a card, and applied to a byte read it served whatever file carried the bare name `index` in answer to a request for a directory. Canonicalization now takes the addressing, and `runOperation` reads it off the name — sound for the same reason answering definition-free is, now that no declaration can take the name. A trailing slash, a query string and a fragment still normalize for both. The two file-meta values come from one query rather than one lookup each, which a byte response routed through here would pay per request. `OperationSourceResult` also carries `size`, which a facade needs for `Content-Length` and to decide whether it can offer a `Range` at all, and two comments that overclaimed are narrowed: the byte routes build a validator from a content hash only for a `.json` or an executable extension and from `lastModified` otherwise, and the metadata describes the handle as it opened while the bytes are read from it afterwards. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt --- packages/base/operations.ts | 29 ++++++++--- .../host/tests/integration/operations-test.ts | 51 +++++++++++++++---- .../tests/card-operations-dispatch-test.ts | 4 ++ .../card-operations/dispatch.ts | 37 +++++++++++--- .../card-operations/read-source.ts | 26 ++++++++-- .../runtime-common/card-operations/types.ts | 26 ++++++++-- packages/runtime-common/realm.ts | 27 +++++----- .../tests/card-operations-dispatch-test.ts | 44 ++++++++++++++++ 8 files changed, 196 insertions(+), 48 deletions(-) diff --git a/packages/base/operations.ts b/packages/base/operations.ts index 681c826f4bf..0b1b7875916 100644 --- a/packages/base/operations.ts +++ b/packages/base/operations.ts @@ -82,14 +82,24 @@ export const BASE_OPERATIONS = [ export type BaseOperationName = (typeof BASE_OPERATIONS)[number]; -// The base operations no declaration may name as its `base`. A stored-bytes -// read serves what is on disk: there is no payload to reshape, no program -// stage to run, and no result to project, so a declaration built on it would -// describe work nothing carries out. Refusing at the decorator is what keeps -// the realm's dispatch free to answer it without consulting a definition — -// the two rules are the same rule, read from opposite ends. +// The base operations a declaration may neither build on nor be named after. +// A stored-bytes read serves what is on disk: there is no payload to reshape, +// no program stage to run, and no result to project, so a declaration built on +// it would describe work nothing carries out. +// +// Both halves of that refusal matter, because a name and a base are +// independent. The realm answers one of these by name without reading a +// definition at all, so a declaration under the name — whatever base it +// builds on — would be dispatched straight past: the built-in would run and +// the author's operation would never be reached. Refusing the name is what +// makes answering it definition-free correct, and refusing the base is what +// stops the behavior being reached under some other name. const NOT_DECLARABLE: readonly BaseOperationName[] = ['readSource']; +function isNotDeclarable(name: string): boolean { + return NOT_DECLARABLE.includes(name as BaseOperationName); +} + // ============================================================================ // Typed references // @@ -472,6 +482,11 @@ export const operation = function ( ); } let owner = assertOperationTarget(target, key); + if (isNotDeclarable(key)) { + throw new Error( + `${declarationLabel(owner, key)}: "${key}" is a reserved operation name — a "${key}" serves the bytes stored at the def's URL, which the realm answers without reading a definition, so a declaration under this name would never be reached`, + ); + } assertNameAvailable(owner, key); if (typeof descriptor?.initializer !== 'function') { throw new Error( @@ -712,7 +727,7 @@ function assertValidDeclaration( `${label}: \`base\` must name the built-in behavior this operation builds on — one of ${quoteList(BASE_OPERATIONS)}`, ); } - if (NOT_DECLARABLE.includes(base)) { + if (isNotDeclarable(base)) { throw new Error( `${label}: a "${base}" operation serves the bytes stored at the def's URL, so there is nothing for a declaration to specialize or rebind`, ); diff --git a/packages/host/tests/integration/operations-test.ts b/packages/host/tests/integration/operations-test.ts index eae8cbe7743..67bb21bd588 100644 --- a/packages/host/tests/integration/operations-test.ts +++ b/packages/host/tests/integration/operations-test.ts @@ -29,6 +29,17 @@ let card: (typeof OperationsModule)['card']; let bxl: (typeof OperationsModule)['bxl']; let linkTo: (typeof OperationsModule)['linkTo']; +// The entry `getOperations` synthesizes for a base operation a def carries. +// The cast is load-bearing rather than convenience: `OperationDeclaration` +// deliberately cannot express `base: 'readSource'`, because nothing may +// declare one and the authoring types are the first place that is refused — +// while `getOperations` still reports the entry every card and file def +// carries. That asymmetry lives here rather than being spelled out at each +// expectation. +function implied(base: string): OperationsModule.OperationDeclaration { + return { base } as OperationsModule.OperationDeclaration; +} + // Compile-time assertions. The call does nothing at run time; it fails to // type-check unless the two types are identical, so the call is the assertion. type Identical = @@ -172,13 +183,13 @@ module('Integration | operations', function (hooks) { assert.deepEqual( getOperations(CardDef), { - read: { base: 'read' }, - readSource: { base: 'readSource' }, - create: { base: 'create' }, - update: { base: 'update' }, - delete: { base: 'delete' }, - query: { base: 'query' }, - transform: { base: 'transform' }, + read: implied('read'), + readSource: implied('readSource'), + create: implied('create'), + update: implied('update'), + delete: implied('delete'), + query: implied('query'), + transform: implied('transform'), }, 'a card def carries every base operation, implied by the def type', ); @@ -189,7 +200,7 @@ module('Integration | operations', function (hooks) { ); assert.deepEqual( getOperations(FileDef), - { read: { base: 'read' }, readSource: { base: 'readSource' } }, + { read: implied('read'), readSource: implied('readSource') }, "a file's metadata is read-only, so a file def carries only its two reads", ); assert.deepEqual( @@ -459,9 +470,29 @@ module('Integration | operations', function (hooks) { } return Redacted; }, - /serves the bytes stored at the def's URL/, + /reserved operation name/, 'and it cannot be specialized under its own name either', ); + + // The name is reserved independently of the base, because the two are + // independent everywhere else: a declaration is invoked under its name and + // carried out by its base. The realm answers this name without reading a + // definition, so a declaration under it — whatever base it builds on — + // would be dispatched straight past, and the built-in would run in place + // of what the author wrote. + assert.throws( + () => { + class Sneaky extends CardDef { + @operation static readSource = { + base: 'read', + output: { redacted: true }, + }; + } + return Sneaky; + }, + /reserved operation name/, + 'a declaration cannot take the name by building on another base', + ); }); test('the decorator rejects an operation name that is already a static', function (assert) { @@ -1432,7 +1463,7 @@ module('Integration | operations', function (hooks) { class Bare extends cardAPI.BaseDef {} assert.deepEqual( getOperations(Bare), - { read: { base: 'read' }, readSource: { base: 'readSource' } }, + { read: implied('read'), readSource: implied('readSource') }, 'the two reads are what every addressable def shares', ); assert.throws( diff --git a/packages/realm-server/tests/card-operations-dispatch-test.ts b/packages/realm-server/tests/card-operations-dispatch-test.ts index b589013d01e..67af2b62f09 100644 --- a/packages/realm-server/tests/card-operations-dispatch-test.ts +++ b/packages/realm-server/tests/card-operations-dispatch-test.ts @@ -122,6 +122,10 @@ module(basename(import.meta.filename), function () { await runSharedTest(cardOperationsDispatchTests, assert, {}); }); + test('the realm root is a directory, not a stored file', async function (assert) { + await runSharedTest(cardOperationsDispatchTests, assert, {}); + }); + test('a type has no stored bytes to read', async function (assert) { await runSharedTest(cardOperationsDispatchTests, assert, {}); }); diff --git a/packages/runtime-common/card-operations/dispatch.ts b/packages/runtime-common/card-operations/dispatch.ts index 0642d44dccd..3b75e774874 100644 --- a/packages/runtime-common/card-operations/dispatch.ts +++ b/packages/runtime-common/card-operations/dispatch.ts @@ -55,11 +55,11 @@ export interface OperationCore { readFileAsText(localPath: LocalPath): Promise; // The stored bytes at a local path, opened the way the realm's own byte // serve opens them, with the realm's own refusals applied: a path it will - // not serve — an empty one, an `_`-prefixed realm endpoint, a directory — - // answers undefined, and so does one that is not there. Unlike - // `fileMetaDocument` this does not decline a card's `.json`: that path names - // the card's stored source, which is exactly what a stored-bytes read is - // for. + // not serve — the realm root, which is a directory, or a name nothing is + // stored under — answers undefined. It applies no refusal to a name as + // such, which is what keeps this in step with the byte routes: they serve a + // card's `.json` and an `_`-prefixed file alike, so a read of stored bytes + // has to reach both. // // `content` is unread until it is touched. The adapter opens a real stream // on first touch, so a caller that wants only the metadata must leave it @@ -397,7 +397,15 @@ export async function runOperation( request: OperationRequest, opts: RunOperationOptions = {}, ): Promise { - let target = canonicalizeTarget(core, request.target); + // A definition-free name is a read of stored bytes, which addresses a path + // rather than a card — so the realm root stays the realm root rather than + // resolving to the index card. Reading the addressing off the name is sound + // for the same reason answering it without a definition is: no declaration + // can take one of these names, so the name settles which behavior this is + // before anything is read. + let target = canonicalizeTarget(core, request.target, { + rootNamesIndexCard: !isDefinitionFreeOperation(request.name), + }); let canonical: OperationRequest = target === request.target ? request : { ...request, target }; let scope = newOperationScope(core); @@ -504,9 +512,20 @@ export function pathsFor(core: OperationCore): RealmPaths { // Idempotent, and deliberately tolerant: a URL that does not parse, or that // belongs to another realm, is returned untouched so the refusal for it comes // from the code that has something to say about it. +export interface CanonicalizeOptions { + // Whether the realm root names the realm's index card. It does for a read of + // a card, which is what makes the root readable at all. It does not for a + // read of stored bytes, which addresses a path: the root is the realm's + // directory, so resolving it to `index` there would serve whatever file + // happens to carry that bare name in answer to a request for a directory. + // Defaults to true — a caller addressing paths says so. + rootNamesIndexCard?: boolean; +} + export function canonicalizeTarget( core: OperationCore, target: OperationTarget, + opts: CanonicalizeOptions = {}, ): OperationTarget { if (target.kind !== 'instance') { return target; @@ -522,7 +541,7 @@ export function canonicalizeTarget( } catch { return target; } - if (localPath === '') { + if (localPath === '' && (opts.rootNamesIndexCard ?? true)) { localPath = 'index' as LocalPath; } // A `.json` path is deliberately left alone. It names a card's stored source @@ -531,6 +550,10 @@ export function canonicalizeTarget( // Resolving it to the card instead would answer a question nobody asked, and // would be the one spelling where the extension test and the rest of the // core disagreed. + // + // Everything else here is common to both addressings: a trailing slash, a + // query string and a fragment all name the thing they hang off, whether that + // thing is a card or a file. let canonical = paths.fileURL(localPath).href; return canonical === target.url ? target diff --git a/packages/runtime-common/card-operations/read-source.ts b/packages/runtime-common/card-operations/read-source.ts index 21395bac1ae..86c4516140e 100644 --- a/packages/runtime-common/card-operations/read-source.ts +++ b/packages/runtime-common/card-operations/read-source.ts @@ -49,9 +49,20 @@ import type { OperationCore, RunOperationOptions } from './dispatch.ts'; // * The redirects. An extension-less URL naming `foo.gts`, or a card id // naming its `.json`, is resolved by the facade — this executor takes the // resolved path and reads it. A read has no redirect to give. -// * The response around the bytes. The `ETag` built from `version` and its -// source variant, `Last-Modified`, `x-created`, the 304, `Range` and the -// source cache are all the facade's, computed from what comes back here. +// * The response around the bytes. `Last-Modified`, `x-created`, the +// validator, the 304 and the source cache are all the facade's, computed +// from what comes back here. Which validator is the facade's choice too, +// and the byte routes do not make one choice: the source route builds an +// `ETag` from a content hash for a `.json` or an executable extension, and +// from `lastModified` for everything else. A `Range` needs more than this +// result carries — the adapter's bounded-read capability does not travel +// through it — so a facade serving 206s holds the handle itself. +// +// * The pairing of the metadata with the bytes. `lastModified` is the stat +// taken when the handle opened and the body is read from it later, so a +// write landing in between pairs one with the other, exactly as it does +// for the byte routes reading the same handle. A caller that cannot +// tolerate that has to revalidate after reading rather than before. // * A batching envelope over operations does not carry this one at all: // bytes do not belong in a JSON batch, and a stream cannot be one member // of one. @@ -64,8 +75,12 @@ export async function readSourceOperation( ): Promise { // `runOperation` canonicalized the target already; doing it again is a no-op // and keeps a direct caller of this executor addressing the same path - // dispatch would have. - let target = canonicalizeTarget(core, request.target); + // dispatch would have — including the addressing, since a stored-bytes read + // names a path and the realm root is that realm's directory rather than its + // index card. + let target = canonicalizeTarget(core, request.target, { + rootNamesIndexCard: false, + }); let url = instanceTargetURL({ ...request, target }); let localPath = localPathFor(core, url); let file = await core.openStoredFile(localPath); @@ -100,6 +115,7 @@ export async function readSourceOperation( lastModified: file.lastModified, created: meta.createdAt ?? null, version: meta.version ?? null, + size: file.size, }; if (opts.headersOnly) { return result; diff --git a/packages/runtime-common/card-operations/types.ts b/packages/runtime-common/card-operations/types.ts index eea93e40bce..ad3698c9380 100644 --- a/packages/runtime-common/card-operations/types.ts +++ b/packages/runtime-common/card-operations/types.ts @@ -268,9 +268,10 @@ export interface OperationHeadResult { } // The stored bytes of a resource, and what the byte-serve headers are computed -// from. This is what the source and byte-serve routes answer with: a card -// instance's `.json`, a module's text, an image's bytes — the resource exactly -// as it sits on disk, with no assembly and no index read behind it. +// from. This is the representation the source and byte-serve routes answer +// with: a card instance's `.json`, a module's text, an image's bytes — the +// resource exactly as it sits on disk, with no assembly and no index read +// behind it. export interface OperationSourceResult { // Inferred from the path's extension by `inferContentType`, which is what // both byte routes infer theirs with. A path with no extension the platform @@ -285,14 +286,29 @@ export interface OperationSourceResult { // than filling it in. created: number | null; // The content hash of the stored bytes — the same identity the rest of the - // project calls `version`, and what the source route's `ETag` is built - // from. Null only where the realm can neither recall nor compute one. + // project calls `version`. It identifies these bytes and no others, which is + // what a validator needs, but it is not by itself the byte routes' `ETag`: + // the source route builds one from a content hash for a `.json` or an + // executable extension and from `lastModified` for everything else, so a + // facade reproducing those validators chooses between the two. Null only + // where the realm can neither recall nor compute a hash. version: string | null; + // The byte size, where the adapter knew it from the stat it already + // performed, and absent where knowing it would cost reading the bytes. A + // facade needs it for `Content-Length` and to decide whether it can offer a + // `Range` at all. The bounded-read capability itself does not travel here — + // it is a function on the adapter's handle — so a facade serving 206s reads + // from the handle rather than from this result. + size?: number; // The bytes. Absent in the headers-only mode, which is the whole difference // between the two: a `HEAD` reports the metadata above and would discard // this. Whatever form the realm's file adapter produced — a string, a byte // array, or an unread stream — so a caller hands it to a response body // rather than materializing it. + // + // The metadata above describes the handle as it opened; the bytes are read + // from it afterwards. A write landing in between pairs one with the other, + // the same way it does for a byte route reading the same handle. body?: OperationSourceBody; } diff --git a/packages/runtime-common/realm.ts b/packages/runtime-common/realm.ts index 6dcce4a1bd0..33e32995106 100644 --- a/packages/runtime-common/realm.ts +++ b/packages/runtime-common/realm.ts @@ -73,6 +73,7 @@ import { removeFileMeta, getCreatedTime, getContentMeta, + getFileMetaForPaths, } from './file-meta.ts'; import { systemError, @@ -5843,32 +5844,30 @@ export class Realm { // wants is its call to make. // // The hash is computed from its own handle, so it never consumes the one a - // read is serving from. Two notes on cost for whoever routes a byte response - // through here: hashing reads the whole file (`computeContentHash` samples - // large content, but only after materializing it), and this is one row - // lookup more than `getSourceOrRedirect` pays, since that hashes bytes it - // has already materialized. Both lookups here are single-row reads on - // `realm_file_meta`'s primary key. + // read is serving from, and it costs a full read of the file when it is + // reached (`computeContentHash` samples large content, but only after + // materializing it). Both values come from one row, so this is one query on + // `realm_file_meta`'s primary key rather than a lookup per value — worth + // holding to, since a byte response routed through here pays it per request. async #operationStoredFileMeta( localPath: LocalPath, observedSize?: number, ): Promise { let persisted = this.#dbAdapter - ? await getContentMeta(this.#dbAdapter, this.url, localPath) - : { contentHash: undefined, contentSize: undefined }; + ? (await getFileMetaForPaths(this.#dbAdapter, this.url, [localPath])).get( + localPath, + ) + : undefined; let describesThisFile = - persisted.contentHash !== undefined && + persisted?.contentHash !== undefined && observedSize !== undefined && persisted.contentSize === observedSize; - let version = describesThisFile ? persisted.contentHash : undefined; + let version = describesThisFile ? persisted!.contentHash : undefined; if (version === undefined) { let fileRef = await this.#operationStoredFile(localPath); version = fileRef ? await computeContentHashFromRef(fileRef) : undefined; } - return { - version, - createdAt: await this.getCreatedTime(localPath), - }; + return { version, createdAt: persisted?.createdAt }; } private async getFileMeta( diff --git a/packages/runtime-common/tests/card-operations-dispatch-test.ts b/packages/runtime-common/tests/card-operations-dispatch-test.ts index 983adc72584..52eb49ca3b0 100644 --- a/packages/runtime-common/tests/card-operations-dispatch-test.ts +++ b/packages/runtime-common/tests/card-operations-dispatch-test.ts @@ -940,6 +940,11 @@ const tests = Object.freeze({ `${path} hands back exactly what the adapter produced`, ); assert.strictEqual(result.lastModified, 1699); + assert.strictEqual( + result.size, + typeof content === 'string' ? content.length : content.byteLength, + `${path} carries the size a Content-Length is set from`, + ); } } }, @@ -1072,6 +1077,45 @@ const tests = Object.freeze({ assert.strictEqual(error.code, 'target-not-found'); }, + 'the realm root is a directory, not a stored file': async (assert) => { + // A card read resolves the realm root to the realm's index card, which is + // what makes the root readable at all. A stored-bytes read addresses a + // path, and the root is the realm's own directory — so it must not resolve + // to `index` and serve whatever file happens to carry that bare name. + let { core } = stub({ stored: { index: 'not the index card' } }); + for (let url of [REALM, REALM.replace(/\/$/, '')]) { + let error = await refusalFrom(() => + runOperation(core, invoke({ kind: 'instance', url }, 'readSource')), + ); + assert.strictEqual( + error.code, + 'target-not-found', + `${url} names a directory: ${error.detail}`, + ); + } + + // The bare name still reads when it is asked for by name. + let byName = await runOperation( + core, + invoke({ kind: 'instance', url: `${REALM}index` }, 'readSource'), + ); + assert.true(isSourceResult(byName), 'the file itself is readable'); + if (isSourceResult(byName)) { + assert.strictEqual(byName.body, 'not the index card'); + } + + // And the card read still resolves the root to the index card, which is + // the behavior the two addressings differ on. + let card = await runOperation( + core, + invoke({ kind: 'instance', url: REALM }, 'read'), + ); + assert.true( + isDocumentResult(card), + 'a card read of the root serves the index card', + ); + }, + 'a type has no stored bytes to read': async (assert) => { let { core, calls } = stub(); let error = await refusalFrom(() => From 6479de6302658ee23a1a424fd38d77cff3a0dea9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 16:15:43 +0000 Subject: [PATCH 04/12] Include readSource in the inherited-operations key list `getOperations` reports the base operations a def type carries alongside the declared ones, so the expectation for a subclass's merged set names every base operation. It was missing the stored-bytes read. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt --- packages/host/tests/integration/operations-test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/host/tests/integration/operations-test.ts b/packages/host/tests/integration/operations-test.ts index 67bb21bd588..4547f37993d 100644 --- a/packages/host/tests/integration/operations-test.ts +++ b/packages/host/tests/integration/operations-test.ts @@ -137,6 +137,7 @@ module('Integration | operations', function (hooks) { 'listMine', 'query', 'read', + 'readSource', 'transform', 'update', ], From 0264612ba34de181c5cd2ce304c2aa1d25770c5e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 17:14:59 +0000 Subject: [PATCH 05/12] Read a stored-bytes body once in the underscore-prefixed test The body is whatever the file adapter produced, which under Node is a single-use stream: reading it twice yields the bytes and then nothing. Hold the first read and compare it against both the literal and what the source route serves, which is also the one pass a facade putting the body on a response gets. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt --- packages/realm-server/tests/card-operations-core-test.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/realm-server/tests/card-operations-core-test.ts b/packages/realm-server/tests/card-operations-core-test.ts index c65145ffc46..2999b9d024d 100644 --- a/packages/realm-server/tests/card-operations-core-test.ts +++ b/packages/realm-server/tests/card-operations-core-test.ts @@ -470,7 +470,12 @@ module(basename(import.meta.filename), function () { ); let source = sourceOf(result); assert.strictEqual(source.contentType, 'text/markdown'); - assert.strictEqual(await textOf(source.body), '# notes'); + // Read once and hold it. The body is whatever the adapter produced — a + // single-use stream under Node — so reading it twice yields the bytes + // and then nothing, which is what a facade putting it on a response + // gets too: one pass. + let bytes = await textOf(source.body); + assert.strictEqual(bytes, '# notes'); let response = await realmRequest .get('/_notes.md') @@ -480,7 +485,7 @@ module(basename(import.meta.filename), function () { 200, `the source route serves it too: ${response.text}`, ); - assert.strictEqual(await textOf(source.body), response.text); + assert.strictEqual(bytes, response.text); }); test('a stored-bytes read never reports a version that describes other bytes', async function (assert) { From a78eaf90eaefdbc13aa36a924ce80aedc37b0577 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 19:24:27 +0000 Subject: [PATCH 06/12] Populate the source version on the byte routes' own terms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A review of the stored-bytes read turned up that its central claims did not describe its behavior, and that resolving `version` cost far more than the comments said. `version` is now populated exactly where the byte routes build a validator from a content hash — a `.json` or an executable extension — and reports the absence everywhere else, which is what those routes do: `getSourceOrRedirect` takes its `bypassCache` path for anything else and bases the `ETag` on `lastModified`, computing no hash at all. The previous fallback hashed unconditionally, and `ensureFileCreatedAt` inserts a row carrying only `created_at`, so that reached a full buffering read of every file the realm had not itself written with a hash — an image or a video included, and in the headers-only mode, whose whole promise is to leave the body alone. That mode now reports no version rather than buying one with a read it declined; the two modes cannot contradict each other, since one answers with a hash where the other answers with nothing. Where the bytes are read they are read from the caller's own handle and handed back with the hash, so one open serves both and `version` describes the body it is returned with. Previously the fallback opened a second handle, so an overwrite between the two opens produced a version for bytes never served — the failure the size check exists to prevent. The definition-free justification was false and is restated. The ordinary path resolves these names for an instance target whether or not a definition resolves, because `defKindFor` takes an instance target's kind from its URL and never from the entry; and a `.gts` does have an entry, the file def its extension names. What skipping the lookup buys is the lookup, on the hottest path the realm has, plus fixing the addressing before anything reads. What makes skipping safe is the name reservation, not the other way round. Lowering now refuses a reserved name too, so no stored definition can carry one. The decorator only governs what it lowers, and a definition-cache row carries no code version and is not re-derived until something invalidates it — so an entry written before the reservation existed would have been dispatched past rather than run. The reserved set moves to `card-operations/types.ts`, which dispatch and lowering can both reach; the authoring decorator enforces the same list from inside a card module. `getOperations` returns `CarriedOperation`, since `OperationDeclaration` is a closed union that cannot express a synthesized entry for a name nothing may declare — a consumer testing for one got "no overlap". `OperationSourceResult.size` reports null rather than being absent, to match the two values beside it. And two comments are corrected: the version claim, which ignored that `computeContentHash` samples above its whole-content limit, and the `_`-prefix rationale, which described the raw byte serve as a registered route and missed that a path under a prefix claimed before the router is reachable through this read and through no byte route. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt --- packages/base/operations.ts | 29 ++- .../tests/card-operations-core-test.ts | 71 ++++--- .../tests/card-operations-dispatch-test.ts | 12 ++ .../card-operations/dispatch.ts | 80 +++++--- .../runtime-common/card-operations/index.ts | 3 + .../card-operations/lowering.ts | 25 +++ .../card-operations/read-source.ts | 81 +++++--- .../runtime-common/card-operations/types.ts | 54 ++++-- packages/runtime-common/realm.ts | 60 ++++-- .../tests/card-operations-dispatch-test.ts | 182 ++++++++++++++++-- 10 files changed, 459 insertions(+), 138 deletions(-) diff --git a/packages/base/operations.ts b/packages/base/operations.ts index 0b1b7875916..ec460096b91 100644 --- a/packages/base/operations.ts +++ b/packages/base/operations.ts @@ -91,9 +91,10 @@ export type BaseOperationName = (typeof BASE_OPERATIONS)[number]; // independent. The realm answers one of these by name without reading a // definition at all, so a declaration under the name — whatever base it // builds on — would be dispatched straight past: the built-in would run and -// the author's operation would never be reached. Refusing the name is what -// makes answering it definition-free correct, and refusing the base is what -// stops the behavior being reached under some other name. +// the author's operation would never be reached. Refusing the name here is +// what keeps a new declaration out of that state, and refusing the base is +// what stops the behavior being reached under some other name. Lowering +// refuses the name too, so no stored definition can carry one either. const NOT_DECLARABLE: readonly BaseOperationName[] = ['readSource']; function isNotDeclarable(name: string): boolean { @@ -375,6 +376,19 @@ export type OperationDeclaration = | ReadOperationDeclaration | QueryOperationDeclaration; +// A base operation a def carries with nothing declared on it. It is not a +// declaration and the union above deliberately cannot express one: an author +// writes no clauses for a base operation, and the two `NOT_DECLARABLE` names +// cannot be written at all, so a declaration type that admitted them would +// invite exactly what the decorator refuses. `getOperations` returns both +// shapes, so a consumer reading `base` to dispatch gets every operation a def +// carries — including the ones no `OperationDeclaration` could name. +export interface ImpliedOperation { + readonly base: BaseOperationName; +} + +export type CarriedOperation = OperationDeclaration | ImpliedOperation; + // The operations declared on a def, read off the class type. Keyed by // operation name, so an invocation surface can be typed from the class alone. // @@ -525,14 +539,11 @@ export const operation = function ( // relied on to carry one, so lower from `getDeclaredOperations`. export function getOperations( classOrInstance: BaseDef | typeof BaseDef, -): Record { +): Record { let owner = defConstructorFor(classOrInstance, 'getOperations'); - let operations = emptyOperationRecord(); + let operations = emptyOperationRecord() as Record; for (let base of impliedOperations(owner)) { - // A base operation with nothing declared on it is the declaration - // `{ base }`; the cast is only because a union does not narrow from a - // computed discriminant. - operations[base] = { base } as OperationDeclaration; + operations[base] = { base }; } return Object.assign(operations, declaredOperations(owner)); } diff --git a/packages/realm-server/tests/card-operations-core-test.ts b/packages/realm-server/tests/card-operations-core-test.ts index 2999b9d024d..776bd9e0e96 100644 --- a/packages/realm-server/tests/card-operations-core-test.ts +++ b/packages/realm-server/tests/card-operations-core-test.ts @@ -494,43 +494,63 @@ module(basename(import.meta.filename), function () { // overwritten out of band therefore has a row describing bytes that are // gone, and reporting that hash would let a conditional GET answer 304 // for content that changed. - await testRealm.write('notes.md', '# first'); - let recorded = sourceOf( - await runOperation( - testRealm.operationCore, - request( - { kind: 'instance', url: `${testRealmHref}notes.md` }, - 'readSource', + // + // A module, so this is also a path whose validator the byte routes build + // from a content hash — which is what makes recomputing one warranted + // when the row cannot be trusted. + let read = async (localPath: string) => + sourceOf( + await runOperation( + testRealm.operationCore, + request( + { kind: 'instance', url: `${testRealmHref}${localPath}` }, + 'readSource', + ), ), - ), - ); + ); + + await testRealm.write('notes.gts', 'export const first = 1;'); assert.strictEqual( - recorded.version, - computeContentHash('# first'), + (await read('notes.gts')).version, + computeContentHash('export const first = 1;'), 'a realm write records the hash of what it wrote', ); - // Straight to disk, so nothing refreshes the row. - writeFileSync(join(testRealmPath, 'notes.md'), '# second, and longer'); - let overwritten = sourceOf( - await runOperation( - testRealm.operationCore, - request( - { kind: 'instance', url: `${testRealmHref}notes.md` }, - 'readSource', - ), - ), + // Straight to disk, so nothing refreshes the row. The new bytes are a + // different length, which is what the recorded hash is checked against. + writeFileSync( + join(testRealmPath, 'notes.gts'), + 'export const second = 2; // and longer', ); + let overwritten = await read('notes.gts'); assert.strictEqual( await textOf(overwritten.body), - '# second, and longer', + 'export const second = 2; // and longer', 'the read serves the bytes that are on disk', ); assert.strictEqual( overwritten.version, - computeContentHash('# second, and longer'), + computeContentHash('export const second = 2; // and longer'), 'and `version` identifies those bytes rather than the recorded ones', ); + + // The same overwrite on a path whose validator rests on `lastModified` + // rather than a hash. The recorded hash is equally untrustworthy, and + // reading an image or a video to replace it is a cost the route serving + // it would not pay — so the answer is the absence, never the stale hash. + await testRealm.write('notes.md', '# first'); + writeFileSync(join(testRealmPath, 'notes.md'), '# second, and longer'); + let unhashed = await read('notes.md'); + assert.strictEqual( + await textOf(unhashed.body), + '# second, and longer', + 'the bytes on disk are still what is served', + ); + assert.strictEqual( + unhashed.version, + null, + 'and no version is reported rather than one that describes other bytes', + ); }); test('a headers-only stored-bytes read reports the same metadata with no body', async function (assert) { @@ -547,7 +567,10 @@ module(basename(import.meta.filename), function () { ); assert.strictEqual(headers.body, undefined, 'no bytes in this mode'); assert.strictEqual(headers.contentType, 'text/markdown'); - assert.strictEqual(typeof headers.version, 'string'); + // A fixture file, so the realm has no hash recorded for it, and a `.md` + // is not a path whose validator is built from one. The mode reports the + // absence rather than reading the body it declined to return. + assert.strictEqual(headers.version, null); let withBody = sourceOf( await runOperation( diff --git a/packages/realm-server/tests/card-operations-dispatch-test.ts b/packages/realm-server/tests/card-operations-dispatch-test.ts index 67af2b62f09..e4363199c07 100644 --- a/packages/realm-server/tests/card-operations-dispatch-test.ts +++ b/packages/realm-server/tests/card-operations-dispatch-test.ts @@ -122,6 +122,18 @@ module(basename(import.meta.filename), function () { await runSharedTest(cardOperationsDispatchTests, assert, {}); }); + test('a version read from the bytes is the version of the bytes served', async function (assert) { + await runSharedTest(cardOperationsDispatchTests, assert, {}); + }); + + test('a headers-only read asks for no version it would have to read bytes for', async function (assert) { + await runSharedTest(cardOperationsDispatchTests, assert, {}); + }); + + test('an adapter that reports no size still reads', async function (assert) { + await runSharedTest(cardOperationsDispatchTests, assert, {}); + }); + test('the realm root is a directory, not a stored file', async function (assert) { await runSharedTest(cardOperationsDispatchTests, assert, {}); }); diff --git a/packages/runtime-common/card-operations/dispatch.ts b/packages/runtime-common/card-operations/dispatch.ts index 3b75e774874..348bbef7983 100644 --- a/packages/runtime-common/card-operations/dispatch.ts +++ b/packages/runtime-common/card-operations/dispatch.ts @@ -3,6 +3,7 @@ import { urlNamesFile } from '../file-def-code-ref.ts'; import { readOperation } from './read.ts'; import { readSourceOperation } from './read-source.ts'; import { + DEFINITION_FREE_BASE_OPERATIONS, OperationFailure, type BaseOperation, type OperationDefinition, @@ -67,21 +68,27 @@ export interface OperationCore { openStoredFile( localPath: LocalPath, ): Promise; - // The write-time record behind a path's bytes: the content hash a - // stored-bytes read reports as `version`, and when the realm first saw the - // path. Resolving the hash is the realm's job rather than the executor's, - // because the realm is the side that can both read its own file-meta row and - // open a second handle on the bytes to hash them — which is what it takes to - // answer without consuming the handle a read is about to serve. + // The version and creation time of the bytes at `file`, resolved by the + // realm because it owns both its file-meta row and the policy for when a + // content hash is worth reading bytes to get. // - // `observedSize` is the byte size of the handle the caller is reading from, - // where its adapter knew it from a stat rather than from the bytes. The - // realm needs it to tell whether its recorded hash still describes the file: - // a `version` that identifies bytes other than the ones it is returned with - // is what a conditional GET would build a wrong validator from. + // `file` is the handle the caller will serve from, not just its path: the + // realm validates its recorded hash against that handle's size, since a + // `version` identifying bytes other than the ones it is returned with is + // what a conditional GET would build a wrong validator from. Where the + // realm has to read the bytes to hash them it reads them from this handle + // and returns them as `bytes`, so one open serves both the hash and the + // body and the two cannot describe different files. + // + // `mayReadBytes` is false for a headers-only read, which returns no body and + // so must leave the handle's `content` untouched. The realm then answers + // from its row alone and reports no version where it has none recorded — + // an absence a caller can act on, rather than a value bought with a read it + // asked not to pay. storedFileMeta( localPath: LocalPath, - observedSize?: number, + file: OperationStoredFile, + opts: { mayReadBytes: boolean }, ): Promise; // Whether the realm's ignore rules exclude this URL. An ignored path is // never visited, so no amount of waiting produces an index row for it. @@ -133,11 +140,17 @@ export interface OperationStoredFile { } export interface OperationStoredFileMeta { - // The content hash of the stored bytes, absent where the realm can neither - // recall nor compute one. + // The content hash of the stored bytes, absent where the realm has none + // recorded and reading the bytes to compute one is not warranted for this + // path. version?: string; // Epoch seconds, absent where the realm holds no record of this path. createdAt?: number; + // The bytes, present exactly when the realm read them to hash them. Serving + // these rather than the handle's own `content` is what makes `version` + // describe the body it is returned with — and the handle has been consumed + // producing them, so a caller that has these must serve these. + bytes?: Uint8Array; } // `CachingDefinitionLookup`, narrowed to the one read an operation makes. @@ -252,22 +265,32 @@ const ALLOWED_BASE_OPERATIONS: Readonly< // // A definition is consulted for two reasons — to find a declaration of the // requested name, and to learn a type target's def kind — and a stored-bytes -// read needs neither. Nothing may declare one (the authoring decorator refuses -// it, and so does the declared branch below), so no declaration can take the -// name; and a type has no stored bytes, so there is no type-target form whose -// kind would have to be resolved. +// read needs neither. An instance target's kind comes from its URL, and +// nothing may declare one of these names, so there is nothing in a definition +// that could change the answer. +// +// What skipping the lookup buys is the lookup: a byte read is the hottest +// path the realm has, and resolving a definition for one costs a cache read +// that cannot affect the outcome — for a `.gts` it is a read of the file def +// its extension names. It also fixes the addressing before anything reads, +// which is what lets `runOperation` tell a path read from a card read by name +// alone. // -// That is what makes skipping the lookup correct rather than merely cheaper. A -// module path is the case that needs it: `.gts` is a registered extension, so -// resolving a definition for one means a cache lookup on the file def its -// extension names, and a path whose type nothing can resolve would then refuse -// a read of bytes that are plainly on disk. Definition-free means -// definition-free. +// It is not what makes the answer correct. The ordinary path reaches the same +// built-in for an instance target whether or not a definition resolves, since +// `defKindFor` never consults one. What makes *skipping* safe is the name +// reservation: were a declaration able to take one of these names, resolving +// before the lookup would run the built-in in its place. +// A null-prototype record over the shared list, so a wire-supplied name is +// looked up the same prototype-safe way every other name on this path is. const DEFINITION_FREE_OPERATIONS: Readonly< Partial> -> = { - readSource: true, -}; +> = Object.assign( + Object.create(null) as Partial>, + Object.fromEntries( + DEFINITION_FREE_BASE_OPERATIONS.map((name) => [name, true]), + ), +); function isBaseOperation(name: string): name is BaseOperation { return own(CARD_DEF_OPERATIONS, name) !== undefined; @@ -734,8 +757,7 @@ function notAllowed( let because = target.kind === 'instance' && kind ? `a ${kind} allows ${describeAllowed(kind)}` - : `a type carries only what its definition declares, and a "${base}" ` + - `runs against an instance`; + : `a "${base}" runs against an instance, and a type is not one`; return new OperationFailure({ id: targetId(target), status: 405, diff --git a/packages/runtime-common/card-operations/index.ts b/packages/runtime-common/card-operations/index.ts index e1ccbded150..0ed43117a97 100644 --- a/packages/runtime-common/card-operations/index.ts +++ b/packages/runtime-common/card-operations/index.ts @@ -10,6 +10,7 @@ export { runOperation, } from './dispatch.ts'; export type { + CanonicalizeOptions, OperationCore, OperationDefinitionLookup, OperationIndexQueryEngine, @@ -23,7 +24,9 @@ export { readSourceOperation } from './read-source.ts'; export { lowerQueryOperation } from './query.ts'; export type { QueryInvocation } from './query.ts'; export { + DEFINITION_FREE_BASE_OPERATIONS, OperationFailure, + isDefinitionFreeBaseOperation, isDocumentResult, isHeadResult, isIdentityResult, diff --git a/packages/runtime-common/card-operations/lowering.ts b/packages/runtime-common/card-operations/lowering.ts index 1c715644d0c..5f2dafac20b 100644 --- a/packages/runtime-common/card-operations/lowering.ts +++ b/packages/runtime-common/card-operations/lowering.ts @@ -10,6 +10,7 @@ import { paramKeysRead, usesVolatileCall, } from './bxl-emit.ts'; +import { isDefinitionFreeBaseOperation } from './types.ts'; import type { LowerOperationDeclarationsResult, OperationDefinition, @@ -134,6 +135,30 @@ export async function lowerOperationDeclarations( let issues: OperationLoweringIssue[] = []; for (let name of Object.keys(raw)) { let sink = new IssueSink(name); + if (isDefinitionFreeBaseOperation(name)) { + // The realm answers these names without reading a definition, so a + // stored operation under one would be dispatched straight past rather + // than run. The authoring decorator refuses the name; refusing it here + // too is what keeps it out of a type's entry, which outlives the code + // that built it — a definition-cache row carries no code version and is + // not re-derived until something invalidates it. + let operation: OperationDefinition = { + base: 'read', + deterministic: true, + invalid: true, + issues: [ + { + code: 'reserved-name', + operation: name, + path: name, + message: `"${name}" is a reserved operation name — the realm serves it from the bytes stored at the target's URL and reads no definition to do so`, + }, + ], + }; + operations[name] = operation; + issues.push(...operation.issues!); + continue; + } let operation = await lowerOperation(raw[name], sink, context); if (sink.issues.length > 0) { operation.invalid = true; diff --git a/packages/runtime-common/card-operations/read-source.ts b/packages/runtime-common/card-operations/read-source.ts index 86c4516140e..ea3d5de3592 100644 --- a/packages/runtime-common/card-operations/read-source.ts +++ b/packages/runtime-common/card-operations/read-source.ts @@ -16,23 +16,25 @@ import type { OperationCore, RunOperationOptions } from './dispatch.ts'; // // A stored-bytes read serves a resource exactly as it sits on disk: a card // instance's `.json`, a `.gts` or `.ts` module's text, an image's or a PDF's -// bytes. It is the third of the realm's three reads — the document one is -// `read`, assembled from the search index; this one is the representation the -// realm's two byte routes serve, the `card+source` `GET`/`HEAD` and the raw -// byte serve. +// bytes. Where `read` serves the realm's indexed view of a target, assembled +// from the search index, this serves the representation the realm's byte +// routes serve — the `card+source` `GET`/`HEAD` and the raw byte serve. // // Two things separate it from `read`, and both come from what it serves: // // * It consults no definition. `read` needs a type's definition to assemble -// a document; bytes need only a path. A module has no `adoptsFrom` and no -// definition-cache entry, so gating its source on a lookup would refuse a -// read of bytes that are plainly there. Dispatch resolves the name before -// it would reach one, and this executor asks for none. +// a document; bytes need only a path, so a definition cannot change what +// this answers. Dispatch resolves the name before it would read one — see +// `DEFINITION_FREE_OPERATIONS` for what that buys and what it rests on — +// and this executor asks for none. // // * It touches the index not at all. There is no row to peek and no -// generation to join on, so a `readSource` costs one file open and one -// file-meta row — never a search read. That is also why it can answer for -// a path the index has no row for, and for one it never will. +// generation to join on, so it costs one file open and one file-meta row +// — never a search read. That is also why it can answer for a path the +// index has no row for, and for one it never will. Where the realm has no +// recorded hash and the path is one whose validator is built from one, it +// also reads the bytes it is about to serve, to hash them; it reads them +// once and serves those. // // Two modes, as `read` has: // @@ -42,7 +44,11 @@ import type { OperationCore, RunOperationOptions } from './dispatch.ts'; // adapter's `content` untouched rather than reading and // discarding it, which matters concretely: `content` is a lazy // getter that opens a real stream on first touch, so touching it -// to throw it away would strand one. +// to throw it away would strand one. So it reports no `version` +// where the realm has none recorded: an absence a caller can act +// on, rather than a value bought with the read it asked not to +// pay. The two modes never contradict each other — one answers +// with a hash where the other answers with nothing. // // What stays outside, and what a facade routing here has to keep: // @@ -54,15 +60,20 @@ import type { OperationCore, RunOperationOptions } from './dispatch.ts'; // from what comes back here. Which validator is the facade's choice too, // and the byte routes do not make one choice: the source route builds an // `ETag` from a content hash for a `.json` or an executable extension, and -// from `lastModified` for everything else. A `Range` needs more than this -// result carries — the adapter's bounded-read capability does not travel -// through it — so a facade serving 206s holds the handle itself. +// from `lastModified` for everything else, computing no hash at all on +// that second path. `version` is populated on the same terms, so a facade +// reproducing either validator has what that route uses and nothing it +// does not. A `Range` needs more than this result carries — the adapter's +// bounded-read capability does not travel through it — so a facade serving +// 206s holds the handle itself. // -// * The pairing of the metadata with the bytes. `lastModified` is the stat -// taken when the handle opened and the body is read from it later, so a -// write landing in between pairs one with the other, exactly as it does -// for the byte routes reading the same handle. A caller that cannot -// tolerate that has to revalidate after reading rather than before. +// * The pairing of `lastModified` with the bytes. It is the stat taken when +// the handle opened, and a streamed body is read from that handle later, +// so a write landing in between pairs one with the other — exactly as it +// does for the byte routes reading the same handle. `version` is not +// exposed to that window: it either describes a file of the size this +// handle reported, or it was computed from the very bytes returned +// alongside it. // * A batching envelope over operations does not carry this one at all: // bytes do not belong in a JSON batch, and a stream cannot be one member // of one. @@ -101,24 +112,30 @@ export async function readSourceOperation( detail: `${url.href} does not exist in realm ${core.realmURL}`, }); } - // `file.path` rather than the requested path: they are the same here, since - // the facade resolved any fallback before dispatching, but the content type - // has to describe the bytes that actually arrived rather than the name they - // were asked for. - let contentType = inferContentType(file.path); - // The size travels with the request for the version so the realm can check - // its recorded hash against the very handle these bytes come from, rather - // than against whatever a later stat would see. - let meta = await core.storedFileMeta(localPath, file.size); + // Everything below describes `file.path`, not the path that was asked for. + // They are the same here, since the facade resolves any fallback before + // dispatching, but a result has to describe the bytes that actually arrived + // rather than the name they were requested under — and it has to pick one of + // the two for all of its members, or a fallback would leave the content type + // describing one file and the version another. + let servedPath = file.path; + // The handle goes with the request, not just its path: the realm checks its + // recorded hash against this handle's size, and where it has to read bytes + // to hash them it reads them from this handle and hands them back. + let meta = await core.storedFileMeta(servedPath, file, { + mayReadBytes: !opts.headersOnly, + }); let result: OperationSourceResult = { - contentType, + contentType: inferContentType(servedPath), lastModified: file.lastModified, created: meta.createdAt ?? null, version: meta.version ?? null, - size: file.size, + size: file.size ?? null, }; if (opts.headersOnly) { return result; } - return { ...result, body: file.content }; + // `meta.bytes` when the realm read them to hash them — the handle is spent + // producing those, and they are the bytes `version` describes. + return { ...result, body: meta.bytes ?? file.content }; } diff --git a/packages/runtime-common/card-operations/types.ts b/packages/runtime-common/card-operations/types.ts index ad3698c9380..b5953b8b678 100644 --- a/packages/runtime-common/card-operations/types.ts +++ b/packages/runtime-common/card-operations/types.ts @@ -168,7 +168,11 @@ export type OperationLoweringIssueCode = // A raw BXL program that does not parse. | 'invalid-program' // A declared query the realm's own query grammar refuses. - | 'invalid-query'; + | 'invalid-query' + // An operation declared under a name the realm resolves without reading a + // definition. Such a name is answered before a stored entry is consulted, so + // an operation kept under it would never run. + | 'reserved-name'; // A problem found while lowering one operation. Recorded, never thrown: // definition build is decoupled in time from the edit that introduced the @@ -208,6 +212,24 @@ export interface LowerOperationDeclarationsResult { // card authoring surface, which only loads inside a card module. export type BaseOperation = BaseOperationName; +// The base operations the realm resolves without reading a definition, which +// is also the set of names nothing may be declared under: the realm answers +// one of these before it would consult a type's entry, so an operation stored +// under the name would be dispatched straight past rather than run. +// +// Stated here because both ends of that rule need it and this module is the +// one both can reach — dispatch, which does the resolving, and lowering, which +// keeps such a name out of a stored entry. The authoring decorator enforces +// the same list from inside a card module, where it can refuse the +// declaration outright. +export const DEFINITION_FREE_BASE_OPERATIONS: readonly BaseOperation[] = [ + 'readSource', +]; + +export function isDefinitionFreeBaseOperation(name: string): boolean { + return (DEFINITION_FREE_BASE_OPERATIONS as readonly string[]).includes(name); +} + // What an operation runs against. An `instance` target is an existing card or // file, addressed by URL — the identity of a thing that already has stored // state. A `type` target names a class instead, for the operations that have @@ -286,20 +308,26 @@ export interface OperationSourceResult { // than filling it in. created: number | null; // The content hash of the stored bytes — the same identity the rest of the - // project calls `version`. It identifies these bytes and no others, which is - // what a validator needs, but it is not by itself the byte routes' `ETag`: - // the source route builds one from a content hash for a `.json` or an - // executable extension and from `lastModified` for everything else, so a - // facade reproducing those validators chooses between the two. Null only - // where the realm can neither recall nor compute a hash. + // project calls `version`. + // + // Two things a facade building a validator from it has to know. It is not by + // itself the byte routes' `ETag`: the source route builds one from a hash for + // a `.json` or an executable extension and from `lastModified` for + // everything else, and `version` is populated on those same terms, so it is + // null exactly where that route computes no hash. And `computeContentHash` + // samples above its whole-content limit, so a large file's hash covers its + // head, tail and length rather than all of it — `isSampledContentHash` tells + // one from the other, and the realm's own `ETag` joins a sampled hash with + // `lastModified` rather than trusting it alone. version: string | null; // The byte size, where the adapter knew it from the stat it already - // performed, and absent where knowing it would cost reading the bytes. A - // facade needs it for `Content-Length` and to decide whether it can offer a - // `Range` at all. The bounded-read capability itself does not travel here — - // it is a function on the adapter's handle — so a facade serving 206s reads - // from the handle rather than from this result. - size?: number; + // performed, and null where knowing it would cost reading the bytes — the + // same way the two values above report what the realm cannot say. A facade + // needs it for `Content-Length` and to decide whether it can offer a `Range` + // at all. The bounded-read capability itself does not travel here — it is a + // function on the adapter's handle — so a facade serving 206s reads from the + // handle rather than from this result. + size: number | null; // The bytes. Absent in the headers-only mode, which is the whole difference // between the two: a `HEAD` reports the metadata above and would discard // this. Whatever form the realm's file adapter produced — a string, a byte diff --git a/packages/runtime-common/realm.ts b/packages/runtime-common/realm.ts index 33e32995106..72f3c8a6629 100644 --- a/packages/runtime-common/realm.ts +++ b/packages/runtime-common/realm.ts @@ -146,6 +146,7 @@ import { } from './index.ts'; import type { OperationCore, + OperationStoredFile, OperationStoredFileMeta, } from './card-operations/dispatch.ts'; import type { FromScratchResult } from './tasks/indexer.ts'; @@ -747,6 +748,18 @@ export function ifNoneMatchMatches(headerValue: string, etag: string): boolean { .some((token) => token.trim().replace(/^W\//, '') === normalizedEtag); } +// Whether the byte routes build this path's validator from a content hash. +// `getSourceOrRedirect` caches and hashes a `.json` or an executable +// extension — a card's stored source, or a module — and takes its +// `bypassCache` path for everything else, where the `ETag` rests on +// `lastModified` and no hash is computed at all. A stored-bytes read reports +// `version` on the same terms: the value is there for the routes that build a +// validator from one, and no read of an image or a video is spent producing a +// hash the route serving it would not use. +function storedPathIsHashed(localPath: LocalPath): boolean { + return localPath.endsWith('.json') || hasExecutableExtension(localPath); +} + // Cheap helper for the source endpoint: returns the content fingerprint of the // body when the ref has already been materialized to a string or Uint8Array. // Returns undefined for stream refs (the caller falls back to lastModified). @@ -3179,8 +3192,8 @@ export class Realm { readFileAsText: async (localPath) => (await this.readFileAsText(localPath))?.content, openStoredFile: (localPath) => this.#operationStoredFile(localPath), - storedFileMeta: (localPath, observedSize) => - this.#operationStoredFileMeta(localPath, observedSize), + storedFileMeta: (localPath, file, opts) => + this.#operationStoredFileMeta(localPath, file, opts), isIgnored: (url) => this.isIgnored(url), fileMetaDocument: (localPath) => this.#operationFileMetaDocument(localPath), @@ -5535,12 +5548,19 @@ export class Realm { // Its `.json` refusal is right for metadata — a `.json` is a card's source, // not a file with metadata of its own — and wrong for this, because a card's // stored source is exactly what a stored-bytes read reads. Its `_`-prefix - // refusal does not describe the byte routes at all: the `card+source` - // GET/HEAD and the raw byte serve are both registered on `/.*` and refuse no - // name, `upsertCardSource` writes whatever path it is given, and only the - // specific registered `_` endpoints are routed away from the file handlers. - // So an `_`-prefixed file a caller stored is a file the realm serves, and - // refusing it here would make this the one read that could not reach it. + // refusal does not describe the byte routes: the `card+source` GET/HEAD is + // registered on `/.*` and refuses no name, the raw byte serve is whatever + // `fallbackHandle` reaches when the router does not claim the request, and + // `upsertCardSource` writes whatever path it is given. So an `_`-prefixed + // file a caller stored is a file those routes serve, and refusing it here + // would make this the one read that could not reach it. + // + // The prefixes the router or `handle` do claim are the exception in the + // other direction: a path under one is answered by that endpoint rather than + // from disk, so bytes stored beneath it — `_screenshot/…`, say, whose GET is + // claimed before the router — are reachable through this read and through no + // byte route. Worth knowing when a facade routes here; not worth a + // name-based refusal, which is what got this wrong in the first place. // // What is left is the path that names no file at all. `#adapter.openFile` // answers undefined for a directory and for a path that is not there, so @@ -5851,23 +5871,27 @@ export class Realm { // holding to, since a byte response routed through here pays it per request. async #operationStoredFileMeta( localPath: LocalPath, - observedSize?: number, + file: OperationStoredFile, + { mayReadBytes }: { mayReadBytes: boolean }, ): Promise { let persisted = this.#dbAdapter ? (await getFileMetaForPaths(this.#dbAdapter, this.url, [localPath])).get( localPath, ) : undefined; - let describesThisFile = + let createdAt = persisted?.createdAt; + if ( persisted?.contentHash !== undefined && - observedSize !== undefined && - persisted.contentSize === observedSize; - let version = describesThisFile ? persisted!.contentHash : undefined; - if (version === undefined) { - let fileRef = await this.#operationStoredFile(localPath); - version = fileRef ? await computeContentHashFromRef(fileRef) : undefined; - } - return { version, createdAt: persisted?.createdAt }; + file.size !== undefined && + persisted.contentSize === file.size + ) { + return { version: persisted.contentHash, createdAt }; + } + if (!mayReadBytes || !storedPathIsHashed(localPath)) { + return { createdAt }; + } + let bytes = await fileContentToBytes({ content: file.content }); + return { version: computeContentHash(bytes), createdAt, bytes }; } private async getFileMeta( diff --git a/packages/runtime-common/tests/card-operations-dispatch-test.ts b/packages/runtime-common/tests/card-operations-dispatch-test.ts index 52eb49ca3b0..2aa257a8054 100644 --- a/packages/runtime-common/tests/card-operations-dispatch-test.ts +++ b/packages/runtime-common/tests/card-operations-dispatch-test.ts @@ -63,6 +63,15 @@ interface StubOptions { // value for. storedVersions?: Record; storedCreatedAt?: Record; + // Paths for which the realm has no recorded hash but will read the bytes to + // compute one — its behavior for a path whose validator is built from a + // hash. The stub reads the handle and hands the bytes back, as the realm + // does, so the executor is held to serving those rather than the handle it + // no longer owns. + storedHashOnRead?: Record; + // An adapter that reports no `size`, which the realm cannot validate a + // recorded hash against. + sizelessAdapter?: boolean; } interface Stub { @@ -71,7 +80,11 @@ interface Stub { // What each `storedFileMeta` call was asked about, for the one part of the // version contract that is the core's: handing the realm the size of the // handle whose bytes it is returning. - metaCalls: { localPath: string; observedSize: number | undefined }[]; + metaCalls: { + localPath: string; + size: number | undefined; + mayReadBytes: boolean; + }[]; } // `getInstance` matches `i.url` / `i.file_alias`, so a row answers to the @@ -102,6 +115,8 @@ function stub(opts: StubOptions = {}): Stub { stored = {}, storedVersions = {}, storedCreatedAt = {}, + storedHashOnRead = {}, + sizelessAdapter = false, } = opts; let core: OperationCore = { @@ -213,20 +228,50 @@ function stub(opts: StubOptions = {}): Stub { return content; }, lastModified: 1699, - size: typeof content === 'string' ? content.length : content.byteLength, + ...(sizelessAdapter + ? {} + : { + size: + typeof content === 'string' + ? content.length + : content.byteLength, + }), }; }, - async storedFileMeta(localPath, observedSize) { + async storedFileMeta(localPath, file, { mayReadBytes }) { calls.push('storedFileMeta'); - // Recorded rather than acted on: deciding whether a recorded hash still - // describes the file is the realm's, so what the core owes is passing - // the size of the handle it is reading from. The realm-server suite - // holds the realm to the decision. - metaCalls.push({ localPath, observedSize }); - return { - version: storedVersions[localPath], - createdAt: storedCreatedAt[localPath], - }; + // What the core owes is the handle it is reading from and whether the + // bytes may be read; the policy over those is the realm's, and the + // realm-server suite holds the realm to it. The shape is mirrored here + // so the executor is exercised against both of the realm's answers. + metaCalls.push({ localPath, size: file.size, mayReadBytes }); + let createdAt = storedCreatedAt[localPath]; + let recorded = Object.prototype.hasOwnProperty.call( + storedVersions, + localPath, + ) + ? storedVersions[localPath] + : undefined; + if (recorded !== undefined) { + return { version: recorded, createdAt }; + } + let onRead = Object.prototype.hasOwnProperty.call( + storedHashOnRead, + localPath, + ) + ? storedHashOnRead[localPath] + : undefined; + if (onRead === undefined || !mayReadBytes) { + return { createdAt }; + } + // Reading the handle, as the realm does — which spends it, so the bytes + // come back with the version. + let content = file.content; + let bytes = + typeof content === 'string' + ? new TextEncoder().encode(content) + : (content as Uint8Array); + return { version: onRead, createdAt, bytes }; }, async isIgnored() { return false; @@ -271,6 +316,19 @@ function invoke(target: OperationTarget, name: string) { }; } +// The stub's bodies are a string or a byte array, never a stream, so one pass +// is enough — but going through a helper keeps a case from reading a body +// twice by accident, which a real single-use handle would answer with nothing. +async function textOfBody(body: unknown): Promise { + if (typeof body === 'string') { + return body; + } + if (body instanceof Uint8Array) { + return new TextDecoder().decode(body); + } + throw new Error(`expected a materialized body, got ${typeof body}`); +} + // Rethrows anything that is not an operation failure, so a genuine bug reports // itself rather than being read as the refusal under test. async function refusalFrom( @@ -921,8 +979,9 @@ const tests = Object.freeze({ [ { localPath: path, - observedSize: + size: typeof content === 'string' ? content.length : content.byteLength, + mayReadBytes: true, }, ], `${path} asks for its version against the handle being read`, @@ -1077,6 +1136,103 @@ const tests = Object.freeze({ assert.strictEqual(error.code, 'target-not-found'); }, + 'a version read from the bytes is the version of the bytes served': async ( + assert, + ) => { + // Where the realm has no hash recorded it reads the handle to compute one, + // which spends that handle. So the bytes come back with the version and + // the executor has to serve those — serving the spent handle instead would + // hand back an empty body under a hash of the real one. + let { core, calls } = stub({ + stored: { 'person-1.json': '{"data":{"type":"card"}}' }, + storedHashOnRead: { 'person-1.json': 'hash-of-read-bytes' }, + }); + let result = await runOperation( + core, + invoke({ kind: 'instance', url: `${REALM}person-1.json` }, 'readSource'), + ); + assert.true(isSourceResult(result)); + if (isSourceResult(result)) { + assert.strictEqual(result.version, 'hash-of-read-bytes'); + assert.deepEqual( + result.body, + new TextEncoder().encode('{"data":{"type":"card"}}'), + 'the body is the bytes the version was computed from', + ); + } + assert.strictEqual( + calls.filter((call) => call === 'storedContent').length, + 1, + 'and the handle is touched once in total — the realm reads it, the ' + + 'executor serves what that read produced rather than reaching for it ' + + 'again', + ); + }, + + 'a headers-only read asks for no version it would have to read bytes for': + async (assert) => { + // The mode returns no body, so it must not buy a version with a read of + // one. It reports the absence instead, which no facade can build a wrong + // validator from — and the body mode's answer for the same path is a + // hash, so the two never contradict each other. + let opts = { + stored: { 'person-1.json': '{"data":{"type":"card"}}' }, + storedHashOnRead: { 'person-1.json': 'hash-of-read-bytes' }, + }; + let target: OperationTarget = { + kind: 'instance', + url: `${REALM}person-1.json`, + }; + + let headers = await runOperation( + stub(opts).core, + invoke(target, 'readSource'), + { headersOnly: true }, + ); + assert.true(isSourceResult(headers)); + if (isSourceResult(headers)) { + assert.strictEqual( + headers.version, + null, + 'no version, rather than one bought with the read it declined', + ); + assert.strictEqual(headers.body, undefined); + } + + let bytes = await runOperation( + stub(opts).core, + invoke(target, 'readSource'), + ); + assert.true(isSourceResult(bytes)); + if (isSourceResult(bytes)) { + assert.strictEqual( + bytes.version, + 'hash-of-read-bytes', + 'while the mode that does read the bytes reports their hash', + ); + } + }, + + 'an adapter that reports no size still reads': async (assert) => { + // A recorded hash cannot be checked against a handle whose length the + // adapter never stated, so the realm treats it as undescribed. The result + // reports the size it does not have rather than omitting the member. + let { core } = stub({ + stored: { 'sample.md': '# hi' }, + storedVersions: { 'sample.md': 'recorded' }, + sizelessAdapter: true, + }); + let result = await runOperation( + core, + invoke({ kind: 'instance', url: `${REALM}sample.md` }, 'readSource'), + ); + assert.true(isSourceResult(result)); + if (isSourceResult(result)) { + assert.strictEqual(result.size, null); + assert.strictEqual(await textOfBody(result.body), '# hi'); + } + }, + 'the realm root is a directory, not a stored file': async (assert) => { // A card read resolves the realm root to the realm's index card, which is // what makes the root readable at all. A stored-bytes read addresses a From 910e5202f2ad9eebdedb5806e5b555c30933c3f1 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 20:15:59 +0000 Subject: [PATCH 07/12] Read a source version in bounded ranges rather than by streaming the file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `version` was bought with a full read of the file whenever the realm had no recorded hash to hand back — more than the byte route it claims parity with pays, since `contentHashFromMaterializedRef` hashes only content the route already holds and falls back to `lastModified` otherwise. The headers-only mode declined that read and so reported no version at all, which left the two modes disagreeing about a validator for the same file. `computeContentHash` already samples above `CONTENT_HASH_WHOLE_LIMIT_BYTES`: the value is the byte length plus a hash of the head and one of the tail. A length comes from a stat, so that value can be assembled from two bounded reads and is byte-identical to hashing the whole content — `computeContentHashFromRanges` does exactly that, and the content-hash suite holds the two forms to the same string at every size boundary and pins which ranges are asked for. The realm reads its fallback fingerprint that way, through the handle's `createRangeStream`, so hashing costs at most the whole-hash limit however large the file, and never touches `content` — which is the single-use body a full read returns and a headers-only read leaves alone. Both modes therefore report the same version at the same cost, `mayReadBytes` and the bytes-back channel are gone, and an adapter offering no bounded read simply has no version to report rather than an unbounded read taken on its behalf. A short range read means the file is no longer the one the stat described, so the fingerprint is abandoned rather than reported. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt --- .../tests/card-operations-core-test.ts | 72 +++++- .../tests/card-operations-dispatch-test.ts | 8 +- .../realm-server/tests/content-hash-test.ts | 68 ++++++ .../card-operations/dispatch.ts | 41 ++-- .../card-operations/read-source.ts | 40 ++-- packages/runtime-common/content-hash.ts | 37 +++ packages/runtime-common/index.ts | 1 + packages/runtime-common/realm.ts | 97 ++++++-- .../tests/card-operations-dispatch-test.ts | 221 +++++++++++------- 9 files changed, 426 insertions(+), 159 deletions(-) diff --git a/packages/realm-server/tests/card-operations-core-test.ts b/packages/realm-server/tests/card-operations-core-test.ts index 776bd9e0e96..71fbc6dcd1e 100644 --- a/packages/realm-server/tests/card-operations-core-test.ts +++ b/packages/realm-server/tests/card-operations-core-test.ts @@ -1,6 +1,6 @@ import QUnit from 'qunit'; const { module, test } = QUnit; -import { writeFileSync } from 'fs'; +import { rmSync, writeFileSync } from 'fs'; import { basename, join } from 'path'; import type { Test, SuperTest } from 'supertest'; import type { RealmHttpServer as Server } from '../server.ts'; @@ -10,6 +10,8 @@ import { SupportedMimeType, baseRealm, computeContentHash, + isSampledContentHash, + CONTENT_HASH_WHOLE_LIMIT_BYTES, rri, } from '@cardstack/runtime-common'; import { @@ -533,6 +535,23 @@ module(basename(import.meta.filename), function () { computeContentHash('export const second = 2; // and longer'), 'and `version` identifies those bytes rather than the recorded ones', ); + let overwrittenHeaders = sourceOf( + await runOperation( + testRealm.operationCore, + request( + { kind: 'instance', url: `${testRealmHref}notes.gts` }, + 'readSource', + ), + { headersOnly: true }, + ), + ); + assert.strictEqual( + overwrittenHeaders.version, + overwritten.version, + 'and the mode that returns no body reports the same one — a ' + + 'fingerprint is read in bounded ranges of the file rather than out ' + + 'of the body, so declining the body costs no part of it', + ); // The same overwrite on a path whose validator rests on `lastModified` // rather than a hash. The recorded hash is equally untrustworthy, and @@ -553,6 +572,49 @@ module(basename(import.meta.filename), function () { ); }); + test('a stored-bytes read fingerprints a file larger than it reads', async function (assert) { + // Above `CONTENT_HASH_WHOLE_LIMIT_BYTES` the fingerprint is the byte + // length plus a hash of the head and one of the tail, so the realm + // assembles it from two bounded reads and the stat — the same value + // hashing the whole file yields, for a read that does not grow with the + // file. Written straight to disk so there is no row to recall it from, + // and under an extension whose validator is built from a hash, which is + // what sends the read at the file in the first place. + let content = new Uint8Array(CONTENT_HASH_WHOLE_LIMIT_BYTES + 4096); + for (let i = 0; i < content.length; i++) { + content[i] = (i * 31 + 7) & 0xff; + } + writeFileSync(join(testRealmPath, 'large.json'), content); + let large = sourceOf( + await runOperation( + testRealm.operationCore, + request( + { kind: 'instance', url: `${testRealmHref}large.json` }, + 'readSource', + ), + { headersOnly: true }, + ), + ); + // Off the realm's directory before anything can fail, so nothing that + // runs after this reads a fixture this case invented. + rmSync(join(testRealmPath, 'large.json')); + assert.strictEqual( + large.size, + content.length, + 'the size comes from the stat', + ); + assert.true( + isSampledContentHash(large.version!), + 'the fingerprint samples rather than covering the whole file', + ); + assert.strictEqual( + large.version, + computeContentHash(content), + 'and it is the same value hashing the whole content produces, so one ' + + 'compares equal to the other wherever they meet', + ); + }); + test('a headers-only stored-bytes read reports the same metadata with no body', async function (assert) { let target: OperationTarget = { kind: 'instance', @@ -567,10 +629,6 @@ module(basename(import.meta.filename), function () { ); assert.strictEqual(headers.body, undefined, 'no bytes in this mode'); assert.strictEqual(headers.contentType, 'text/markdown'); - // A fixture file, so the realm has no hash recorded for it, and a `.md` - // is not a path whose validator is built from one. The mode reports the - // absence rather than reading the body it declined to return. - assert.strictEqual(headers.version, null); let withBody = sourceOf( await runOperation( @@ -582,7 +640,9 @@ module(basename(import.meta.filename), function () { assert.deepEqual( metadata, headers, - 'the two modes agree on every value a response header is computed from', + 'the two modes agree on every value a response header is computed ' + + 'from — `version` included, which is why neither mode may reach ' + + 'for the body to produce one', ); }); diff --git a/packages/realm-server/tests/card-operations-dispatch-test.ts b/packages/realm-server/tests/card-operations-dispatch-test.ts index e4363199c07..6a4ef5b6899 100644 --- a/packages/realm-server/tests/card-operations-dispatch-test.ts +++ b/packages/realm-server/tests/card-operations-dispatch-test.ts @@ -122,11 +122,15 @@ module(basename(import.meta.filename), function () { await runSharedTest(cardOperationsDispatchTests, assert, {}); }); - test('a version read from the bytes is the version of the bytes served', async function (assert) { + test('a version read from the file costs the body nothing', async function (assert) { await runSharedTest(cardOperationsDispatchTests, assert, {}); }); - test('a headers-only read asks for no version it would have to read bytes for', async function (assert) { + test('both modes report the same version, neither paying for the other', async function (assert) { + await runSharedTest(cardOperationsDispatchTests, assert, {}); + }); + + test('an adapter with no bounded read reports no version', async function (assert) { await runSharedTest(cardOperationsDispatchTests, assert, {}); }); diff --git a/packages/realm-server/tests/content-hash-test.ts b/packages/realm-server/tests/content-hash-test.ts index 9d4572c5aca..5e23b8e9c06 100644 --- a/packages/realm-server/tests/content-hash-test.ts +++ b/packages/realm-server/tests/content-hash-test.ts @@ -7,6 +7,7 @@ import { CONTENT_HASH_TAIL_BYTES, CONTENT_HASH_WHOLE_LIMIT_BYTES, computeContentHash, + computeContentHashFromRanges, isSampledContentHash, } from '@cardstack/runtime-common'; @@ -154,4 +155,71 @@ module(basename(import.meta.filename), function () { assert.notStrictEqual(flip(0), base, 'the first byte is covered'); assert.notStrictEqual(flip(len - 1), base, 'the last byte is covered'); }); + + test('the ranged form returns the same fingerprint as the whole form', async function (assert) { + // What lets a caller holding a file read a fingerprint out of it without + // streaming it, and still have that value compare equal to one computed + // and stored from whole content. A different value either way would make + // the cheap read a different fingerprint rather than a cheaper one. + for (let length of [ + 0, + 1, + 1024, + CONTENT_HASH_WHOLE_LIMIT_BYTES, + CONTENT_HASH_WHOLE_LIMIT_BYTES + 1, + OVER_LIMIT, + CONTENT_HASH_WHOLE_LIMIT_BYTES * 8, + ]) { + let content = bytes(length); + let ranged = await computeContentHashFromRanges( + length, + async (start, count) => content.subarray(start, start + count), + ); + assert.strictEqual( + ranged, + computeContentHash(content), + `${length} bytes hash the same either way`, + ); + } + }); + + test('the ranged form reads no more than the whole-hash limit', async function (assert) { + // The I/O bound, stated the same way the CPU bound is: as a property of + // which ranges are asked for rather than as a measurement. A file eight + // times the limit is read for the limit, so the read does not grow with + // the file. + let length = CONTENT_HASH_WHOLE_LIMIT_BYTES * 8; + let content = bytes(length); + let reads: { start: number; count: number }[] = []; + await computeContentHashFromRanges(length, async (start, count) => { + reads.push({ start, count }); + return content.subarray(start, start + count); + }); + assert.deepEqual( + reads, + [ + { start: 0, count: CONTENT_HASH_HEAD_BYTES }, + { + start: length - CONTENT_HASH_TAIL_BYTES, + count: CONTENT_HASH_TAIL_BYTES, + }, + ], + 'exactly the head and the tail, and nothing in between', + ); + assert.strictEqual( + reads.reduce((total, { count }) => total + count, 0), + CONTENT_HASH_WHOLE_LIMIT_BYTES, + 'which totals the limit whatever the size above it', + ); + }); + + test('empty content is hashed without a read at all', async function (assert) { + let reads = 0; + let hash = await computeContentHashFromRanges(0, async () => { + reads++; + return new Uint8Array(); + }); + assert.strictEqual(hash, computeContentHash(new Uint8Array())); + assert.strictEqual(reads, 0, 'there are no bytes to ask for'); + }); }); diff --git a/packages/runtime-common/card-operations/dispatch.ts b/packages/runtime-common/card-operations/dispatch.ts index 348bbef7983..1aff014f528 100644 --- a/packages/runtime-common/card-operations/dispatch.ts +++ b/packages/runtime-common/card-operations/dispatch.ts @@ -1,3 +1,4 @@ +import type { Readable } from 'stream'; import { RealmPaths, type LocalPath } from '../paths.ts'; import { urlNamesFile } from '../file-def-code-ref.ts'; import { readOperation } from './read.ts'; @@ -69,26 +70,21 @@ export interface OperationCore { localPath: LocalPath, ): Promise; // The version and creation time of the bytes at `file`, resolved by the - // realm because it owns both its file-meta row and the policy for when a - // content hash is worth reading bytes to get. + // realm because it owns both its file-meta row and the policy for which + // paths carry a content hash at all. // - // `file` is the handle the caller will serve from, not just its path: the + // `file` is the handle the caller will serve from, not just its path. The // realm validates its recorded hash against that handle's size, since a // `version` identifying bytes other than the ones it is returned with is - // what a conditional GET would build a wrong validator from. Where the - // realm has to read the bytes to hash them it reads them from this handle - // and returns them as `bytes`, so one open serves both the hash and the - // body and the two cannot describe different files. - // - // `mayReadBytes` is false for a headers-only read, which returns no body and - // so must leave the handle's `content` untouched. The realm then answers - // from its row alone and reports no version where it has none recorded — - // an absence a caller can act on, rather than a value bought with a read it - // asked not to pay. + // what a conditional GET would build a wrong validator from, and where it + // has to read the file to fingerprint it, it reads bounded ranges of that + // same handle. What it never reads is the handle's `content`: that is the + // single-use body a full read returns and a headers-only read leaves + // untouched, so both modes reach the same version at the same cost and + // neither spends the other's. storedFileMeta( localPath: LocalPath, file: OperationStoredFile, - opts: { mayReadBytes: boolean }, ): Promise; // Whether the realm's ignore rules exclude this URL. An ignored path is // never visited, so no amount of waiting produces an index row for it. @@ -137,20 +133,23 @@ export interface OperationStoredFile { // The byte size where the adapter knows it from the stat it already // performed, and absent where knowing it would cost reading the bytes. size?: number; + // A bounded read of `[start, end]`, both inclusive, present where the + // adapter can serve one without materializing the rest. It is what lets a + // version be read from a file without streaming it, so an adapter that + // offers none simply has no version to report for a path the realm holds no + // record of. + createRangeStream?: ( + start: number, + end: number, + ) => ReadableStream | Readable; } export interface OperationStoredFileMeta { // The content hash of the stored bytes, absent where the realm has none - // recorded and reading the bytes to compute one is not warranted for this - // path. + // recorded and this is not a path whose validator is built from one. version?: string; // Epoch seconds, absent where the realm holds no record of this path. createdAt?: number; - // The bytes, present exactly when the realm read them to hash them. Serving - // these rather than the handle's own `content` is what makes `version` - // describe the body it is returned with — and the handle has been consumed - // producing them, so a caller that has these must serve these. - bytes?: Uint8Array; } // `CachingDefinitionLookup`, narrowed to the one read an operation makes. diff --git a/packages/runtime-common/card-operations/read-source.ts b/packages/runtime-common/card-operations/read-source.ts index ea3d5de3592..415868d9fe9 100644 --- a/packages/runtime-common/card-operations/read-source.ts +++ b/packages/runtime-common/card-operations/read-source.ts @@ -32,9 +32,9 @@ import type { OperationCore, RunOperationOptions } from './dispatch.ts'; // generation to join on, so it costs one file open and one file-meta row // — never a search read. That is also why it can answer for a path the // index has no row for, and for one it never will. Where the realm has no -// recorded hash and the path is one whose validator is built from one, it -// also reads the bytes it is about to serve, to hash them; it reads them -// once and serves those. +// recorded hash for a path whose validator is built from one, it reads +// the file to fingerprint it, in ranges bounded by the fingerprint's own +// shape rather than by the file's size. // // Two modes, as `read` has: // @@ -44,11 +44,12 @@ import type { OperationCore, RunOperationOptions } from './dispatch.ts'; // adapter's `content` untouched rather than reading and // discarding it, which matters concretely: `content` is a lazy // getter that opens a real stream on first touch, so touching it -// to throw it away would strand one. So it reports no `version` -// where the realm has none recorded: an absence a caller can act -// on, rather than a value bought with the read it asked not to -// pay. The two modes never contradict each other — one answers -// with a hash where the other answers with nothing. +// to throw it away would strand one. Every value it reports is +// therefore identical to the one the other mode reports for the +// same handle, `version` included — a fingerprint is read in +// bounded ranges of its own rather than out of the body, so +// neither mode pays for the other's and the two cannot +// disagree about what a validator identifies. // // What stays outside, and what a facade routing here has to keep: // @@ -70,10 +71,11 @@ import type { OperationCore, RunOperationOptions } from './dispatch.ts'; // * The pairing of `lastModified` with the bytes. It is the stat taken when // the handle opened, and a streamed body is read from that handle later, // so a write landing in between pairs one with the other — exactly as it -// does for the byte routes reading the same handle. `version` is not -// exposed to that window: it either describes a file of the size this -// handle reported, or it was computed from the very bytes returned -// alongside it. +// does for the byte routes reading the same handle. `version` sits in the +// same window and narrows it the two ways it can: a recorded hash is +// reported only for a file of the size this handle stat'd, and a +// fingerprint read from the file is abandoned rather than reported if the +// ranges it reads no longer add up to that size. // * A batching envelope over operations does not carry this one at all: // bytes do not belong in a JSON batch, and a stream cannot be one member // of one. @@ -120,11 +122,9 @@ export async function readSourceOperation( // describing one file and the version another. let servedPath = file.path; // The handle goes with the request, not just its path: the realm checks its - // recorded hash against this handle's size, and where it has to read bytes - // to hash them it reads them from this handle and hands them back. - let meta = await core.storedFileMeta(servedPath, file, { - mayReadBytes: !opts.headersOnly, - }); + // recorded hash against this handle's size, and reads bounded ranges of this + // handle where it has to fingerprint the file itself. + let meta = await core.storedFileMeta(servedPath, file); let result: OperationSourceResult = { contentType: inferContentType(servedPath), lastModified: file.lastModified, @@ -135,7 +135,7 @@ export async function readSourceOperation( if (opts.headersOnly) { return result; } - // `meta.bytes` when the realm read them to hash them — the handle is spent - // producing those, and they are the bytes `version` describes. - return { ...result, body: meta.bytes ?? file.content }; + // The first and only touch of `content`, which is where a streaming adapter + // opens its stream. + return { ...result, body: file.content }; } diff --git a/packages/runtime-common/content-hash.ts b/packages/runtime-common/content-hash.ts index 0a04e4826e9..b89b21d2add 100644 --- a/packages/runtime-common/content-hash.ts +++ b/packages/runtime-common/content-hash.ts @@ -68,3 +68,40 @@ export function computeContentHash(content: string | Uint8Array): string { export function isSampledContentHash(contentHash: string): boolean { return contentHash.startsWith(`${SAMPLED_MARKER}:`); } + +// The same fingerprint as `computeContentHash`, assembled from bounded reads +// instead of from the whole content. +// +// Above the limit the value is a function of three things — the byte length, +// the head and the tail — and a length comes from a stat rather than a read, +// so nothing above the limit needs the middle to produce an identical value. +// That gives the sampling an I/O bound to match the CPU bound it already has: +// `readRange` is asked for at most `CONTENT_HASH_WHOLE_LIMIT_BYTES` whatever +// the file's size, and for exactly the head and tail the whole-content path +// hashes, so the two forms return the same string for the same content. A +// caller can reach for whichever is cheaper where it stands without the two +// ever disagreeing about what a fingerprint identifies, which is what lets a +// value computed one way validate against one stored the other. +// +// `size` is the caller's own measure of the content, and above the limit it is +// part of the value. A `readRange` that answers from content of a different +// length than `size` describes is reading something else by then, so a caller +// that cannot pin the two to one snapshot has to treat a short read as a +// failure rather than as bytes. +export async function computeContentHashFromRanges( + size: number, + readRange: (start: number, length: number) => Promise, +): Promise { + if (size <= 0) { + // Empty content has one hash and no bytes to read for it. + return md5(new Uint8Array()); + } + if (size <= CONTENT_HASH_WHOLE_LIMIT_BYTES) { + return md5(await readRange(0, size)); + } + let head = md5(await readRange(0, CONTENT_HASH_HEAD_BYTES)); + let tail = md5( + await readRange(size - CONTENT_HASH_TAIL_BYTES, CONTENT_HASH_TAIL_BYTES), + ); + return `${SAMPLED_MARKER}:${size}:${head}:${tail}`; +} diff --git a/packages/runtime-common/index.ts b/packages/runtime-common/index.ts index d9b29814dc0..fe230a91c73 100644 --- a/packages/runtime-common/index.ts +++ b/packages/runtime-common/index.ts @@ -1259,6 +1259,7 @@ export { } from './write-size-validation.ts'; export { computeContentHash, + computeContentHashFromRanges, isSampledContentHash, CONTENT_HASH_WHOLE_LIMIT_BYTES, CONTENT_HASH_HEAD_BYTES, diff --git a/packages/runtime-common/realm.ts b/packages/runtime-common/realm.ts index 72f3c8a6629..5a9ebc1ad23 100644 --- a/packages/runtime-common/realm.ts +++ b/packages/runtime-common/realm.ts @@ -241,7 +241,11 @@ import { fileSizeLimitFor, validateWriteSize, } from './write-size-validation.ts'; -import { computeContentHash, isSampledContentHash } from './content-hash.ts'; +import { + computeContentHash, + computeContentHashFromRanges, + isSampledContentHash, +} from './content-hash.ts'; import { resolveFileDefCodeRef, urlNamesFile } from './file-def-code-ref.ts'; import type { Utils } from './matrix-backend-authentication.ts'; @@ -753,13 +757,64 @@ export function ifNoneMatchMatches(headerValue: string, etag: string): boolean { // extension — a card's stored source, or a module — and takes its // `bypassCache` path for everything else, where the `ETag` rests on // `lastModified` and no hash is computed at all. A stored-bytes read reports -// `version` on the same terms: the value is there for the routes that build a -// validator from one, and no read of an image or a video is spent producing a -// hash the route serving it would not use. +// `version` on the same terms, so a facade reproducing either validator has +// what that route uses and spends nothing on a value the route serving an +// image or a video would not look at. function storedPathIsHashed(localPath: LocalPath): boolean { return localPath.endsWith('.json') || hasExecutableExtension(localPath); } +// A handle's content fingerprint, read in bounded ranges rather than by +// streaming the file. +// +// The cost is bounded by the fingerprint's own shape: +// `computeContentHashFromRanges` asks for the whole content up to +// `CONTENT_HASH_WHOLE_LIMIT_BYTES` and for a fixed head and tail above it, so +// no file costs more than that limit to hash however large it is, and the +// value is the same one hashing the whole content would produce. +// +// Nothing here touches `content`. That keeps this off the handle a body is +// served from, which matters twice: `content` is a lazy getter on every +// streaming adapter, so reading it here would strand a stream for a +// headers-only read, and it is single-use, so it would take the bytes a full +// read is about to return. Both modes therefore reach the same fingerprint at +// the same cost, and neither pays for the other's. +// +// Undefined rather than an unbounded read where the adapter cannot serve a +// range or does not know the size without reading the bytes: a bounded read +// is worth a validator and an unbounded one is not, so an adapter declares +// the bounded read by implementing `createRangeStream` and the value is +// simply absent for one that does not. +async function contentHashFromRanges( + file: Pick, +): Promise { + let { size, createRangeStream } = file; + if (size === undefined || !createRangeStream) { + return undefined; + } + try { + return await computeContentHashFromRanges(size, async (start, length) => { + let bytes = await fileContentToBytes({ + // `createRangeStream` bounds are inclusive on both ends. + content: createRangeStream(start, start + length - 1), + }); + if (bytes.length !== length) { + // The file is no longer the one `size` describes, and a fingerprint + // built from a file that moved under the read identifies neither + // version of it. + throw new Error( + `read ${bytes.length} of ${length} bytes at offset ${start}: content changed while hashing`, + ); + } + return bytes; + }); + } catch { + // Every consumer of a version handles its absence, so a failed read costs + // the validator rather than the response the bytes are for. + return undefined; + } +} + // Cheap helper for the source endpoint: returns the content fingerprint of the // body when the ref has already been materialized to a string or Uint8Array. // Returns undefined for stream refs (the caller falls back to lastModified). @@ -3192,8 +3247,8 @@ export class Realm { readFileAsText: async (localPath) => (await this.readFileAsText(localPath))?.content, openStoredFile: (localPath) => this.#operationStoredFile(localPath), - storedFileMeta: (localPath, file, opts) => - this.#operationStoredFileMeta(localPath, file, opts), + storedFileMeta: (localPath, file) => + this.#operationStoredFileMeta(localPath, file), isIgnored: (url) => this.isIgnored(url), fileMetaDocument: (localPath) => this.#operationFileMetaDocument(localPath), @@ -5851,28 +5906,25 @@ export class Realm { // disk. `persistFileMeta` is reached from the realm's own write path and // nowhere else, so a file overwritten out of band (a deploy rsync, an // operator editing the volume) keeps a row describing bytes that are gone. - // Handing that hash back would be worse than computing one slowly: it is - // what a conditional GET builds its validator from, so a stale one answers - // 304 for content that changed. + // Handing that hash back would be worse than computing one: it is what a + // conditional GET builds its validator from, so a stale one answers 304 for + // content that changed. // // So the row is trusted only where the length it recorded matches the handle - // the caller is reading from — `observedSize`, taken from the adapter's stat - // rather than from the bytes — and the bytes are hashed otherwise. An - // out-of-band overwrite that preserves the exact byte length is the one case - // this does not catch; closing it needs either an unconditional hash on - // every read or an mtime on the row, and which of those the byte facade - // wants is its call to make. + // the caller is reading from — that handle's `size`, taken from the + // adapter's stat rather than from its bytes — and the fingerprint is read + // from the file otherwise, in the bounded ranges `contentHashFromRanges` + // describes. An out-of-band overwrite that preserves the exact byte length + // is the one case the length check does not catch; closing it needs an mtime + // on the row to validate against, and whether the byte facade wants that is + // its call to make. // - // The hash is computed from its own handle, so it never consumes the one a - // read is serving from, and it costs a full read of the file when it is - // reached (`computeContentHash` samples large content, but only after - // materializing it). Both values come from one row, so this is one query on + // Both values come from one row, so this is one query on // `realm_file_meta`'s primary key rather than a lookup per value — worth // holding to, since a byte response routed through here pays it per request. async #operationStoredFileMeta( localPath: LocalPath, file: OperationStoredFile, - { mayReadBytes }: { mayReadBytes: boolean }, ): Promise { let persisted = this.#dbAdapter ? (await getFileMetaForPaths(this.#dbAdapter, this.url, [localPath])).get( @@ -5887,11 +5939,10 @@ export class Realm { ) { return { version: persisted.contentHash, createdAt }; } - if (!mayReadBytes || !storedPathIsHashed(localPath)) { + if (!storedPathIsHashed(localPath)) { return { createdAt }; } - let bytes = await fileContentToBytes({ content: file.content }); - return { version: computeContentHash(bytes), createdAt, bytes }; + return { version: await contentHashFromRanges(file), createdAt }; } private async getFileMeta( diff --git a/packages/runtime-common/tests/card-operations-dispatch-test.ts b/packages/runtime-common/tests/card-operations-dispatch-test.ts index 2aa257a8054..d92ab0eddb7 100644 --- a/packages/runtime-common/tests/card-operations-dispatch-test.ts +++ b/packages/runtime-common/tests/card-operations-dispatch-test.ts @@ -10,6 +10,7 @@ import { type OperationError, type OperationTarget, } from '../card-operations/index.ts'; +import { fileContentToBytes } from '../stream.ts'; import type { CodeRef } from '../code-ref.ts'; import type { Definition } from '../definitions.ts'; import type { SharedTests } from '../helpers/index.ts'; @@ -63,15 +64,18 @@ interface StubOptions { // value for. storedVersions?: Record; storedCreatedAt?: Record; - // Paths for which the realm has no recorded hash but will read the bytes to - // compute one — its behavior for a path whose validator is built from a - // hash. The stub reads the handle and hands the bytes back, as the realm - // does, so the executor is held to serving those rather than the handle it - // no longer owns. - storedHashOnRead?: Record; + // Paths for which the realm has no recorded hash but reads one out of the + // file — its behavior for a path whose validator is built from a hash. The + // stub reads it through the handle's bounded range and never through + // `content`, as the realm does, so a case can hold both modes to the same + // version without either spending the body the other returns. + storedRangeHash?: Record; // An adapter that reports no `size`, which the realm cannot validate a // recorded hash against. sizelessAdapter?: boolean; + // An adapter offering no bounded read, which leaves the realm no way to + // fingerprint an unrecorded file short of streaming the whole of it. + rangelessAdapter?: boolean; } interface Stub { @@ -83,7 +87,6 @@ interface Stub { metaCalls: { localPath: string; size: number | undefined; - mayReadBytes: boolean; }[]; } @@ -115,8 +118,9 @@ function stub(opts: StubOptions = {}): Stub { stored = {}, storedVersions = {}, storedCreatedAt = {}, - storedHashOnRead = {}, + storedRangeHash = {}, sizelessAdapter = false, + rangelessAdapter = false, } = opts; let core: OperationCore = { @@ -218,6 +222,10 @@ function stub(opts: StubOptions = {}): Stub { if (content === undefined) { return undefined; } + let bytes = + typeof content === 'string' + ? new TextEncoder().encode(content) + : content; return { path: localPath, // A getter, as every streaming adapter's is, and it records the touch: @@ -228,23 +236,34 @@ function stub(opts: StubOptions = {}): Stub { return content; }, lastModified: 1699, - ...(sizelessAdapter + ...(sizelessAdapter ? {} : { size: bytes.byteLength }), + // A separate read of a bounded extent, which is what lets a version + // be read without spending `content`. Recorded on its own, so a case + // can tell a fingerprint read from the file apart from one recalled + // from a row. + ...(rangelessAdapter ? {} : { - size: - typeof content === 'string' - ? content.length - : content.byteLength, + createRangeStream: (start: number, end: number) => { + calls.push('storedRangeRead'); + let extent = bytes.subarray(start, end + 1); + return new ReadableStream({ + start(controller) { + controller.enqueue(extent); + controller.close(); + }, + }); + }, }), }; }, - async storedFileMeta(localPath, file, { mayReadBytes }) { + async storedFileMeta(localPath, file) { calls.push('storedFileMeta'); - // What the core owes is the handle it is reading from and whether the - // bytes may be read; the policy over those is the realm's, and the + // What the core owes is the handle whose bytes it is returning, not just + // that handle's path; the policy over it is the realm's, and the // realm-server suite holds the realm to it. The shape is mirrored here - // so the executor is exercised against both of the realm's answers. - metaCalls.push({ localPath, size: file.size, mayReadBytes }); + // so the executor is exercised against each of the realm's answers. + metaCalls.push({ localPath, size: file.size }); let createdAt = storedCreatedAt[localPath]; let recorded = Object.prototype.hasOwnProperty.call( storedVersions, @@ -255,23 +274,23 @@ function stub(opts: StubOptions = {}): Stub { if (recorded !== undefined) { return { version: recorded, createdAt }; } - let onRead = Object.prototype.hasOwnProperty.call( - storedHashOnRead, + let fromRanges = Object.prototype.hasOwnProperty.call( + storedRangeHash, localPath, ) - ? storedHashOnRead[localPath] + ? storedRangeHash[localPath] : undefined; - if (onRead === undefined || !mayReadBytes) { + if (fromRanges === undefined || !file.createRangeStream || !file.size) { + // No record and nothing to read one out of — the absence a facade + // falls back from, rather than an unbounded read of the file. return { createdAt }; } - // Reading the handle, as the realm does — which spends it, so the bytes - // come back with the version. - let content = file.content; - let bytes = - typeof content === 'string' - ? new TextEncoder().encode(content) - : (content as Uint8Array); - return { version: onRead, createdAt, bytes }; + // Read out of the file, as the realm does: through a bounded extent of + // the handle, leaving `content` for the body. + await fileContentToBytes({ + content: file.createRangeStream(0, file.size - 1), + }); + return { version: fromRanges, createdAt }; }, async isIgnored() { return false; @@ -981,7 +1000,6 @@ const tests = Object.freeze({ localPath: path, size: typeof content === 'string' ? content.length : content.byteLength, - mayReadBytes: true, }, ], `${path} asks for its version against the handle being read`, @@ -1136,16 +1154,14 @@ const tests = Object.freeze({ assert.strictEqual(error.code, 'target-not-found'); }, - 'a version read from the bytes is the version of the bytes served': async ( - assert, - ) => { - // Where the realm has no hash recorded it reads the handle to compute one, - // which spends that handle. So the bytes come back with the version and - // the executor has to serve those — serving the spent handle instead would - // hand back an empty body under a hash of the real one. + 'a version read from the file costs the body nothing': async (assert) => { + // Where the realm has no hash recorded it reads one out of the file. What + // it must not read it out of is the body: `content` is single-use, so a + // version taken from there would hand back an empty body under a hash of + // the real one. let { core, calls } = stub({ stored: { 'person-1.json': '{"data":{"type":"card"}}' }, - storedHashOnRead: { 'person-1.json': 'hash-of-read-bytes' }, + storedRangeHash: { 'person-1.json': 'hash-from-ranges' }, }); let result = await runOperation( core, @@ -1153,65 +1169,96 @@ const tests = Object.freeze({ ); assert.true(isSourceResult(result)); if (isSourceResult(result)) { - assert.strictEqual(result.version, 'hash-of-read-bytes'); - assert.deepEqual( - result.body, - new TextEncoder().encode('{"data":{"type":"card"}}'), - 'the body is the bytes the version was computed from', + assert.strictEqual(result.version, 'hash-from-ranges'); + assert.strictEqual( + await textOfBody(result.body), + '{"data":{"type":"card"}}', + 'the whole body is still there to serve', ); } + assert.true( + calls.includes('storedRangeRead'), + 'the version came out of a bounded read of the file', + ); assert.strictEqual( calls.filter((call) => call === 'storedContent').length, 1, - 'and the handle is touched once in total — the realm reads it, the ' + - 'executor serves what that read produced rather than reaching for it ' + - 'again', + 'and `content` is touched exactly once — by the executor, for the body', ); }, - 'a headers-only read asks for no version it would have to read bytes for': - async (assert) => { - // The mode returns no body, so it must not buy a version with a read of - // one. It reports the absence instead, which no facade can build a wrong - // validator from — and the body mode's answer for the same path is a - // hash, so the two never contradict each other. - let opts = { - stored: { 'person-1.json': '{"data":{"type":"card"}}' }, - storedHashOnRead: { 'person-1.json': 'hash-of-read-bytes' }, - }; - let target: OperationTarget = { - kind: 'instance', - url: `${REALM}person-1.json`, - }; + 'both modes report the same version, neither paying for the other': async ( + assert, + ) => { + // A `HEAD` and a conditional `GET` decide on the same file, so a version + // that appeared in one and not the other would leave a facade holding two + // answers for one validator. Reading it out of a bounded extent rather + // than out of the body is what lets the headers-only mode report the same + // hash while still touching no stream it would have to discard. + let opts = { + stored: { 'person-1.json': '{"data":{"type":"card"}}' }, + storedRangeHash: { 'person-1.json': 'hash-from-ranges' }, + }; + let target: OperationTarget = { + kind: 'instance', + url: `${REALM}person-1.json`, + }; - let headers = await runOperation( - stub(opts).core, - invoke(target, 'readSource'), - { headersOnly: true }, - ); - assert.true(isSourceResult(headers)); - if (isSourceResult(headers)) { - assert.strictEqual( - headers.version, - null, - 'no version, rather than one bought with the read it declined', - ); - assert.strictEqual(headers.body, undefined); - } + let { core, calls } = stub(opts); + let headers = await runOperation(core, invoke(target, 'readSource'), { + headersOnly: true, + }); + assert.true(isSourceResult(headers)); + if (isSourceResult(headers)) { + assert.strictEqual(headers.version, 'hash-from-ranges'); + assert.strictEqual(headers.body, undefined); + } + assert.false( + calls.includes('storedContent'), + 'and it got there without touching the bytes it is not returning', + ); - let bytes = await runOperation( - stub(opts).core, - invoke(target, 'readSource'), + let bytes = await runOperation( + stub(opts).core, + invoke(target, 'readSource'), + ); + assert.true(isSourceResult(bytes)); + if (isSourceResult(bytes) && isSourceResult(headers)) { + let { body: _body, ...metadata } = bytes; + assert.deepEqual(metadata, headers, 'every value agrees, version too'); + } + }, + + 'an adapter with no bounded read reports no version': async (assert) => { + // A version is worth a bounded read of a file and is not worth an + // unbounded one, so an adapter that cannot serve a range has none to give + // for a path the realm holds no record of. The absence is a facade's cue + // to fall back to the modification time, which is there either way. + let { core, calls } = stub({ + stored: { 'person-1.json': '{"data":{"type":"card"}}' }, + storedRangeHash: { 'person-1.json': 'hash-from-ranges' }, + rangelessAdapter: true, + }); + let result = await runOperation( + core, + invoke({ kind: 'instance', url: `${REALM}person-1.json` }, 'readSource'), + ); + assert.true(isSourceResult(result)); + if (isSourceResult(result)) { + assert.strictEqual(result.version, null); + assert.strictEqual(result.lastModified, 1699); + assert.strictEqual( + await textOfBody(result.body), + '{"data":{"type":"card"}}', + 'and the bytes read as they always did', ); - assert.true(isSourceResult(bytes)); - if (isSourceResult(bytes)) { - assert.strictEqual( - bytes.version, - 'hash-of-read-bytes', - 'while the mode that does read the bytes reports their hash', - ); - } - }, + } + assert.strictEqual( + calls.filter((call) => call === 'storedContent').length, + 1, + 'the body is read once, and nothing is read twice for a hash', + ); + }, 'an adapter that reports no size still reads': async (assert) => { // A recorded hash cannot be checked against a handle whose length the From 4ae5b61dbb07cc8ac898198d8e55d844c0572482 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 20:17:15 +0000 Subject: [PATCH 08/12] State the definition-free branch's basis by reference rather than restating it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The branch comment claimed a module path's definition lookup cannot succeed. A `.gts` resolves to the file def its extension names, which has a cache entry, so the lookup does succeed — and the constant's own comment says so. Point at that one justification instead of carrying a second, wrong one. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt --- packages/runtime-common/card-operations/dispatch.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/runtime-common/card-operations/dispatch.ts b/packages/runtime-common/card-operations/dispatch.ts index 1aff014f528..33ed210ff94 100644 --- a/packages/runtime-common/card-operations/dispatch.ts +++ b/packages/runtime-common/card-operations/dispatch.ts @@ -333,11 +333,11 @@ export async function resolveOperation( ): Promise { assertInRealm(core, target); if (isDefinitionFreeOperation(name)) { - // Before the lookup, not merely without it: for a module path the lookup - // cannot succeed, and reaching it at all would gate a read of bytes on a - // definition. The kind still decides whether the target carries the - // operation, so it comes from the target alone — which is where an - // instance target's kind comes from anyway. + // Before the lookup, not merely without it — see + // `DEFINITION_FREE_OPERATIONS` for what that rests on. The kind still + // decides whether the target carries the operation, so it comes from the + // target alone, which is where an instance target's kind comes from + // anyway. let kind = definitionFreeKind(target); if (!kind || !own(ALLOWED_BASE_OPERATIONS[kind], name)) { throw notAllowed(target, name, kind, name); From 97260cb3965d2feb481605fae8a8f8a9e2de715c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 21:19:09 +0000 Subject: [PATCH 09/12] Fingerprint every stored path, not only the ones a route validates on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The version was withheld for paths outside `.json` and the executable extensions, on the grounds that the source route builds those paths' ETag from `lastModified` and computes no hash — so reading one was a cost the serving route would not pay. Reading a fingerprint is now bounded by the hash's own shape rather than by the file's size, so that cost argument no longer holds, and what the gate withholds is a content identity for exactly the large media a caching facade would most want a strong validator for. Which validator a route builds stays that route's own choice; the result carries both members for either. An absent version now means only that the realm could neither recall one nor read one within a bounded cost — an adapter with no bounded read, or none that knows a size without reading the bytes. The out-of-band overwrite test now covers a module and a `.md` as the two sides of the line the byte routes draw, asserting both report the new bytes' fingerprint, since what a stale row means does not depend on the extension it sits under. The large-file case moves to an extension the source route would validate on `lastModified`, which is the case the gate skipped. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt --- .../tests/card-operations-core-test.ts | 33 +++++++++++-------- .../card-operations/dispatch.ts | 8 ++--- .../card-operations/read-source.ts | 17 +++++----- packages/runtime-common/realm.ts | 23 +++++-------- .../tests/card-operations-dispatch-test.ts | 17 +++++----- 5 files changed, 48 insertions(+), 50 deletions(-) diff --git a/packages/realm-server/tests/card-operations-core-test.ts b/packages/realm-server/tests/card-operations-core-test.ts index 71fbc6dcd1e..040fd671109 100644 --- a/packages/realm-server/tests/card-operations-core-test.ts +++ b/packages/realm-server/tests/card-operations-core-test.ts @@ -497,9 +497,10 @@ module(basename(import.meta.filename), function () { // gone, and reporting that hash would let a conditional GET answer 304 // for content that changed. // - // A module, so this is also a path whose validator the byte routes build - // from a content hash — which is what makes recomputing one warranted - // when the row cannot be trusted. + // Run against a module and against a `.md`, the two sides of the line + // the byte routes draw for their own validators, since a stored-bytes + // read draws none: what a stale row means does not depend on the + // extension it sits under. let read = async (localPath: string) => sourceOf( await runOperation( @@ -553,10 +554,11 @@ module(basename(import.meta.filename), function () { 'of the body, so declining the body costs no part of it', ); - // The same overwrite on a path whose validator rests on `lastModified` - // rather than a hash. The recorded hash is equally untrustworthy, and - // reading an image or a video to replace it is a cost the route serving - // it would not pay — so the answer is the absence, never the stale hash. + // The same overwrite on a path the source route would validate on + // `lastModified` rather than on a hash. The recorded hash is equally + // untrustworthy here, and the read that replaces it is equally bounded, + // so this answers with the new bytes' fingerprint too — which route asks + // for one is the facade's business, not this read's. await testRealm.write('notes.md', '# first'); writeFileSync(join(testRealmPath, 'notes.md'), '# second, and longer'); let unhashed = await read('notes.md'); @@ -567,8 +569,8 @@ module(basename(import.meta.filename), function () { ); assert.strictEqual( unhashed.version, - null, - 'and no version is reported rather than one that describes other bytes', + computeContentHash('# second, and longer'), + 'and `version` identifies them rather than the recorded bytes', ); }); @@ -578,18 +580,21 @@ module(basename(import.meta.filename), function () { // assembles it from two bounded reads and the stat — the same value // hashing the whole file yields, for a read that does not grow with the // file. Written straight to disk so there is no row to recall it from, - // and under an extension whose validator is built from a hash, which is - // what sends the read at the file in the first place. + // and under an extension the source route would validate on + // `lastModified`: the bounded read is what makes fingerprinting every + // path affordable, so this is the case that would be tempting to skip + // and is exactly the one a large media serve wants a strong validator + // for. let content = new Uint8Array(CONTENT_HASH_WHOLE_LIMIT_BYTES + 4096); for (let i = 0; i < content.length; i++) { content[i] = (i * 31 + 7) & 0xff; } - writeFileSync(join(testRealmPath, 'large.json'), content); + writeFileSync(join(testRealmPath, 'large.bin'), content); let large = sourceOf( await runOperation( testRealm.operationCore, request( - { kind: 'instance', url: `${testRealmHref}large.json` }, + { kind: 'instance', url: `${testRealmHref}large.bin` }, 'readSource', ), { headersOnly: true }, @@ -597,7 +602,7 @@ module(basename(import.meta.filename), function () { ); // Off the realm's directory before anything can fail, so nothing that // runs after this reads a fixture this case invented. - rmSync(join(testRealmPath, 'large.json')); + rmSync(join(testRealmPath, 'large.bin')); assert.strictEqual( large.size, content.length, diff --git a/packages/runtime-common/card-operations/dispatch.ts b/packages/runtime-common/card-operations/dispatch.ts index 33ed210ff94..0caa1bc76bb 100644 --- a/packages/runtime-common/card-operations/dispatch.ts +++ b/packages/runtime-common/card-operations/dispatch.ts @@ -70,8 +70,8 @@ export interface OperationCore { localPath: LocalPath, ): Promise; // The version and creation time of the bytes at `file`, resolved by the - // realm because it owns both its file-meta row and the policy for which - // paths carry a content hash at all. + // realm because it owns both its file-meta row and how a fingerprint is + // reached for a path the row does not describe. // // `file` is the handle the caller will serve from, not just its path. The // realm validates its recorded hash against that handle's size, since a @@ -145,8 +145,8 @@ export interface OperationStoredFile { } export interface OperationStoredFileMeta { - // The content hash of the stored bytes, absent where the realm has none - // recorded and this is not a path whose validator is built from one. + // The content hash of the stored bytes, absent only where the realm can + // neither recall one nor read one within a bounded cost. version?: string; // Epoch seconds, absent where the realm holds no record of this path. createdAt?: number; diff --git a/packages/runtime-common/card-operations/read-source.ts b/packages/runtime-common/card-operations/read-source.ts index 415868d9fe9..3aef6def65f 100644 --- a/packages/runtime-common/card-operations/read-source.ts +++ b/packages/runtime-common/card-operations/read-source.ts @@ -32,9 +32,9 @@ import type { OperationCore, RunOperationOptions } from './dispatch.ts'; // generation to join on, so it costs one file open and one file-meta row // — never a search read. That is also why it can answer for a path the // index has no row for, and for one it never will. Where the realm has no -// recorded hash for a path whose validator is built from one, it reads -// the file to fingerprint it, in ranges bounded by the fingerprint's own -// shape rather than by the file's size. +// recorded hash it reads the file to fingerprint it, in ranges bounded by +// the fingerprint's own shape rather than by the file's size — so this +// answers for a path of any size at a cost that does not grow with it. // // Two modes, as `read` has: // @@ -62,11 +62,12 @@ import type { OperationCore, RunOperationOptions } from './dispatch.ts'; // and the byte routes do not make one choice: the source route builds an // `ETag` from a content hash for a `.json` or an executable extension, and // from `lastModified` for everything else, computing no hash at all on -// that second path. `version` is populated on the same terms, so a facade -// reproducing either validator has what that route uses and nothing it -// does not. A `Range` needs more than this result carries — the adapter's -// bounded-read capability does not travel through it — so a facade serving -// 206s holds the handle itself. +// that second path. Both members are here for either choice — a content +// identity for every path, whether or not the route serving it asks for +// one, since reading it is bounded and withholding it would only narrow +// what a facade can validate on. A `Range` needs more than this result +// carries — the adapter's bounded-read capability does not travel through +// it — so a facade serving 206s holds the handle itself. // // * The pairing of `lastModified` with the bytes. It is the stat taken when // the handle opened, and a streamed body is read from that handle later, diff --git a/packages/runtime-common/realm.ts b/packages/runtime-common/realm.ts index 5a9ebc1ad23..875f92d6013 100644 --- a/packages/runtime-common/realm.ts +++ b/packages/runtime-common/realm.ts @@ -752,18 +752,6 @@ export function ifNoneMatchMatches(headerValue: string, etag: string): boolean { .some((token) => token.trim().replace(/^W\//, '') === normalizedEtag); } -// Whether the byte routes build this path's validator from a content hash. -// `getSourceOrRedirect` caches and hashes a `.json` or an executable -// extension — a card's stored source, or a module — and takes its -// `bypassCache` path for everything else, where the `ETag` rests on -// `lastModified` and no hash is computed at all. A stored-bytes read reports -// `version` on the same terms, so a facade reproducing either validator has -// what that route uses and spends nothing on a value the route serving an -// image or a video would not look at. -function storedPathIsHashed(localPath: LocalPath): boolean { - return localPath.endsWith('.json') || hasExecutableExtension(localPath); -} - // A handle's content fingerprint, read in bounded ranges rather than by // streaming the file. // @@ -5919,6 +5907,14 @@ export class Realm { // on the row to validate against, and whether the byte facade wants that is // its call to make. // + // Every path is fingerprinted, an image or a video included. Which validator + // a byte route builds is that route's own choice — `getSourceOrRedirect` + // rests its `ETag` on a content hash for a `.json` or an executable + // extension and on `lastModified` for everything else — but the read that + // produces a hash is bounded, so declining to answer for the paths one route + // happens not to ask about would withhold a content identity rather than + // save anything worth saving. + // // Both values come from one row, so this is one query on // `realm_file_meta`'s primary key rather than a lookup per value — worth // holding to, since a byte response routed through here pays it per request. @@ -5939,9 +5935,6 @@ export class Realm { ) { return { version: persisted.contentHash, createdAt }; } - if (!storedPathIsHashed(localPath)) { - return { createdAt }; - } return { version: await contentHashFromRanges(file), createdAt }; } diff --git a/packages/runtime-common/tests/card-operations-dispatch-test.ts b/packages/runtime-common/tests/card-operations-dispatch-test.ts index d92ab0eddb7..6dce8f131a9 100644 --- a/packages/runtime-common/tests/card-operations-dispatch-test.ts +++ b/packages/runtime-common/tests/card-operations-dispatch-test.ts @@ -64,11 +64,10 @@ interface StubOptions { // value for. storedVersions?: Record; storedCreatedAt?: Record; - // Paths for which the realm has no recorded hash but reads one out of the - // file — its behavior for a path whose validator is built from a hash. The - // stub reads it through the handle's bounded range and never through - // `content`, as the realm does, so a case can hold both modes to the same - // version without either spending the body the other returns. + // Paths for which the realm has no recorded hash and reads one out of the + // file instead. The stub reads it through the handle's bounded range and + // never through `content`, as the realm does, so a case can hold both modes + // to the same version without either spending the body the other returns. storedRangeHash?: Record; // An adapter that reports no `size`, which the realm cannot validate a // recorded hash against. @@ -1098,10 +1097,10 @@ const tests = Object.freeze({ 'a stored-bytes read reports a version the realm never recorded as absent': async (assert) => { - // The realm resolves the hash — from its own row, or by hashing the - // bytes when the row carries none — so a null here means it could do - // neither. Reporting the absence is what lets the facade omit an `ETag` - // rather than emit one that identifies nothing. + // The realm resolves the fingerprint — from its own row, or by reading + // bounded ranges of the file when the row carries none — so a null here + // means it could do neither. Reporting the absence is what lets the + // facade omit an `ETag` rather than emit one that identifies nothing. let { core } = stub({ stored: { 'sample.md': '# hi' } }); let result = await runOperation( core, From 5b64f68322eba8c3d8cb3aacdfe679750c89b6ea Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 18:13:56 +0000 Subject: [PATCH 10/12] Say what the version contract is, and hold the reserved-name lists equal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three comment and coverage gaps from review, none of them behavior. `OperationSourceResult.version` still documented the policy that predated fingerprinting every path: null wherever the source route builds no hash from one. The code reports a content identity for every path, and the realm-server suite asserts exactly that for a `.md`. This is the type a facade author reads, so as written it told them to skip `version` for the large media the bounded read covers. The executor's cost claim said the fingerprint read does not grow with the file. The bound is min(size, `CONTENT_HASH_WHOLE_LIMIT_BYTES`) — a ceiling, not a flat cost, since below the limit the whole file is read. Worth stating precisely because the read is the common path rather than the exception: indexing records a path's creation time without hashing it, so every path the realm has indexed but never written through reaches it, and the fingerprint resolves before the headers-only return, so a HEAD pays it too. `NOT_DECLARABLE` and `DEFINITION_FREE_BASE_OPERATIONS` are one decision in two homes, and dispatch skipping the definition lookup is sound only while the decorator refuses the same names. `base/operations.ts` cannot import the constant — the runtime-common barrel carries only the types from `card-operations/types.ts`, and reaching the value pulls in the entry that type-checks bxl — so the guard is executable instead: a host case asserting every member of that list is refused by the decorator, which fails the day the two diverge. Lowering's `reserved-name` branch gains a test too, driven from a raw record since the decorator makes it unreachable through a declaration; that branch is all that keeps a stored entry from having the built-in run in its place. Also: the decorator's guidance offered bases it refuses three lines later, since both messages built their lists from unfiltered vocabularies. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt --- packages/base/operations.ts | 21 ++++++--- .../host/tests/integration/operations-test.ts | 36 +++++++++++++++ .../tests/unit/operation-lowering-test.ts | 45 ++++++++++++++++++- .../card-operations/read-source.ts | 11 ++++- .../runtime-common/card-operations/types.ts | 15 ++++--- packages/runtime-common/realm.ts | 9 ++-- 6 files changed, 119 insertions(+), 18 deletions(-) diff --git a/packages/base/operations.ts b/packages/base/operations.ts index 88e4eb216b8..716609f9377 100644 --- a/packages/base/operations.ts +++ b/packages/base/operations.ts @@ -97,6 +97,17 @@ export type BaseOperationName = (typeof BASE_OPERATIONS)[number]; // refuses the name too, so no stored definition can carry one either. const NOT_DECLARABLE: readonly BaseOperationName[] = ['readSource']; +// A base-operation list with the reserved names dropped, for the messages that +// tell an author which bases are open to them. The checks below still run over +// the unfiltered lists — what a def type carries and what a reserved name +// refuses are separate questions — but guidance that named a reserved base +// would point somewhere the very next check rejects. +function declarable( + names: readonly BaseOperationName[], +): readonly BaseOperationName[] { + return names.filter((name) => !isNotDeclarable(name)); +} + function isNotDeclarable(name: string): boolean { return NOT_DECLARABLE.includes(name as BaseOperationName); } @@ -378,9 +389,9 @@ export type OperationDeclaration = // A base operation a def carries with nothing declared on it. It is not a // declaration and the union above deliberately cannot express one: an author -// writes no clauses for a base operation, and the two `NOT_DECLARABLE` names -// cannot be written at all, so a declaration type that admitted them would -// invite exactly what the decorator refuses. `getOperations` returns both +// writes no clauses for a base operation, and a `NOT_DECLARABLE` name cannot +// be written at all, so a declaration type that admitted one would invite +// exactly what the decorator refuses. `getOperations` returns both // shapes, so a consumer reading `base` to dispatch gets every operation a def // carries — including the ones no `OperationDeclaration` could name. export interface ImpliedOperation { @@ -735,7 +746,7 @@ function assertValidDeclaration( let base = declaration.base; if (!isBaseOperationName(base)) { throw new Error( - `${label}: \`base\` must name the built-in behavior this operation builds on — one of ${quoteList(BASE_OPERATIONS)}`, + `${label}: \`base\` must name the built-in behavior this operation builds on — one of ${quoteList(declarable(BASE_OPERATIONS))}`, ); } if (isNotDeclarable(base)) { @@ -751,7 +762,7 @@ function assertValidDeclaration( if (!implied.includes(base)) { throw new Error( `${label}: this def type carries only ${quoteList( - implied, + declarable(implied), )}, so it cannot declare a "${base}" operation`, ); } diff --git a/packages/host/tests/integration/operations-test.ts b/packages/host/tests/integration/operations-test.ts index 4547f37993d..2558f38c6c9 100644 --- a/packages/host/tests/integration/operations-test.ts +++ b/packages/host/tests/integration/operations-test.ts @@ -1,6 +1,8 @@ import { getService } from '@universal-ember/test-support'; import { module, test } from 'qunit'; +import { DEFINITION_FREE_BASE_OPERATIONS } from '@cardstack/runtime-common/card-operations'; + import type { Loader } from '@cardstack/runtime-common/loader'; import { setupCardLogs, setupLocalIndexing } from '../helpers'; @@ -496,6 +498,40 @@ module('Integration | operations', function (hooks) { ); }); + test('the decorator refuses every name the realm answers definition-free', function (assert) { + // The two lists are one decision with two homes: dispatch skips the + // definition lookup for `DEFINITION_FREE_BASE_OPERATIONS`, and that is + // sound only while the decorator refuses the same names — otherwise a + // declaration takes one, the built-in answers, and nothing reports the + // declaration that never ran. `base/operations.ts` cannot import the + // constant (the `runtime-common` barrel carries only the types from + // `card-operations/types.ts`, and reaching the value pulls in the entry + // that type-checks bxl), so this case is what holds them equal: adding a + // definition-free operation to the runtime list alone fails here. + // + // The decorator is a plain function, so a name from the list drives it + // directly — decorator syntax cannot spell a computed one. `base: 'read'` + // is deliberate: it is the hole that matters, a reserved name declared on + // a base that is otherwise allowed. + assert.ok( + DEFINITION_FREE_BASE_OPERATIONS.length > 0, + 'the list is non-empty, so the loop below asserts something', + ); + for (let name of DEFINITION_FREE_BASE_OPERATIONS) { + assert.throws( + () => { + class Shadow extends CardDef {} + operation(Shadow, name, { + initializer: () => ({ base: 'read' }), + }); + return Shadow; + }, + /reserved operation name/, + `${name} is refused as a declaration name`, + ); + } + }); + test('the decorator rejects an operation name that is already a static', function (assert) { assert.throws( () => { diff --git a/packages/host/tests/unit/operation-lowering-test.ts b/packages/host/tests/unit/operation-lowering-test.ts index 3389931eac3..c9db9c9027b 100644 --- a/packages/host/tests/unit/operation-lowering-test.ts +++ b/packages/host/tests/unit/operation-lowering-test.ts @@ -13,7 +13,10 @@ import { type Definition, type LowerOperationDeclarationsResult, } from '@cardstack/runtime-common'; -import { lowerOperationDeclarations } from '@cardstack/runtime-common/card-operations'; +import { + DEFINITION_FREE_BASE_OPERATIONS, + lowerOperationDeclarations, +} from '@cardstack/runtime-common/card-operations'; import ENV from '@cardstack/host/config/environment'; import { shimExternals } from '@cardstack/host/lib/externals'; @@ -1035,4 +1038,44 @@ module('Unit | operation lowering', function (hooks) { 'the base operations a def type carries are not declarations, so there is nothing to lower', ); }); + + test('a reserved name is refused rather than lowered', async function (assert) { + // The decorator refuses these names, so no class can carry one and this + // branch is unreachable through a real declaration — which is why it is + // driven from a raw record. It is worth driving: a type's entry outlives + // the code that built it, and this refusal is the only thing standing + // between a stored entry under a definition-free name and the built-in + // running in place of it. The realm resolves the name before reading any + // definition, so a lowered entry here would be dispatched straight past + // rather than reported. + let { field, contains, CardDef } = api; + class Reserved extends CardDef { + static displayName = 'Reserved'; + @field title = contains(StringField); + } + shim({ Reserved }); + + for (let name of DEFINITION_FREE_BASE_OPERATIONS) { + let result = await lowerOperationDeclarations( + { [name]: { base: 'read' } } as Record< + string, + OperationsModule.OperationDeclaration + >, + { + definition: buildDefinition(Reserved), + lookupDefinition, + identifyCard: (target) => identifyCard(target), + }, + ); + assert.deepEqual( + codes(result), + ['reserved-name'], + `${name} is recorded as reserved`, + ); + assert.true( + result.operations[name]?.invalid, + `${name} is marked invalid rather than dropped, so a consumer reading the entry sees the refusal`, + ); + } + }); }); diff --git a/packages/runtime-common/card-operations/read-source.ts b/packages/runtime-common/card-operations/read-source.ts index 3aef6def65f..aab4f254984 100644 --- a/packages/runtime-common/card-operations/read-source.ts +++ b/packages/runtime-common/card-operations/read-source.ts @@ -33,8 +33,15 @@ import type { OperationCore, RunOperationOptions } from './dispatch.ts'; // — never a search read. That is also why it can answer for a path the // index has no row for, and for one it never will. Where the realm has no // recorded hash it reads the file to fingerprint it, in ranges bounded by -// the fingerprint's own shape rather than by the file's size — so this -// answers for a path of any size at a cost that does not grow with it. +// the fingerprint's own shape: min(size, `CONTENT_HASH_WHOLE_LIMIT_BYTES`) +// — the whole file below that limit, a fixed head and tail above it. So +// the read has a ceiling no file can exceed, not a flat cost. Worth +// sizing for rather than treating as the exception: indexing records a +// path's creation time without hashing it, so every path the realm has +// indexed but never written through has no recorded hash and reaches this +// read, and both modes pay it — the fingerprint is resolved before the +// headers-only return, so a `HEAD` of an unrecorded file reads as much of +// it as a full read would. // // Two modes, as `read` has: // diff --git a/packages/runtime-common/card-operations/types.ts b/packages/runtime-common/card-operations/types.ts index b5953b8b678..a5f880bc57e 100644 --- a/packages/runtime-common/card-operations/types.ts +++ b/packages/runtime-common/card-operations/types.ts @@ -313,12 +313,15 @@ export interface OperationSourceResult { // Two things a facade building a validator from it has to know. It is not by // itself the byte routes' `ETag`: the source route builds one from a hash for // a `.json` or an executable extension and from `lastModified` for - // everything else, and `version` is populated on those same terms, so it is - // null exactly where that route computes no hash. And `computeContentHash` - // samples above its whole-content limit, so a large file's hash covers its - // head, tail and length rather than all of it — `isSampledContentHash` tells - // one from the other, and the realm's own `ETag` joins a sampled hash with - // `lastModified` rather than trusting it alone. + // everything else, so which of the two to reproduce is the facade's choice + // — but a content identity is reported for every path, whether or not the + // route serving it asks for one, so the choice is never forced by an absent + // value. Null means only that the realm could neither recall a fingerprint + // nor read one within a bounded cost. And `computeContentHash` samples above + // its whole-content limit, so a large file's hash covers its head, tail and + // length rather than all of it — `isSampledContentHash` tells one from the + // other, and the realm's own `ETag` joins a sampled hash with `lastModified` + // rather than trusting it alone. version: string | null; // The byte size, where the adapter knew it from the stat it already // performed, and null where knowing it would cost reading the bytes — the diff --git a/packages/runtime-common/realm.ts b/packages/runtime-common/realm.ts index 2f97dde268a..71fbd0224df 100644 --- a/packages/runtime-common/realm.ts +++ b/packages/runtime-common/realm.ts @@ -765,10 +765,11 @@ export function ifNoneMatchMatches(headerValue: string, etag: string): boolean { // streaming the file. // // The cost is bounded by the fingerprint's own shape: -// `computeContentHashFromRanges` asks for the whole content up to -// `CONTENT_HASH_WHOLE_LIMIT_BYTES` and for a fixed head and tail above it, so -// no file costs more than that limit to hash however large it is, and the -// value is the same one hashing the whole content would produce. +// `computeContentHashFromRanges` asks for min(size, +// `CONTENT_HASH_WHOLE_LIMIT_BYTES`) — the whole content up to that limit, and +// a fixed head and tail above it. So the read has a ceiling no file can +// exceed rather than a flat cost, and the value is the same one hashing the +// whole content would produce. // // Nothing here touches `content`. That keeps this off the handle a body is // served from, which matters twice: `content` is a lazy getter on every From 9332491ff01b929ec86a163f774b93961d3da8a6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 18:30:32 +0000 Subject: [PATCH 11/12] Call the operation decorator by its runtime signature in the list guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `operation` is exported as `PropertyDecorator` — TypeScript's two-argument shape — while the Babel legacy decorator it actually is takes a third descriptor argument carrying the declaration object. The guard that holds the reserved-name lists equal drives the decorator directly, since decorator syntax cannot spell a computed name, and passed three arguments against the two-argument type. Cast at the call site to the real runtime signature, for the same mismatch the export's own `as unknown as PropertyDecorator` exists for. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt --- packages/host/tests/integration/operations-test.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/host/tests/integration/operations-test.ts b/packages/host/tests/integration/operations-test.ts index 2558f38c6c9..41dbd7a2fbb 100644 --- a/packages/host/tests/integration/operations-test.ts +++ b/packages/host/tests/integration/operations-test.ts @@ -517,11 +517,21 @@ module('Integration | operations', function (hooks) { DEFINITION_FREE_BASE_OPERATIONS.length > 0, 'the list is non-empty, so the loop below asserts something', ); + // `operation` is exported as `PropertyDecorator` — TypeScript's two-arg + // shape — while the Babel legacy decorator it actually is takes a third + // descriptor argument, which is where the declaration object arrives. The + // cast asks for the real runtime signature, the same mismatch the export's + // own `as unknown as PropertyDecorator` exists for. + let applyOperation = operation as unknown as ( + target: unknown, + key: string, + descriptor: { initializer: () => unknown }, + ) => void; for (let name of DEFINITION_FREE_BASE_OPERATIONS) { assert.throws( () => { class Shadow extends CardDef {} - operation(Shadow, name, { + applyOperation(Shadow, name, { initializer: () => ({ base: 'read' }), }); return Shadow; From d70f9f61e38b472b0cdb52fe069d728c7faba4b4 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 19:03:24 +0000 Subject: [PATCH 12/12] Leave the declarable-base guidance out of the reserved-name change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Filtering reserved names out of `assertValidDeclaration`'s two guidance messages rewrote one of them from `carries only "read", "readSource"` to `carries only "read"`, which two existing cases assert verbatim. They surfaced as errors rather than failures because `assert.throws` rethrows the original when its pattern misses. The filter reads better than what it replaced — guidance that names a base the next check refuses points an author nowhere — but it is a separate change from reserving the name, and carrying it here means rewriting those two expectations alongside it. Restore the unfiltered lists; the messages are worth revisiting on their own. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt --- packages/base/operations.ts | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/packages/base/operations.ts b/packages/base/operations.ts index 716609f9377..637f59f718d 100644 --- a/packages/base/operations.ts +++ b/packages/base/operations.ts @@ -97,17 +97,6 @@ export type BaseOperationName = (typeof BASE_OPERATIONS)[number]; // refuses the name too, so no stored definition can carry one either. const NOT_DECLARABLE: readonly BaseOperationName[] = ['readSource']; -// A base-operation list with the reserved names dropped, for the messages that -// tell an author which bases are open to them. The checks below still run over -// the unfiltered lists — what a def type carries and what a reserved name -// refuses are separate questions — but guidance that named a reserved base -// would point somewhere the very next check rejects. -function declarable( - names: readonly BaseOperationName[], -): readonly BaseOperationName[] { - return names.filter((name) => !isNotDeclarable(name)); -} - function isNotDeclarable(name: string): boolean { return NOT_DECLARABLE.includes(name as BaseOperationName); } @@ -746,7 +735,7 @@ function assertValidDeclaration( let base = declaration.base; if (!isBaseOperationName(base)) { throw new Error( - `${label}: \`base\` must name the built-in behavior this operation builds on — one of ${quoteList(declarable(BASE_OPERATIONS))}`, + `${label}: \`base\` must name the built-in behavior this operation builds on — one of ${quoteList(BASE_OPERATIONS)}`, ); } if (isNotDeclarable(base)) { @@ -762,7 +751,7 @@ function assertValidDeclaration( if (!implied.includes(base)) { throw new Error( `${label}: this def type carries only ${quoteList( - declarable(implied), + implied, )}, so it cannot declare a "${base}" operation`, ); }