Report a visitor's explain refusal as the 401 they receive, and remove the unused OperationScope.proposed

[habdelra]

Important

Merge this before cardstack/boxel-skills#174, which documents a visitor's explain refusal as the 401 this PR makes explain report.

This PR makes two changes to how the operation policy's create lane and explain describe themselves.

Explain reports the refusal a visitor actually receives. An explain's refusal is documented as the { status, code } its actor would receive. For an actor who isn't signed in, explain reported the gate's refusals as a non-reader's 404 target-not-found, or as the gate's 403 for a write on a realm anyone may read. That covered no-grant, predicate-false, reads-actor, and a write whose acting user doesn't resolve. The realm answers such a visitor 401 actor-required. The realm's rule now lives in one place, refusalSeenBy in card-operations/types.ts beside refusalForNonReader, together with the AUTHENTICATION_REQUIRED error it returns. Realm#refusalSeenBy answers with it and explain reports it, so the two can't disagree. A realm anyone may read declines a visitor's write outright, because the realm asks whether a caller it declined a write could read only of a signed-in caller. Explain's coarseDeclinedFor now derives the same, so a visitor's refused write there is reported as the same 401. Signed-in actors are unchanged. The new test in realm-policy-anonymous-named-operation-test.ts explains each of those refusals for a visitor, sends the same request as one, and checks that both answers are 401 actor-required, on a private realm and on a realm anyone may read.

OperationScope.proposed is gone. It held a create's staged payload: stageWriteEntry set it, and derive dropped it for every other invocation. No production code read it. The gate reads a GateScope that leaves the payload out, because the payload names the type the caller claims. A create against a type is judged by the card it mints once staged, and a named create anchored on a card by that card. The field is removed from OperationScope, ScopeInvocation, newOperationScope and derive, and staging returns the entry's scope unchanged. OperationScope's and ResolvedEnvelopeEntry.scope's comments now say what the gate reads. The staging tests now assert on the staged entry's payload, and the derive test now checks that a trace, the member that belongs to one invocation, doesn't carry over.

Not in this PR: on a realm anyone may write, explain reports a visitor's declared write that reads actor() as allowed, while the realm refuses it 401. Tracked in CS-13613.

Linear: CS-13348, CS-13612

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

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

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-06T23:25:18.820684Z	139fbb5	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.

[habdelra]

[Claude Code 🤖] This review checked that nothing else reads proposed, that the edited comments are accurate, that a create is still staged and judged the same way, and that the rewritten tests can still fail. It did not run the realm-server suite.

No behavior problems: nothing in the repo reads the field, and the write lock judges a create from the card handed to dischargePendingDecision, never from the scope. One comment on ResolvedEnvelopeEntry.scope is now stale (blocking), and the derive test lost its only check of what does not carry over (non-blocking).

1. Update the stale scope comment on ResolvedEnvelopeEntry. Details are in the file comment on envelope.ts.
2. Replace the duplicated caller assertion in the derive test with a check that a trace does not carry over. Details are in the comment on card-operations-dispatch-test.ts.
3. Nit, in a file outside the diff: the GateScope comment in gate.ts still says "A create's proposed document is not among them". That reads as if the scope GateScope narrows had such a field. Since OperationScope's comment now covers this, the sentence can be dropped or reworded to talk about the create's payload. Non-blocking.

[habdelra]

[Claude Code 🤖] The scope member's comment on ResolvedEnvelopeEntry still says the scope carries, "once stageWriteEntry has run — the document a create would write". After this change staging returns the entry's scope unchanged, so that clause now describes a field that no longer exists. Suggested wording:

  // This entry's own view of the batch's scope: the batch's caller and row
  // memo.
  scope: OperationScope;

Regression (a stale comment this PR leaves behind). Blocking, because the ticket's bar is that no comment still presents the payload as part of the scope.

[habdelra]

[Claude Code 🤖] Fixed in 6533a8f: the scope comment on ResolvedEnvelopeEntry now says only what the scope carries, the batch's caller and row memo.

[habdelra]

[Claude Code 🤖] Two problems with this test:

1. The new assertion repeats assert.deepEqual(entry.caller, batch.caller) from four lines up. It can only fail if derive mutated entry in place, and scopeFor always builds a fresh object, so it never fails.
2. With proposed gone, nothing tests the other half of derive's contract: the part that does not carry over. Its comment now says "a trace belongs to one invocation and never does", and derive is the only place a trace is attached (capabilities.ts does scope.derive({ trace })). The removed proposed assertions were the only check that a per-invocation member is dropped.

Swapping the duplicate for a trace check would cover it:

let traced = entry.derive({ trace: someTrace });
assert.strictEqual(traced.trace, someTrace);
assert.strictEqual(
  traced.derive({}).trace,
  undefined,
  'a trace is one invocation’s and does not carry over',
);

Regression in coverage, not in behavior. Non-blocking.

[habdelra]

