Repository navigation
Add the judgment calls to the realm-policy-authoring skill - #175
Merged
habdelra merged 3 commits intoOct 6, 2026
Merged
Conversation
A new section, "Choosing what to grant", covers the decisions the policy engine accepts either way: a named operation over a raw update when a grant rests on a field, keeping authorization-bearing fields out of a grant's reach, when a snapshot grant's index-lag window is acceptable, why read and query are separate grants, and how to read an explanation. Each rule is tied to a scenario in the school example realms. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Staging Workspace Sync SuccessfulSuccessfully synced changes to staging workspace Sync Details |
habdelra
commented
Oct 6, 2026
habdelra
left a comment
habdelra
left a comment
Contributor
Author
There was a problem hiding this comment.
[Claude Code 🤖] This review checked §13's claims about the platform against boxel main (the gate, explain, gate-trace, executors, coordinator and policy compile) and the school example realms and their test. It also checked the membership spelling, audience and evergreen wording. It didn't review the prose style of the sections this PR leaves alone.
One blocking gap: the rule against raw writes and the checklist line that restates it both leave out raw appendContainsMany, which can extend teacherIds directly. The checklist line, read literally, also rules out read grants. Everything else is accuracy or wording.
- Name
appendContainsManyin §13's raw-write rule. Blocking. See the "Keep authorization-bearing fields out of reach" thread. - Limit the checklist line to write grants and add
appendContainsMany. Blocking. Suggestion in the thread. - A snapshot grant lays the index under the stored source. Stored fields stay current, so the opening of "When
snapshot: trueis acceptable" and explain item 4 overstate the gap. Non-blocking. - The lead-teacher example in the read/query split shows read versus write, and no school scenario backs "can mean different things". Non-blocking.
threwgivesfailedonly when no other grant held. Nit.
The membership spellings are all any(. == actor()) and parenthesized inside or. The skill names no server logs and no internal source paths. The skill text has no ticket IDs and no temporal wording. card-operations-authoring is unchanged.
Comment on lines
+1168
to
+1172
| **Never grant raw `update`, `transform` or `delete` on a type whose predicate | ||
| reads a field those operations can change, unless every caller who passes the | ||
| predicate is trusted with the consequence.** `update` and `transform` rewrite | ||
| the field. `delete` removes it along with the card, so any caller the predicate | ||
| admits decides alone whether the card and its roster go on existing. |
Contributor
Author
There was a problem hiding this comment.
[Claude Code 🤖] This list leaves out raw appendContainsMany, and that base writes exactly the fields membership predicates read. §3 lists it as a base every card carries. The raw entry lets the caller choose the field and the items (AppendContainsManyEntry in card-operations/executors.ts takes field/items or fields). So a grant of appendContainsMany on Classroom with .teacherIds | any(. == actor()) lets Alice add anyone to teacherIds. That is the escalation this section exists to prevent, and a reader who follows the rule as written would allow it.
Please name appendContainsMany here next to update and transform (it extends the field rather than rewriting it). The checklist line has the same gap; see the comment there.
Regression in the new guidance. Blocking.
Contributor
Author
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in 88b3866. Raw appendContainsMany is now in the list, and the prose says why: the caller names both the field and the items, so it can add anyone to teacherIds.
Comment on lines
+1322
to
+1324
| - No grant can write the field its own predicate reads. Every grant that | ||
| rests on a field names a named operation, not a raw `update`, `transform` or | ||
| `delete`, unless everyone it admits is trusted with that write (§13). |
Contributor
Author
There was a problem hiding this comment.
[Claude Code 🤖] Read literally, "Every grant that rests on a field names a named operation" also covers read grants, and so it fails the school's own read grants. Both of them rest on fields. The line means write grants only. It also leaves out raw appendContainsMany (see the comment on §13's "Keep authorization-bearing fields out of reach"). Suggested wording:
Suggested change
| - No grant can write the field its own predicate reads. Every grant that | |
| rests on a field names a named operation, not a raw `update`, `transform` or | |
| `delete`, unless everyone it admits is trusted with that write (§13). | |
| - No grant can write the field its own predicate reads. Every write grant | |
| that rests on a field names a named operation, not a raw `update`, | |
| `transform`, `appendContainsMany` or `delete`, unless everyone it admits is | |
| trusted with that write (§13). |
Regression. Blocking, together with the §13 list.
Contributor
Author
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in 88b3866. The checklist line now reads "Every write grant that rests on a field…" and lists update, transform, appendContainsMany and delete, so read grants are outside it.
Comment on lines
+1203
to
+1209
| A snapshot grant decides on the card as the index last saw it (§8). That opens | ||
| a time-of-check gap the length of the index's delay. Suppose a classroom has a | ||
| computed `headTeacher`, or a `searchable` link to its coordinator's roster card. | ||
| Take someone off the roster, and their grant keeps holding until the classroom | ||
| is indexed again. The delay is usually short, but it grows while the realm's | ||
| index is busy. The gap applies to writes too: the write lock judges a snapshot | ||
| grant against the same indexed copy. |
Contributor
Author
There was a problem hiding this comment.
[Claude Code 🤖] "Decides on the card as the index last saw it" says more than the realm does, and it contradicts §8's "lays the card's indexed values under its stored ones". snapshotInput in card-operations/gate.ts lays the row under the stored source. Stored fields are read as of now, and only computed values and the fields of a searchable linked card come from the index. A linked card's fields also count only while the stored link still names the card the index expanded. Two things follow for this paragraph:
- Re-pointing the classroom's coordinator link takes effect at once. The old coordinator stops matching and the new one fails closed until the classroom is indexed. The gap opens only when the linked card's own fields change, or the inputs of a computed value.
- In "Take someone off the roster", the gap comes from editing the roster card. It doesn't come from changing the classroom's link.
Suggested opening: "A snapshot grant reads computed values and linked cards' fields as the index last saw them (§8); stored fields stay current." Then make the example an edit to the coordinator's roster card. Item 4 of "Reading an explanation" (lines 1290–1293) needs the same narrowing: a did-not-hold after an edit to a stored field the predicate reads is not the index catching up.
Regression (inaccurate new prose). Non-blocking, but this is the mechanism the whole subsection rests on.
Contributor
Author
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in 88b3866. The section now says that a snapshot grant reads stored fields as they are now, and only its computed values and searchable linked cards' fields as the index last saw them. The example names the lagging case (the computed value's inputs, or the id on the linked roster card) and says that re-pointing the link takes effect at once. Explain item 4 is narrowed the same way.
Comment on lines
+1255
to
+1258
| - **They can mean different things.** Being allowed to open a card you were | ||
| sent a link to is different from being allowed to enumerate every card like | ||
| it. A lead teacher in the school reads the classroom they lead but can't log | ||
| an activity there. The same split applies between reading and listing. |
Contributor
Author
There was a problem hiding this comment.
[Claude Code 🤖] The lead-teacher example shows a different split from the one this bullet argues for. Reading a classroom versus appending an activity is read versus a write, not read versus listing. "The same split applies between reading and listing" asks the reader to make that jump themselves. In the school, the read and listMySchedules predicates are identical, so no scenario there shows the two lanes meaning different things. That breaks the rule that every rule in this section maps to a school scenario. Two options:
- Drop the lead-teacher sentence and keep the bullet's first sentence as the whole argument.
- Give a hypothetical and label it as one, for example "a substitute who can open the classroom they were sent but shouldn't enumerate every classroom".
I'd take (1). Non-blocking.
Contributor
Author
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in 88b3866. I dropped the lead-teacher sentence. The bullet now argues only open-versus-enumerate, which the provider's ad-hoc search in the same subsection illustrates.
Comment on lines
+1296
to
+1297
| 5. **`threw` is a bug in the policy, not a refusal.** The answer is | ||
| `decision: "failed"` and the invocation would answer 500 to a realm reader. |
Contributor
Author
There was a problem hiding this comment.
[Claude Code 🤖] decision: "failed" happens only when no other grant held. Grants union: firstHolding in card-operations/gate.ts keeps evaluating after a throw and admits on any grant that holds. An explanation can therefore show one grant threw and still be allowed, and a reader of this item would take that as contradicting it. Suggested wording: "If no other grant held, the answer is decision: "failed" …; if another held, the throw still hides a bug for every card that grant doesn't admit."
Non-blocking nit.
Contributor
Author
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in 88b3866. Item 5 now says failed applies only when no other grant held, and that an explanation can be allowed with a threw grant inside it. The bug is still there, and it refuses the first card that only the throwing grant could admit.
…nsMany and a throw beside a held grant Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 6, 2026
…ng a snapshot link Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
richardhjtan
approved these changes
Oct 6, 2026
This was referenced Oct 6, 2026
This was referenced Oct 6, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
Merge order: this PR waits on none of the boxel anonymous-access work. Its one constraint is that it merges before #174, which merges this PR in and renumbers the skill's sections. The rest of the chain, for context:
Compile a policy grant's opt-in to callers who aren't signed in boxel#6534: compile a grant's anonymous opt-in(merged)5a. Say that no policy grant reaches a path the realm ignores #178: lands before step 5
This PR: can merge any time before step 5. A separate boxel
feat(skills):pin bump (CS-13108, not opened yet) ships this PR and boxel-skills#177 without waiting on this chain.Each "not opened yet" line gets its PR URL as soon as that PR opens.
In service of the operation-permission policies project, which lets a realm's policy admit callers the realm's own permissions turn away. This PR adds the decisions an author has to make that the engine can't make for them. The
realm-policy-authoringskill already covers what a policy card accepts. The engine compiles a policy that lets a caller widen their own access as readily as one that doesn't, so this guidance has to live in the skill.The new §13, "Choosing what to grant", covers:
updategranted onteacherIdslets the caller rewriteteacherIds. A named operation such asappendActivitywrites only what its declaration fills. This is why a grant on a named operation never grants its base.update,transform,appendContainsManyordeletewhere the predicate reads a field those operations change, unless everyone the predicate admits is trusted with that change. Three places to keep the authorizing data instead: another realm the policy grants nothing in, a mirror only the realm's writers update, or anonGrantableoperation.snapshot: trueis acceptable. The cost is a gap as long as the index's delay, for the computed values and linked cards' fields the predicate reads. That's fine for low-stakes reads and for query grants, which already answer from the index. It isn't fine for writes, revocation for cause, or anything you'd have to undo by hand. The alternative is a stored mirror, which has its own sync gap.readandqueryas separate grants. Repeating a predicate across the two is the cost of the separation. The lanes are judged differently, can mean different things, and change independently.aclfirst,reasonagainstrefusal, whatno-grant(with no rules, or withgrants: []) andpredicate-falseeach point at,tierbeforeoutcome,threw, and chasing an unexpectedallowedthroughadmittedBy. It ends with the edit loop: explain the draft before saving, then the live policy after.Each rule is tied to a scenario in the school example realms that §10's worked call already uses: Alice's refused raw
updateof her own classroom'steacherIds, the provider's ad-hoc search that finds nothing, and theteacherIds/providerIdmirrors. §8 and §11 now point at the new section, the checklist (now §14) gains two lines, and the skill's description,index.mdand the glossary entry mention the judgment calls. No other section is renumbered, so the existing cross-references into §7–§12 still hold.After this merges, a boxel PR titled
feat(skills):bumpsBOXEL_SKILLS_VERSIONso the CLI plugin copy picks this up.🤖 Generated with Claude Code