Skip to content

fix(plugin-sharing): re-evaluate position and team sharing rules for the user whose membership changes - #24790

Merged
objectstack-fleet[bot] merged 7 commits into
mainfrom
claude/sharing-position-recompute
Oct 20, 2026
Merged

objectstack-fleet[bot] merged 7 commits into
mainfrom
claude/sharing-position-recompute

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Security fix (maintainer-ordered; no public card; detail withheld under the disclosure ruling).

Clause-②: no

What changed

@objectstack/plugin-sharing now re-evaluates position-recipient and team-recipient sharing rules for the affected user when the membership behind the recipient changes.

  • New hooks, recipient-membership-recompute.ts. afterInsert, afterUpdate and afterDelete on sys_user_position and sys_team_member, bound under their own package id so a rule rebind cannot remove them. The plugin binds them next to the business-unit recompute.
  • Which writes trigger it. Each insert and each delete triggers it. An update triggers it only when its patch touches a column the expansion reads: the user, the position or team, the organization, or the validity window. For each such write, every rule whose recipient is the position or team the row names (before or after the write) is re-evaluated for the user the row names.
  • Organization scope. An organization's rule is re-evaluated only for rows of its own organization. A platform-global rule is re-evaluated for every row. Team expansion has no organization scope, so neither does its trigger.
  • SharingRuleService.reconcileRuleForUser(rule, userId). The user-scoped pass. Its outcome equals the whole-rule pass restricted to that user. If the rule reaches the user, the user ends up with one grant per matched record at the rule's level. If it does not reach the user (inactive rule, switched-off position, or the user no longer holds the recipient), the user ends up with none. Withdrawal goes through SharingService.revoke, one grant at a time, the same way the whole-rule pass withdraws.
  • User-scoped holder checks. PositionGraphService.holdsPosition and teamHasMember / holdsRole in team-graph.ts read the same rows the expansions read, narrowed to one user. The expansions and the checks build their predicates from one shared function per table, so the two cannot drift apart.
  • Docs. content/docs/permissions/sharing-rules.mdx now lists the new reconcile moment in its "Switching a rule off" table. The sentence in permissions-matrix.mdx about when rules are re-evaluated now names it too.

The invariant restored

When a position is assigned to a user or removed from one, every sharing rule whose recipient is that position is re-evaluated for that user in the same write path. A new holder gains the rule's grants. A removed holder loses them before the write returns, and nobody has to re-evaluate the rule by hand. Team membership follows the same rule. Other users' grants are not touched, and neither are rules that name other recipients.

Cost

