feat(plugin-sharing): one INFO line when isSystem writes materialise zero sharing grants (#6783) - #6963
Merged
Conversation
…zero sharing grants (#6783) Demand 3 of #4707, maintainer-ruled 2026-08-06. The sharing-rule record-write hooks skip isSystem sessions, so a seed run lands rows on an object an ACTIVE rule covers and creates no sys_record_share rows. The skip is correct — the kernel:bootstrapped backfill heals it — but it was completely silent, which is indistinguishable from a broken sharing configuration (hotcrm#640). afterInsert and afterUpdate now emit SYSTEM_WRITE_SKIP_NOTICE once per object per hook-binding generation, carrying the ruled wording verbatim plus the object and its active rules. One line per batch, never per row. Deliberately unchanged: the skip itself, the absence of any new switch, and afterDelete's silence — a delete skips revocation, not materialisation, and no re-evaluation or restart can reach a grant whose record is gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 8 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
os-zhuang
marked this pull request as ready for review
August 9, 2026 06:40
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
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.
Fixes #6783
Demand 3 of #4707, maintainer-ruled 2026-08-06 (
诉求 3(做)). Execution, not a decision.The defect: silence, not behaviour
plugin-sharing's record-write hooks return early on(ctx?.session as any)?.isSystem. So a seed run — or any internal write batch — lands rows on an object an active sharing rule covers and materialises zerosys_record_sharerows.That skip is correct. The
kernel:bootstrappedbackfill (backfillRuleGrantsinsharing-plugin.ts) reconciles every rule andevaluateRuleis idempotent, so the state heals. What was wrong is that nothing said so. hotcrm#640: a fresh install, 9 active rules, 9 accounts matching their criteria, users holding the right positions — andsys_record_shareempty. Every visible layer said "configured"; the only way to learn otherwise was to query the table, find it empty, and read this package's source.Mechanism
packages/plugins/plugin-sharing/src/rule-hooks.ts:A new exported constant
SYSTEM_WRITE_SKIP_NOTICEcarries the ruling's wording verbatim, behind the package tag every other line in the file uses:The two skips that drop grant materialisation —
afterInsertandafterUpdate— call anoteSystemWriteSkipped(objectName)helper before returning. The early-return itself is byte-for-byte the same decision it was.The helper is latched by a per-object set built inside
bindRuleHooks, and logs{ object, rules: [active rule names] }as metadata. It claims the latch before logging and swallows a throwing logger, so a log sink that is down cannot turn an observability line into a failed seed row, nor into one throw per row.One line per batch, not per row. That boundary is the shape of the fix, not an optimisation: the defect is silence, and a per-row flood is the same defect with a different symptom. A seed batch of N rows on one object produces exactly one line; an insert batch plus an update batch on the same object still produces one. The latch re-arms with the binding generation —
bindRuleRebindTriggersunbinds and re-binds this package on everysys_sharing_rulewrite — so a changed rule set gets its own notice instead of inheriting the previous generation's silence.Conclusion per authority face
bindRuleHooksskipsactive === falserules when building its object set, so an object whose only rule is inactive gets no hooks bound at all — silence is guaranteed upstream of the notice. Pinned by asserting the bound-hook list is empty, then that the write is silent.sys_record_shareis empty alongside the line, so the assertion is about the outcome and not only about the logger.warnanderrornever carry this text.The one face that is worded around rather than detected
The card asks that an active rule which legitimately matches nothing stay silent. Whether a given seeded row satisfies a rule's criteria is answerable only by running
recordMatches— theengine.findper row that the skip exists to avoid. Detecting it here would cost the skip its entire reason to exist, and re-implementing criteria matching in memory would fork the engine's filter semantics inside a plugin.So the line is worded as a statement about the write path ("materialisation did not run, here is where the answer comes from"), never as a claim that grants were owed. It is therefore true in both cases and is not a false alarm when the rule genuinely matched nothing.
The observable half of that face is pinned: a rule that is active, a row that is written, and zero grants materialised because the row is outside the criteria produces no line — because the defect only exists on the
isSystempath.Deliberately NOT done
sys_record_sharerow is created, updated or revoked by this PR.isSystem: truehas at least three distinct, undocumented side effects across three packages — each one has cost an app-side bug #4707 is decided — theisSystemconcept is not being split — and this card introduces nothing that could reopen it.warn/error. The ruling asked for INFO.afterDeletestays silent, and this is a decision, not an omission. A delete skips revocation, not materialisation, and the remedy the line names would be false there:evaluateRuleiterates records that still exist, so neither re-evaluating a rule nor restarting can reach a grant whose record is gone (the orphan named at the tail of 共享规则 hook 对谓词式(multi)写入不重算:if (!id) return让sys_record_share授权在批量更新后变陈旧 #4779). That class belongs torecord-share-cascade.ts— which stashes for system writes on its own account (记录删除后source: 'manual'的sys_record_share仍是孤儿 —— #4779 的 afterDelete 只覆盖规则共享,且只覆盖有规则的对象 #5103) — and to the boot orphan sweep. A line here would point an operator at a repair that cannot run. Pinned by a test, and by mutation C below.beforeUpdate/beforeDeletestash skips stay silent. They are row-set plumbing, not materialisation; noticing there would double-count withafterUpdate.packages/plugins/plugin-sharingplus one changeset. Nocontent/docs/releases/edit.Reverse verification
Direction predicted and written down before each run. All three matched exactly — no unpredicted movement in either direction. 14 tests in the new file.
noteSystemWriteSkipped(objectName)call sites, keep the constant and helperafterDeleteskipTwo readings worth stating plainly rather than rounding off:
re-arms on rebindexpects 2 notices from 1 insert + rebind + 1 insert; with no latch at all it also gets 2. It cannot distinguish "the latch re-armed" from "there is no latch", which is why the per-object and per-batch cases carry that half of the contract. Recorded here rather than counted as a pass.Taken out with
git diffto a patch file plusgit checkout --, nevergit stash— the stash stack lives in the common.gitand is shared by every worktree.Gates
pnpm --filter @objectstack/plugin-sharing testpnpm --filter @objectstack/plugin-sharing typechecktsc --noEmit, no output)eslinton both touched files,--no-inline-configcheck:engine-double-contractpinnedon both verbs (assertEngineDeleteDispatch,assertEngineUpdateDispatch); the shrink-only baseline is untouched and gains no rowcheck:nul-bytesgrep -naPself-scan of all three changed files, cleancheck:empty-changesetcheck:error-code-casing,check:route-envelope,check:durability-log-level,check:kernel-hook-pairs,check:wildcard-fallthroughcheck:i18npackages/cli/dist/commands/i18n/extract.js) and reports "Nothing was checked". Pre-existing environment state, not this change: no translation bundle, authoring key or locale file is touched herePremise
Re-measured against
origin/main@68feaadd6, which includes #6909 (merged into this package 2026-08-09T04:08:21Z). Premise stands. Measured, not assumed:isSystemearly returns inrule-hooks.tsare still at:157,:165,:180,:194— fix(sharing): defer to an app-authored RLS widener before hard-refusing a by-id write (#5493) #6909 touched the by-id write gate insharing-plugin.ts, not this file, so the card's anchors did not drift after all.isSystemand the hooks skip them deliberately is atsharing-plugin.ts:61, not:47as the card records. Same comment, same meaning; only the address moved.bindRuleHooksbinds only for rules withactive !== false, and all four hooks return before any materialisation when the session isisSystem.🤖 Generated with Claude Code
Generated by Claude Code