SharingRuleService.reconcile aborts a rule's whole pass when one grant is refused by the engine's system-write organization rule — that rule's stale revocations stay undone (residue of #14484)

[os-project-manager]

Filed by the director seat (objectstack #12708, summon #10, session session_01ShyhexkB2d1AeRZ85tgAAe) out of the contract review of PR #14726 (#14484). The PR discloses this shape itself (changeset paragraph "(a)"); it is carried here as the follow-up rather than blocking the landing, for the reasons in the review verdict. domain:services, pm:queue, S.

The shape

After #14484, sys_record_share is tenant-scoped in the #13491 ledger, so an organization-less system insert on it is refused loudly on a walled install (ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED). SharingService.grant resolves the organization on every path that can, but two paths resolve none:

• a platform-global sharing rule (organization_id = null; its sweep runs unscoped) materialising a grant on an organization-less record;
• a direct grant whose read of the shared record's organization failed (deliberately not substituted with the acting session's, the review's blocking fix).

On the rule path, SharingRuleService.reconcile / reconcileForRecord has no per-grant catch: the refusal propagates and that rule's reconcile aborts mid-loop. Grants already written in the pass stay; the remaining grants and the stale-row revocations of that pass do not happen. The boot backfill logs the rule and continues to the next; the write hooks catch so a user write is never failed.

Why it is a card

The revocation half is the security-relevant half: a stale over-grant of that rule persists across every pass that meets the same organization-less record, and that record keeps aborting the pass until it is repaired. The shape needs three things at once (walled install, platform-global rule, organization-less record in a tenant-scoped object), and the organization-less population is the one #13491 / #13564 are draining, so it is narrow; but it is reachable on existing walled deployments today (cloud#1232 measured organization-less rows), and before #14484 those passes completed (writing NULL rows). It is loud (logged), not silent, which is why it did not block the review.

What to build

• A per-grant catch in both reconcile loops: a grant refused by the engine's organization rule is logged (rule, object, record id, the engine's code), counted in the pass result, and the pass continues — the remaining grants and, above all, the stale-row revocations still run.
• Pin: a platform-global rule over two records, one organization-less, on an isolated posture: the organization-less grant is refused and counted, the other record's grant lands, the stale row of a third record is revoked in the same pass. Ablation: remove the catch, the revocation pin goes red.
• Do not change the refusal itself, the direct-grant path's read-failed ⇒ null rule, or the eleven bare-context reads (measure: census the dependents of the SQL driver's orWhereNull tenant-wall carve-out before deciding its future (NULL org_id rows are globally visible on shared-DB walled deployments) #13564, with the maintainer).

Clause-② no (no accept-set change; a loop keeps going instead of aborting). @objectstack/plugin-sharing changeset patch.

Related: #14484 (the ruling), PR #14726 (the landing and its changeset paragraph (a)), #13491 (the ledger), #13564 (the reads), objectstack #12708 (director seat ledger).

[os-sales]

Claim: domain:services execution seat, session session_01AUF1NoViznQK32gqpK8wS8, round R22. Branch claude/issue-14754-reconcile-per-grant-catch.

Comments read before dispatch: 0 — this card carries no comments and therefore no ruling. Its body is a complete specification, filed by the director seat out of PR #14726's contract review; the "What to build" section is the brief.

Why it is dispatchable now. This card lands in SharingRuleService.reconcile / reconcileForRecord, and this lane held it serialized behind PR #14726, which claimed sharing-rule-service.ts. #14726 merged at 08:55:14Z, so the file is released and the residue can be built on a base that actually contains the change it is residue of — which is the reason it was held rather than dispatched in parallel.

⛔ Not to be widened into #14726's territory: SharingService.grant's organization resolution is what #14726 just landed, including the review's blocking fix. This card is the per-grant catch in the two reconcile loops, nothing more.

The security half is the revocation half. A refused grant aborting the pass mid-loop means that rule's stale revocations do not happen — a stale over-grant persists across every pass that meets the same organization-less record. Any acceptance here has to show both halves: that one refused grant no longer aborts the pass, and that the stale revocation actually occurs.

---

Generated by Claude Code

[claude]

os-dev-report

{
  "issue": 14754,
  "status": "done",
  "branch": "claude/issue-14754-reconcile-per-grant-catch",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/14930",
  "premise_still_valid": true,
  "summary": "Base verified to contain #14726 before any edit: `git merge-base --is-ancestor 3f64fe6c6 HEAD` succeeded, 3f64fe6c6 being the squashed landing of PR #14726 (#14484); worktree cut from origin/main at 13b520069. Both reconcile loops now attempt each grant through one shared helper, `grantOrAbsorbOrganizationRefusal` (sharing-rule-service.ts:1316), which absorbs ONLY ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED, logs it with rule/object/record/recipient/code, and returns a boolean the caller counts; every other error rethrows. reconcile is at 1352, reconcileForRecord at 1416. The count is reported through a new plugin-local type SharingRuleReconcilePassResult (line 105) extending the spec's SharingRuleEvaluationResult with grantsRefused, because packages/spec is another lane and was not touched. One further change inside both loops: the desired row leaves the stale set BEFORE the grant is attempted, so a refused upsert can never fall through into the revoke loop and retract a still-desired grant (this card's defect inverted). THREE THINGS IN THE ORDER/CARD TURNED OUT FALSE OR IMPRECISE. (1) Sweep order: the card and the order both assume a refused grant can strand 'the remaining grants'. Measured, the engine returns organization-less rows LAST (raw driver order rec_first,rec_orgless,rec_last; engine filtered read rec_first,rec_last,rec_orgless) because the driver's NULL-org compatibility arm is appended to the scoped arm. So a refusal is nearly always one of the last attempts and what an abort destroyed was almost entirely the revoke loop. This STRENGTHENS the card: the revocation half is very nearly the only half. The fixture therefore carries TWO organization-less records so the second refusal is an order-independent continuation witness. (2) Refusal reachability: the engine rule gates the INSERT half only. An organization-less row that already exists takes the update half, which grant sends unscoped and which reaches the NULL row, so it is UPDATED, not refused. A refusal therefore cannot reach the `cur` branch of either loop today; the delete-before-attempt reordering is behaviour-neutral defensive correctness, and this boundary is pinned as its own case. (3) Clause-2 and the changeset level both differ from the card; see the fields below. The card said it has no comments; it has exactly one, my own seat's claim comment, which carries no ruling.",
  "tests": "All figures from head 7919e8036 unless stated. PACKAGE SUITE: `pnpm --filter @objectstack/plugin-sharing test` -> `Test Files 33 passed (33)` / `Tests 779 passed (779)`, which includes the three cases in record-share-organization-stamp.test.ts that DELIBERATELY pin the abort on the other error a pass meets here (scoped update half answering RECORD_NOT_FOUND, left standing by the 2026-09-02 review) - they are the witness that the catch stayed narrow. TYPECHECK: `pnpm --filter @objectstack/plugin-sharing typecheck` exit 0, and its own verdict line `check:test-typecheck: OK` confirms the new test file is really type-checked rather than excluded. NEW PIN: packages/plugins/plugin-sharing/src/reconcile-refused-grant-continues.test.ts, 10 cases, real SqlDriver on better-sqlite3 :memory: behind a real ObjectQL, isolated posture, platform-global rule. ACCEPTANCE HALF 1 (pass continues): evaluateRule RESOLVES; asserts grantsRefused 2, grantsCreated 2, matchedRecords 4; a separate continuation witness asserts the measured attempt order [rec_first, rec_last, rec_orgless, rec_orgless_2] and that the first refusal index is less than the last attempt index. ACCEPTANCE HALF 2 (the security half, pinned SEPARATELY): a stale rule row seeded on an unmatched record is asserted GONE from the table after the pass, with grantsRevoked 1 alongside grantsRefused 2; the reconcileForRecord twin asserts grantsRefused 1 + grantsRevoked 1 with the table empty afterwards; plus a cross-rule case where one rule's refusal no longer aborts evaluateAllForRecord so the following rule still reconciles and revokes. ABLATION, direction predicted before running (pins redden and the revocation pin is among them). Mutation: the absorption line replaced by an unconditional rethrow. Disk proof BEFORE the run: guard-line literal count 1 -> 0, injected marker count 1, git hash-object dc843b134fe9758985d2440f989b28e97bd42c03 -> 0b433b304454815bf42a7b13b8a15aadb4b67320. MUTANT LEG: `Tests 7 failed | 22 passed (29)` - reddened were both HALF 1 pins, HALF 2 the revocation pin, the log pin, the boundary pin and both reconcileForRecord pins; correctly still green were the bare-refusal control, the narrow-catch pin, the org-carrying control and all 19 record-share-organization-stamp cases. RESTORE LEG: proven by blob hash equality back to dc843b13 (equal to the HEAD blob, non-empty), zero markers, and an EMPTY `git diff HEAD`; re-run gave `Tests 29 passed (29)`. The script carried `trap restore EXIT INT TERM` with an absolute REPO_ROOT path and restored via `git checkout HEAD -- path`, never a bare checkout. REBUILD: no build was needed for either leg and this was verified rather than assumed - the test imports ./sharing-rule-service.js RELATIVELY, so vitest reads the mutated source directly, not a dist artifact; the cross-package deps (objectql, driver-sql) come from dist and were unchanged and pre-built. GATES: 62 families derived by `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack` from the real change set (38 before the docs commit; re-derived to 62 after it added content/docs/**, and the 24 new ones were all run). Every exit code captured redirect-then-read, never across a pipe. 60 green. check-system-context-census exited 1 - a REAL finding I caused: my 37-line insertion moved two context?.isSystem reads from :157/:382 to :194/:419, pure line rot; repaired with the gate's own --fix (second commit, touching only content/docs/permissions/system-context.mdx) and re-run green. check-test-completeness exited 3 = NOT MEASURED per its own verdict text (no turbo test log to hand it) - reported as NOT MEASURED, not as red or green. RATCHET FAMILIES RE-RUN ON THE FINAL HEAD 7919e8036 after the docs commit: check-system-context-census, check:engine-double-contract, check:where-matcher, check:query-options-erasure, check:type-check-debt - all exit 0 (the last re-measured 21 ledger entries, 549 raw errors, none above its recorded number). LINT: not a narrowing - `eslint . --no-inline-config --format json` over the WHOLE repo completed inside the foreground cap: 5786 files as counted by ESLint's own output, 0 errors, 0 warnings. DOWNSTREAM: `turbo run typecheck --filter='...@objectstack/plugin-sharing'` - PREFIX form, i.e. consumers OF the package, the direction a contract change lands in - 89 tasks successful. REVERSE VERIFICATION that consumers read the REBUILT .d.ts rather than a cache: a probe importing the package by NAME and reading grantsRefused compiles clean (exit 0), while a deliberate typo is rejected `TS2551: Property 'grantsRefusedTYPO' does not exist on type 'SharingRuleReconcilePassResult'. Did you mean 'grantsRefused'?` - the error names the new key, so the declaration under test is the fresh one. A first attempt at this probe returned TS5112 (tsconfig present but not loaded) on both legs; that was read as NOT MEASURED and re-run with --ignoreConfig, not reported as a result.",
  "mcp_calls": "0 - the whole run went through repo-scoped REST (probed first: GET on the repo returned 200) plus git and curl; no MCP GitHub tool was called",
  "open_questions": [],
  "out_of_scope_findings": []
}

Generated by Claude Code

---

Generated by Claude Code

[claude]

os-dev-report

{
  "issue": 14754,
  "status": "blocked",
  "branch": "claude/issue-14754-reconcile-per-grant-catch",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/14930",
  "premise_still_valid": true,
  "summary": "PATCH ROUND 1. The red gate is real and reproduced; the proposed in-lane fix is MEASURED UNSOUND, so per the coordinator's own instruction I stopped rather than inventing a workaround. Nothing was committed this round: head is still 7919e8036, tree clean, PR still draft. THE PROPOSED FIX IS NOT SAFE. `instanceof SystemWriteOrganizationRequiredError` fails across the CJS/ESM split, measured, not assumed. @objectstack/objectql declares BOTH realms in its own package.json exports (import -> dist/index.mjs, require -> dist/index.js). Loading the class through each from inside plugin-sharing: `SAME CLASS IDENTITY (A === B): false`; an instance built in the ESM realm gives `instanceof A (same realm): true` but `instanceof B (CROSS-REALM): false`; the code compare survives the split (`true`). That is precisely the hazard the class's own docblock names ('Identified by `code` rather than `instanceof` ... so the check survives crossing a package boundary where two copies of this module can exist'), and the repo ships a `check:dual-build-cjs-loads` gate because consumers really do load the CJS build. The failure would be silent and its consequence is exactly this card's defect returning: the refusal stops being absorbed, propagates, and the pass aborts mid-loop, losing the stale revocations again - in the deployment shape the card exists for. So instanceof trades a lint red for a reintroduced security defect. Declined. THE GATE HAS NO THIRD MECHANISM. I read scripts/check-error-code-provenance.ts in full: the only two remedies are a ledger row under the stamping package's owner key or a PROVENANCE_WAIVERS entry, both in packages/spec/src/api/error-code-ledger.zod.ts. There is no in-file marker or opt-out at the stamp site. AN IN-LANE SPELLING DOES EXIST AND I AM DELIBERATELY NOT TAKING IT. Measured: replacing the named constant with a bare inline literal comparison turns the gate GREEN (exit 0, 305 stamp sites, 'OK - every registered-code stamp site is listed'), because the gate's published patterns are objlit/assign/constdef and it declares itself blind to a bare binary comparison. Taking it would be gate evasion by re-spelling the literal, which the coordinator forbade, and it is substantively worse code: it deletes the type annotation `SystemWriteOrganizationRequiredError['code']` that is the only thing currently stopping the spelling from drifting. Same objection to matching on `err.name`. RECOMMENDED FALLBACK, and a better variant of it. Open the domain:spec card as planned, but note the correct entry is probably a PROVENANCE_WAIVERS row rather than an owner-key row: plugin-sharing does not EMIT this code, it MATCHES a code the engine emits, and @objectstack/objectql's owner key ALREADY lists it (error-code-ledger.zod.ts:532). The waiver mechanism is the gate's documented answer for 'another package's door owns the emission'. A consumer matching a producer's code is arguably a class the waiver vocabulary does not yet name, which is exactly why it is an adjudication for the owning lane and not a mechanical entry. Worth considering alongside it: have @objectstack/objectql PUBLISH a recognizer for its own refusal, so no consumer ever re-spells the literal and the whole class closes rather than this one site - that edits objectql, not spec, but it is beyond this card's stated scope so I did not take it unilaterally.",
  "tests": "GATE, RED REPRODUCED AT 7919e8036: `pnpm --filter @objectstack/spec check:error-code-provenance` -> exit 1 (captured redirect-then-read, never across a pipe), verdict `FAIL - 1 stamp site(s) of a registered code with no provenance row: @objectstack/plugin-sharing stamps 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED' (constdef) at packages/plugins/plugin-sharing/src/sharing-rule-service.ts:87`. NO GREEN LEG EXISTS TO REPORT: I did not commit a fix, so the requested red-then-green pair is deliberately incomplete at the green end. Re-run at the end of the round still exit 1, confirming the red is standing and my probes restored cleanly. REALM MEASUREMENT (the decisive one), run from inside packages/plugins/plugin-sharing so bare-specifier resolution is the real one: ESM specifier resolved to file:///.../packages/objectql/dist/index.mjs, CJS to /.../packages/objectql/dist/index.js; `SAME CLASS IDENTITY (A === B): false`; `instA instanceof A (same realm): true`; `instA instanceof B (CROSS-REALM): false`; `instA.code: ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED`; `code compare survives the split: true`. Probe removed afterwards; tree clean. SPELLING PROBE, with the same disk-proof and restore discipline as the ablation: mutation confirmed on disk (blob changed, inline literal present 1), gate exit 0 with `OK - every registered-code stamp site is listed under its own owner key or carries a recorded waiver (8 waiver(s), all live)`; restore proven by blob-hash equality against the HEAD blob and an EMPTY `git diff HEAD`, under `trap restore EXIT INT TERM` with an absolute REPO_ROOT and `git checkout HEAD -- path`. Recorded as a measurement only; NOT adopted. SUITE AND TYPECHECK re-run at 7919e8036 after all probing: `Test Files 33 passed (33)` / `Tests 779 passed (779)`, typecheck exit 0 with `check:test-typecheck: OK`. `git status --porcelain` empty - the tree is byte-identical to the pushed head, so no probe leaked. ABLATION NOT REDONE, and the reason is that the condition for redoing it did not arise: the coordinator scoped the re-run to 'if instanceof replaces a string compare'. It does not - the absorption predicate is unchanged from the round-1 measurement (`err?.code !== ENGINE_ORGANIZATION_REFUSAL_CODE`), because no code changed this round. The round-1 mutant leg (`Tests 7 failed | 22 passed (29)`, revocation pin among the reddened) therefore still describes this head exactly.",
  "mcp_calls": "0 for this round as well (2 REST writes: none - report comment only); cumulative for the card: 0 MCP GitHub calls, everything via repo-scoped REST, git and curl",
  "open_questions": [
    {
      "question": "How should plugin-sharing's match on the engine's ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED be recorded, given that instanceof is measured unsound and both of the gate's remedies live in packages/spec?",
      "options": [
        "A - PROVENANCE_WAIVERS entry in packages/spec naming '@objectstack/objectql' as the owner key (which already lists the code at error-code-ledger.zod.ts:532), with the evidence that plugin-sharing MATCHES rather than emits. domain:spec lane.",
        "B - owner-key row adding the code under '@objectstack/plugin-sharing'. Same file, same lane, but it asserts plugin-sharing EMITS the code, which is false.",
        "C - have @objectstack/objectql publish a recognizer for its own refusal, so consumers stop re-spelling the literal; closes the whole class, edits objectql not spec, but is outside this card's stated scope.",
        "D - inline the literal in plugin-sharing to fall into the gate's declared blind spot. Measured green, but it is evasion and deletes the type anchor that prevents drift."
      ],
      "recommendation": "A, with C as the durable follow-up. A is the gate's own documented shape for 'another package's door owns the emission' and it records the true relationship - objectql emits, plugin-sharing matches - as a decision on the record rather than a false emitter claim (B). It is one line in another lane, so it unblocks a p2 security fix cheaply. C is strictly better long-term because it removes every consumer's need to spell the literal at all, and it is the only option that also survives the CJS/ESM split that killed instanceof; it belongs in its own card against objectql. D is refused: the gate publishes that blind spot as a known limit, so exploiting it converts a recorded decision into an unrecorded one, and it would delete the SystemWriteOrganizationRequiredError['code'] annotation that is the current drift guard."
    }
  ],
  "out_of_scope_findings": [
    "NOT filed as an issue - handed to the seat as requested: check:error-code-provenance was NOT derived by scripts/pm/dispatch-gates.mjs. Confirmed second instance of #14880. Evidence: `--commands` output does not contain 'provenance' at all (grep returns nothing), and `--residue` places it in the SILENT bucket with 'pnpm --filter @objectstack/spec run check:error-code-provenance [lint.yml] names: scripts/js-comment-mask.mjs, packages/spec/src/api/error-code-ledger.zod, packages/spec/src/api/error-code-ledger.zod.ts' plus the annotation 'artifact roster: all 3 declared literal(s) name tracked FILES - artifacts this gate names (a baseline, an allowlist of current members), not a population it declares.' So it declares only the artifacts it READS (the comment masker and the ledger) and never its actual scan surface, which its own header states is 'every package src/ tree under packages/'. My diff sits squarely inside that surface, so it scored an ordinary silent verdict and fell outside the --commands harvest. The residue text names the remedy itself: 'If it scans packages, the fix belongs there: declare the scan surface beside the roster (the subtree spelling, packages/**), after which it is MATCHED here.' It was NOT derived-and-skipped; my 62-family run was a faithful harvest of what the script emitted."
  ]
}

Generated by Claude Code

---

Generated by Claude Code