Skip to content

Add readSource, the stored-bytes read, as a base operation - #6071

Open
habdelra wants to merge 15 commits into
mainfrom
cs-12912-card-ops-readsource-base-operation-the-stored-bytes-read-for
Open

Add readSource, the stored-bytes read, as a base operation#6071
habdelra wants to merge 15 commits into
mainfrom
cs-12912-card-ops-readsource-base-operation-the-stored-bytes-read-for

Conversation

@habdelra

@habdelra habdelra commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Background and Goal

A realm serves three reads, and the operation core models one of them:

  • The document. GET with Accept: application/vnd.card+json returns a card as a JSON:API document assembled from the search index, or a file's metadata document. That is read.
  • The source. GET/HEAD with Accept: application/vnd.card+source returns the stored text of a resource exactly as it sits on disk — a card instance's .json, a .gts/.ts module. getSourceOrRedirect serves it; the code editor and the CLI live on that route.
  • The bytes. GET on an image, a PDF or a markdown file with any other Accept returns the raw bytes with an inferred content type, through serveLocalFile.

The last two had no operation to dispatch through. That matters beyond tidiness: authorization layered on operations gates only what flows through operations, so every read path outside them is an unguarded exit. It also leaves a file's most interesting representation unreachable — a file def instance whose read returns metadata but whose bytes are not readable is a strange thing to have.

This adds readSource, the read of a resource's stored bytes, as a base operation. Nothing is routed to it, so the change is fully additive: every existing handler and suite is untouched and read is unchanged.

Where to start

  • packages/runtime-common/card-operations/read-source.ts — the executor, and a module header stating which parity a byte facade can rely on and which four things stay the facade's.
  • packages/runtime-common/card-operations/dispatch.tsDEFINITION_FREE_OPERATIONS, the branch in resolveOperation that answers before the definition lookup, the two new OperationCore collaborators, and the addressing option on canonicalizeTarget.
  • packages/runtime-common/content-hash.tscomputeContentHashFromRanges, the existing fingerprint assembled from bounded reads.
  • packages/base/operations.tsBASE_OPERATIONS, READ_ONLY, and NOT_DECLARABLE with its two refusals.
  • packages/runtime-common/tests/card-operations-dispatch-test.ts — the new cases against a stub that records every collaborator call.
  • packages/realm-server/tests/card-operations-core-test.ts — the same operation obtained from a real test realm and held against what the card+source GET serves.

