Skip to content

Commit 97f4f8c

Browse files
os-justinclaude
andauthored
refactor(plugin-sharing,spec): retire the ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED provenance waiver with its stamp site (#19668)
Fixes #16160 Clause-②: no ## What this does Retires the `ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED` provenance waiver **together with its stamp site**. Both halves ship here, and neither can ship alone: `check:error-code-provenance` reconciles a waiver in three directions at once, so removing the stamp site alone reddens it on the stale waiver, and removing the waiver alone reddens it on the orphaned site. That is measured below, not asserted. - `packages/plugins/plugin-sharing/src/sharing-rule-service.ts` — the local `ENGINE_ORGANIZATION_REFUSAL_CODE` constant is gone. The per-grant catch now asks `isSystemWriteOrganizationRequiredError(err)`, and the `warn` that reports an absorbed refusal names `SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE`. Both come from `@objectstack/objectql`, whose barrel comment says in so many words that they exist so a consumer performs the `code` compare "without acquiring a `check:error-code-provenance` stamp site of its own". - `packages/spec/src/api/error-code-ledger.zod.ts` — the matching `PROVENANCE_WAIVERS` entry is deleted. It carried its own expiry: *"Removed together with the stamp site when #14936 lands and objectql publishes a recognizer."* ## It was four references, exactly as triage counted Triage's line-by-line read recorded 「删常量会连带四处引用,取卡人排期时按 4 处算,不是 2 处」. Located by symbol, not by line — every number had drifted by +21 since that read: | triage's line | line on this tree | what it is | disposition | |---|---|---|---| | `:137-138` | `:138-139` | the constant itself, typed `SystemWriteOrganizationRequiredError['code']` | deleted; the type-only import goes with it | | `:1631` | `:1652` | docblock `{@link ENGINE_ORGANIZATION_REFUSAL_CODE}` | repointed at `SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE` | | `:1645` | `:1666` | docblock `{@link ENGINE_ORGANIZATION_REFUSAL_CODE}` | reworded to name the recognizer | | `:1683` | `:1704` | `if (err?.code !== ENGINE_ORGANIZATION_REFUSAL_CODE) throw err;` | now `if (!isSystemWriteOrganizationRequiredError(err)) throw err;` | | `:1691` | `:1712` | `code: ENGINE_ORGANIZATION_REFUSAL_CODE,` | now `code: SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE,` — see the judgement below | Four references plus the declaration. After the edit `git grep ENGINE_ORGANIZATION_REFUSAL_CODE` over `packages/` returns nothing. ## The `:1691` judgement, stated rather than carried along The card and triage both flagged this site as a separate judgement, on the reading that plugin-sharing **emits** the code there in its own refusal envelope. **On this tree that reading does not hold, and this PR records why.** `:1712` is not an envelope. It is the second argument of `this.logger?.warn?.(message, meta)` — a structured log field on the refusal the catch has just absorbed. The enclosing method's own contract is explicit that nothing leaves it carrying this code: *"Returns `true` when the grant landed, `false` when it was refused — never throws for the refusal, and the caller counts what it is told."* So plugin-sharing recognises this code and reports what it absorbed; it emits it nowhere. Two consequences, both deliberate: - **The waiver's premise 「Matches the code, never emits it」 survives the re-read** — it was never resting on the site the card suspected. Nothing here needs re-adjudicating, and the waiver is retired on its own stated expiry rather than on a changed premise. - **So the site takes the published constant, and this is not the "larger change" branch.** There is no re-emission to stop, so widening the card into one would be a change with no defect under it. The engine's own `SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE` is the right value for a log field that must name the code, and an identifier in `code:` position is not a `check:error-code-provenance` stamp site — the `objlit` pattern matches a quoted literal only. - **It is pinned, and was before this PR.** `field-recipient.test.ts` asserts the warn payload carries `code: 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED'`, and separately that a grant refused with any other code rethrows. Both stay green, so the swap is proven to have moved neither the reported code nor the width of the catch. ## Premise re-measured on this tree, against the built artifact The card's whole basis is that `@objectstack/objectql` publishes the recognizer. Measured at `packages/objectql/dist/index.d.ts`, not at source: ``` 2231:declare const SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE: "ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED"; 2277:declare function isSystemWriteOrganizationRequiredError(err: unknown): boolean; 3246:export { …, SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE, …, isSystemWriteOrganizationRequiredError, … }; ``` plugin-sharing declares `@objectstack/objectql` in `dependencies` and its tsconfig carries no `paths` remap, so the compiler reads that declaration file through the package `exports`. A negative control proves the reading is live rather than cached: renaming the imported member to one objectql does not export turns the package typecheck red with ``` src/sharing-rule-service.ts(39,3): error TS2724: '"@objectstack/objectql"' has no exported member named 'isSystemWriteOrganizationRequiredErrorNOSUCHEXPORT'. Did you mean 'isSystemWriteOrganizationRequiredError'? ``` and the restore is proven byte-identical (`blob == HEAD`, `git diff HEAD` empty). ## `check:error-code-provenance` — the load-bearing gate, before and after Both readings are from `pnpm --filter @objectstack/spec run check:error-code-provenance`, exit code captured before any pipe. **Before** (exit 0): ``` scanned 2411 files; 339 registered-code stamp site(s): 322 listed, 17 waived OK — every registered-code stamp site is listed under its own owner key or carries a recorded waiver (10 waiver(s), all live) ``` **After** (exit 0): ``` scanned 2411 files; 338 registered-code stamp site(s): 322 listed, 16 waived OK — every registered-code stamp site is listed under its own owner key or carries a recorded waiver (9 waiver(s), all live) ``` Sites 339 → 338, waived 17 → 16, waivers 10 → 9, and `listed` unmoved at 322 — the drop triage asked to be confirmed, in each of the three counts. `--report` before the edit named exactly one waived site for the pair, `packages/plugins/plugin-sharing/src/sharing-rule-service.ts:138 (constdef)`; it is absent after. ## "One PR or neither" — measured, in both directions Two one-legged ablations through `scripts/ablation-replace.mjs`, each committed-state, each with its on-disk landing proven and its restore proven (`blob == HEAD`, `git diff HEAD` empty). Predicted direction before running: red, both. **A. The stamp site back, the waiver still gone** — the catch re-spelled as a local `*_CODE` constdef: ``` scanned 2411 files; 339 registered-code stamp site(s): 322 listed, 16 waived 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:1705 — not listed under its own owner key ``` **B. The waiver back, the stamp site still gone**: ``` scanned 2411 files; 338 registered-code stamp site(s): 322 listed, 16 waived FAIL — 1 provenance-waiver problem(s): waiver @objectstack/plugin-sharing → ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED matches NO stamp site in the scan — its subject is gone (or moved beyond the published patterns); remove the waiver, or extend STAMP_PATTERNS … ``` Each half reddens the gate on its own, as the card said. The green above is therefore a reading, not a vacuum. Neither ablation left anything behind: the working tree is clean at the final commit. ## Verification All readings taken at `17e22cbd1` (the final commit, `origin/main` merged in at `8f30c188a`). - **Derived gates: 83 derived, 83 run, 0 NOT-MEASURED, 0 UNRUN, all exit 0.** Derived with `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` after the changeset existed, exit codes captured before any pipe, reconciled with `--ran` carrying each command's code. Two gates first answered `exit 3` PREREQUISITE NOT MET rather than a verdict — `check:dual-build-cjs-loads` and `check:i18n`; both prerequisites were built and both re-ran green (`104 published require entry point(s) across 67 package(s) load`; `9 package(s) — all bundles in sync`). - **`pnpm --filter @objectstack/spec --filter @objectstack/plugin-sharing run typecheck`** — exit 0, both packages including their script and test layers. - **`pnpm --filter @objectstack/plugin-sharing test`** — `Test Files 37 passed (37)`, `Tests 913 passed (913)`. - **`pnpm --filter @objectstack/spec test`** — `Test Files 512 passed (512)`, `Tests 14953 passed | 1 todo (14954)`. - **`pnpm --filter @objectstack/spec check:generated`** — `All 15 generated artifacts are up to date`. Nothing to regenerate: no schema, no export and no authorable key moved. - **`pnpm lint` (repo-wide, not narrowed)** — `eslint . --no-inline-config`, exit 0, **6996 files linted, 0 errors, 0 warnings**, file count read from `--format json`. - Heavy runs went through `scripts/pm/os-verify-lock.sh` with `OS_VERIFY_LOCK_SLOT=issue-16160`; every verdict is its `VERDICT command-exit` line, never a bare shell status. ## Changeset `patch` for both `@objectstack/spec` and `@objectstack/plugin-sharing`, judged against each `package.json`'s `files[]` after a build rather than by inspection: - `@objectstack/plugin-sharing` ships `dist`. Its built `dist/index.js` and `dist/index.mjs` now contain `isSystemWriteOrganizationRequiredError` and no longer contain `ENGINE_ORGANIZATION_REFUSAL_CODE` (positive control: an unrelated symbol in the same file still matches). Published bytes moved. - `@objectstack/spec` ships both `dist` **and** `src/**/*.zod.ts`, and `error-code-ledger.zod.ts` matches that second entry — so the edited file ships twice over, compiled and verbatim. The retired waiver's `reason` text is absent from `dist` after the build (positive control: a sibling waiver's reason still matches). So `skip-changeset` is not available to this diff on either package. ## Acceptance notes - `sharing-service.ts` mentions the literal `ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED` in two docblocks. Those are comments, the gate masks comments before scanning, and both statements are accurate about the engine's refusal — an observation, not a finding, and deliberately untouched. - Nothing in the (a)/(b)/(c) filing classes turned up on this diff. ## 维护者速读(草稿) ### 改了什么 删掉了一条「到期债务」:`plugin-sharing` 过去自己抄了一份引擎错误码的字面量,台账里为此挂了一条豁免记录。现在引擎自己把识别函数发布出来了,`plugin-sharing` 改用它,那条豁免记录随之删除。两处改动必须同一个 PR,分开做任何一半都会让门禁变红。 ### 为什么改 那条豁免记录自己写了到期条件——「等 objectql 发布识别器后,与打码点一起删除」——而**没有任何门禁会在条件满足时提醒任何人**。不删,它就从一条有期限的记录变成永久居留,下一个读台账的人会以为这是个长期决定。本次改动同时消掉一处重复的字符串字面量:抄写的那一份一旦和引擎抛出的不一致,失效形态是一个**永远不会触发的 catch**,不是一个失败的测试。 ### 风险与代价(含回滚) 风险低。对外错误面零变化:`ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED` 仍然注册在 `@objectstack/objectql` 名下,`ERROR_CODE_LEDGER` 与 `StandardErrorCode` 的成员一个没动,本次只动了 `PROVENANCE_WAIVERS` 数组里的一个元素。吸收范围也没变宽:两个既有测试分别钉住「吸收这一个码」与「别的码照样抛出」,都是绿的。回滚 = revert 这一个 PR;两处改动在同一次提交里,不存在只回滚一半的形态。 ### 席位意见 ### 你要做的 无需动作。这是记账债务的按期兑付,不是行为变更;`Clause-②: no`,两个包各一条 `patch` changeset 已随 PR 提交。 --- _Generated by [Claude Code](https://claude.ai/code/session_01Sfe5YjBLwB9J3y8fvm2xq1)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f2681c5 commit 97f4f8c

3 files changed

Lines changed: 36 additions & 36 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@objectstack/spec': patch
3+
'@objectstack/plugin-sharing': patch
4+
---
5+
6+
`plugin-sharing` recognises the engine's organization refusal through objectql's own published recognizer instead of a locally re-spelled literal, and the `PROVENANCE_WAIVERS` row that excused that local spelling is retired with it (#16160).
7+
8+
Clause-②: no
9+
10+
The waiver carried its own expiry in its `reason`: *removed together with the stamp site when objectql publishes a recognizer*. It does, so both halves land here — `check:error-code-provenance` reconciles a waiver in three directions at once (the `registeredUnder` key still lists the code, the waived package still does not, and the scan still finds a site for the pair), so removing either half alone reddens the gate on the other.
11+
12+
- **`ENGINE_ORGANIZATION_REFUSAL_CODE` is gone.** It was a `constdef` stamp site in `plugin-sharing/src/sharing-rule-service.ts` for a code this package only ever RECOGNISES — `@objectstack/objectql` is the emitter and already carries the row. The per-grant catch now asks `isSystemWriteOrganizationRequiredError(err)`, and the `warn` that reports an absorbed refusal names `SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE`. Both are imported from `@objectstack/objectql`, which exports them for exactly this: a consumer performs the `code` compare without authoring the string, so it acquires no stamp site of its own and cannot drift from what the engine throws.
13+
- **Nothing about the absorbed set moves.** The catch stays as narrow as it was — one engine refusal absorbed, everything else rethrown unchanged — and `plugin-sharing` still emits this code nowhere: the surviving mention is a structured log field on the refusal it just absorbed, not a refusal envelope of its own.
14+
- **No error-code membership moves.** `ERROR_CODE_LEDGER` and `StandardErrorCode` are untouched; `ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED` stays registered under `@objectstack/objectql` exactly as before. The only ledger change is one `PROVENANCE_WAIVERS` element, 10 waivers → 9, and the gate's site census 339 → 338 with `listed` unchanged at 322.

‎packages/plugins/plugin-sharing/src/sharing-rule-service.ts‎

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,22 @@ import type { ExecutionContext } from '@objectstack/spec/kernel';
2323
// enforcement hole this closes gets re-opened one seam over.
2424
import { isRowActive } from '@objectstack/core';
2525
// [#14754] The engine's organization refusal for a system write on a
26-
// tenant-scoped object (#8844). Imported as a TYPE only: the runtime check is a
27-
// `code` compare, which is the convention that class itself documents ("a
28-
// caller that catches it identifies it by `code`") so the check survives the
29-
// package boundary where two copies of that module can exist. Typing the
30-
// literal FROM the class is what keeps the two spellings from drifting — a
31-
// typo here would not be a failing test, it would be a catch that never fires.
32-
import type { SystemWriteOrganizationRequiredError } from '@objectstack/objectql';
26+
// tenant-scoped object (#8844). The runtime check is a `code` compare, which is
27+
// the convention that refusal itself documents ("a caller that catches it
28+
// identifies it by `code`") so the check survives the package boundary where
29+
// two copies of that module can exist — `instanceof` is unsound across the dual
30+
// build and fails SILENTLY, as a catch that never fires.
31+
//
32+
// [#14936] The recognizer and the code are taken FROM the engine rather than
33+
// re-spelled here. The engine publishes both precisely so a consumer can
34+
// perform the compare without authoring the string itself: a local literal is a
35+
// second spelling that can drift from what the engine throws, and it acquires a
36+
// `check:error-code-provenance` stamp site in this package for a code this
37+
// package only ever RECOGNISES.
38+
import {
39+
isSystemWriteOrganizationRequiredError,
40+
SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE,
41+
} from '@objectstack/objectql';
3342
import type { SharingEngine } from './sharing-service.js';
3443
import type { SharingService } from './sharing-service.js';
3544
import { normalizeAccessLevel, normalizeStoredAccessLevel } from './access-level.js';
@@ -129,15 +138,6 @@ function rowFromRule(row: any): SharingRuleRow {
129138
};
130139
}
131140

132-
/**
133-
* [#14754] The one engine refusal a reconcile pass absorbs per grant.
134-
*
135-
* Spelled once, and typed from the engine's own declaration so it cannot drift
136-
* from the code the engine actually throws.
137-
*/
138-
const ENGINE_ORGANIZATION_REFUSAL_CODE: SystemWriteOrganizationRequiredError['code'] =
139-
'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED';
140-
141141
/**
142142
* [#14754] What one reconcile pass did, plus the grants the engine's
143143
* organization rule REFUSED in it.
@@ -1649,7 +1649,7 @@ export class SharingRuleService implements ISharingRuleService {
16491649
*
16501650
* `sys_record_share` is tenant-scoped in the #13491 ledger (#14484), so on a
16511651
* walled install an organization-less system insert on it is refused loudly
1652-
* with {@link ENGINE_ORGANIZATION_REFUSAL_CODE}. `SharingService.grant`
1652+
* with {@link SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE}. `SharingService.grant`
16531653
* resolves the organization on every path that can; a platform-global rule
16541654
* (`organization_id = null`, its sweep unscoped) materialising onto an
16551655
* organization-less record resolves none, and meets the refusal.
@@ -1663,8 +1663,9 @@ export class SharingRuleService implements ISharingRuleService {
16631663
*
16641664
* ## Why the catch is narrow, and must stay narrow
16651665
*
1666-
* ONLY {@link ENGINE_ORGANIZATION_REFUSAL_CODE} is absorbed; everything else
1667-
* rethrows unchanged. Two reasons, and the second is measured:
1666+
* ONLY the engine's organization refusal is absorbed — recognised through
1667+
* the engine's own {@link isSystemWriteOrganizationRequiredError}; everything
1668+
* else rethrows unchanged. Two reasons, and the second is measured:
16681669
*
16691670
* - A catch-all would swallow real defects — a driver outage, a criteria
16701671
* bug, a permission failure — and report a pass that "completed" having
@@ -1701,15 +1702,15 @@ export class SharingRuleService implements ISharingRuleService {
17011702
);
17021703
return true;
17031704
} catch (err: any) {
1704-
if (err?.code !== ENGINE_ORGANIZATION_REFUSAL_CODE) throw err;
1705+
if (!isSystemWriteOrganizationRequiredError(err)) throw err;
17051706
this.logger?.warn?.(
17061707
'[sharing-rule] grant refused by the engine organization rule — counted, pass continues',
17071708
{
17081709
rule: rule.name ?? rule.id,
17091710
object: rule.object_name,
17101711
record: recordId,
17111712
recipient: recipientId,
1712-
code: ENGINE_ORGANIZATION_REFUSAL_CODE,
1713+
code: SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE,
17131714
},
17141715
);
17151716
return false;

‎packages/spec/src/api/error-code-ledger.zod.ts‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1653,21 +1653,6 @@ export const PROVENANCE_WAIVERS: readonly ProvenanceWaiver[] = [
16531653
'protocol layer — the registered emitter — turns it into the 403 the wire carries ' +
16541654
'(ADR-0010 §3.3). Spec ships schemas and pure helpers, never an HTTP door.',
16551655
},
1656-
{
1657-
package: '@objectstack/plugin-sharing',
1658-
code: 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED',
1659-
registeredUnder: '@objectstack/objectql',
1660-
reason: 'Matches the code, never emits it (#14754, adjudicated on #14937 — maintainer ' +
1661-
'ruling A, 2026-09-04): `ENGINE_ORGANIZATION_REFUSAL_CODE` in ' +
1662-
'`plugin-sharing/src/sharing-rule-service.ts` is a `constdef` the per-grant catch in BOTH ' +
1663-
'reconcile loops compares an incoming `err.code` against, so exactly one engine refusal is ' +
1664-
'absorbed and a refused grant no longer aborts the pass or its stale-row revocations. The ' +
1665-
'emitter is `@objectstack/objectql` (`SystemWriteOrganizationRequiredError`, ' +
1666-
'tenancy/system-write-organization.ts) and the objectql owner key already carries the row ' +
1667-
'(#8844). Recognising a code is not emitting it; the named constant is typed FROM the ' +
1668-
'engine\'s own declaration so it cannot drift from what the engine throws. Removed together ' +
1669-
'with the stamp site when #14936 lands and objectql publishes a recognizer.',
1670-
},
16711656
{
16721657
package: '@objectstack/types',
16731658
code: 'VALIDATION_FAILED',

0 commit comments

Comments
 (0)