Repository navigation
Commit 2551404
test(plugin-security): the delegated-organization-membership parity test boots once per posture and asks only the probes it asserts (#26872)
Fixes #26754
Clause-②: no
Metadata-protocol: none
## 维护者速读
- **改了什么**:只改一个测试文件。`delegated-organization-membership` 的 parity 测试,由「每个
posture 一个大 `it`」改为「每个 posture 只启动一次栈,三个小 `it`(读、导出、插入)共用」,每个 `it`
只调用它真正断言的探针。产品代码零改动,不发版。
- **为什么改**:这个测试在 CI 分片负载下逼近并越过 vitest 默认的 5 秒超时,已经把 merge queue
的构建踢出队列;每踢一次,排在后面的所有 PR 都要重建。
- **风险与回滚**:风险低。原有 5 条断言逐字保留(非空断言在导出 `it` 里多放一份);对三个探针各做一次变异,都只让对应的那个
`it` 变红。回滚就是 revert 本 PR。
-
**先看**:`packages/plugins/plugin-security/src/delegated-organization-membership.test.ts`
里 parity `describe` 的 `beforeAll`/`afterAll` 共享栈与三个 `it`;其次是 `boot()`
新增的 `lifetime` 选项(不加它,共享的引擎会在第一个 `it` 后被文件级 `afterEach` 销毁)。
- **两分钟验证**:`pnpm --filter @objectstack/plugin-security exec vitest run
src/delegated-organization-membership.test.ts -t 'parity: each probe
agrees'`,预期 `Tests 6 passed | 64 skipped (70)`,空闲机器上每个 parity `it` 约
0.1–0.3 秒。
- **需要你做的**:无,席位落地。
## What changes
One file:
`packages/plugins/plugin-security/src/delegated-organization-membership.test.ts`.
No product source, no timeout raised, nothing skipped or quarantined.
- **The `[#25107 · ADR-0090 D10] parity` describe.** Per posture
(`group`, `isolated`), one stack is booted and seeded in `beforeAll` and
destroyed in `afterAll`. Three `it`s share it, so a failure names the
operation:
- a read: find ⇔ `canReadObject`, find ⇔ the read explanation;
- an export: find ⇔ `canExport`;
- an insert: insert ⇔ `canWriteObject` insert. It runs last, being the
only one that writes.
Each `it` asks only the probes it asserts. The old single `it` called
`probeAnswers` (nine probes per cell), asserted four of them, and threw
the other five away: the four field projections and the update probe.
Those five stay pinned, per context and per object, in the two probe
describes above it.
- **`boot()` gains `lifetime: 'describe'`.** It hands the engine to the
caller's `afterAll` instead of the file's per-test `afterEach`, which
would otherwise destroy a shared engine after the first `it`. Every
other caller is unchanged (default `'test'`).
### Where the time went (measured before choosing the cut)
This is the old `it`, instrumented in a throwaway copy, alone and warm,
at `--maxWorkers=1`, two rounds per posture, on base `0ea90c7a18`:
| share of the old per-posture `it` (411–510 ms) | measured |
|:--|:--|
| boot and seed (engine init and schema sync, seed rows, plugin init and
start, `seedForReads`) | **10.4–11.6%** |
| the engine operations (9 finds, 9 inserts) | 15.7–17.6% |
| the probes (81 calls) | **71.4–73.9%** |
| … of which the five probes never asserted | **29.8–31.4%** |
| … of which the read explanation (asserted) | 21.5–23.1% |
So the boot was not the cost, which falsifies the dispatch's mechanism
assumption. The parity `it` already booted only once per posture, so
sharing a stack alone would save nothing there. The cut that pays is to
stop asking for answers the test never reads. Sharing the stack is what
lets the three operations become separate `it`s without paying three
boots.
### Why one stack per posture is sound
I measured it directly. On one stack per posture, every read-side
outcome (find ok, `canReadObject`, `canExport`, explain `allowed`,
`canWriteObject` insert) was read for all nine cells, then all nine
parity inserts were run, then every outcome was read again. Result:
`outcomes-equal=true` on both postures. The rows a find returns did
change (`rows-equal=false`, for example `parity_4` appears for
`delegated in org_a · qa_account`), but no parity assertion reads rows,
only whether an operation is refused. The old `it` already ran later
cells' finds after earlier cells' inserts. The insert `it` now runs
last, so the read and export `it`s see no parity row at all.
### Assertions, before and after
| | before | after |
|:--|:--|:--|
| `expect(` lines in the parity describe (textual) | 5 | 6 |
| assertions executed per run (two postures) | 74 (= 2 × (9 cells × 4 +
1)) | 76 (= 2 × (19 + 10 + 9)) |
| `it`s in the file | 66 | 70 |
All five original assertion lines are present byte-for-byte (compared
modulo indentation, by script; none missing, none reworded). The one
added line is the original non-vacuity pin
(`expect(refused).toEqual(...)`), repeated in the export `it`, because
that `it` runs its own find and would otherwise pass vacuously when run
alone.
## Tests
### Timings: the slowest parity `it`, before and after
**The leg.** The card's CI leg was a Test Core shard: turbo
`--concurrency=4` × `VITEST_MAX_WORKERS=3` on 4 vCPU, so up to 12
runnable test threads. I approximated it on this 4-vCPU container as the
file at `--maxWorkers=3` (`VITEST_MAX_WORKERS=3`) beside **11 pure-CPU
`node` busy loops**: 12 runnable threads. Other agents' unlocked work on
the box (builds, a dogfood run) came on top, uncontrolled; load average
per run is in the logs below. This is a CPU-share approximation, not a
real shard.
**Paired: the fair comparison.** The original file (a byte-identical
untracked copy of base `0ea90c7a18`'s version, blob `356eb401`) and the
new file ran in the **same** vitest invocation, under the same burners,
on head `e92c25a1b4`. The two files' unchanged control `it`s ("what the
rule leaves alone in the probes") agree within each run, so both saw
comparable load.
| run | original: slowest parity `it` | new: slowest parity `it` |
speed-up | control `it`, original / new |
|:--|--:|--:|--:|:--|
| 1 | 4,159 ms | 1,857 ms | 2.2× | 2,461 / 3,054 ms |
| 2 | 4,151 ms | 1,523 ms | 2.7× | 2,413 / 2,449 ms |
| 3 | 2,957 ms | 1,045 ms | 2.8× | 1,401 / 1,511 ms |
**Sequential, same leg.** Before on base `0ea90c7a18`, after on head
`e92c25a1b4`:
| run | before `group` | before `isolated` | after: slowest of 6 parity
`it`s |
|:--|--:|--:|--:|
| 1 | **5,403 ms, timed out** | 4,830 ms | 1,486 ms |
| 2 | 3,843 ms | **5,249 ms, timed out** | 1,690 ms |
| 3 | 4,509 ms | **6,126 ms, timed out** | 1,885 ms |
Before, 3 of 3 sequential runs were red at vitest's 5,000 ms default.
Each failing record's stack anchors at the parity `it`'s definition (old
line 648), which is how vitest 4's JSON reporter records a timeout.
After, 6 of 6 loaded runs (3 sequential, 3 paired) are green, and the
slowest parity `it` in any of them is **1,885 ms**. Caveat: the
sequential after-runs met a lighter box than the before-runs (their
control `it`s ran about 1.5× faster), which is why the paired table
above is the comparison to quote.
**Alone** (`--maxWorkers=3`, no burners): before `group` 897 ms and
`isolated` 823 ms; after, `group` read / export / insert 315 / 153 / 167
ms and `isolated` 286 / 116 / 115 ms.
### The reshaped `it`s still bite: one-shot mutation check (nothing
committed)
The diff changes no product behaviour, so no ablation was owed. To show
that none of the three new `it`s is vacuous, I mutated each probe on
head `e92c25a1b4` in turn through `scripts/ablation-replace.mjs` (wrap
mode: anchor must hit once, write verified on disk, restore verified).
The mutation made the probe admit exactly the refused context: `if
(context?.onBehalfOf && context?.tenantId === 'org_d') return
Promise.resolve(true);` at the top of `SecurityPlugin.canReadObject`,
`canExport` and `canWriteObject`, in `security-plugin.ts`. The engine's
find and insert do not route through these three entry points.
| mutated probe | marker on disk | red `it`s (both postures) | green
`it`s | first failure |
|:--|:--|:--|:--|:--|
| `canReadObject` | 1 | read | export, insert | `delegated in org_d ·
qa_account: canReadObject ⇔ find: expected true to be false` |
| `canExport` | 1 | export | read, insert | `delegated in org_d ·
qa_account: canExport ⇔ find (an export reads as a find): expected true
to be false` |
| `canWriteObject` | 1 | insert | read, export | `delegated in org_d ·
qa_account: canWriteObject insert ⇔ insert: expected true to be false` |
After each leg: `ok restored: blob == HEAD (fe67e27) and git diff
HEAD is empty`, and the marker count went back to 0. A first attempt was
a **no-op**: its replacement re-contained the anchor, the tool refused
before measuring ("the anchor count moved 1 -> 1"), and it was
re-anchored and re-run as above.
### Package suite and typecheck (head `e92c25a1b4`)
- `vitest run --maxWorkers=3` over the whole package: `Test Files 231
passed (231)`, `Tests 5036 passed | 45 skipped (5081)`.
- `pnpm --filter @objectstack/plugin-security typecheck`: exit 0, ending
`check:test-typecheck: OK — @objectstack/plugin-security's test layer
compiles under packages/plugins/plugin-security/tsconfig.test.json`.
That program lists this file (`tsc -p tsconfig.test.json --listFiles`: 1
hit, 231 test files).
- `pnpm exec eslint
packages/plugins/plugin-security/src/delegated-organization-membership.test.ts`:
exit 0, no output.
- Not run locally, left to CI: the root `test:repo` project.
## Acceptance notes
- **Dispatch mechanism assumptions, measured.** (1) "The cost is the
per-posture boot plus seeding" is **falsified**: about 10% of the `it`;
the probes are about 72%, and a third of the `it` was probes it never
asserted. (2) "Sharing is safe only if the insert does not change what
the read and the export see": the insert changes the rows a later find
returns, but no asserted outcome. The isolation here is by order (the
one writing `it` runs last), not by distinct rows: the parity cells must
cover the same three objects.
- **Observation, not filed.** With the parity `it`s fixed, the slowest
`it`s left in this file are the three "what the rule leaves alone in the
probes" admitted-probe `it`s, each running `probeAnswers` over three
objects. On the same leg they reached 1.9–3.9 s before and 1.5–3.1 s
after, under 5 s in every run. They are outside this card, and nothing
observed here failed.
- **No changeset.** `files[]` is `dist`, `README.md` and `CHANGELOG.md`.
After `pnpm --filter @objectstack/plugin-security build`, this diff's
symbols (`parityContexts`, `lifetime: 'describe'`, the new `it` titles)
have 0 hits in those paths, while the positive controls (`canExport(`,
`delegatedOrganizationRefusal`) hit 4 files each. Nothing publishes. Per
the dispatch, the `skip-changeset` label is the seat's to apply; I did
not apply it.
- **Base sync.** `origin/main` (then `8eef9a8db4`) was merged in at
`e92c25a1b4` before every reading above. Upstream moved code on the
measured path: the objectql validate split, a driver-sql move, and
#26801's explain-another-user existence check. The last runs only when
`request.userId` names another user, and the parity read passes none.
The 9 commits that landed since touch nothing under
`packages/plugins/plugin-security`, and neither in-flight
plugin-security PR the claim named had landed (`git log
0ea90c7..origin/main`, with a control commit found in the same
range).
## Verification
The gate union ran on head **`e92c25a1b4`** (`git rev-parse --short
HEAD`), the head of this PR. It used the list re-derived on that head
with no paths, `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack`: 52 commands, identical to the pre-merge
derivation. Each exit code was captured before any pipe.
- 50 exited 0, each printing its own pass line.
- 2 exited 3 with `PREREQUISITE NOT MET`: **NOT MEASURED** locally, not
a pass and not a fail. `check:dual-build-cjs-loads` needs every
package's `dist/` (56 absent in this worktree). `check:i18n` needs the
CLI's compiled `i18n extract` and its config closure. Both run in CI's
`Lint & Repo Gates`, and neither reads a test file.
- Reconciliation, `dispatch-gates --ran` over the recorded `COMMAND ::
exit CODE` lines: `52 derived famil(ies) accounted for — 50 run, 2
NOT-MEASURED (2 DERIVED from a recorded exit 3)`.
<details>
<summary>Gate verdict lines (head e92c25a)</summary>
```text
exit 0 node scripts/check-closing-keyword-parity.mjs
exit 0 node scripts/check-closing-keyword-parity.mjs --self-test :: 40 assertions, 5 mutations of the shipped parsers each driven to red
exit 0 node scripts/check-comment-mask-adoption.mjs :: OK, 14 private comment-stripper(s), all 14 recorded
exit 0 node scripts/check-comment-mask-adoption.mjs --self-test :: PASS (0 failure(s))
exit 0 node scripts/check-comment-mask-corpus.mjs :: 18497 files, 0 disagree, 0 unparseable
exit 0 node scripts/check-dts-emitted.mjs --self-test :: all assertions passed
exit 0 node scripts/check-issue-citations.mjs
exit 0 node scripts/check-plugin-teardown-shape.mjs :: 88 Plugin implementation(s)
exit 0 node scripts/check-plugin-teardown-shape.mjs --self-test :: 48 cases pass
exit 0 node scripts/check-tenant-audit-census.mjs :: OK, 244 write call sites certified
exit 0 node scripts/check-tenant-audit-census.mjs --self-test :: 27 cases pass
exit 0 node scripts/ci/assert-turbo-remote-cache.mjs --self-test :: OK (37 case(s) across 7 batteries)
exit 0 node scripts/ci/plan-test-core-shards.mjs --self-test :: 23 case(s) across 5 batteries
exit 0 node scripts/ci/prune-turbo-cache.mjs --self-test :: OK (25 case(s) across 5 batteries)
exit 0 node scripts/docs-audit/check-affected-docs.mjs
exit 0 node scripts/docs-audit/check-drift-comment.mjs :: 66 cases pass across 5 fixture diff(s)
exit 0 pnpm --filter @objectstack/spec run check:duration-unit-keys :: 209 unit-declaring numeric key(s)
exit 0 pnpm check:ci-filter-parity :: OK, all 23 build input(s)
exit 0 pnpm check:closed-ceremony-text :: 4637 shipped file(s) read
exit 0 pnpm check:cross-package-test-inputs :: OK, 31 package(s) read outside themselves, all declared
exit 0 pnpm check:doc-authoring :: sibling-package prose ids hold the baseline
exit 0 pnpm check:driver-memory-census :: OK
exit 0 pnpm check:dts-closure :: 113/113 declared declaration file(s) present across 18 package(s)
exit 3 pnpm check:dual-build-cjs-loads :: PREREQUISITE NOT MET, nothing was measured
exit 0 pnpm check:engine-double-contract
exit 0 pnpm check:gitlink-declared :: OK (21664 index entries, 0 gitlink(s))
exit 3 pnpm check:i18n :: PREREQUISITE NOT MET, nothing was checked
exit 0 pnpm check:i18n-stale-fill :: OK (11 bundle set(s), no new stale fills)
exit 0 pnpm check:issue-citations
exit 0 pnpm check:keyed-text-bounds
exit 0 pnpm check:lean-entry-closure :: admitted set held exactly (15 packages)
exit 0 pnpm check:logger-receiver-detach
exit 0 pnpm check:nul-bytes :: OK (scanned 21562 text file(s), no raw ASCII control bytes)
exit 0 pnpm check:objectql-double-limit :: no files added
exit 0 pnpm check:org-identifier :: OK
exit 0 pnpm check:page-declaration-shape
exit 0 pnpm check:platform-object-tenancy-census :: matches the tree
exit 0 pnpm check:published-files :: 73 publishable package(s) covered
exit 0 pnpm check:query-options-erasure :: no files added
exit 0 pnpm check:refd-timer-probe
exit 0 pnpm check:registry-log-declared
exit 0 pnpm check:rest-log-spy-declared :: OK
exit 0 pnpm check:slot-lookup :: no files added
exit 0 pnpm check:sourcemap-no-sources-content :: 307 map(s), none embed source text
exit 0 pnpm check:system-context-census :: OK
exit 0 pnpm check:test-source-alias :: OK, 119 packages with tests scanned
exit 0 pnpm check:tier-file-adoption
exit 0 pnpm check:type-check-coverage
exit 0 pnpm check:type-check-debt :: surplus none, every entry sits exactly at its measurement
exit 0 pnpm check:undeclared-dep-imports
exit 0 pnpm check:watch-hint-literal :: 74 declaration(s) across 4 rostered name(s)
exit 0 pnpm check:where-matcher :: no files added
```
</details>
<details>
<summary>Run logs: the load leg and the box per run</summary>
```text
before (base 0ea90c7), 11 burners, --maxWorkers=3:
run 1 vitest-exit=1 wall 133s loadavg 6.27 -> 14.56 group 5403 (timed out) isolated 4830
run 2 vitest-exit=1 wall 125s loadavg 14.56 -> 16.70 group 3843 isolated 5249 (timed out)
run 3 vitest-exit=1 wall 121s loadavg 16.70 -> 15.19 group 4509 isolated 6126 (timed out)
after (head e92c25a), 11 burners, --maxWorkers=3: read / export / insert, group then isolated
run 1 vitest-exit=0 wall 94s loadavg 4.28 -> 11.95 1339 / 651 / 497 1486 / 311 / 528
run 2 vitest-exit=0 wall 89s loadavg 11.95 -> 13.57 1216 / 680 / 776 1690 / 620 / 552
run 3 vitest-exit=0 wall 87s loadavg 13.57 -> 14.00 1600 / 502 / 493 1885 / 682 / 812
paired (head e92c25a, original copy + new file in one invocation), 11 burners:
run 1 vitest-exit=0 wall 82s original 4159 / 2876 new 1178 / 887 / 763 1857 / 668 / 518
run 2 vitest-exit=0 wall 88s original 3975 / 4151 new 1523 / 842 / 629 1469 / 112 / 186
run 3 vitest-exit=0 wall 70s original 2957 / 2657 new 1045 / 375 / 408 941 / 378 / 376
```
Every run went through `scripts/pm/os-verify-lock.sh`, which excludes
other locked runs but not unlocked sibling work, so these are shared-box
figures. The burners were started and killed by PID inside the run
script; nothing was killed by name.
</details>
---
_Generated by [Claude
Code](https://claude.ai/code/session_01QSeKYmQXM3rHJcPnrEPk4B)_
Co-authored-by: Claude <noreply@anthropic.com>1 parent 35ee2ad commit 2551404
1 file changed
Lines changed: 77 additions & 29 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
65 | 65 | | |
66 | 66 | | |
67 | 67 | | |
68 | | - | |
| 68 | + | |
69 | 69 | | |
70 | 70 | | |
71 | 71 | | |
| |||
178 | 178 | | |
179 | 179 | | |
180 | 180 | | |
181 | | - | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
182 | 186 | | |
183 | 187 | | |
184 | 188 | | |
| |||
202 | 206 | | |
203 | 207 | | |
204 | 208 | | |
205 | | - | |
| 209 | + | |
206 | 210 | | |
207 | 211 | | |
208 | 212 | | |
| |||
644 | 648 | | |
645 | 649 | | |
646 | 650 | | |
| 651 | + | |
| 652 | + | |
| 653 | + | |
| 654 | + | |
| 655 | + | |
| 656 | + | |
| 657 | + | |
| 658 | + | |
| 659 | + | |
| 660 | + | |
| 661 | + | |
| 662 | + | |
| 663 | + | |
| 664 | + | |
| 665 | + | |
647 | 666 | | |
648 | | - | |
649 | | - | |
650 | | - | |
651 | | - | |
652 | | - | |
653 | | - | |
654 | | - | |
655 | | - | |
656 | | - | |
657 | | - | |
658 | | - | |
659 | | - | |
660 | | - | |
661 | | - | |
662 | | - | |
663 | | - | |
664 | | - | |
665 | | - | |
666 | | - | |
667 | | - | |
668 | | - | |
669 | | - | |
| 667 | + | |
| 668 | + | |
| 669 | + | |
| 670 | + | |
| 671 | + | |
| 672 | + | |
| 673 | + | |
| 674 | + | |
| 675 | + | |
| 676 | + | |
| 677 | + | |
| 678 | + | |
| 679 | + | |
| 680 | + | |
| 681 | + | |
| 682 | + | |
| 683 | + | |
| 684 | + | |
| 685 | + | |
| 686 | + | |
| 687 | + | |
| 688 | + | |
670 | 689 | | |
671 | | - | |
672 | | - | |
673 | | - | |
674 | | - | |
| 690 | + | |
| 691 | + | |
| 692 | + | |
| 693 | + | |
| 694 | + | |
| 695 | + | |
| 696 | + | |
| 697 | + | |
| 698 | + | |
| 699 | + | |
| 700 | + | |
| 701 | + | |
| 702 | + | |
| 703 | + | |
| 704 | + | |
| 705 | + | |
| 706 | + | |
| 707 | + | |
| 708 | + | |
| 709 | + | |
| 710 | + | |
| 711 | + | |
| 712 | + | |
| 713 | + | |
| 714 | + | |
| 715 | + | |
| 716 | + | |
| 717 | + | |
| 718 | + | |
| 719 | + | |
| 720 | + | |
| 721 | + | |
| 722 | + | |
675 | 723 | | |
676 | 724 | | |
677 | 725 | | |
| |||
0 commit comments