The business-unit recompute does not recompute every rule on every graph write, and it queues its grant half. The reason is that a BU write can move many recipients at once. This change addresses the same cost concern by narrowing the scope instead of deferring the work. A membership row moves exactly one user. Per written row the pass costs:

  • one rule listing;
  • for each rule that names the recipient: an indexed holder check (never the recipient's whole member list), that user's grants of the rule, and, only when the rule reaches the user, the rule's criteria read plus the grants the user lacks.

A bulk import of N rows therefore costs N user-scoped passes and never runs a whole-rule pass.

Tests

  • src/recipient-membership-recompute.test.ts: a real ObjectQL engine over SqlDriver (better-sqlite3), so the engine itself supplies each hook context (inserted row, update and delete pre-image, per-row dispatch of a batch insert). Pins:
    • a new holder gains the rule's grants;
    • a removed holder loses them, and both read (buildReadFilter) and edit (checkEdit → deny) are refused;
    • moving an assignment to another user, or to another position, moves the grants with it;
    • a validity window that closes counts as a removal;
    • a second assignment of the same position, or a membership role naming it, keeps the grants;
    • an unrelated position's grants are untouched and its rule is never re-evaluated;
    • other holders' grants are untouched;
    • an assignment in another organization does not re-evaluate an organization's rule;
    • an update that cannot change who holds the position re-evaluates nothing;
    • a bulk import of 40 assignments makes 20 user-scoped passes (for the named position), no whole-rule pass, and only user-scoped assignment reads;
    • after the user-scoped passes, a whole-rule evaluateRule has nothing left to create, update or revoke;
    • team: a new member gains, a removed member loses;
    • binding, write classification and organization pinning.
  • src/position-graph.test.ts: holdsPosition answers exactly expandPositionUsers(...).includes(user) across users, positions, organization scopes, validity windows and the membership-role source, and it reads one user only.
  • Ablations, each run once with scripts/ablation-replace.mjs and restored to the HEAD blob (git diff HEAD empty):
    1. No membership hook bound: 13 of 18 red.
    2. Organization pin removed: 2 of 18 red.
    3. Holder check replaced by the full expansion: the bulk pin red.
    4. The user-scoped withdrawal revokes nothing: 7 of 18 red (every removal, move and team-removal pin).

Readings at 99997e91e:

  • pnpm --filter @objectstack/plugin-sharing test: 51 files, 1192 tests passed.
  • pnpm --filter @objectstack/plugin-sharing typecheck: exit 0. The test layer compiles, and test-typecheck-debt.json holds its 3 existing pinned signatures with no change.
  • The new and changed files on their own: 26 tests passed.
  • Ablations 1 to 3 ran at 6d46619c7; the lines they mutate are unchanged since then. Ablation 4 ran at 99997e91e.

Gates

All gate families that node scripts/pm/dispatch-gates.mjs --commands derives for this change set: 99 families, run locally at 99997e91e.

  • 98 families exit 0. check:type-check-debt ran under the verify lock: exit 0, 26 raw tsc errors, none above its recorded number.
  • node scripts/check-plugin-teardown-shape.mjs --self-test exits 3 (PREREQUISITE NOT MEASURED). Its pinned positive control is not reachable from this shallow clone. CI runs it.
  • dispatch-gates --ran reconciliation: 99 derived, 99 run, 0 unrun.
  • pnpm --filter @objectstack/spec build and check:generated ran after merging origin/main: exit 0, all 14 generated artifacts up to date.

The full test battery and the type-check lanes are CI's.


Generated by Claude Code

…the user whose membership changed

A write to sys_user_position, to the sys_member.role transition source, or
to sys_team_member now re-evaluates every sharing rule whose recipient the
row names, for the one user the row names, in the same write path. A new
holder gains the rule's grants and a former holder loses them before the
write returns.

WIP: tests and changeset follow.

Claude-Session: https://claude.ai/code/session_01KqkTQNLcxVc588eZpJmp1r
Co-authored-by: Claude <noreply@anthropic.com>
…et and docs

Real-engine pins: a new holder gains the rule's grants, a removed holder
loses them with read and edit refused, moves and window changes follow the
holding, an unrelated position and other holders are untouched, an
organization's rule is pinned to its own rows, a bulk import stays
user-scoped. holdsPosition is pinned equal to the expansion.

Claude-Session: https://claude.ai/code/session_01KqkTQNLcxVc588eZpJmp1r
Co-authored-by: Claude <noreply@anthropic.com>
The membership-role transition source stays part of the holder check (a
user who still holds the position there keeps the grants), but its writes
are not watched: its values are the closed set of membership grades.

Claude-Session: https://claude.ai/code/session_01KqkTQNLcxVc588eZpJmp1r
Co-authored-by: Claude <noreply@anthropic.com>
…e test double

The user-scoped withdrawal reuses the sharing service's revoke, as the
whole-rule pass does, so the change adds no new engine write call site.
The position-graph equivalence double applies the caller's limit.

Claude-Session: https://claude.ai/code/session_01KqkTQNLcxVc588eZpJmp1r
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/xl documentation Improvements or additions to documentation tests tooling labels Oct 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-sharing, touching 50 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/plugins/plugin-sharing/src/bu-tree-recompute.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

40 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 46bb5e3e5405fad31ccba6fe571e98b9234aac78.

⛔ 12 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/plugins/plugin-sharing/src/bu-tree-recompute.ts) — pages documenting those are invisible to this run
  • 1 anchor(s) matched too much of the corpus to be a work list: organization_id (literal, 35 pages)
  • 3 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 9 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 46bb5e3e5405fad31ccba6fe571e98b9234aac78 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from ca02625e0e7f3b8376e86c4d52a755dd99f56b34 — the merge of head 99997e91e92721fc4950855a0b2e7a43ca1ad5dd into base 46bb5e3e5405fad31ccba6fe571e98b9234aac78, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin ca02625e0e7f3b8376e86c4d52a755dd99f56b34 && git checkout ca02625e0e7f3b8376e86c4d52a755dd99f56b34
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 46bb5e3e5405fad31ccba6fe571e98b9234aac78 99997e91e92721fc4950855a0b2e7a43ca1ad5dd && git checkout -B drift-repro 46bb5e3e5405fad31ccba6fe571e98b9234aac78 && git merge --no-ff 99997e91e92721fc4950855a0b2e7a43ca1ad5dd

node scripts/docs-audit/affected-docs.mjs --json 46bb5e3e5405fad31ccba6fe571e98b9234aac78

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 46bb5e3e5405fad31ccba6fe571e98b9234aac78 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对

ACCEPT · PR #24790 at 99997e91e · repo:hotcrm seat 2 (the maintainer's direct dispatch; no public card, by the maintainer's ruling) · os-support-ai · session session_01KqkTQNLcxVc588eZpJmp1r · 2026-10-20T06:31Z

  1. Shape:
    • Draft, base main. It opens with the mandated security first line and Clause-②: no, and the body carries no closing keyword and names no issue. Assignee os-support-ai.
    • 11 files, +1130 / −13, all in packages/plugins/plugin-sharing, plus two docs sentences and one changeset. check-governed-merges --pr: 0 of 11 paths hit the register, 1143 lines, under the human-merge threshold.
    • The commits carry the model-free trailer pair, with 0 model-identifier lines.
  2. The diff, read:
    • recipient-membership-recompute.ts binds afterInsert, afterUpdate and afterDelete on sys_user_position and sys_team_member, under their own package id, at priority 190.
    • Unknown means affected: an update is judged by the expansion columns its patch carries, and an unreadable payload or organization counts as "affected", never "unchanged".
    • Scope: each write re-evaluates only the rules naming the recipient, for the one user the row names, pinned to the row's organization for an organization rule.
    • Failure isolation: one rule's failure for one user does not stop the rest.
    • SharingRuleService.reconcileRuleForUser withdraws through SharingService.revoke, as the whole-rule pass does.
    • The holder checks and the expansions share one predicate builder per table, so they cannot drift apart.
  3. The open question, the seat's call: A, keep the team leg. team is a declared recipient kind. The same mechanism, about 20 lines and one pin, enforces it the way position is enforced. Leaving a declared recipient half-enforced is the shape the platform's basic ruling refuses.
  4. Changeset and docs, read sentence by sentence: the changeset states the triggering writes, the scope and cost, and the new public method. It says "No spec key, endpoint or configuration changes", which the diff confirms. Both docs sentences name the new reconcile moment and nothing more.
  5. Gates:
    • The dev's local run on 99997e91e: 51 files, 1192 tests passed, typecheck exit 0, all 99 derived families run. One prerequisite is NOT MEASURED locally (check-plugin-teardown-shape --self-test, which needs a non-shallow clone); CI runs it.
    • Four ablations went red in the predicted direction, and each restore is proven blob-equal.
    • CI on 99997e91e at this review: 15 checks success, 5 skipped, 18 running. Landing waits for every check.
  6. Acceptance notes (not rework):
    • A membership write whose hook context lacks the changed row (the missing path) is logged and reconciled only for the user that can be read.
    • A failed reconcile is logged and does not fail the operator's write, as in the rest of the hook family. The rule converges on its next evaluation.

Landing: once every check on the head is green, this seat brings main in through the relay if it has moved, then flips the PR ready and arms auto-merge.


Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 20, 2026 06:58
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 20, 2026 06:58
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 20, 2026
Merged via the queue into main with commit 0b7286f Oct 20, 2026
42 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/sharing-position-recompute branch October 20, 2026 07:28
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Landed · repo:hotcrm seat 2 · session session_01KqkTQNLcxVc588eZpJmp1r · 2026-10-20T07:28Z

Reading Probe Result
PR #24790 GET /pulls/24790 Merged 2026-10-20T07:28Z through the merge queue, on the reviewed head 99997e91e (ACCEPT 6284923571 on this PR). Every check was success on that head (36 success, 5 skipped), and the queue re-ran the subscribing workflows on its rebuilt generation.
origin/main GET /commits/main 0b7286ff9, the merge commit itself. Its message carries 0 model-identifier lines.

State: there is no card to close. This was the maintainer's direct dispatch, with no public card by their ruling. The membership recompute ships in the next @objectstack/plugin-sharing release.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/xl tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants