You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit b70a119
Browse filesBrowse the repository at this point in the historyBrowse files
fix(plugin-sharing): let system writes materialize sharing rules — drop the isSystem skips in bindRuleHooks (#13533) (#14528)
* fix(plugin-sharing): let system writes materialize sharing rules
`bindRuleHooks` returned early on `ctx.session.isSystem` in `afterInsert` and
`afterUpdate`, so a system-context write that moved a record into a sharing
rule's criteria materialized no `sys_record_share` row. Approval write-back is
exactly that write — an approval node with `lockRecord: true` mirrors the
decision onto the subject record under a system context, because while the
record is locked no other write can land — so a teammate who depended on the
criteria rule could not see an approved record until somebody re-evaluated the
rule or restarted the server.
Removes three early returns: the two named above, and the one on the
`beforeUpdate` / `beforeDelete` row-set stash they depended on. That third one
is load-bearing: without a stash `readAffectedRows` reports the row set as
`unbounded`, which would have sent every single-row system update down the
object-wide revoke plus asynchronous re-grant branch instead of the per-record
recompute. Retires the `SYSTEM_WRITE_SKIP_NOTICE` channel that announced the
skip.
`afterDelete` keeps its skip on separate grounds — its payload is revocation,
which `record-share-cascade.ts` delivers for system writes on its own account.
The two pins of the old behaviour are reversed and registered rather than
deleted: `system-write-skip-notice.test.ts` is renamed to
`system-write-materialisation.test.ts` and carries a row-by-row register of
every old expectation against its new counterpart, and `bulk-recompute.test.ts`
keeps its case under a docblock recording what it used to assert and why.
`content/docs/permissions/system-context.mdx` is updated because
`check:system-context-census` requires it: the page anchors every `isSystem`
read site and states census-derived counts, and three sites vanished.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
* chore(sharing): regenerate the isSystem census page and the engine-double ledger after merging main
Discharges the os-regen deferral the merge commit recorded.
`content/docs/permissions/system-context.mdx` is os-regen-driven and both sides
moved it, so the merge sequence took main's side and this commit re-applies the
branch's hand-written half on top: row 30 now describes the one surviving skip
(`afterDelete`, whose payload `record-share-cascade.ts` delivers) instead of all
four, rough edge 2 records the reversal rather than the retired INFO notice, and
the seven census-derived counts move with the population. Re-derived on the
merged tree rather than carried over — `check:system-context-census` reports
"OK — 106 elevation read sites in 20 packages across 45 files, all anchored;
140 anchors resolve, 27 declared non-read".
`scripts/engine-double-contract.pinned.json` learns the test file's new path.
The two rows are repathed, not renumbered: `--write` reports "692 (file, verb)
row(s), 2 added or grown, 2 lost. No seam losses — this regeneration only
records new seams."
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
* chore(sharing): re-anchor the isSystem census page after the second main merge
Patch round 1. `content/docs/permissions/system-context.mdx` is os-regen-driven
and both sides moved it again, so the merge stopped there and neither side could
simply win: main's #14383 (#12981 batch 9) moved row 37's `share-link-service.ts`
anchors by +6, while this branch moved row 36's `sharing-plugin.ts` anchor by +11
and rewrote row 30 plus rough edge 2. The resolution takes row 37 from main and
row 36 from this branch — the two sides moved different rows, so there was no
real overlap, only the driver's refusal to text-merge a generated page.
Verified by re-deriving rather than by inspection: `check:system-context-census`
reports "OK — 106 elevation read sites in 20 packages across 45 files, all
anchored; 140 anchors resolve, 27 declared non-read".
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
* docs(sharing): patch round 2 — deferred-revoke precision, bu-tree docblock, operator cost sentence
Documentation-only follow-up on the contract review's non-blocking §5 notes
2, 3 and 4. No code path changes: the only .ts edit is a docblock body.
- system-context.mdx row 30: "Lose: nothing" glossed the one shape where the
cascade's delivery is the queued orphan sweep rather than a synchronous
revoke. The cell now says the revoke is delivered but deferred on the
unbounded shape.
- bu-tree-recompute.ts: the docblock still described bindRuleHooks'
materialisation skip as current. It now says what bindRuleHooks does after
this change and why this file's own hooks never skipped system writes.
- changeset: one sentence on the operator-visible cost of seed- and
import-time system writes on rule-covered objects, and "unexported" becomes
"not exported from the package entry point". Level stays patch.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
---------
Co-authored-by: Claude <noreply@anthropic.com>
Copy file name to clipboardExpand all lines: content/docs/permissions/system-context.mdx
+35-17Lines changed: 35 additions & 17 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -9,9 +9,9 @@ the seed loader replaying package fixtures, a plugin's boot reconciler, a
9
9
service self-write, a migration.
10
10
11
11
This page is **the authority** for what that flag actually does. It exists
12
-
because the flag is not one concept: it is a single boolean read at **109
12
+
because the flag is not one concept: it is a single boolean read at **106
13
13
distinct sites across 20 packages**, and knowing three of those behaviours gives
14
-
no hint that the other hundred-and-six exist. Every documented app-side bug
14
+
no hint that the other hundred-and-three exist. Every documented app-side bug
15
15
traced to `isSystem` had the same shape — the metadata was complete and correct,
16
16
and the gap was observable only by querying the resulting rows.
17
17
@@ -124,18 +124,18 @@ that silently does not happen.
124
124
125
125
### 3. Sharing (`plugin-sharing`)
126
126
127
-
The largest single consumer — **20 of the 109 sites**.
127
+
The largest single consumer — **17 of the 106 sites**.
128
128
129
129
| # | Behaviour when `isSystem`| What you get / what you lose | Anchor |
130
130
|:--|:---|:---|:---|
131
-
| 30 |**Sharing-rule grant materialisation is skipped on all four record-write hooks**| Lose: **no `sys_record_share` rows are created**. A fully configured sharing rule grants **nothing** on seeded data until a rule is re-evaluated or the boot backfill runs. This is the behaviour that motivated #4707. Since #6783 the skip is no longer silent — it emits an INFO notice (rough edge 2) |`rule-hooks.ts:250`, `:274`, `:293`, `:322`|
131
+
| 30 | **Sharing-rule REVOCATION is skipped on the record-`afterDelete` hook** — and on that hook only | Lose: nothing permanently — the revoke is **delivered, but deferred on the unbounded shape**. The payload belongs to another subscriber: `record-share-cascade.ts` binds on every sharing-capable object and stashes for system writes on its own account (#5103). When the deleted ids are enumerable it revokes inline; when they are not — a predicate delete whose row set the stash could not resolve — it hands the reclaim to a queued background orphan sweep instead, so the share rows outlive the deleted records until that sweep runs, with the boot orphan sweep behind it. No surviving record loses access either way, and a restart re-runs the same sweep. This is one subscriber declining work another owns, not elevation silencing a consequence. ⚠️ **Grant MATERIALISATION no longer asks** — the `afterInsert` / `afterUpdate` skips, and the `before*` stash skip that fed them, were removed by the 2026-08-31 ruling on #13533; a system write materialises exactly as a user write does | `rule-hooks.ts:292` |
132
132
| 31 | Sharing write verdict short-circuits to `allow`| Get: writes pass the sharing gate unconditionally |`plugin-sharing/src/sharing-service.ts:654`|
133
133
| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement |`plugin-sharing/src/sharing-service.ts:920`, `:1007`, `:1597`|
134
134
| 33 |`grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) |`plugin-sharing/src/sharing-service.ts:1208`|
135
135
| 34 |`revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile |`plugin-sharing/src/sharing-service.ts:1286` (guard at `:1311`) |
136
136
| 35 |`listShares()` skips the management gate | Get: full enumeration of who can see a record |`plugin-sharing/src/sharing-service.ts:1338`|
137
-
| 36 |`sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing`|`sharing-plugin.ts:1077`|
138
-
| 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link creation/resolution while the policy is off |`plugin-sharing/src/share-link-service.ts:449`, `:503`, `:507`, `:580`, `:610`|
137
+
| 36 |`sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing`|`sharing-plugin.ts:1088`|
138
+
| 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link **creation** while the policy is off — resolution is **not** bypassed since #14033 (`publicSharing.enabled` is a standing policy held at every redemption): a link minted this way does not resolve until the block is enabled|`plugin-sharing/src/share-link-service.ts:449`, `:503`, `:507`, `:580`, `:610`|
139
139
| 38 | Sharing-rule provenance stamp skipped | Lose: the row is not marked as an admin customization — seeder / `defineRule` / boot reconcilers are "the package door" |`sharing-rule-provenance.ts:47`|
140
140
| 39 | Sharing-rule service write + delete paths return early | Lose: the manage-rules gate on the service surface, and the platform-global-rule delete guard |`sharing-rule-service.ts:157`, `:382`|
141
141
@@ -217,14 +217,32 @@ should recognise it instead of re-deriving it.
217
217
(`plugin-security/src/claim-seed-ownership.ts`). Any *third* system write
218
218
path gets neither. If you add one, stamp ownership yourself.
219
219
220
-
2.**Sharing materialisation is skipped, and now says so.** Row 30 produces
220
+
2.**Sharing materialisation is no longer skipped — the rough edge is closed,
221
+
and it is recorded here rather than deleted.** This entry used to describe
221
222
"configured but inert": nine installed sharing rules, matching records,
222
-
correct positions — and `sys_record_share` empty. The boot backfill does
223
-
eventually fix it, so the behaviour is not wrong; it was undiscoverable.
224
-
The INFO notice tracked as #6783**has since shipped**
225
-
(`rule-hooks.ts:274`, `:293` call `noteSystemWriteSkipped`), so the skip is
226
-
now observable — the earlier edition of this page said it was not, and that
227
-
sentence was already stale when the anchors were re-censused.
223
+
correct positions — and `sys_record_share` empty, repaired only by a
224
+
re-evaluation or a restart. The reading that made it a rough edge rather
225
+
than a defect was that the boot backfill eventually fixes it, so the
226
+
behaviour was correct and merely undiscoverable; #6783 shipped an INFO
227
+
notice on that reasoning.
228
+
229
+
That reading did not survive contact with a runtime write. An approval
230
+
node's write-back is a system write — `lockRecord: true` means only a
231
+
platform write can land while the record is locked — and it happens long
232
+
after boot, so no backfill was coming: a manager approved a request and the
233
+
teammate who depended on the criteria rule `status == "approved"` could not
234
+
see it at all. The maintainer ruled on 2026-08-31 (#13533) that the skip was
235
+
a **bug**, because a sharing rule's declared semantics is a published
236
+
promise and `isSystem` names the operator, never a consequence that need not
237
+
happen. Both materialisation skips and the notice that announced them are
238
+
gone; row 30 is now the `afterDelete` skip alone, which survives on the
239
+
separate ground that another subscriber delivers that payload.
240
+
241
+
⚠️ The observability half of that reading is worth keeping in mind
242
+
independently: the only signal a builder ever had was one INFO line, and
243
+
nothing in the docs or the tests said "after approval, when does the team
244
+
see it?". A compensating path that exists but that nobody can be expected to
245
+
know about is not a compensating path.
228
246
229
247
3.**Strict write observability is inert under elevation.** Row 22: a caller
230
248
that asked to be told loudly about dropped fields is told nothing, because
@@ -251,7 +269,7 @@ Ownership injection, `readonly` bypass and sharing materialisation are
251
269
independent decisions, and a seed loader plausibly wants the first two but not
252
270
the third. The concept is nevertheless **staying as one boolean**:
253
271
254
-
-**Shipped semantics.**`isSystem` is a published contract with 109 read sites
272
+
-**Shipped semantics.**`isSystem` is a published contract with 106 read sites
255
273
in 20 packages. Splitting it is a breaking contract change across all of them.
256
274
(The ruling was taken when the census read 80 sites in 18 packages; the count
257
275
has grown, which strengthens rather than weakens the argument.)
@@ -308,12 +326,12 @@ still holds equal to the census on every pull request:
308
326
| Appearances of the bare identifier `isSystem` in non-test sources | 813 | — |
309
327
| — parsed as a declaration | 22 | ✅ |
310
328
| — parsed as an object-literal / type key (producers and option objects) | 310 | — |
311
-
| — parsed as a property **read**|115| ✅ |
329
+
| — parsed as a property **read**|112| ✅ |
312
330
| — parsed in some other syntactic position (a local, a cast, a conditional) | 9 | ✅ |
313
331
| — the remainder: text inside comments and string literals | 358 | — |
314
332
| Of those reads: reads of one of the unrelated metadata fields | 6 | ✅ |
315
-
| Of those reads: reads of `ExecutionContext.isSystem`|**109**| ✅ |
316
-
| — behaviour-bearing (rows 1–61 above) |105| ✅ |
333
+
| Of those reads: reads of `ExecutionContext.isSystem`|**106**| ✅ |
334
+
| — behaviour-bearing (rows 1–61 above) |102| ✅ |
317
335
| — carry the flag onward only (rows 62–65 above) | 4 | ✅ |
318
336
| Packages containing at least one elevation read |**20**| ✅ |
319
337
| Files containing at least one elevation read | 45 | ✅ |
0 commit comments