Key decisions and non-obvious mechanics

  • Definition-free rests on the name reservation, not on a lookup that would fail. What skipping the definition lookup buys is the lookup — a byte read is the hottest path the realm has, and for a .gts the lookup is a read of the file def its extension names, which cannot affect the outcome. But that is not what makes it 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 below: were a declaration able to take the name, resolving before the lookup would run the built-in in its place. Skipping also fixes the addressing before anything reads, which is what lets runOperation tell a path read from a card read by name alone.

  • A type target refuses without a lookup either. A type has no stored bytes, and that answer does not depend on which kind of def the type turns out to be, so it is operation-not-allowed rather than target-not-found. A FieldDef type refuses for that reason and not because a field def carries nothing — which is why the definition-free branch has to come before resolveOperation's existing refusal of a type target with no resolvable definition, rather than after it.

  • The name is reserved, and so is the base — a name and a base are independent. A declaration wins over the built-in of the same name, which is how an author specializes read or rebinds delete onto transform. A stored-bytes read is the one behavior that cannot be reached that way: it serves what is on disk, so there is no payload to reshape, no program stage to run and no result to project. Refusing the base stops the behavior being reached under another name. Refusing the name is what makes answering it definition-free correct — the realm resolves it before reading any definition, so a declaration under that name, whatever base it built on, would be dispatched straight past and the built-in would run in place of what the author wrote. The authoring decorator refuses both; lowering refuses a reserved name so no stored definition can carry one, reporting it as an invalid operation with a reserved-name issue rather than dropping it silently; and dispatch refuses a stored definition that carries the base regardless. OperationDeclaration also cannot express base: 'readSource', so the authoring types refuse it before the decorator does.

  • A stored-bytes read addresses a path; a card read addresses a card. canonicalizeTarget resolves the realm root to the realm's index card, which is what makes the root readable as a card at all, and is wrong for bytes — the root is the realm's directory, and resolving it to index would serve whatever file carries that bare name in answer to a request for a directory. So canonicalization takes the addressing, and runOperation reads which one applies off the request name and passes it down, so dispatch and the executor never address different things. Reading it off the name is sound for the same reason answering the operation without a definition is: no declaration can take the name. Everything else canonicalization does is common to both — 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.

  • The exhaustive table did its job. CARD_DEF_OPERATIONS is Readonly<Record<BaseOperation, true>>, so adding the name to BaseOperation failed to compile until each def kind said whether it carries it: card-def and file-def do, field-def does not. CLAUSE_KEYS in the authoring API is exhaustive the same way.

  • version identifies the bytes it is returned with, and costs a bounded read at most. That is its whole job, and the realm's realm_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 — a deploy rsync, an operator editing the volume — keeps a row describing bytes that are gone, and version is what a conditional GET builds its validator from. So the row is trusted only where the length it recorded matches the handle being read, and the fingerprint is read out of the file otherwise — for every path, an image or a video included.

    Reading it costs at most CONTENT_HASH_WHOLE_LIMIT_BYTES, however large the file. computeContentHash already samples above that limit: the value is the byte length plus a hash of the head and a hash 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 each asks for. The realm reads through the handle's createRangeStream, the bounded-read capability FileRef already carries for Range serving.

    Two things this does not claim. An out-of-band overwrite preserving the exact byte length still yields the recorded hash; closing that needs an mtime column to validate against, and whether the byte facade wants one is its call. And the realm detects no out-of-band write anywhere: getSourceOrRedirect's own #sourceCache holds a stale ref and hash across one too. What is specific to a persisted row is that it survives a restart, which is what made the cold-process deploy case worth guarding.

  • version 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 — those paths compute no hash at all, and contentHashFromMaterializedRef will not read bytes to get one. Which validator to build stays the facade's choice, and both members are here for either: a content identity is reported for every path, whether or not the route serving it happens to ask for one. Reporting it only where a route asks would cost nothing to keep and would withhold a strong validator for exactly the large media a caching facade most wants one for.

  • Neither mode pays for the other's bytes. The fingerprint is read in bounded ranges of the file rather than out of the body, and nothing on the version path touches the handle's content — which is a lazy getter that opens a real stream on first touch and is single-use. So a headers-only read leaves it alone and still reports the same version a full read reports, and a full read still has its whole body to serve. Two modes that disagreed about a validator for one file would leave a facade holding two answers; the suites pin the agreement from both ends, and pin that content is touched exactly once in the mode that returns a body and not at all in the mode that does not. 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 is a moving file, not bytes. If the ranges no longer add up to the size the stat reported, the file is not the one being described and a fingerprint of it identifies neither version — so it is abandoned rather than reported. Every consumer of a version handles its absence, so that costs the validator rather than the response.

  • The metadata and the bytes are not one snapshot. lastModified is the stat taken when the handle opened and the body is read from it afterwards, so a write landing in between pairs one with the other. That is the window the byte routes already have, reading the same handle the same way; reproducing it is the parity this operation is for, and closing it would be a change to those handlers.

  • Every way there is nothing to read arrives as one refusal. openStoredFile keeps only the empty-path check; #adapter.openFile supplies the rest by answering undefined for a directory and for a missing path. There is deliberately no name-based refusal. Neither of openFileForMetadata's two — its .json refusal and its _-prefix refusal — describes the byte routes: the card+source GET/HEAD and the raw byte serve are 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 a card's .json and a stored _notes.md are both files the realm serves, and refusing either here would make this the one read that could not reach them.

  • Never target-not-indexed. That code promises waiting will resolve the absence, and what it waits for is the index. A read of bytes has nothing to wait for, so a card whose .json is on disk does not change the answer for a path whose own bytes are missing.

  • Absences are reported rather than filled in. An absent version means only that the realm could neither recall a fingerprint nor read one within a bounded cost — an adapter offering no bounded read, or one that cannot state a size without reading the bytes, which also reports no size. A path the realm holds no record of has no creation time. Each is null rather than a substituted value — the byte serve omits x-created in that case rather than substituting the modification time, so null carries the absence through and lets a facade make the same choice.

  • The core gets no VirtualNetwork and no network capability. The two new collaborators are plain functions the realm binds to its own file adapter and file-meta row, in the same shape as the ones already there. Both file-meta values come from one row read, since a byte response routed through here pays it per request.

  • OperationCore.readSource is renamed readFileAsText, after the realm method it is bound to. One name meaning both "the target's source as text" and the operation would have been a trap in a module where both appear.

  • getOperations returns what a def carries, declared or implied. Adding a second implied read made the old return type — a record of declarations — describe something a built-in is not. It now returns CarriedOperation, either an OperationDeclaration or an ImpliedOperation naming the base it resolves to.

What stays outside

The redirects (an extension-less URL naming foo.gts, a card id naming its .json), the response around the bytes — Last-Modified, x-created, the validator, the 304, the source cache — and the content type's own Accept negotiation all stay at the facade, which hands the executor a resolved path. Range needs more than the result carries: size travels, so a facade can set Content-Length and decide whether a 206 is possible at all, but the adapter's bounded-read capability is a function on its handle rather than plain data, so a facade serving 206s reads from the handle. 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.

Test plan

Ran locally:

  • packages/runtime-common/tests/card-operations-dispatch-test.ts — 37 cases, 150 assertions, all passing, driven directly through the shared-tests module. Twelve are new: the bytes, inferred content type and size for a card's .json, a module, an image (a Uint8Array handed back undecoded) and a stored _notes.md, each also asserting that the size of the handle being read travels with the request for its version; the call ledger showing one file open, one file-meta row and one touch of the bytes with no definition lookup and no index read at all, for a .gts path — the case the constraint exists for; a version read out of the file leaving the whole body still there to serve and touching content exactly once; both modes reporting that same version with the headers-only mode touching content not at all; an adapter with no bounded read reporting no version rather than having one streamed for it; an adapter that reports no size still reading; an unrecorded version and created reported as null; a missing path and a directory refusing target-not-found and never target-not-indexed; the realm root refusing in both its spellings while the bare name index still reads by name and a card read of the root still serves the index card; a type target and a field-def type target refusing operation-not-allowed with an empty call ledger; and a stored definition built on readSource being refused.
  • The shim in packages/realm-server/tests/card-operations-dispatch-test.ts declares exactly the shared module's 37 test names — checked mechanically, since a shim that misses one runs it nowhere and reports nothing.
  • packages/realm-server/tests/content-hash-test.ts — 13 cases, 37 assertions, all passing, run against the real module: the ranged form returning the same string as the whole form at every size boundary from empty through eight times the limit, the ranges it asks for being exactly the head and the tail and totalling the whole-hash limit, and empty content hashing with no read at all.
  • lint:types clean for runtime-common, realm-server and host; eslint and prettier clean for every changed file. All of the above re-run after merging main in.

Left to CI. This sandbox cannot boot the realm-server suite: its setup needs Synapse on :8008 and the prerender manager on :4222, and a Matrix login failure surfaces as an unhandled rejection that fails a test before its body runs. Postgres, the host app and Chromium can all be brought up here; the Synapse image cannot be pulled, so the two suites below ran nowhere locally.

  • packages/realm-server/tests/card-operations-core-test.ts — nine new cases against a real test realm: a card instance's source held byte-for-byte against the card+source GET with the served ETag built from the reported version; a module's text likewise; an image written through the realm coming back undecoded with image/png; a stored _notes.md reached by the read and held against the same GET; a module overwritten with writeFileSync so nothing refreshes its row, asserting the read serves the new bytes, reports their fingerprint rather than the recorded one, and reports the same one in the mode that returns no body; the same overwrite on a .md, the other side of the line the byte routes draw for their own validators, reporting that same new fingerprint since what a stale row means does not depend on the extension it sits under; a file larger than the whole-hash limit written straight to disk under an extension the source route would validate on lastModified, whose reported version is sampled and equals what hashing the whole content produces; the headers-only mode agreeing with the body mode on every value a header is computed from; a missing path and a real directory refusing target-not-found; and a card-def and a field-def type target refusing operation-not-allowed.
  • packages/host/tests/integration/operations-test.ts — the def-type tables now list both reads, plus new cases that a readSource cannot be declared on a card def or a file def, cannot be specialized under its own name, and cannot take that name by building on another base.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-10T14:29:33.570563Z a7a6f3b PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a7a6f3b6cb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/runtime-common/realm.ts Outdated
Comment thread packages/runtime-common/realm.ts Outdated
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Preview deployments

Host Test Results

    1 files  ±0      1 suites  ±0   2h 30m 38s ⏱️ - 2m 3s
4 742 tests ±0  4 728 ✅ +2  14 💤 ±0  0 ❌ ±0 
4 757 runs  ±0  4 743 ✅ +4  14 💤 ±0  0 ❌  - 2 

Results for commit d70f9f6. ± Comparison against earlier commit 9332491.

Realm Server Test Results

    1 files  ±0    214 suites  ±0   1h 17m 26s ⏱️ + 4m 58s
2 832 tests ±0  2 832 ✅ ±0  0 💤 ±0  0 ❌ ±0 
2 871 runs  ±0  2 871 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit d70f9f6. ± Comparison against earlier commit 9332491.

…e names

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved findings cover declaration-name collisions, root-target handling, snapshot consistency, ETag/range parity, and duplicate metadata queries.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds readSource as a definition-free base operation for reading stored resource bytes and metadata.

Changes:

  • Adds executor, types, exports, dispatch, and realm bindings.
  • Registers the operation as read-only and non-declarable.
  • Adds shared, realm-server, and host test coverage.
File summaries
File Summary
packages/runtime-common/tests/card-operations-dispatch-test.ts Tests dispatch, byte reads, metadata, and refusal cases.
packages/runtime-common/realm.ts Binds stored-file and metadata access.
packages/runtime-common/card-operations/types.ts Defines stored-file results and adapter types.
packages/runtime-common/card-operations/read.ts Updates the existing text-read boundary.
packages/runtime-common/card-operations/read-source.ts Executes stored-byte reads.
packages/runtime-common/card-operations/index.ts Exports the new operation APIs.
packages/runtime-common/card-operations/dispatch.ts Adds definition-free dispatch and collaborators.
packages/realm-server/tests/card-operations-dispatch-test.ts Registers shared dispatch tests.
packages/realm-server/tests/card-operations-core-test.ts Tests realm-backed source reads.
packages/host/tests/integration/operations-test.ts Tests operation declarations and restrictions.
packages/base/operations.ts Registers and validates the base operation.
Review details

Suppressed comments (4)

packages/runtime-common/card-operations/dispatch.ts:306

  • [Claude Code 🤖] This early return assumes that no declaration can use the readSource name, but the decorator only rejects declarations whose base is readSource. A valid @operation static readSource = { base: 'read' } (or another allowed base) is therefore skipped here and the built-in byte reader silently runs instead of the declared operation. Reserve the readSource operation name during authoring and keep the persisted-definition path consistent with that invariant.
export async function resolveOperation(

packages/runtime-common/card-operations/read-source.ts:100

  • [Claude Code 🤖] The metadata lookup happens after openStoredFile, while body remains a lazy handle. On the Node adapter, openFile snapshots lastModified immediately but opens the path only when content is touched, so a concurrent write can pair an old mtime with a new version and body (or the reverse). The facade's ETag can then validate a different representation than it serves; read metadata and bytes from one consistent snapshot or revalidate the pair before returning.
  // 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,

packages/runtime-common/card-operations/types.ts:290

  • [Claude Code 🤖] This says version is what the source-route ETag is built from, but the existing byte handler's non-JSON/non-executable bypassCache path calls serveLocalFile without an etagBase, so images and PDFs use lastModified as the ETag base. A facade using this result for conditional requests would not reproduce those validators; either expose the exact ETag inputs or narrow this parity claim to responses that actually use the content hash.
  // 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;

packages/runtime-common/card-operations/types.ts:296

  • [Claude Code 🤖] OperationStoredFile preserves size, but this result drops it along with the adapter's range capability when readSourceOperation returns. For the Node adapter body is a lazy ReadStream, so a facade consuming only this result cannot reproduce the existing Content-Length/Accept-Ranges/Range behavior without reopening or materializing the file. Carry the size/range metadata through the operation result, or keep this operation from claiming parity with those handlers.
  // 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;
  • Files reviewed: 11/11 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/base/operations.ts Outdated
Comment thread packages/runtime-common/card-operations/read-source.ts Outdated
Comment thread packages/runtime-common/realm.ts Outdated
…eta query

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt

Copy link
Copy Markdown
Contributor Author

[Claude Code 🤖] The four suppressed comments in the Copilot review have no threads of their own, so answering them here. Three are addressed in 699f46df; one I have deliberately not changed.

dispatch.ts:306 — declaration name vs. base. The same finding as the inline thread on operations.ts, and the serious one. Fixed: the decorator reserves the name, not just the base. Detail on that thread.

types.ts:290version is not the byte routes' ETag. Correct, and the comment overclaimed. The source route builds a validator from a content hash only for a .json or an executable extension; everything else takes the bypassCache path and gets an ETag based on lastModified, with no hash computed at all. The comment now says what version is — an identity for these bytes and no others, which is what a validator needs — and says explicitly that reproducing the byte routes' validators means choosing between the hash and lastModified. The executor's module header carries the same correction.

types.ts:296 — the result drops size and range capability. Half fixed, half narrowed, because the two halves are different. size now travels on OperationSourceResult: the executor already had it from the adapter's stat, a facade needs it for Content-Length and to decide whether it can offer a Range at all, and dropping it was a straight omission. The bounded-read capability is not the same kind of thing — createRangeStream is a function on the adapter's handle, and putting it in the result would push the adapter's interface through an operation result that is otherwise plain data. So a facade serving 206s reads from the handle rather than from this result, and the header now says so rather than implying parity it does not have.

read-source.ts:100 — metadata and bytes are not one snapshot. Real, and deliberately unchanged. openFile stats when the handle opens and the body is read from it later, so a write landing in between pairs an old lastModified with new bytes. That is not introduced here: getSourceOrRedirect and serveLocalFile read the same handle the same way, so the byte routes have exactly this window, and readSource reproducing it is the parity this ticket is for. Closing it would mean revalidating after the read across all byte serving — a change to those handlers, not to this operation, and one that would put this PR's diff somewhere it does not belong. I have named the window in the executor header instead, so a facade author meets it rather than discovering it. Happy to file it separately if you would like it tracked.


Generated by Claude Code

`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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt
@habdelra
habdelra changed the base branch from cs-12791-card-ops-operation-core-types-dispatch-the-read-executor to main September 10, 2026 20:02
…file

`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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt
…tating it

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt
@habdelra
habdelra requested a review from a team September 10, 2026 20:44
…dsource-base-operation-the-stored-bytes-read-for
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt
…dsource-base-operation-the-stored-bytes-read-for
…dsource-base-operation-the-stored-bytes-read-for

@backspace backspace left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] Convergence pass over the bot findings and the invariants they moved: verified each fix as a change on the head commit rather than against its description, then re-ran the definition-free, addressing and version claims against the code around them. The realm-server and host suites were left to CI, which is green.

No blocking issues — approving. The three items below are comment accuracy and test coverage; none of them changes what the code does, and none needs to hold the merge.

Where each earlier finding landed

  1. Stale version after an out-of-band overwrite — resolved. The row is trusted only where its content_size matches the handle's stat, and contentHashFromRanges reads the fingerprint otherwise; pinned for a module and for a .md in a stored-bytes read never reports a version that describes other bytes.
  2. Underscore-prefixed stored files — resolved. #operationStoredFile keeps only the empty-path check, and _notes.md reads in both the shared and realm-server suites.
  3. A declaration taking the name while building on another base — resolved, and this was the one that mattered. isNotDeclarable(key) runs before assertNameAvailable, lowering refuses the key with a reserved-name issue, and the host suite covers readSource declared on base: 'read' as well as on its own base.
  4. The realm root serving a bare index file — resolved. The addressing travels on CanonicalizeOptions.rootNamesIndexCard rather than being preserved only inside the executor, and the shared case pins both root spellings refusing while index still reads by name and a card read of the root still serves the index card.
  5. Two queries per stored-bytes read — resolved. One getFileMetaForPaths read supplies both values.
  6. Metadata and bytes are not one snapshot — accepted as parity rather than closed. Confirmed the byte routes have the same window: serveLocalFile serves ref.lastModified from the stat taken at open and reads ref.content afterwards.
  7. ETag/range paritysize travels; the bounded-read capability stays on the handle. The version half of that comment is now out of step with the code — item 1 below.

Recommendations

  1. The version doc comment on OperationSourceResult states the opposite of what the code and the suite do — see the thread on types.ts.
  2. The executor header's cost claim holds only above the whole-hash limit, and the ranged read is the steady state rather than the exception — see the thread on read-source.ts.
  3. The reserved-name list has two homes and nothing pins them equal; the lowering branch that enforces it has no test — see the thread on base/operations.ts.

Adjacent, not asked of this change

assertValidDeclaration builds its "this def type carries only …" message from impliedOperations, and BASE_OPERATIONS fills the "base must name … one of" message, so both now offer an author a base that isNotDeclarable refuses three lines earlier. Filtering NOT_DECLARABLE out of those two messages would keep the guidance pointing somewhere an author can actually go.

Comment on lines +313 to +321
// 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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] This says version is null wherever the source route computes no hash. The code reports one for every path: #operationStoredFileMeta falls through to contentHashFromRanges whenever the row's content_size does not match the handle's stat, extension immaterial — and a stored-bytes read never reports a version that describes other bytes asserts exactly that for notes.md, the case chosen because the source route validates it on lastModified. read-source.ts's own header states the contract the code keeps ("a content identity for every path, whether or not the route serving it asks for one"), so the two disagree.

Worth correcting rather than leaving: this is the type a facade author reads, and as written it tells them to skip version for exactly the large media the bounded fingerprint covers.

Regression (comment only), non-blocking.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] Confirmed and fixed. This is a regression and the diagnosis is exact: the comment is the pre-gate-removal policy left verbatim.

version used to be populated only where the source route builds a validator from a hash, on the reasoning that reading one for an image was a cost the route serving it would not pay. Removing the gate made that reasoning obsolete — the fingerprint is read in bounded ranges, so withholding it saved nothing and denied a strong validator to exactly the large media that most wants one. I updated read-source.ts's header and OperationStoredFileMeta in dispatch.ts and missed this one, which is the worst of the three to miss for the reason you name: it is the type a facade author reads, and read-source.ts's header is not.

Rewritten to say what the code does — which of the two validators to reproduce is the facade's choice, a content identity is reported for every path whether or not the route serving it asks for one, and null means only that the realm could neither recall a fingerprint nor read one within a bounded cost. The sampled-hash half of the comment was accurate and stands.

Verified against the assertion you cite: a stored-bytes read never reports a version that describes other bytes expects computeContentHash('# second, and longer') for notes.md, so the suite does pin the behavior the comment now describes.


Generated by Claude Code

Comment on lines +34 to +37
// 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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] The bound is min(size, CONTENT_HASH_WHOLE_LIMIT_BYTES), not a constant — below the limit computeContentHashFromRanges asks for readRange(0, size), so the fingerprint read is the whole file. "a cost that does not grow with it" holds only above the limit.

What makes that worth stating is how often the read is reached. ensureFileCreatedAt (index-runner/visit-file.ts) inserts a row carrying created_at and nothing else, so every path the realm has indexed but never written through Realm.write has a row whose content_hash is null and falls through to the ranged read — a realm seeded on disk is in that state for all of its files, not just one someone overwrote out of band. And storedFileMeta runs before the headersOnly return, so a HEAD of an unrecorded 4 MB file reads 4 MB.

The ask is the sentence rather than the behavior: state min(size, limit) here and in contentHashFromRanges's header, so a facade sizing a HEAD does not read the cost as flat. If you would rather keep the claim as written, the shape that earns it is letting the headers-only mode decline an unrecorded fingerprint. Non-blocking either way.

Follow-up.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] Confirmed and fixed as the sentence, not the behavior.

The overclaim is real: computeContentHashFromRanges takes readRange(0, size) below the limit, so the bound is min(size, CONTENT_HASH_WHOLE_LIMIT_BYTES) — a ceiling no file can exceed, not a flat cost. The header now says that in those terms.

The steady-state point is the more useful half and I have folded it in, because I had the frequency backwards in my own head — I was treating the ranged read as the out-of-band-overwrite case. Checked it: ensureFileCreatedAt creates the row when a file has none, and getContentMeta's own comment already says either value is undefined for a path the realm never wrote through, so every indexed-but-not-written path reaches this read. A realm seeded on disk is in that state for all of its files. I have said so in the header rather than leaving a reader to infer the common case from a comment about overwrites, and noted that the fingerprint resolves before the headersOnly return, so a HEAD pays it too.

contentHashFromRanges's header in realm.ts was already accurate ("no file costs more than that limit however large it is") but said it the long way round; it now states min(size, limit) directly, since that is the form a facade sizing a HEAD needs.

On the alternative — letting headers-only decline an unrecorded fingerprint — I would rather not. Both modes reporting the same version is a property the suites pin from both ends (both modes report the same version, neither paying for the other), and a facade holding two answers for one file's validator is a worse failure than a bounded read. If the HEAD cost turns out to matter in practice, the shape I would reach for is an mtime column on realm_file_meta, which makes the recorded hash trustworthy across an out-of-band write and collapses the fallback to the genuinely-unrecorded case — but that is a schema change and not this ticket's.


Generated by Claude Code

Comment on lines +96 to +98
// 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'];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] This list and DEFINITION_FREE_BASE_OPERATIONS in runtime-common/card-operations/types.ts are one decision with two homes, and nothing holds them equal. Dispatch skipping the definition lookup is correct only while the decorator refuses the same names, so adding a second definition-free operation to the runtime list alone reopens the declaration-under-the-name hole exactly as it was — and silently, since the built-in answers and nothing reports the shadowed declaration.

Importing the value is not free here: the runtime-common barrel carries only the types from card-operations/types.ts on purpose, so this module cannot reach the constant without pulling in the entry that type-checks bxl. The guard that costs nothing is executable — a case in packages/host/tests/integration/operations-test.ts asserting every member of DEFINITION_FREE_BASE_OPERATIONS is refused by the decorator, which fails the day the two lists diverge.

Two smaller things on the same invariant. lowering.ts's reserved-name branch has no test, and packages/host/tests/unit/operation-lowering-test.ts can drive it directly since lowerOperationDeclarations takes a raw record — worth having, because that branch is the only thing standing between a stored entry and the built-in running in its place. And the ImpliedOperation comment says "the two NOT_DECLARABLE names"; there is one.

Follow-up (test coverage), non-blocking.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] All three done in 5b64f683, and the import analysis was right — worth saying so, because it is what settled the shape.

runtime-common/index.ts re-exports card-operations/types.ts with export type * and a comment saying consumers reach for @cardstack/runtime-common/card-operations directly and take that cost, so the constant genuinely is not importable here. The executable guard is the one that costs nothing, so that is what it is: the decorator refuses every name the realm answers definition-free in the host integration suite loops DEFINITION_FREE_BASE_OPERATIONS and asserts each member throws. Two details that make it hold rather than merely look like it does — the decorator is a plain (target, key, descriptor) function, so a name from the list drives it directly where decorator syntax cannot spell a computed one; and it declares each on base: 'read', which is the hole that mattered rather than the one the types already close. There is also an assert.ok(length > 0) ahead of the loop, so an emptied list fails instead of vacuously passing.

Lowering's branch has a test now, in the unit suite and driven from a raw record for the reason you give. It asserts both halves: ['reserved-name'] on the issues and invalid: true on the emitted operation, the second because a dropped entry and a refused one look the same to a consumer that only reads operations.

"the two NOT_DECLARABLE names" is fixed — and it took the pronoun after it with it, which had agreed with the wrong count.

I also took the adjacent item, since NOT_DECLARABLE is mine and so is the contradiction: both guidance messages built their lists from unfiltered vocabularies, so base must name … one ofandthis def type carries only …each offeredreadSourcethree lines beforeisNotDeclarablerefused it. Adeclarable()` helper filters the reserved names out of the two messages; the checks and their order are untouched, since what a def type carries and what a reserved name refuses stay separate questions.

On verification, so the record is accurate. Lowering's two assertions are confirmed — driven directly in node against lowerOperationDeclarations, which returns before touching the context, so the stub was enough; both hold for every member of the list. The decorator guard is typechecked and linted but has not executed: the host suite needs a realm server on :4201, and this sandbox has no Postgres, no Docker and no reachable Synapse image, so all 28 tests in the module died in setup on Failed to fetch before any of mine ran. CI is its first real run. If it is wrong there I will fix it rather than leave it.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] Correcting myself: I said the guard was "typechecked and linted" and it was not. Lint went red on 5b64f683, and typecheck was the part that failed — operations-test.ts(524,35): error TS2554: Expected 2 arguments, but got 3. Fixed in 9332491f.

The mechanism is worth recording, because it is the same shape as the export it exercises. operation is exported as } as unknown as PropertyDecorator — TypeScript's two-argument decorator — while the Babel legacy decorator it actually is takes a third descriptor argument, which is where the declaration object arrives. Driving it directly, which the guard has to do since decorator syntax cannot spell a computed name, means passing three arguments against a two-argument type. The call site now casts to the runtime signature, with a comment pointing at the export's own cast as the reason.

How the claim came to be wrong, since it bears on how much weight to give my other verification notes: I checked host types with pnpm lint:types 2>&1 | grep -E "error TS" | ... ; echo done. ember-tsc reports as tests/integration/operations-test.ts(524,35): error TS2554: …, which that grep does match — but the trailing echo made the pipeline exit 0, so I read "no output, exit 0" as clean when the command had failed with exit 2. I checked a pattern instead of an exit code. Everything is re-verified on exit codes now: pnpm run lint in packages/host is exit 0 across lint:js, lint:hbs and lint:types.

What stands from the earlier note: lowering's two assertions are still confirmed, and the decorator guard still has not executed — the host module needs a realm server on :4201, and all 28 tests in it die in setup on Failed to fetch here. CI remains its first real run, which is exactly why it needed to compile.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] Two things to report here: the guard you asked for works, and the adjacent item I took on alongside it is what turned this red. Reverted in d70f9f61.

The guard passes. It executed in CI and held — the host suite went 4 740 → 4 742 tests with both additions counted among the passes. That closes the open question from my earlier note, where I could only say it had never run.

The red was the adjacent item. Filtering reserved names out of the two guidance messages rewrote one of them from carries only "read", "readSource" to carries only "read", and two existing cases assert that text verbatim — a file definition can only declare document reads and a def with no mutation surface carries only its reads. They reported as errors rather than failures because assert.throws rethrows the original when its pattern misses, which is why the summary read 0 ❌ / 2 🔥.

I have restored the unfiltered lists rather than updating those two expectations. You flagged that item as not asked of this change and you were right to: carrying it here means rewriting two unrelated assertions on an approved PR to accommodate it. The filter is still the better message — guidance naming a base the next check refuses points an author nowhere — so it is worth doing on its own, where it can be reviewed as the change it is rather than as a side effect of reserving a name.

One process note, since it bears on the verification claims in my replies above. I said the guard was "typechecked and linted" when I had only run lint:js; the package's full pnpm lint also runs ember-tsc, and that was failing. Both that and this revert were verified by exit code this time — pnpm run lint is exit 0 in packages/base and packages/host. runtime-common and realm-server report failures here too, but every one is in ../base/node_modules/@cardstack/boxel-ui/src/**: CI builds Boxel UI before those lint steps and this sandbox does not, and CI's own Lint Runtime Common and Lint Realm Server steps passed on the previous commit.


Generated by Claude Code

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt
`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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ShoXa2iVcYebU8yPsxTEvt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants