Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-security,plugin-sharing,plugin-webhooks,platform-objects,service-messaging,spec): scope five tenant-scoped declared unique indexes per organization (#8554) - #8599
Merged
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 6 package(s): 109 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also reference the affected code. These are read-only:
|
os-zhuang
marked this pull request as ready for review
August 14, 2026 01:10
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 14, 2026
qq9340100 pushed a commit
that referenced
this pull request
Aug 14, 2026
Discharges the merge commit's placeholder: node scripts/check-i18n-bundles.mjs --write over the merged tree. The result carries both sides — main's #8599 source wording and this branch's source-tracking rewrites — as the extractor's own output, not a hand-picked side. Only the en file changed; the 30 translated-locale bundles were rewritten in place and came back byte-identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jqe56GnYFddggeAyfkZFVz
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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#8554
Five platform objects declared
unique: trueon a table-level index. On a declared index that is the positional spelling of'global'— the listed columns verbatim — so each was an installation-wide key on a tenant-scoped object. Fourth act of the class ruled on 2026-08-13, aftersys_user_preference/sys_capability(#8461) andsys_position(#8556).1. The probe came first, per object — and all five reproduced
R2 makes the live probe the required first step: #8468 established the shape for
sys_position, not for these five. Predictions were recorded before each run. Real engine (SqlDriver, better-sqlite3),OS_TENANCY_POSTURE=isolated, driving the real shipped declarations imported from source — not hand-copied fixtures — withorganization_idderived fromresolveInjectedSystemColumnsrather than typed in.sys_permission_setuniq_sys_permission_set_name(name)sys_sharing_ruleuniq_sys_sharing_rule_name(name)sys_webhookuniq_sys_webhook_name(name)sys_email_templateuniq_sys_email_template_name_locale(name, locale)sys_notification_preferenceuniq_sys_notification_preference_user_id_topic_channel(user_id, topic, channel)No object contradicted the static read, so there is no fork to report. One honest note on process:
sys_email_template's first probe run answered500on every insert — that was my harness (a filler namingbodyinstead of the requiredbody_html, and omittinglabel), not the object. Fixed and re-run before drawing any conclusion; a 500 is not a 409 and I did not count it as one.The two composite cases got a second control that varies only a trailing column (
locale;channel) and is accepted before and after. That is what proves the installation-wide key was the composite rather than its leading column, and it is pinned.2. What changed
unique: 'organization', spelled explicitly (R1 — no positional default in either direction), each with the measured probe recorded at the declaration.schema-drift.ts— see A1. Verified byte-identical toorigin/mainby object hash3a16c6f5e297d81eb63b07781664c6dbddf2a1fe, not by eye.packages/spec/src/security/permission.zod.tsdescribe()→ regeneratesreferences/security/permission.mdxpackages/spec/src/automation/webhook.zod.tsdescribe()→ regeneratesreferences/automation/webhook.mdxandreferences/integration/connector.mdx, which embeds the same webhook schema. I did not expect the third page; it came out ofgen:docsand is correct.sys_permission_set's fielddescriptionand its clone-dialoghelpText(the exact moment an admin types a new name), andsys_webhook's fielddescription.content/docs/automation/webhooks.mdx— hand-written, corrected by hand.3. Ablations: colour predicted, then measured
Both predicted before running, both committed-then-reverted (never stashed), and the reverts proven by hash.
Ablation A — revert only the five declarations to bare
true.The driver suite staying green is the point, not a miss: it carries its own copied fixtures and imports no declaration. That is exactly why each package pin asserts "matches the fixture the driver suite copies, entry for entry". And as on #8556, the test named "declares exactly one unique index" stays green under the ablation —
trueis truthy and the filter still matches. A pin's name is not its reach, which is why the scope assertion istoBe('organization')and never a truthiness check.Ablation B — revert only #8461's declared-index arm in
schema-drift.ts.Predicted: fresh-database sections stay green; the deployed-installation block goes red; the decisive assertion answers 409 where 201 is expected; and the three negative guards stay green for the wrong reason (they assert emptiness, and an ablated arm returns empty for everything). Predicted 26 red / 59 green.
Measured: 26 red / 59 green, decisive line:
With that arm ablated, a fully applied non-destructive migration still answers 409 cross-organization, on all five objects. Every fresh-database test stayed green throughout — a fresh-schema suite cannot see this defect at all.
Both ablations' predictions were correct this time. I flag that rather than claim credit: #8556's review round found its real information in the wrong predictions, and two clean sweeps mean these ablations confirmed a mechanism already understood rather than discovering a new one. The genuinely wrong prediction on this card is in §6.
4. The deployed-installation half is the load-bearing part
For each of the five, the suite builds an installation that already has the old global index plus real rows (two tenant rows and one organization-less platform row), then migrates it. A named harness guard asserts the seeded database really carries the pre-fix index, that it holds 3 rows, and that the defect is live on it — without it the whole block could be exercising a fresh schema with every assertion still passing.
It then pins, per object: exactly one
replace_unique_indexfinding categorisedsafe, zero orphandrop_indexfindings, applies without--allow-destructive, keeps every row, converges to no drift, and both halves hold on the migrated database. Also pinned: deploying the new code is not by itself the fix —initObjectsis additive, so until the retirement is applied the old index keeps enforcing and the cross-organization create still 409s. The changeset says this to operators in those terms.Anti-vacuity twins throughout: every "409 flips to 201" has a same-organization duplicate that must still be refused, plus an organization-less pair that must still be refused among themselves. A fix that removed uniqueness instead of scoping it would be a worse defect and is indistinguishable from the real fix by the 201 assertion alone.
One harness fact worth recording: seeding the platform row with the same key as the tenant row made
seedDeployedthrow a raw UNIQUE violation and took 30 tests red at once — because under the pre-fix global index those two rows genuinely collide. That is the defect, not a harness accident, and the fixture now carries a distinctplatformKeywith the reason written next to it.5. PM assumptions
legacyUniqueReplacementsemits exactly one entry per object, withlegacyColumnsequal to the whole listed key (not the leading column) for both composites.schema-drift.tsuntouched.sys_notification_preferencelands on the name-truncation path, which I flagged in the claim comment before touching anything: its natural replacement name isuniq_sys_notification_preference_organization_id_user_id_topic_channel— 70 characters, pastINDEX_NAME_MAX = 60— sobuildIndexNametruncates it touniq_sys_notification_preference_a22d7d27while the 54-character legacy name is emitted verbatim. The two therefore differ and thelegacyName === replacement.nameguard correctly does not fire. This matters because if those two names had ever collapsed to one string, the guard would read the respelling as "nothing was superseded" and emit no migration at all — the declaration would change, a fresh database would look right, and every deployed installation would keep the global index forever. Section 6 of the suite pins it.organization_idinjected on all five: CONFIRMED, derived from the built value viaresolveInjectedSystemColumns(tenancydeclundefined,plan.tenanttrue, injected setid, organization_id, created_at, created_by, updated_at, updated_by), never a regex over source. Pinned in each package, because if it ever goes false the'organization'spelling has no column to key on and the fix is silently inert.sys_permission_set(three sites, one of them a generated reference page) andsys_webhook(three sites, two generated pages) carried them.sys_sharing_rule,sys_email_templateandsys_notification_preferencecarry none. All corrected at source; a final sweep confirms no bare uniqueness claim about any of the five survives.6. What I got wrong, and the sixth and seventh instances
A4 by execution, and my prediction was wrong. I predicted the tenancy-filtered sweep would leave exactly one object (
sys_setting, #8555) after my fix. Measured onorigin/main:lintUnscopedDeclaredIndexesover 76 loadable definitions gives 41 raw findings, of which 20 — not 6 — are tenant-scoped. My five are correctly gone afterwards, so A4's before/after holds. But the card's stated method (rule verdict × tenancy filter) is not what produced its five-object table; an unstated human judgement about which keys are authored names was applied on top, and that undeclared step is where instances were dropped.Chasing that down is where the real information was:
sys_notification_subscription(the direct sibling of one of my five, same directory, same ADR-0030 layer,topic+principalauthored from the Setup grid) andsys_audience_binding_suggestion.(package_id, permission_set_name, anchor)— the same triple for every tenant that installs the same package — so the second and every later organization to install a package silently never gets its binding-suggestion row, and its admin is never asked to bind the package's default permission set. That is live onmaintoday, independent of this PR.sys_job, a secondsys_setting's unique key is installation-wide on a tenant-scoped object — but unlike the #8323 class it has a real argument for staying that way #8555-shaped judgement case (engine-owned; if the job catalogue is a property of the deployment then the correct answer is the opposite one, an explicit'global'). Filed withoutpm:queue, with the semantic argument stated, exactly as the ruling requires for a non-inheriting instance. It also writes down the three-category triage (S6 composites / opaque-id keys / the defect class) so the next sweep does not re-derive it.Neither is addressed here — R5 bounds this card's scope at exactly five objects.
A second finding, and this one already affects a merged PR. Editing a field's
descriptiondoes not update the translation bundles:node scripts/check-i18n-bundles.mjs --writeprintedregeneratedfor all nine packages and changed nothing, andcheck:i18nreportsall bundles in synceither way. That is #8543 (dispatched to another seat), measured here. Its consequence is that #8556's landed source correction leftsys_position's en bundle still saying "Unique machine name for the position" onmainright now — the corrected source and the shipped English text disagree.I did not touch the generator (#8543 owns it) and did not touch
sys_position(out of scope). For my own objects I followed the bundle header's own documented workflow — "Edit translations in place; re-run extract (with --merge) to fill new gaps. Do not hand-edit the structure — only the leaf string values" — and corrected the leaf values in all four shipped locales. Without that, this PR would have claimed to correct the published text while the text an admin actually reads in Setup still asserted the old contract.Verification
@objectstack/spec10520 / 397 files ·plugin-security1084 / 56 ·plugin-sharing574 / 22 ·plugin-webhooks86 / 7 ·platform-objects362 / 19 ·service-messaging224 / 21 ·driver-sql1565 passed + 52 skipped / 97. Typecheck green on all seven.check:nul-bytes,check:changeset-gate-self-tests,check:cross-package-test-inputs,check:docs-audit-scope,check:merge-driver,check:objectui-changeset,check:quick-reference-counts,check:role-word,check:spec-parsed-alias,check:test-source-alias,check:type-source-resolution,check:query-options-erasure,check:i18n,check:doc-formula-expressions,check:dev-prereqs,check:adr-0087-registration,check:changeset-no-major,check:empty-changeset,check:authorable-surface,check:docs,check:generated,check:api-surface,check:export-origins.check:query-options-erasurewent RED and was repaired at the source, not by raising the ceiling. The new driver suite grew the test surface 240 → 245 sites via{} as anyon query options. Removing the casts (they were never needed —find/countaccept{}) returned it to 240, at the ceiling. ⛔ No ledger was grown anywhere.check:type-check-debt— the--re-measurehalf first refused to run, naming@objectstack/platform-objectsand@objectstack/service-realtimeas having type entry points older than their sources ([finding][devx]check:type-check-debt的 ledger 余量会让新写的 pin 变哑：mongodb 曾有 33 条余量吞掉一次真实回退，另有 5 条目前带 4–19 余量 #6376). It refused rather than measuring through a stale.d.ts, which is the gate working. Built both directly and re-ran: 33 ledger entries re-measured, 1969 raw errors, none above its recorded number, surplus none.scripts/pm/dispatch-gates.mjs, which surfacedcheck:doc-formula-expressions,check:dev-prereqs,check:merge-driver,check:spec-parsed-alias,check:adr-0087-registration,check:changeset-no-major,check:empty-changesetandcheck:query-options-erasure— none of which the dispatch brief named, and the last of which was the one that actually failed.origin/mainwas merged and the entire union plus every suite re-run after the final commit (A green gate union proves nothing about the head it is reported against — nothing ties the run to a commit, so a mid-round union goes stale silently #8550).Breaking: no — a pure relaxation. Shipped as⚠️ But it is not a no-op for operators: the changeset states that
patch, same call #8556 made for the same shape.os migrate applyis required and why deploying alone is insufficient.Generated by Claude Code