Skip to content

Replace a file's content and append a line to one, through the operation core - #6130

Open
habdelra wants to merge 7 commits into
cs-12793-card-ops-the-transform-executor-bxl-programs-over-a-cardsfrom
cs-12925-card-ops-update-on-a-filedef-replace-content-and-appendline
Open

habdelra wants to merge 7 commits into
cs-12793-card-ops-the-transform-executor-bxl-programs-over-a-cardsfrom
cs-12925-card-ops-update-on-a-filedef-replace-content-and-appendline

Conversation

@habdelra

@habdelra habdelra commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Background

A file in a realm — an image, a PDF, a markdown document, a CSV, a log — is described by a FileDef. Its metadata (name, contentType, contentHash, contentSize) is derived from the bytes and is read-only, so unlike a card there is no JSON:API document to change: what a file is, is its content. Until now the operation core gave files two ways in, both reads — read, which returns the metadata document, and readSource, which returns the bytes.

This adds the two writes. update on a file replaces its content wholesale, and appendLine adds one newline-terminated line to a text file without reading what is already there. Both go through the batch coordinator, so a file change is a first-class member of an all-or-nothing batch alongside card changes: one write lock, one index job, one index event, and nothing written if any entry fails.

Two things motivate putting file writes on the operation core at all. A fine-grained authorization policy layered on operations can only gate what flows through operations, so a write path left outside it is an unguarded exit. And some file edits are append-only by nature — a log, a ledger, an audit trail — and should not cost the file.

Why appending needs its own primitive

Every write the realm serves today replaces a file's whole content. RealmAdapter has openFile, write and remove, so adding one line to a log means reading the log, concatenating, and writing all of it back. For a file of a few hundred megabytes that is the whole cost of the operation, paid to add sixty bytes.

So the adapter gains append(path, contents), implemented for the node filesystem with an append-mode write, and _commitBatchUnlocked — the realm's one commit covering everything a caller changes — gains a leg for it beside the writes and the removals. An appended file gets the same per-file treatment a written one does: the size ceiling, the index-initiation event, the own-write tracking that stops the file watcher re-reporting this replica's own change, the byte-cache drop, the peer notification, and a realm_file_meta row. It joins the same single index job and the same single index event.

The version. Every write reports a version — the content fingerprint the realm records for the file, which a caller holds to tell whether the file has moved on and to name the base its next write is computed against. After an append that fingerprint has to describe the whole file, which sounds like it means reading all of it. It does not: computeContentHash already samples above CONTENT_HASH_WHOLE_LIMIT_BYTES (5 MB), where a fingerprint is the byte length plus a hash of the head and a hash of the tail. So the append leg assembles the version with computeContentHashFromRanges, which asks the adapter for at most 5 MB however large the file is and produces exactly the value hashing the whole content would have produced. Adding a line to a file of any size costs the line plus a bounded read — the same machinery, and the same bound, the appendContainsMany splice write already uses.

Ordering, and one file changing twice. The commit writes first, then appends, then removes. That ordering is what makes "replace this log's content, then add a line to it" land the way the entries were sent. When a path is in both legs it is one file changing twice, not two files changing, so the append's readings replace what the write recorded rather than joining them — two realm_file_meta rows for one path in one statement is an error Postgres raises outright.

Which entry means which target

The coordinator was built when every target was a card, so it derived a target's stored path by appending .json to the entry's href, read every target's file as text before any executor ran, and committed writes and removals only. A file's href is its path, and a file replaced wholesale has no merge base to read.

The entry's payload is what says which target it means, and it says so without anything having to classify a URL:

  • update carrying a JSON:API document is a card's — the patch is merged over the card's stored document at href + .json.
  • update carrying content is a file's — the bytes replace what is stored at href.
  • appendLine is a file's too, and reads nothing at all.

That matters because a URL is the one thing that cannot settle the question. Dispatch classifies an instance target by its extension, and the extension table does not name every stored file — a .log, a .css, a .yml holds bytes and serves them, and each is classified as a card. Reading the intent off the payload means neither the caller nor the coordinator has to guess.

So the pre-read is now per entry rather than per batch. A card is read whole, because a patch merges over it. A file whose content is being replaced is not read: what the batch keeps for one is its version, assembled from the same bounded ranges as above, which is all a baseVersion comparison needs. An appendLine's target is not read at all, and is not opened at all — establishing that the file is there is a stat.

What a file write refuses

Two stored things are addressed by their own path and are not files in this sense: a module's source, and a card's stored .json. Replacing either wholesale through the envelope would be writing code or a card while saying it was changing a file, so both are refused — a module by its extension, a card's source by what its bytes hold, using the same isCardDocumentString predicate the realm's own size classification uses to decide which ceiling a write is held to. Stored JSON that is not a card is a file like any other and is writable. A card addressed without an extension is refused too, and told that its update carries a patch rather than content, rather than being told nothing is there.

rawSource: true is how the one caller that must reach those says so. It replaces the bytes stored at the path verbatim, whatever they are, and creates the file when nothing is there — which is what today's card+source POST does for modules and for a card's raw .json. Nothing on the envelope path sets it; the flag is defined here so the facade ticket that migrates that route only has to set it.

appendLine refuses everything whose name says a line of text does not belong at the end of it: binary content (an unknown extension resolves to a binary type and is refused with them, which is the byte-preserving side to be wrong on), a JSON document — what follows a JSON document's closing brace is no longer one, which is why a card's stored source is the case that matters most — a module, and a card. It also refuses a line that already contains a line break: the terminator is the operation's to add, so one call appends one line and the version it reports describes the file the caller asked for.

One known gap in that classification. mime-types resolves neither .jsonl nor .ndjson, so inferContentType answers application/octet-stream for them and they are turned away as binary — and newline-delimited JSON is the one format whose whole point is that a line gets appended to it. Fixing it properly means adding both to CONTENT_TYPE_OVERRIDES and to the textual-application-type list, which is not a local change: isBinaryFilename is what boxel-cli uses to choose between the binary and text wire formats when it syncs, reads and writes files, so reclassifying an extension changes how a published package transfers those files. That deserves its own change with the CLI's own coverage, so it is left as a follow-up. The behavior here is a refusal rather than a corruption, so the gap costs a caller an error, not a file.

A file's size ceiling is applied to what an append adds rather than to what the file will hold. The limit is over the bytes a caller hands the realm, and a file grown past it one line at a time is what an append-only file is; measuring the result would mean knowing the file's length, which is the question an append exists in order not to ask.

Composing inside one batch

Two entries naming one file compose the way two entries naming one card do. Two appendLines are joined in order and reach the file once, and both report the version the commit left it at, since one commit is what produced it. An update followed by an appendLine keeps both, in that order.

Two entries that both replace one file are refused, and so is the other direction. Two card patches compose because the second merges over the bytes the first staged; a wholesale replacement merges over nothing, so a second one would drop the first entry's content while that entry still reported success — and carried the second entry's version as its own. An appendLine followed by an update on the same file is the same objection: the commit writes before it appends, so the earlier entry's line would land after the later entry's content instead of being replaced by it. Reordering would silently discard the append, so the pair is refused and the two changes are sent as separate batches. An update on a file an earlier entry changed without reading is refused for the same reason.

The commit's own conflict check covers an append against a path the batch removes, too. Nothing expressible today reaches it — a removal always names a card's .json and an append refuses every JSON content type — but that invariant lives in the executors rather than in the commit, and without the check a delete that ever named a file would have the commit recreate the path, append to it, announce it as added, and then unlink it.

What this does not change

Existing write / writeMany / delete behavior, the existing source POST and binary upload routes, read and readSource. No transport reaches these executors yet: runOperation still answers 501 for every write, and the envelope endpoint and the source-POST facade are their own tickets. One consequence of that is worth naming for whichever lands first — dispatch's allow table admits a file-only behavior only for a target its extension classifies as a file, so appendLine on a telemetry.log is refused before its executor runs. That gate has to let a card-classified instance target through and leave the file/card discrimination to the executor, which is what this PR builds; the table already carries a comment saying so, and changing it belongs with the transport that first routes a write through dispatch.

Tests

The coordinator's decisions — which files it would write, what it refuses, and whether it commits at all — are pinned against a stub in realm-server/tests/card-operations-file-write-test.ts, where "nothing is committed" is the property itself rather than a proxy for it: a file's content replaced at its own path, a base version reported for a file the batch never read, each of the four things a file update refuses and the verbatim replacement that reaches two of them, a line added without the file appearing anywhere on the batch's read surface, two lines joined into one append, an update and an append composing, the reverse being refused, and each way an append can be aimed at something that is not a text file.

The rest only exists against a real realm and is covered in realm-server/tests/card-operations-commit-test.ts: the bytes on disk after an update and after an append, the version each reports matching the content hash of what is actually stored, one index job and one index event per batch, a file-only batch of two updates and an append, the same batch with a failing last entry leaving all three files byte-identical with no job and no event, a mixed batch of a card create with a local id, a card update linking to it, a file update and an append committing together, and a failing entry of either kind leaving the other kind untouched. The last case appends to a 32 MB file and counts what the commit asks the adapter for, asserting the total is within CONTENT_HASH_WHOLE_LIMIT_BYTES — the ceiling a fingerprint is assembled under however large the file is. It counts bytes rather than watching the heap deliberately: every read on this path yields Uint8Arrays, whose backing stores heapUsed does not count, so a version computed by reading the file end to end would register as no growth at all and a heap-based assertion could not fail for the regression it names.

🤖 Generated with Claude Code

habdelra and others added 4 commits September 15, 2026 10:38
Every write the realm serves replaces a file's whole content, so adding a
line to a log costs the log. The adapter gains an append primitive, the
commit gains a leg for it, and a batch entry may stage one — joined in
order per path, so two lines added to one file reach it once.

The appended file's version is assembled from bounded reads rather than
from bytes in hand, which is the same value hashing the whole content
would produce at a cost no file can exceed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An update now takes a file as well as a card, and which one it means is
read off the payload it carries: a JSON:API document is a patch to merge,
content is a file's bytes to replace. A file's url is its path, so nothing
is appended to it to find the bytes; only a card's gets `.json`.

A module's source and a card's stored `.json` are addressed the same way
and are refused, since replacing either wholesale would write code or a
card while saying it was changing a file. The `card+source` POST reaches
them with `rawSource`, which the envelope never sets.

Alongside it, `appendLine` adds one newline-terminated line to a text
file. Every check it makes is one the file's name and a stat can answer,
because the target is never opened: refusing binary content, a JSON
document, a module and a card, and establishing the file is there.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A batch may replace a log's content and add a line to it, which is one
file changing twice. Everything the write recorded describes bytes the
file no longer holds, so the append's readings replace them — two
realm_file_meta rows for one path in one statement is an error Postgres
raises outright.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The coordinator's decisions are checked against a stub, where "nothing is
committed" is the property itself rather than a proxy for it: which file a
write lands at, what each of the two writes refuses, the line an append
adds without the file appearing anywhere on the batch's read surface, and
how two entries naming one file compose.

Holds its own stub rather than a shared one, since nothing in the host
runs this suite.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 15, 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-15T15:18:51.185821Z 47f185b 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: 47f185bafb

ℹ️ 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 on lines +761 to +765
if (stored && isCardDocumentString(stored.content)) {
refuse(
`${url.href} holds a card's stored source; an update on a card is ` +
`the merge its document describes, not a replacement of its bytes`,
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Distinguish collection JSON from stored card source

When an ordinary .json file contains a JSON:API collection such as {"data":[]}, this check rejects a content replacement as though the file were a card. isCardDocumentString accepts both single-card and collection documents, but collection documents never become card instance rows, so they remain regular files and should still support file updates; use the single-card predicate for this refusal.

Useful? React with 👍 / 👎.

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 🤖] Reachable, and narrower than it reads: the collection branch of isCardDocumentString accepts any data array whose every member is a card resource, which an empty array satisfies vacuously — {"data": []} classifies as a card document, so a data file holding it is refused with "holds a card's stored source". A collection of real card resources is the other case; anything else ({"data": [1, 2]}, {"data": {…}}) already falls through.

This repo also already draws that line the other way, for the same reason and with the reason written down — realm.ts's not-indexed-yet marker parses the source and gates on isSingleCardDocument, over a comment saying a collection document "never becomes an instance row". So a .json holding one is served as a file and indexed as a file, and turning a replacement of it away as a card's source contradicts what the realm does with it everywhere else.

Worth noting before switching the predicate: the comment above this check justifies the choice as "the same isCardDocumentString predicate the realm's own size classification uses", and that stays true of writeSizeType — a collection document keeps the card ceiling and keeps counting as an instance for the mid-loop index flush. So narrowing this to isSingleCardDocument is right, but the justification above it needs rewording rather than keeping, since the two predicates would then no longer be the same one.

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 🤖] Fixed in e6c79f0. The refusal now parses the stored content and gates on isSingleCardDocument, so a .json holding a JSON:API collection is replaceable like any other file — pinned by a case that replaces one holding {"data": []}.

The justification above it is reworded rather than kept, as you noted: writeSizeType still classifies a collection document as a card for the ceiling, so the two predicates are no longer the same one and saying they are would be false. The coordinator's targetRead carried the same sentence and is reworded with it — it now says why a .json file target is the one whose content the batch reads, without borrowing the size classification's authority for it.

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

github-actions Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Preview deployments

Host Test Results

    1 files      1 suites   2h 29m 27s ⏱️
4 819 tests 4 805 ✅ 14 💤 0 ❌
4 834 runs  4 820 ✅ 14 💤 0 ❌

Results for commit d3d06cf.

Realm Server Test Results

    1 files    226 suites   1h 18m 27s ⏱️
3 121 tests 3 121 ✅ 0 💤 0 ❌
3 160 runs  3 160 ✅ 0 💤 0 ❌

Results for commit d3d06cf.

@habdelra habdelra left a comment

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 🤖] This review went after the commit's new append leg and the coordinator's per-entry pre-read — what the batch records for a file it never opened, how the two legs compose when one path is in both, and whether either new executor's refusals can be reached around. The declaration, lowering and dispatch tables this builds on were not re-examined, and I did not run the suites.

No blocking issues. One behavioral gap worth fixing before merge — a file whose write leg short-circuits and whose append then lands is never announced as changed — and the rest are decisions or a test that cannot fail.

Both bot findings hold, and I have confirmed each in its own thread with the reachability evidence. The isCardDocumentString one is narrower than it reads but real: {"data": []} satisfies that predicate, and this repo already draws the card line at isSingleCardDocument for exactly this reason.

  1. Announce the path from the append leg's already-written branch — reply in the bot thread on realm.ts.
  2. Decide whether an update on a file keeps its bounded-read property through the commit, or say that it does not — thread on readPreState.
  3. Give the bounded-memory case an assertion that can fail — thread on card-operations-commit-test.ts.
  4. Two pairs commitStaged's conflict matrix lets through: a second update on one file, and an append after a delete — thread on commitStaged.
  5. .jsonl / .ndjson are refused as binary — thread on stageAppendLine.
  6. collect drops the length check its twin in realm.ts carries — thread on collect.

Adjacent, not this PR: one path in both legs produces two file-system events while trackOwnWrite records a single echo-suppression key for it, so the watcher can re-report the second as an external change. That suppression describes itself as a best attempt and the cost is one extra reindex of one file — worth knowing for whoever next touches the tracking.

Comment on lines +478 to +483
// What an entry needs from its target is not the same for every entry, so the
// form of the read is the entry's to name (`targetRead`). A card is read whole,
// because a patch merges over it. A file whose content is being replaced is not
// read at all: there is no merge base, and the bytes could be a hundred
// megabytes of log. What the batch keeps for one of those is its version,
// assembled from bounded reads, which is all a `baseVersion` comparison needs.

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 🤖] This bound does not survive the commit for the payload the envelope sends. _commitBatchUnlocked's write leg calls readFileAsText(path, …) for every string content before it writes — the unchanged-bytes short-circuit — and fileContentToText drains the whole file into a string, so replacing the hundred-megabyte log named here still holds the hundred megabytes, on the V8 heap, inside the write lock. The version assembled from ranges here saves the second read of that file, not the first. The Uint8Array payload is the one that skips it (it takes exists() instead), so the bounded case is the one no envelope client can send.

A size comparison settles it without reading anything: openFile already reports size and lastModified, so size !== computeContentSize(content) decides "different" for every replacement that changes the length, and only an equal-size candidate has to be read and compared. Failing that, the honest move is to narrow this comment to what the pre-read does and say the commit reads what the pre-read declined to.

Pre-existing — the short-circuit predates this — but this is the change that routes unbounded file content through it. 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 🤖] Fixed in e6c79f0 with the size comparison. The write leg now opens the file and compares size against computeContentSize(content) before reading anything; only an equal-length candidate is read and compared, and isNewFile comes off that open rather than off the read a file whose length settled it never had. So the bound the comment promises holds through the commit for the envelope's text payload, not just for the Uint8Array the facade will send.

Worth stating plainly: this is the pre-existing short-circuit, and it is load-bearing — a PATCH that changes nothing depends on it to leave the file alone. The comparison only ever turns a read into a stat, never changes the answer, and an adapter that cannot report a size falls through to reading as before.

Comment on lines +549 to +561
async function collect(chunks: AsyncIterable<Uint8Array>): Promise<Uint8Array> {
let read: Uint8Array[] = [];
let total = 0;
for await (let chunk of chunks) {
read.push(chunk);
total += chunk.length;
}
let bytes = new Uint8Array(total);
let at = 0;
for (let chunk of read) {
bytes.set(chunk, at);
at += chunk.length;
}

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 🤖] This is readRangeBytes in realm.ts minus its length check, and that check's own comment gives the reason it is there: a short read yields a fingerprint describing content of one length under a marker claiming another. The value assembled here is what a caller's baseVersion is compared against, so a truncated range read reports a current version as moved on, with nothing said. The expected length is at the call site already (bytes.read(start, start + length)), so refusing a short read costs one comparison. Nit, 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 🤖] Fixed in e6c79f0collect takes the expected length and throws on a short read, with the reason its twin gives. You are right that this one matters more than the twin's: what it assembles is compared against a caller's baseVersion, so a truncated read would not corrupt anything, it would just tell a caller its base had moved when it had not.

Comment on lines +858 to +871
for (let append of change.appends) {
appends.set(
append.path,
`${appends.get(append.path) ?? ''}${append.content}`,
);
appendedBy.set(append.path, index);
}
for (let path of change.deletes) {
deleted.add(path);
// Both legs drop, for the reason the removal wins over either: the file
// is on its way out, so producing content for it and unlinking it in the
// same commit is work with no observable result.
writes.delete(path);
appends.delete(path);

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 pairs this matrix lets through, both the shape of the one it refuses above.

A second update on a file an earlier entry already replaced. writes.set overwrites, the side-load check passes because the path is the later entry's own primaryPath, and the earlier entry's content never reaches the file while that entry reports success carrying the later entry's version. Driven against a stub core: update notes.md → "first" then update notes.md → "second" commits {notes.md: "second"}, and both results report the second version. The reason given a few lines up for refusing append-then-write is "both entries would report success over a file neither of them described" — a wholesale replacement composes over nothing, so unlike two card patches there is nothing that merges the dropped content back in. Refuse it too, or is last-wins the intended rule for file content?

An append after a delete. change.appends is not checked against deleted the way change.writes is. It holds today only because stageDelete always names <path>.json and stageAppendLine refuses every JSON content type — an invariant that lives in another file and that nothing here asserts. A delete that ever names a file would have the commit recreate the path, append to it, announce it as added, and then unlink it. Cheap to pin now, as the symmetric check or as an assertion that says why it cannot happen.

Regression for the first (the pair is only expressible now), missing guard for the second. 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 🤖] Both closed in e6c79f0.

Two replacements. Refused, not last-wins. Your framing is the deciding one: two card patches compose because the second merges over what the first staged, and a wholesale replacement merges over nothing — so the pair is the same hazard the append-then-write refusal already names, and answering it differently was inconsistent rather than intentional. StagedChange.replacesContent is what tells a replacement from a composing patch at the commit, set only by the file branch. Two cases pin it, including one where both entries send identical content, so the pair is refused on what it asks for rather than on what it happens to produce.

Append after a delete. Symmetric check added, with a comment saying it is a guard for an invariant that lives in the executors rather than here. It is unreachable through any entry the coordinator can stage today, so nothing tests it — a test would have to stage a delete naming a non-.json path, which no executor produces.

Comment on lines +876 to +881
let contentType = inferContentType(path);
if (isBinaryContentType(contentType)) {
refuse(
`${url.href} holds ${contentType}, and a line of text appended to ` +
`binary content is not a line of anything`,
);

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 🤖] .jsonl and .ndjson are turned away here rather than by the JSON refusal below: mime-types resolves neither, so inferContentType answers application/octet-stream and the caller is told their newline-delimited JSON log holds binary content. That is the one format whose whole point is that a line gets appended to it, and unlike a JSON document a trailing line leaves it valid — so the byte-preserving default lands on the wrong side of exactly the case this behavior exists for. Two entries in CONTENT_TYPE_OVERRIDES, or deliberate? Follow-up either way, 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 🤖] Real, and deliberately left as a follow-up rather than fixed here.

The fix is not local to this behavior. inferContentType is the realm's content-type authority, and isBinaryFilename over it is what boxel-cli uses to choose between the binary and text wire formats when it syncs, reads and writes files — so adding two entries to CONTENT_TYPE_OVERRIDES plus one to TEXTUAL_APPLICATION_TYPES changes how a published package transfers every .jsonl and .ndjson in a realm, and that classification has mangled files before. It deserves a change that carries the CLI's own coverage.

What this behavior does in the meantime is refuse rather than corrupt, so the gap costs a caller an error and not a file. It is now stated as a known limitation in the PR description so it is not mistaken for a classification anyone thought through.

Comment on lines +1556 to +1560
assert.ok(
growth < SIZE / 4,
`appending held ${Math.round(growth / 1024 / 1024)}MB, which does not ` +
`scale with the ${Math.round(SIZE / 1024 / 1024)}MB already stored`,
);

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 🤖] This assertion cannot fail for the regression it names. heapUsed does not count ArrayBuffer backing stores — measured on this Node: readFileSync(p) of a 64MB file moves heapUsed by 0.0MB and arrayBuffers by 64.0MB, while readFileSync(p, 'utf8') moves heapUsed by 64.0MB. Every read on the append path is bytes (readRangeBytes collects Uint8Array chunks), so an implementation that read the whole file through readRange — the plausible regression, since readRange is what produces the version — registers as ~0 growth here.

The three assertions above do not discriminate either: computeContentHash over the whole 64MB returns the same s1:<len>:<head>:<tail> string computeContentHashFromRanges does, so the version equality and isSampledContentHash both hold for a whole read. As it stands the test pins the file's new length and the version's shape, and nothing in it fails if the file is read end to end.

Two ways to give it teeth. Cheapest: fold arrayBuffers into the reading (heapUsed + arrayBuffers), which catches the byte read the current metric is blind to. Better: count what the adapter is asked for — onRealmSetup already hands out testRealmAdapter, so wrapping its readRange for the duration of the commit and asserting the total is at most CONTENT_HASH_WHOLE_LIMIT_BYTES is deterministic, needs no --expose-gc, and retires the globalThis.gc assertion with it. 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 🤖] You are right that it could not fail, and fixed in e6c79f0 by the second route you suggested. The test now wraps the adapter's readRange for the duration of the commit and asserts the total requested is within CONTENT_HASH_WHOLE_LIMIT_BYTES — deterministic, and it retires the globalThis.gc assertion with it. The file is 32MB rather than 64MB, since the assertion no longer needs the size to outrun a noise floor; it only needs to be past the sampling threshold.

The version-equality assertion stays, because it pins something the byte count does not: that the version reported describes the file on disk. The isSampledContentHash one is gone — as you said, it holds for a whole read too, so with the byte count carrying the claim it was pinning a shape rather than a property.

habdelra and others added 2 commits September 15, 2026 11:42
A replacement of a file's content composes over nothing, so two of them on
one path is the earlier entry's content never landing while it reports
success — the same objection that already refuses appending to a file a
later entry replaces. Refuse it too, and check an append against a removed
path the way a write is checked.

An append announces its own change whether or not the write leg announced
one: a replacement that found the file already holding its bytes records a
result and announces nothing, which left the appended file's change
reaching no subscriber.

A card's stored source is a single card document. A JSON:API collection is
stored and served as a file and never becomes an instance row, so a
replacement of one is a file write.

The unchanged-bytes check rules itself out by length before it reads, so
replacing a file that is large no longer costs its size to discover the
content differs. And the fingerprint a caller compares a base version
against refuses a short read, which would otherwise report a file the
caller is current with as having moved on.

The large-file case counts the bytes the commit asks the adapter for
rather than watching the heap: the reads on that path yield byte arrays,
whose backing stores a heap reading does not count, so it could not fail
for the regression it names.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both branches add an executor and a staged-change leg, so each side's
additions are kept: the entry union carries all three new ops, a staged
change carries both the replacement marker and the run's diagnostics, and
the one `fileExists` on the staging context states both reasons an
executor asks for it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@habdelra
habdelra changed the base branch from cs-12930-card-ops-declaration-and-lowering-catch-up-appendline to cs-12793-card-ops-the-transform-executor-bxl-programs-over-a-cards September 15, 2026 15:51
@habdelra

Copy link
Copy Markdown
Contributor Author

[Claude Code 🤖] Rebased the stack: this now sits on #6131 rather than on #6115 directly, since both branches add an executor and a staged-change leg and would otherwise have conflicted at merge time. The diff above is this PR's own work only.

Every review thread has a reply naming the commit that answers it. The one finding left open is .jsonl / .ndjson being refused as binary content — the description says why it is a follow-up rather than a fix here.

A link check arriving on that side maps a card's id to its stored path,
which the per-entry pre-read here had folded into the read it plans. Both
callers now share one named mapping: a link always names a card, so it
always wants the `.json` the card's own path never carries.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@habdelra
habdelra requested a review from a team September 15, 2026 16:14
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.

1 participant