Skip to content

feat: run an operation's input and output transform stages - #6196

Open
habdelra wants to merge 11 commits into
mainfrom
cs-12798-card-ops-inputoutput-transform-runners-and-the-default-vs
Open

habdelra wants to merge 11 commits into
mainfrom
cs-12798-card-ops-inputoutput-transform-runners-and-the-default-vs

Conversation

@habdelra

@habdelra habdelra commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Background

A card operation is a piece of data, not a piece of code. A card author writes an @operation declaration on their CardDef, the indexer lowers it into the type's definition-cache entry as plain JSON plus BXL — the sandboxed expression language card authors already use for computed fields — and the realm carries it out from that alone, without ever loading the card's module. There are nine built-in behaviors (read, create, update, delete, transform, query, readSource, appendLine, appendContainsMany), and a declaration names one of them as its base.

Around whichever behavior it names, a declaration may carry two optional transform stages, also written in BXL:

  • input runs first, over the payload the caller sent, and produces the payload the operation actually uses. Its job is normalizing or filling in values — supplying a default the caller left out, say.
  • output runs last, over the operation's result, and reshapes or trims it — a read whose output leaves out fields a particular consumer doesn't need, or renames them.

Neither stage ran before this change. Two separate gates refused an operation that declared one: the read executor answered 501 for a read specialized with input or output, and the batch endpoint answered 501 for any entry whose definition carried either. This PR makes both stages real and removes both gates.

This is identity-aware, and it is not access-enforced. A stage may read actor(), and an output may leave a field out of what it projects, but the realm verifies no claim the caller makes and refuses no invocation on the strength of who is asking — the realm's own read/write permission is the whole of what is checked. A field an output omits is still reachable by any caller permitted to read the realm: through the card's plain read, through its stored source, through a search. A projection is a response shape, never a boundary. That is stated in the transforms module, at the envelope handler, and on the authoring API's OperationOutput type, so nobody meets the feature without meeting the caveat.

The order an invocation runs in

Both stages run in runOperation, the dispatch entry, rather than inside any one executor:

  1. input over the payload
  2. the params check, against the payload input produced
  3. the behavior
  4. output over its result

Step 2 reading the transformed payload is what makes an input useful at all — an operation declaring params: { view: StringField } with input: bxl\. + {view: (.view // "summary")}`is invocable with noviewat all, because by the time the check runs the value is there. Put the check first and theinput` could only ever rewrite values the caller had already supplied.

Both stages live in dispatch rather than in an executor for a different reason: a behavior added later cannot quietly be one that ignores them. An operation whose output was skipped answers with more than its author said it would, which is the one failure mode a projection must not have — so the code is arranged so that forgetting is not possible. The writes are the exception and they have to be: they never reach runOperation at all (it still answers 501 for them; they go to the batch coordinator), so the envelope handler runs their two stages itself, input before the entry is staged and output over the committed result.

A new BXL profile

BXL programs run under a profile that says which language constructs and which builtins are available. The existing profiles are compute (permissive), mutation (a write plan), derive (a stored computation), policy/authorization (an access decision) and predicate (something compilable to SQL). A transform is none of those: it is a plain expression over one value, so it gets its own, transform.

transform allows what an expression needs and refuses what an expression has no business doing. It inherits the sandbox bans every non-compute profile carries — jq's assignment operators, user-defined helpers, reduce/foreach, recursive descent, try, label/break, format filters — which is what "no mutation statements" means concretely. It refuses every call with a side effect (env, stderr, halt, error, input, debug) and every call that reads the runtime (builtins, modulemeta, …). It keeps the four request-context builtins, since reading what the request carries is the whole of a transform's job.

It differs from mutation in one way: a volatile call (NOW, RAND, …) is allowed. A mutation plan is replayed by the client's optimistic path, so it must be repeatable; a transform shapes one request's payload or projects one request's result and is never stored or replayed. The operation records the volatility as deterministic: false instead of refusing it — which is what the existing deterministic computation already assumed, since input and output were the only two places it could ever go false.

The profile is enforced in two places, and deliberately: at lowering, so a program reaching outside the dialect becomes a lowering issue an author sees against their own edit; and at evaluation, by the runner. The new @cardstack/bxl/transform sub-entry is what runs one — it scopes the request context around a synchronous evaluation (the context stack unwinds synchronously, so anything deferred past it would read another request's context) and refuses a program that yields anything other than exactly one value. A program yielding none is refused rather than treated as null: a payload silently becoming absent is precisely the failure these stages exist to avoid.

What a customized read does to the plain GET

A CardDef may override read itself. When it does, the projection applies to the ordinary card+json GET of every card of that type — which changes what the response may promise.

The card+json GET is a cacheable, conditional route. It serves Cache-Control: public|private, max-age=0, must-revalidate with an ETag, so a client that has seen the card once sends If-None-Match on every later read and the realm answers 304 cheaply when nothing has moved. The ETag is built from the card's indexed_at, the realm-info hash, its screenshot fingerprint and the request's link shape.

A projected body cannot be described by that validator, and the reason holds whether or not the projection reads actor(): the index row does not move when a type gains, changes or loses an output. A card whose type is given a projection keeps the validator it had, so every stored copy of the unprojected body — in a browser cache, in a CDN, in the realm's own response cache — would go on answering as fresh. So a GET that projects is served Cache-Control: private, no-store, takes no 304 fast path, and is never retained in the response cache. private covers the rest: a projection that reads actor() is per-caller, and no shared cache may hold one.

The ETag is still emitted. It identifies the card's state for a conditional writeIf-Match, which asks about the card rather than about the representation it was read in — and that question is unaffected.

HEAD states the headers the GET would send, so the headers-only read reports whether the full read would project and the HEAD handler follows it: same directive, same absence of a 304.

Cards whose type declares no read are completely untouched — same bytes, same headers, same 304.

What it costs

Deciding this before answering means resolving the target's read definition before the conditional fast path, because the fast path is an answer built without assembling. That resolution is asked only where an answer would be taken from it: on a conditional hit, and on a request a configured response cache would serve or retain. Every other request assembles, and the assembly resolves the same definition anyway. The index-row peek is shared with the handler's own through one OperationScope.

That still left a definition-cache lookup — a SELECT plus a parse of the module's definitions blob — on exactly the two paths that exist to answer without one, so the answer is remembered per card on the realm and the whole map is dropped in clearRealmIndexCaches(). A warm conditional request is back to the single row read it had. Clearing on the index swap is sound rather than merely conservative: the answer is a function of the card's stored adoptsFrom and its type's declarations, a write moves the first and reindexing a module moves the second, and both swap the index — locally and, through the realm_index_updated broadcast, on peer replicas. A foreign realm's module changing without this realm swapping cannot reach it, because a card with foreign-realm dependencies is served no validator at all, so neither fast path is taken and the question is never asked.

A target whose read cannot be resolved — a declaration lowering flagged invalid — takes the assembling path too, so the request answers the real refusal instead of being handed a 304 built from a validator. That answer is not remembered.

A projected assembly is also refused by the response cache's own retention predicate, beside the queryBacked check that is there for the same kind of reason: both are documents that are not a function of their own index row. The handler decides not to populate before assembling, so without that check the decision would rest on the timing of a definition read taken before the body existed.

Refusals

A stage that reads actor() on a request that authenticated nobody (a public realm, no credentials) is refused 401, not 500, before the behavior runs. Credentials would change the outcome, and that is what a 401 says. Whether an operation reads the actor is read off readsActor on the stored definition, which lowering records, so the question is answered without reaching the program text.

A program that fails is the author's 400 whichever of the three ways it failed — it does not parse, it reaches outside the dialect, or it failed on the values this request handed it — carrying the program's own message plus meta.stage and meta.phase. This matches how the transform behavior already reports a failing mutation program. A failure inside the evaluator itself, rather than in the program, is a 500.

Where a failure leaves the realm. An input runs before anything is staged, so a program that fails there refuses the operation with nothing written. An output projects the result of the operation, and a write's result does not exist until the write has committed — so a program that fails there answers 400 over a card that has already changed. The refusal says the caller cannot be told what happened, not that nothing did. This is the one place the batch's all-or-nothing does not reach.

Lowering narrows what can get there and does not close it, which is worth stating precisely because it is what the hole is being accepted on. Lowering refuses a program that does not parse or reaches outside the dialect. It does not ask how many values a program yields, what shape they are, or whether it names a context slot the transport running it fills — so a declaration can be shipped whose write commits and whose projection refuses on every call, with the author finding out at the first invocation rather than at the edit. One of those three is now closed where it can be (empty, the deliberate spelling of "yield nothing", is denied by the profile); "yields exactly one value" is not decidable in general, and the unfilled-slot case wants lowering to know which transport will run the stage, which it does not.

Boundaries in this pass

  • instance() is not supplied. It is a slot on the runner's context and a legal call under the profile, but neither transport fills it today: an operation's stored document is read under the write lock by the behavior itself, and neither a read nor the envelope has one in hand at the moment a stage runs. A program naming it is told the host supplied none. realmConfig() is supplied, on both transports — read at most once per stage, and only where the program names a setting, since a cold read is a parse of the realm's config document.
  • A read's output must stay a JSON:API document — an object with an object data. The card+json response it is served in carries that member and every client reads it. What the projection puts below data is entirely the author's; a program that produces something with no data at all is a 400 naming the reason. A batch entry's projection is held to the weaker rule that it remain an object, since atomic:results is a positional list of result objects.
  • No standalone operations docs page. The posture is stated at the three places a reader meets the feature (the transforms module, the envelope handler, and the authoring API's OperationOutput); the docs page itself belongs with the clinical example and the card-author skill.

Where the changes live

  • packages/bxl/src/bxl/profiles/function-safety.ts, packages/bxl/src/bxl/ast/index.ts — the transform profile: its denied-call set and its node-level bans.
  • packages/bxl/src/transform.ts — the runner, a new @cardstack/bxl/transform sub-entry.
  • packages/runtime-common/card-operations/transforms.tsrunInputTransform / runOutputTransform, the posture, and BXL's surface stated structurally behind an opaque dynamic import (a static one, even type-only, compiles bxl's sources into every package downstream of runtime-common/realm). bxl-mirror-check.ts holds the restatement against the real module.
  • packages/runtime-common/card-operations/dispatch.ts — the four stages in runOperation, the anonymous-actor refusal, and readIsProjected for the facade.
  • packages/runtime-common/card-operations/read.ts, envelope.ts — the two 501 gates removed; entryWithPayload and projectedResult for the batch's two stages.
  • packages/runtime-common/realm.tsgetCard, the card+json HEAD, and the envelope handler's write path.
  • packages/runtime-common/card-operations/bxl-emit.ts — lowering checks input/output under transform rather than compute.

Two holes review found, both closed here

A declared write was never held to its params. Only runOperation ran that check, and the behaviors that write never reach it — they are staged through the batch coordinator — so a declaration requiring a value the caller never sent was carried out anyway. It cannot be left to the executors: they read the payload differently enough that some would never notice, and a delete reads none at all, so the card would go. The check now runs on both transports, after the input stage that may supply the value, which is the order the four stages above describe. This is pre-existing on main for any declared write with params; it becomes reachable for a write carrying a transform, which previously answered 501.

A read carrying only an input was still taking the fast paths. An input changes no byte of the document a read serves — a read answers the target's indexed view and ignores its payload — but it can refuse: the program can fail, the params check runs against what it produced, and a stage that reads actor() turns an anonymous request away. A 304 answers as though none of that happened. So the question the facade asks before taking a fast path is now whether the read carries any stage, not whether it projects one. Whether the body was projected is still decided by the output stage alone, and still reported by the assembly — an input-only read keeps the ordinary cacheable directive, it just has to be run each time rather than answered from a validator.

Testing

packages/realm-server/tests/card-operations-transforms-test.ts (new, own stub, 34 assertions) carries the ordering and the plumbing: that a value an input supplies satisfies the params check and the same operation without its input does not, that a projection replaces the served document while the row its headers are computed from survives untouched, that a headers-only read reports what the GET would answer with, that an anonymous caller gets 401 and a failing program gets a 400 naming its stage. Every case was watched go red under a control — removing the output stage reddens eight of them, and moving the input stage after the params check reddens exactly the ordering one.

packages/bxl/tests/unit/bxl-transform-cli.ts (new, 16 cases) covers the profile and the runner directly: what the dialect allows, what it refuses and at which phase, and that the runtime budget bounds a program.

packages/realm-server/tests/realm-endpoints/operations-test.ts gains the end-to-end half against a real realm whose fixture types declare the stages — an input filling a value, a named read projecting one, a failing output, a declared delete refused before staging because its params were unsatisfied, a read carrying only an input answered 200 rather than 304, and the card+json GET/HEAD of a type whose default read is projected, including that a matching If-None-Match is answered 200 while an ordinary card in the same realm is still answered 304.

The realm-server run is 124/124 across those two files plus card-operations-dispatch-test and card-document-cache-test.

🤖 Generated with Claude Code

habdelra and others added 6 commits September 17, 2026 21:27
An operation's `input` and `output` stages are expressions rather than
plans: they read `.`, the payload or the result they were handed, plus the
request context, and yield one value. `transform` is the profile that says
so — everything `mutation` refuses about side effects and runtime metadata,
volatile calls allowed because nothing a transform computes is stored or
replayed, and `realmConfig` refused because a transform is handed the
payload, the caller and the stored document and nothing else.

`@cardstack/bxl/transform` is the entry that runs one, scoping the request
context around a synchronous evaluation and refusing a program that yields
anything other than exactly one value.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An operation declaration may carry two BXL stages: `input`, which shapes the
payload before the operation uses it, and `output`, which projects the result
before the caller sees it. Both now run, in the one order an invocation has —
`input`, the `params` check against what it produced, the behavior, `output` —
and both run in `runOperation` rather than in an executor, so a behavior added
later cannot quietly answer with more than its author said it would.

A read reports whether its body was projected. The card+json facade reads that
to serve a projected body `private, no-store` and to leave the conditional
fast path and the response cache alone: the validator is built from the index
row, and a row does not move when a type gains or changes an `output`, so a
stored copy of the unprojected body would keep answering as fresh. The `ETag`
is still emitted, because `If-Match` asks about the card rather than about the
representation it was read in.

A stage that reads `actor()` on a request that authenticated nobody is refused
401, so the client learns credentials would change the outcome. A program that
fails is the author's 400 whichever way it failed.

A projection is a response shape and not an access boundary: the realm checks
its own read/write permission and nothing else, so a field an `output` leaves
out is still reachable by any permitted caller.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ours do

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…utoutput-transform-runners-and-the-default-vs
The card+json GET asks whether a read projects before taking either of the
two paths that answer without assembling — the conditional 304 and the shared
response cache. A read whose declaration lowering flagged invalid has a
refusal coming, and reporting it as an ordinary read would answer a caller
holding a validator with a 304 for a card the full request refuses. The
question now has a third answer that says so.

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

@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: 2b678c6423

ℹ️ 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 on lines 4597 to 4599
let staged = writes.map(({ entry, definition }) => {
try {
return batchEntryFor(entry, definition);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Validate transformed write params before committing

After an input is applied, write entries are staged immediately without the validateParams step used by runOperation. For example, a declared delete with a required confirm param and an identity input can now be invoked without confirm; stageDelete does not validate the declaration's params, so the card is deleted even though the request should have been rejected before any write. Validate every transformed write payload against definition.params before calling commitBatch.

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 🤖] Confirmed and fixed in 1594e023ce.

You are right that nothing validated a transformed write payload — and the hole is wider than the input case: validateParams had exactly one call site, runOperation, and no write ever reaches it, so a declared write with required params was never checked at all whether or not it carried a transform. Your delete example is the sharpest form of it, because DeleteEntry carries no payload and stageDelete reads none, so nothing downstream would ever have noticed.

The check is now assertParamsSupplied, exported from dispatch.ts and called by the envelope handler per write entry after the input stage — so the order on that transport matches the one runOperation uses, and a value an input supplies satisfies it. Pinned by a new endpoint test: a declared delete requiring a confirm it was not sent answers 400 naming params("confirm"), and the card is still on disk.

Worth noting for whoever reads this later: the gap is pre-existing on main for any declared write with params; this PR makes it reachable for writes that carry a transform, which previously answered 501.

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 at da460ec07d: this resolves it. assertParamsSupplied runs per write entry between the input stage and batchEntryFor, and writes is filtered off invocationsIn(tree), which walks group members recursively — so an entry nested inside a parallel or serial group is checked too, not just a top-level one. The delete case is covered end to end by the retire fixture, which asserts the file is still on disk after the refusal.

One thing the fix leaves open, raised as its own thread: the stage that now runs ahead of the check is handed no realmConfig, so a write whose input or output reads a setting is refused where a read's is answered.

} catch {
return 'unresolved';
}
return definition.output === undefined ? 'plain' : 'projected';

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 Execute input-only reads before taking cache fast paths

A declared read with an input but no output is classified as plain, so a matching conditional request can return 304 and the response cache can return an existing assembly without running the input, its required-param check, or the anonymous-actor refusal. This is observable when a type gains an input after a client/cache has stored the prior representation, or when an actor-reading input-only read is served from the shared cache. Treat any read with an input stage as requiring assembly, even if its eventual document is unprojected.

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 🤖] Confirmed and fixed in 1594e023ce.

The read shape now answers plain / staged / unresolved, and only plain may be answered without running the read. staged covers a read carrying either stage rather than only a projecting one, which is exactly your point: an input changes no byte of the document a read serves, but it can refuse — the program can fail, the params check runs against what it produced, and a stage reading actor() turns an anonymous request away. A 304 answers as though none of that happened.

The distinction the old shape was conflating is worth keeping separate, so I have: whether the read must run gates the two fast paths, while whether the body was projected still comes from the output stage alone and is reported by the assembly. So an input-only read keeps the ordinary public, max-age=0, must-revalidate directive and its ETag — it simply is never answered from them. Pinned by an endpoint test against a type whose default read carries only an input: the conditional GET comes back 200 with the ordinary directive.

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 at da460ec07d: this resolves it, on both fast paths rather than only the conditional one. readShape is asked whenever matchedEtag || (documentCache && peekEtag) holds, which covers every request that could take either path, and the 304 and the getOrPopulate call are each gated on !assembles — so an input-only read now runs rather than being answered from a validator or from a shared assembly. The staged-report fixture pins the 200-not-304 half, and the ordinary directive it still carries.

unresolved folding into the same branch is right for the same reason: both fast paths answer without assembling, and an invalid declaration's refusal only exists on the assembling path.

The remaining question is cost rather than correctness — the definition lookup this adds is a DB read on exactly those two paths; raised as its own thread on the assembles block.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 18, 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-18T03:33:03.789027Z da460ec Manual request
ℹ️ 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.

@github-actions

github-actions Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Host Test Results

    1 files      1 suites   2h 37m 45s ⏱️
4 895 tests 4 881 ✅ 14 💤 0 ❌
4 910 runs  4 896 ✅ 14 💤 0 ❌

Results for commit 065e804.

Realm Server Test Results

    1 files    245 suites   1h 16m 55s ⏱️
3 647 tests 3 647 ✅ 0 💤 0 ❌
3 698 runs  3 698 ✅ 0 💤 0 ❌

Results for commit 065e804.

habdelra and others added 2 commits September 17, 2026 23:12
The batch is a tree now: an entry is keyed by its position rather than its
index, and the write path fills a Map the coordinator is handed as a schedule.
The two transform stages slot into that unchanged — the transformed entry is
the parsed one with a new payload, so it keeps the position both the staging
schedule and the results are looked up by.

Two fixes on top of the integration:

A declared write is held to its `params` before anything is staged. Only
`runOperation` ran that check, and the behaviors that write never reach it, so
a declaration requiring a value the caller never sent was carried out anyway —
a `delete` reads no payload at all, so nothing downstream would ever have
noticed. The check now runs on both transports, after the `input` stage that
may supply the value.

A read carrying only an `input` no longer takes the conditional or the cached
fast path. An `input` changes no byte of the document a read serves, but it can
refuse: the program can fail, the `params` check runs against what it produced,
and a stage reading `actor()` turns an anonymous request away. A 304 answers as
though none of that happened, so the question the facade asks is now whether
the read carries any stage rather than whether it projects.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`realmConfig()` is a request-context builtin like `params()` and `actor()`,
and a declaration may already call it from an `output` — a file def's
projected read does exactly that, and its lowered program is pinned. The
transform profile now allows it, and the realm supplies the settings the call
answers with.

Read at most once per stage and only where the program names one, since a cold
read is a parse of the realm's config document and most stages name no
setting. The write path keeps its own reader: the coordinator reads the same
value inside the lock, after the drain, so that a batch stages against the
settings it is committing beside.

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

Copy link
Copy Markdown
Contributor Author

@codex review

[Claude Code 🤖] Both findings are fixed at da460ec07d — replies on each thread. A third change came out of the first review pass and is worth a look in its own right: the transform profile no longer denies realmConfig(), because a declaration may already call it from an output and the lowered program is pinned by an existing host test. The realm now supplies the settings the call answers with.

@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: da460ec07d

ℹ️ 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 +4700 to +4705
return {
name: entry.name,
params: paramsFor(entry),
...(entry.href ? { id: entry.href } : {}),
...(caller.actor ? { actor: caller.actor } : {}),
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Supply realm config to batch write transforms

When a declared write uses the newly allowed realmConfig() call in either its input or output, this batch-specific context omits the realmConfig thunk that runStage requires. Reads receive it through OperationCore, but batch writes call this helper, so the transform reaches evaluation without the slot and returns a 400 instead of running. Add the realm config reader here as well.

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 🤖] Confirmed and fixed in 5bb86bc016. #transformContext now carries realmConfig: () => this.getRealmConfig(), the same reader the operation core hands a read.

The shape of the mistake is worth naming: I wired the settings into dispatch's context when I allowed the call, and dispatch serves reads only — writes never reach runOperation. So one declaration's output answered on a read and refused on a write, and on a write that refusal lands after commitBatch. Nothing pinned it because every realmConfig case I had written was a read. There is now an endpoint case for a write whose output reads a setting, which covers the transport rather than the call.

// The handler decides not to retain before assembling — it cannot key a
// projection on a validator that does not move with it — so this reports
// what the assembly turned out to be rather than gating the cache.
projected: boolean;

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 Prevent projected assemblies from entering the shared cache

When a read declaration changes between the readShape preflight and cache population, the assembly can become projected after the handler has already selected getOrPopulate; because CardDocumentCache.retain never checks this new flag, the actor-specific projection can be retained under the unchanged card ETag. If the projection is subsequently removed, the same ETag makes that stale projected body reachable as a normal cache hit. Reject assembly.projected in the cache retention predicate rather than relying solely on the racy preflight.

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 🤖] Agreed and fixed in 5bb86bc016!assembly.projected is now in the retention predicate.

It belongs there on its own terms, not only as cover for the race you describe: the predicate already rejects queryBacked because such a document is not a function of its own index row, and a projection is the same kind of thing — it comes from the type's declaration, which can change without the row moving, and it may differ per caller. Resting that on a preflight read taken before the assembly made it a property of the handler's timing rather than of the cache.

@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 pass went after the two transports agreeing, the load-bearing justifications (the post-commit bound, the profile's deny set, the mirror guard), and whether the card+json fast paths can still reach a projected body. Claims were checked by running the BXL transform runner and the lowering checker directly, by typechecking the mirror pattern, and by typechecking @cardstack/postgres; the realm-server and host suites were not run here, so anything only they can settle is unverified.

No blocking issues. One gap I'd fix in this PR — a write's transform stages are handed no realmConfig, so the same declaration that projects a read cleanly answers 400 after commit on a write — plus a set of justifications that don't reach as far as they claim. The card+json fast paths hold: assembles is computed on both branches that could answer without assembling and gates both, and scope.peekInstance is the same includeErrors: true read the handler made before, so a card whose type declares no read is unchanged. The write path's mutation is safe — entryWithPayload returns a new entry and only resolved/writes share it, while stagedTree and resultsTree key off the tree's positions — and invocationsIn flattens groups, so the params check reaches entries nested inside parallel and serial groups. withRequestContext cannot leak across requests here: the runner is synchronous end to end and the one await in runStage (the settings read) happens before the scope is entered. The opaque-import guard holds — pnpm --filter @cardstack/postgres lint:types reports zero bxl/src diagnostics.

Recommendations, in the order I'd take them:

  1. Supply realmConfig in Realm#transformContext — see the thread on that method in realm.ts.
  2. Soften the post-commit bound, or close its checkable half at lowering, and update the sibling comment on the catch in handleOperations — thread on the output-after-commit block.
  3. Deny empty under the transform profile, or drop the claim that it is denied — thread on runBxlTransform's doc comment.
  4. Add the key-set aliases to the transform mirror guard, which currently cannot see a renamed slot — thread in bxl-mirror-check.ts.
  5. Make the endpoint case for a write's input discriminate, and add one for a write's output succeeding — thread in operations-test.ts.
  6. Flip the spread precedence in entryWithPayload — thread in envelope.ts.
  7. Measure or memoize the definition lookup readShape adds to the 304 and cache-hit paths — thread on the assembles block.

CI is still running on the head commit; nothing red to act on yet.

Adjacent, out of scope: namesRealmConfig is now the second copy of source.includes('realmConfig') in this package, the other in executors.ts. Both are named in each other's comments, so a rename of the builtin would be found — but whoever unifies the two transports' context construction is the natural person to fold them together.

Comment on lines +4700 to +4705
return {
name: entry.name,
params: paramsFor(entry),
...(entry.href ? { id: entry.href } : {}),
...(caller.actor ? { actor: caller.actor } : {}),
};

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 context carries no realmConfig, so a write's input or output naming realmConfig() is refused where a read's is answered. runStage reaches for the settings only when ctx.realmConfig is set, and with the slot absent the builtin throws realmConfig() needs the realm configuration, which the host did not supply in this request's context — a transform error at phase evaluate, which refusal() maps to 400. On an output that 400 lands after commitBatch, so one declaration projects a read cleanly and turns every invocation of a write into "committed, then refused".

Verified on the branch: runBxlTransform('{c: realmConfig("tz")}', …) with no realmConfig slot throws exactly that message; with the slot it answers.

getRealmConfig() is already the reader the coordinator's own core is given a few hundred lines down:

Suggested change
return {
name: entry.name,
params: paramsFor(entry),
...(entry.href ? { id: entry.href } : {}),
...(caller.actor ? { actor: caller.actor } : {}),
};
return {
name: entry.name,
params: paramsFor(entry),
realmConfig: () => this.getRealmConfig(),
...(entry.href ? { id: entry.href } : {}),
...(caller.actor ? { actor: caller.actor } : {}),
};

Nothing pins this either way: the stub suite stubs core.realmConfig and exercises only the dispatch path, and no endpoint case covers a successful output on a write at all. One endpoint case where a write's output reads a setting would cover both.

Regression, and small enough to fix here.

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 5bb86bc016#transformContext carries realmConfig: () => this.getRealmConfig(), and there is now an endpoint case for a write whose output reads a setting, so the transport is covered and not just the call.

Your note that nothing pinned it is the part worth keeping: every realmConfig case I had written was a read, and the stub suite stubs core.realmConfig, so both halves of the coverage looked at the transport that already worked.

Comment thread packages/runtime-common/realm.ts Outdated
Comment on lines +4659 to +4666
// The `output` stage runs after the commit, over the result the wire
// would otherwise carry. It projects what the caller is told about the
// write and cannot unmake one: a program that fails here answers 400
// over a card that has already changed, which is the one place the
// batch's all-or-nothing does not reach. A program that does not parse
// or reaches outside the transform dialect is refused when the
// declaration is lowered, so what gets here is a program that ran on
// values it could not handle.

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 🤖] "what gets here is a program that ran on values it could not handle" is a stronger bound than lowering delivers, and the fixture this PR adds is a counter-example to it.

checkExpressionProgram enforces the transform profile, which governs which constructs and calls a program may use. It says nothing about how many values a program yields or what shape they have. Checked against the branch, lowering returns a clean canonical source for all of:

  • ., . — every invocation then fails outputs.length !== 1 in runBxlTransform
  • .data.attributes.title — every invocation is then refused by projectedResult for not being an object
  • {i: instance("title")}instance() is legal under the profile and no transport fills the slot, so every invocation throws at evaluate

The endpoint fixture broken is the third of those: its write commits and its output refuses, on every call, by construction. So what reaches here is not only data-dependent failure — an author can ship a write that always commits and always answers 400, with nothing telling them until it runs.

Two ways out and they compose: soften the sentence (here, in the transforms.ts header, and in the description), and/or close the checkable half at lowering — a top-level comma and a call to a slot no transport fills are both decidable where the declaration is written.

Either way, the comment on the catch in handleOperations has to move with this. It still reads "nothing was written, no index job was enqueued and no event was broadcast, whichever stage produced it", and this throw lands in exactly that catch, after commitBatch.

Regression; non-blocking, but the bound is what the all-or-nothing hole is being accepted on.

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, and the fixture being a counter-example to the sentence in the same PR is the damning part. Softened in 5bb86bc016, in all three places plus the handleOperations catch, which now says that every stage up to the commit means nothing was written and that an output is the one refusal that reaches it from past that point.

I took the softening rather than the lowering-side close, and the reason is that it would only be a partial close sold as a full one. A top-level comma is decidable; .[] over an array is not, so "yields exactly one value" stays a runtime property either way and the sentence would still need to be true about the remainder. empty was worth closing because it is the deliberate spelling of the thing, and that one is in this commit. The unfilled-slot case is decidable and is the better follow-up — it wants the check to know which transport will run the stage, which is not something lowering knows today.

Comment on lines +101 to +104
* on with an answer the author did not mean to give. A program that yields
* none is refused for the same reason — `empty` is already denied, so the
* only way here is a filter that matched nothing, and a payload silently
* becoming absent is the failure these stages exist to avoid.

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 🤖] empty is not denied under transform. BXL_TRANSFORM_DENIED_CALLS is BXL_DERIVE_CONTROL_DENIED_CALLS plus BXL_METADATA_CALLS, and empty appears in neither — only in BXL_CONTROL_OR_SIDE_EFFECT_CALLS, which nothing in this profile's policy consults. Verified: checkBxlTransform('empty') returns [], as does checkBxlTransform('if .a then empty else . end').

The outcome is still right — a zero-output program is refused a few lines below — so the ask is about the justification, which the next author reads as "the only remaining way to reach zero outputs is a filter that matched nothing".

Adding empty to BXL_TRANSFORM_DENIED_CALLS makes the sentence true and moves the refusal to lowering, where the author meets it against their own edit; the profile's own comment already says it "bans side effects and runtime metadata". Otherwise the clause needs dropping.

Regression (the claim is new here); 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 5bb86bc016 by making the claim true: empty is now in BXL_TRANSFORM_DENIED_CALLS, so the deliberate spelling of "yield nothing" is refused where it is written and the runtime check is left holding only a filter that happened to match nothing — which is what the comment says.

Denying it rather than dropping the clause because a transform answers with exactly one value by contract, so a call whose entire purpose is to yield none has no reading here. Pinned by a case that runs empty and if .a then empty else . end and asserts the refusal comes at the profile phase, not at evaluation.

Comment on lines +102 to +109
export type LocalTransformContextReachesBxl = Assignable<
BxlTransformContext,
LocalTransformProgramContext
>;
export type BxlTransformContextReachesLocal = Assignable<
LocalTransformProgramContext,
BxlTransformContext
>;

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 🤖] These two aliases cannot catch a renamed slot, which is the first of the two drifts the comment above says they hold. Every slot on both context types is optional, so a rename leaves each side with one extra optional property and one absent one, and interface assignability accepts that in both directions. A retyped slot is caught; a renamed one is not — so the very failure named ("a slot spelled differently reaches a program as the host supplied none") passes.

Checked with the repo's tsc: with realmConfig on one side spelled realmSettings on the other, both aliases compile; changing actor from string to number errors.

A key-set pair closes it — errors on that rename, green against the current shapes (both checked):

export type LocalTransformSlotsReachBxl = Assignable<
  Record<keyof BxlTransformContext, unknown>,
  Record<keyof LocalTransformProgramContext, unknown>
>;
export type BxlTransformSlotsReachLocal = Assignable<
  Record<keyof LocalTransformProgramContext, unknown>,
  Record<keyof BxlTransformContext, unknown>
>;

The phase half of the claim does hold — the predicate type on isBxlTransformError carries it, and a rename there breaks BxlTransformEntryReachesLocal.

Regression; 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 5bb86bc016 with the key-set pair.

Verified it discriminates rather than assuming: renaming the slot on the bxl side makes BxlTransformSlotsReachLocal fail with Record<"instance" | "params" | "actor" | "realmConfig", unknown> not satisfying Record<keyof BxlTransformContext, unknown>, and the current shapes compile clean. So the guard now catches the drift its comment names — which it did not before, since every slot being optional made a gained key and a lost one assignable both ways.

Comment on lines +9976 to +9979
if (matchedEtag || (documentCache && peekEtag)) {
assembles =
(await readShape(this.operationCore, url, scope)) !== 'plain';
}

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 🤖] Has the cost of this been measured on the two paths it lands on? readShaperesolveOperationdefinitionForDefinitionLookup.lookupDefinition, and that bottoms out in readFromDatabaseCache: a SELECT against the modules table plus a JSON.parse of that module's whole definitions blob, per call. #inFlight coalesces concurrent identical lookups but holds nothing between requests, so there is no resident cache in front of it — this is a Postgres round trip added to every 304 and to every response-cache hit, which are the two paths that exist to answer without one.

The answer is a function of the target's type and changes only when the module is invalidated, so it is memoizable per module + definition generation the way the realm-info hash already is, which would leave both fast paths at the single row read they have today.

Regression, non-blocking — asking for a number or a memo rather than a change to the shape.

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 🤖] Fair ask, and I have taken the memo rather than the number: 5bb86bc016 remembers the shape per card URL on the realm and drops the whole map in clearRealmIndexCaches(), so a conditional request on a warm realm is back to the single row read it had.

Clearing on index swap is sound rather than merely conservative. The answer is a function of the card's stored adoptsFrom and its type's declarations; a write changes the first and reindexing a module changes the second, and both swap the index — locally and on peer replicas, since that hook is also what the realm_index_updated broadcast drives. The case it does not cover is a foreign realm's module changing without this realm swapping, and that one cannot reach here: a card with foreign-realm dependencies is served no validator at all, so peekEtag stays undefined and neither fast path — nor this question — is reached.

An unresolved answer is deliberately not remembered, since that is a declaration with findings against it that someone is presumably mid-way through fixing.

Comment on lines +706 to +712
let carried: Record<string, unknown> = {};
for (let member of ENVELOPE_MEMBERS) {
if (entry.data && own(entry.data, member) !== undefined) {
carried[member] = entry.data[member];
}
}
throw new OperationFailure({
...(entry.href ? { id: entry.href } : {}),
status: 501,
code: 'internal-error',
title: 'Operation not implemented',
detail:
`operation "${entry.name}" specializes its behavior with ` +
`${stages.join(' and ')}, which a batch does not run`,
meta: { entry: entry.position },
});
return { ...entry, data: { ...carried, ...payload } };

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 🤖] The precedence is the other way round from what the comment promises. { ...carried, ...payload } lets the program's output replace lid or meta, and the program is never handed either — paramsFor strips them before the stage runs — so a program emitting one is overwriting a value it could not read.

Two places that bites. For a base create the whole of data becomes the staged resource, meta.adoptsFrom included. And entry.lid is a top-level member parsed from data.lid before the stage runs, so an overwritten data.lid leaves batchEntryFor's create arm keying the coordinator on the original while the resource it stages carries the new one.

{ ...payload, ...carried } makes the sentence true.

Regression, minor, 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 5bb86bc016{ ...payload, ...carried }, and the comment now says why rather than just what: the program is handed neither member, so anything it emits under those names was overwriting a value it could not read.

The create arm is the case that made it worth more than a comment fix. entry.lid is parsed off data.lid before any stage runs, so an overwritten data.lid would have left the staged resource and the key the coordinator is given naming different cards — and for a base create the whole of data becomes the resource, meta.adoptsFrom included.

Comment on lines +1556 to +1572
test('an input fills a value the params check would have refused', async function (assert) {
let response = await post(
envelope(
invoke('restate', {
href: '/report-restated',
data: { headline: 'Revised' },
}),
),
);

assert.strictEqual(response.status, 200, 'HTTP 200 status');
assert.strictEqual(
storedCard('report-restated.json').data.attributes?.headline,
'Revised',
'the write ran on the payload the input produced',
);
});

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 case passes with the input stage gone. restate declares its input as {headline: params("headline")} and the entry sends data: { headline: 'Revised' }, so the program's output is the payload it was handed, unchanged; assertParamsSupplied passes either way and set: { headline: params('headline') } writes Revised either way. Delete the input clause from the declaration, or delete the envelope's whole input wiring, and the assertion still holds.

That leaves the write path's inputentryWithPayload and the write.entry rewrite, which the dispatch path does not share — with no discriminating end-to-end assertion. The stub suite pins the ordering for a read; nothing pins it for a write.

Sending no headline and giving the input a default covers both halves at once: without the stage the entry is a 400 naming params("headline"), with it the stored headline is a value only the program could have produced.

Regression (new test); 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, and this is the worst kind of test to have shipped: it reported coverage that was never there. Fixed in 5bb86bc016 along the line you suggested — the entry now sends no headline at all, and the input is . + {headline: (.headline // "Restated by default")}, so without the stage the entry is a 400 naming params("headline") and with it the stored value is one only the program could have produced. A second case sends a headline and asserts the caller's own value still wins, so the default is not masking the payload.

That also closes the gap you name underneath it: entryWithPayload and the write.entry rewrite now have an end-to-end assertion that fails without them, which the dispatch path's stub coverage could not speak for.

habdelra and others added 3 commits September 17, 2026 23:58
…ey cover

Seven things review found, and the through-line in most of them is a sentence
claiming more than the code delivers.

A write's transform stages are handed the realm's settings, as a read's are.
One declaration's `output` runs on both transports, so a setting it names has
to be answerable on both — otherwise the same program projects a read and
turns every invocation of a write into "committed, then refused".

The post-commit bound is stated as what it is. Lowering refuses a program that
does not parse or reaches outside the dialect; it does not ask how many values
one yields, what shape they are, or whether it names a context slot the
transport running it fills. So a declaration can be shipped whose write commits
and whose projection refuses every time, and the comment on the batch's catch
no longer claims a refusal always means nothing was written.

`empty` is denied under the profile, which is what the runner's reasoning about
zero-output programs already assumed. The deliberate spelling is now refused
where it is written; a filter that happens to match nothing is what the
runtime check is left holding.

The transform mirror guard catches a renamed slot. Every slot is optional, so
the assignability pair accepted one side gaining a key and losing another — the
exact drift the guard exists for, since a slot filled under a name the builtin
does not read reaches the program as "the host supplied none". A key-set pair
closes it.

An entry's own members win over what its `input` produced. The program is
handed neither `lid` nor `meta`, so anything it emits under those names was
overwriting a value it could not read — and for a base create that reaches
`meta.adoptsFrom`, while an overwritten `lid` would leave the staged resource
and the coordinator's key naming different cards.

A projected assembly is refused by the cache's retention predicate rather than
only by the handler's preflight, which is read before the assembly and so
cannot speak for what it turned out to be.

The read shape is remembered per card between requests and dropped whenever the
index swaps. The lookup behind it is a database read landing on the two paths
that exist to answer without one; the answer moves only with the card's stored
type or its type's declarations, and neither moves without a swap.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The file keeps one report per test that writes one, so its tests do not have
to be ordered against each other. This case had been pointed at the card the
projection test reads.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every other slot on the context is optional for a reason a transport has: an
anonymous request has no actor, a stage may run over no payload, and the
stored document is nobody's to supply yet. The realm's settings are not like
that — a transport able to run a stage can reach them, and one declaration's
`output` runs on whichever transport the caller used. Optional is what let one
of the two supply sites be built without it, so the same program answered on a
read and refused on a write.

The no-stage case now hands in a reader that throws, which says the path
reaches for nothing rather than only that it ends with the same values, and a
new case pins that a stage naming no setting leaves them unread.

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

2 participants