[Claude Code 🤖] Fixed in 6533a8f. The duplicate assertion is gone. The test now derives with a GateTrace, then checks three things: the derived scope carries it, the original scope still has none, and a further derive that names no trace drops it. That is the member-does-not-carry-over check the old proposed assertions covered.

[habdelra]

[Claude Code 🤖] On the GateScope nit in gate.ts: I'm leaving it as it is. The sentence says a create's proposed document is not among the facts the gate reads, and that is still true, with the reason. It doesn't claim the scope has such a field. Several open PRs are editing gate.ts right next to that comment, so a wording-only change there would cost them a conflict for no gain in accuracy.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[habdelra]

[Claude Code 🤖] This review covers the commit that makes explain report a visitor's refusal as the realm sends it. I checked whether explain's refusal matches the realm's answer on every route a visitor can reach: private realms and realms anyone may read, across the gate's refusal reasons. The OperationScope.proposed removal was reviewed earlier and isn't revisited here.

No blocking issues. The realm's own answers are unchanged, and on a private realm explain now agrees with every route, including predicate-threw. One pre-existing gap remains on realms anyone may read, and the new contract makes it matter.

1. On a realm anyone may read, explain's per-operation opensToAnonymous early return answers 401 where the envelope answers 403 for an operation the policy doesn't open to visitors. See the comment on the board half of the new test. Pre-existing and non-blocking; fix it here or file it before merge.
2. The comment on refusalSeenBy claims more coverage than the code gives: three read paths still use refusalForNonReader directly. See the thread in types.ts. Non-blocking.

Adjacent and out of scope: on a realm whose ACL lets anyone write, a visitor's declared write that reads actor() is explained as allowed/acl, because the gate returns a coarse decision. The envelope refuses it 401 at its needsActor check, and explain has no counterpart to that check.

What this review couldn't settle: the realm-policy suites can't run on the local stack right now, so the new test and the 403-vs-401 split above were traced in code, not executed.

[habdelra]

[Claude Code 🤖] On a realm anyone may read, explain and the envelope still disagree about a visitor's write when the policy doesn't open that particular operation to visitors. Can you either close that gap or narrow the stated contract?

• Explain: the early return in explain() (explain.ts, the actor.kind !== 'user' && coarseDeclined !== 'none' && !opensToAnonymous(core, question.operation) block) answers 401 actor-required for any operation missing from policy.anonymous.operations.
• The envelope: it admits the visitor through ANONYMOUS_OPERATIONS (operations: 'any'), so callerFor lets them in whenever the policy opens anything. The gate then filters to anonymous grants, finds none and refuses no-grant. refusalSeenBy passes that through for readDeclined: false, so the visitor gets 403 operation-not-permitted.

On this fixture, explain(BOARD_PETITION, 'update', BOARD_POLICY) reports 401, while invoke('update', …) POSTed to the board without credentials gets 403. A named write the board's policy doesn't open (a second declared operation on Petition) has the same split, and the envelope is the only route that reaches it. I traced this through the code but couldn't run it: the realm-policy suites can't run against the local stack right now.

On a private realm the per-operation check is harmless, because refusalSeenBy turns the gate's refusal into the same 401. The fix that keeps both realms right is to apply the per-operation early return only when coarseDeclined === 'all'. For 'writes', return 401 only when the policy opens nothing to visitors (mirroring callerFor's opened.operations.size > 0), and otherwise let the gate decide. One wrinkle: the card+json PATCH/DELETE/POST routes admit by the specific base operation (ANONYMOUS_CARD_UPDATE etc.), so for update/delete/create those routes send 401 where the envelope sends 403. The contract needs to name which route explain describes. Please also add an unopened operation on the board to this test.

Pre-existing (the early return predates this commit), but this commit and the paired skills doc now state that explain's refusal equals what the realm sends a visitor, so it becomes load-bearing. Non-blocking for the refactor itself; fix it here or file it before merge.

[habdelra]

[Claude Code 🤖] Tracked in CS-13613, together with the review body's case of a realm anyone may write and a declared write that reads actor(). It isn't a change explain can make on its own: on a realm anyone may read, the realm's routes disagree with each other. The envelope admits a visitor whenever the policy opens anything and then refuses with 403, while the card+json writes admit per base operation and answer 401. So the fix is first a decision: does the envelope admit per operation, or does explain say which route it describes? Then explain follows. This PR leaves that early return as it was.

[habdelra]

[Claude Code 🤖] Confirmed, and CS-13613 is narrowed (by a correcting comment on the ticket). On a realm anyone may read, the realm declines a visitor's write as 'all', so the envelope, the card+json writes and explain's early return all answer 401. What's left in CS-13613 is the realm anyone may write with a declared write that reads actor(), plus the capability check's lanes raised in the latest review.

[habdelra]

[Claude Code 🤖] "The realm answers its callers with this … so the two cannot disagree" is broader than the code. Only the envelope (handleOperations) and the card+json write refusal (#cardWriteRefusal) call #refusalSeenBy. The other three read paths call refusalForNonReader directly:

• the source/byte read (the coarseDeclined ? refusalForNonReader(e.error) : e.error in the readSource wrapper),
• the card+json HEAD (refusalForNonReader(e.error).code === 'target-not-found'),
• the card+json read assembly (caller.coarseDeclined ? refusalForNonReader(e.error) : e.error, whose comment already says "see #refusalSeenBy").

Each of those reaches the 401 by its own downstream check instead: #bytesNotThere, or assembly.kind === 'not-found' && requestContext.anonymousCaller. They agree today, but the "cannot disagree" guarantee doesn't cover them. Either route those three through #refusalSeenBy, or reword the comment to name the two routes it does cover. Non-blocking, a wording or consistency fix in this PR.

[habdelra]

[Claude Code 🤖] Fixed in 8cfbdaf. The comment now names the routes that answer with refusalSeenBy (the operations envelope and the card+json writes). It also says that the card+json read and HEAD and the byte serves apply refusalForNonReader themselves and reach the same answer by their own route.

[github-actions]

Host Test Results

    1 files      1 suites   1h 38m 7s ⏱️
5 203 tests 5 192 ✅ 11 💤 0 ❌
5 218 runs  5 207 ✅ 11 💤 0 ❌

Results for commit b5b856f.

Realm Server Test Results

    1 files    319 suites   2h 0m 47s ⏱️
5 087 tests 5 087 ✅ 0 💤 0 ❌
5 146 runs  5 146 ✅ 0 💤 0 ❌

Results for commit b5b856f.

[habdelra]

[Claude Code 🤖] This review covers only the newest commit, which makes explain judge a visitor's write as declined outright. I checked coarseDeclinedFor against Realm#coarseDeclined for each actor kind and lane, the other explain answers that 'all' now feeds, and the routes a visitor can reach. I did not run the realm-policy suites.

No blocking issues. coarseDeclinedFor now matches the realm for a visitor's read and write on a private realm, a realm anyone may read, and a realm anyone may write. The early actor-required return tests only !== 'none', so the change does not touch it.

1. The open thread on the unopened operation (the explain early return, on the anonymous-named-operation test) no longer rests on a true premise. That thread and its reply both assume the envelope gives a visitor's unopened write 403, because readDeclined is false on a realm anyone may read. Under #coarseDeclined it is 'all' for a visitor's POST. So the gate's operation-not-permitted goes through refusalForNonReader to 404 and then to AUTHENTICATION_REQUIRED. The envelope therefore answers 401, the same as the card+json writes and explain's per-operation early return. On a realm anyone may read, the routes no longer disagree, so the decision the follow-up ticket waits on is moot for that case. Could you re-check this and update the ticket's scope to what is left (the realm anyone may write, and the write that reads actor())? Non-blocking.
2. The capability check is the one twin that still treats a visitor as a reader. #capabilityLanes in realm.ts sets coarseDeclined = readAllowed ? 'writes' : 'all' without the signed-in condition, and its comment says the gate judges a visitor's writes "as it judges a reader's". On a realm anyone may read whose policy opens some write to visitors, a visitor's refused write pair comes back non-bare with a reason (operation-not-permitted, policy-predicate-failed, or a create's conditional: true). The invocation itself answers 401 actor-required, and writesRefused claims to report "the code its refusal comes closest to". Setting 'all' there would also put the visitor's reads through the policy, so the fix probably needs bare per lane rather than a different CoarseDeclined. This is pre-existing and not this commit's bug. The new comment in types.ts now states the opposite rule, though, so could you either align the check or file it? Follow-up, non-blocking.

CI on this head is still running. The wording nit on the new types.ts comment is inline.

[habdelra]

[Claude Code 🤖] "A caller who isn't signed in is never one of those on a write" is wrong as written: on a realm anyone may read, the ACL does let a visitor read. The realm just never asks about it for a visitor (#recordCoarsePermission runs #readProbe only when #policyJudges holds, and that needs an authenticatedUser). Since this comment defines the rule refusalSeenBy callers follow, could it say what the realm does instead of claiming something about the ACL? For example:

// lets read the realm is told the refusal as it is, where the realm asks: it
// asks whether a caller it declined a write could read only of a signed-in
// caller, so a visitor's refused write is told as a non-reader's even on a
// realm anyone may read.

Wording on this PR's own comment. Non-blocking.

[habdelra]

[Claude Code 🤖] Fixed in b5b856f, using your wording.

[habdelra]

[Claude Code 🤖] On the #capabilityLanes point in the latest review: agreed, and it's pre-existing, so I've added it to CS-13613 with the per-lane shape you suggested. A visitor's write lane is 'all', and their read lane stays as the ACL allows. The refusalSeenBy comment now says what the realm asks rather than what the ACL allows (b5b856f), so it no longer states the opposite rule.