Skip to content

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

Open
habdelra wants to merge 12 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 12 commits into
mainfrom
cs-12912-card-ops-readsource-base-operation-the-stored-bytes-read-for

Conversation

@habdelra

@habdelra habdelra commented Sep 10, 2026 •
edited
Loading

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.ts — DEFINITION_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.ts — computeContentHashFromRanges, the existing fingerprint assembled from bounded reads.
  • packages/base/operations.ts — BASE_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

@claude
Add readSource, the stored-bytes read, as a base operation
a7a6f3b 
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 •
edited
Loading

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[bot]
chatgpt-codex-connector Bot reviewed Sep 10, 2026
View reviewed changes

@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 •
edited
Loading

Copy link
Copy Markdown
Contributor

Preview deployments

Host Test Results

    1 files  ±0      1 suites  ±0   2h 44m 8s ⏱️ + 5m 46s
4 740 tests +5  4 726 ✅ +5  14 💤 ±0  0 ❌ ±0 
4 755 runs  +5  4 741 ✅ +5  14 💤 ±0  0 ❌ ±0 

Results for commit 842b224. ± Comparison against earlier commit 5d8c8d1.

Realm Server Test Results

    1 files  ± 0    211 suites  +1   1h 16m 30s ⏱️ + 6m 16s
2 799 tests +11  2 799 ✅ +11  0 💤 ±0  0 ❌ ±0 
2 838 runs  +11  2 838 ✅ +11  0 💤 ±0  0 ❌ ±0 

Results for commit 842b224. ± Comparison against earlier commit 5d8c8d1.

@habdelra
habdelra requested a lite review from Copilot September 10, 2026 15:01
@claude
Derive the source version from the bytes, and stop refusing underscor…
27193c0 
…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 reviewed Sep 10, 2026
View reviewed changes

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
@claude
Reserve the readSource name, address paths from the realm root, one m…
699f46d 
…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
@habdelra Claude

habdelra commented Sep 10, 2026

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:290 — version 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

@claude
Include readSource in the inherited-operations key list
6479de6 
`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
@claude
Read a stored-bytes body once in the underscore-prefixed test
0264612 
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
@claude
Populate the source version on the byte routes' own terms
a78eaf9 
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
@claude
Read a source version in bounded ranges rather than by streaming the …
910e520 
…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
@claude
State the definition-free branch's basis by reference rather than res…
4ae5b61 
…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
@claude
Merge remote-tracking branch 'origin/main' into cs-12912-card-ops-rea…
b527644 
…dsource-base-operation-the-stored-bytes-read-for
@claude
Fingerprint every stored path, not only the ones a route validates on
97260cb 
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
@claude
Merge remote-tracking branch 'origin/main' into cs-12912-card-ops-rea…
5d8c8d1 
…dsource-base-operation-the-stored-bytes-read-for
@claude
Merge remote-tracking branch 'origin/main' into cs-12912-card-ops-rea…
842b224 
…dsource-base-operation-the-stored-bytes-read-for
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@habdelra @claude