fix(sharing,driver-sql): a rule authored in Setup actually applies — and switching it off actually withdraws access (#3850)

* fix(sharing,driver-sql): a rule authored in Setup actually applies — and switching it off actually withdraws access (#3821)

**Enabling a rule did nothing to the records already there.** Writing a
`sys_sharing_rule` rebound the per-record hooks, which only makes the rule reach
records written FROM THEN ON. An admin who created a rule and switched it on saw
the recipient's list stay empty until somebody happened to touch each record.

**Switching a rule off did not withdraw access.** Deactivating — or deleting —
left every grant it had already issued in place, and boot backfill only
reconciles ACTIVE rules, so those grants outlived restarts while the UI showed
the rule as disabled. The reconcile existed but was reachable only through
`POST /sharing/rules/:id/evaluate`, which the Console never calls and exposes no
button for.

Each non-system write to `sys_sharing_rule` now also reconciles that rule's
grants, chained behind the existing rebind: insert/update run the same
diff-based `evaluateRule` the REST endpoint runs (it purges when the rule is
inactive), and delete purges directly via the new
`SharingRuleService.revokeRuleGrants` — `evaluateRule` can't help there because
the row is gone (`RULE_NOT_FOUND`), which is also why a rule deleted through the
plain data API used to orphan its grants. Seeding and package bootstrap write
with `isSystem` and are skipped; `kernel:bootstrapped` already backfills those.
Reconciliation is best-effort and never fails the authoring write.

**An unsortable query lost its rows, not just its order.** `SqlDriver.find()`
already recovered from a SELECT projection naming a missing column; the same
failure one clause over — an unknown ORDER BY column — fell through to
`return []`. Because `count()` is a separate statement, the endpoint answered
200 with `records: []` and `total: 3`: the rows are there, none are shown,
nothing is logged. Same family as the `$`-param footgun closed by #2926. It
surfaced through the sharing-rule recipient picker, whose client mangled
`'name asc'` into a list of one-character column names (fixed in objectui).
Rows now outrank their order: the retry ladder drops the projection first, then
the sort, then gives up — an unsortable query returns rows UNORDERED instead of
empty. Non-column errors still propagate.

Also refreshes the `sys_sharing_rule` help text in the zh-CN / ja-JP / es-ES
bundles, which still described `recipient_type` in terms of `department` (the
enum value is `business_unit`) and told admins to enter a queue name for
`recipient_id` (`queue` was removed in ADR-0078). The es-ES option labels for
`position` / `unit_and_subordinates` were translated as "rol".

Browser-verified against the showcase Console: enabling a rule immediately gave
the recipient every matching record, and switching it off took them all away.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(sharing): write the sharing-rule help text for the admin who reads it (#3821)

The field `description`s on `sys_sharing_rule` render as help text under each
input in Setup. They were written as notes for the next engineer:

  接收访问权限的主体类型——求值时会展开为用户授权。`business_unit` 会沿
  parent_business_unit_id 树展开；`team` 为扁平结构（better-auth）；`position`
  展开为该岗位的任职人（岗位为扁平结构，ADR-0090 D3）…（ADR-0057 D5）。

ADR numbers, table and column names, a third-party library, and enum machine
values the dropdown never displays — it shows 用户 / 团队 / 业务单元 / 岗位.

Several were also stale, some of them because of this very issue's UI work:
`recipient_id` still said "fill in a business-unit id / team id / position name"
after it became a record picker; `object_name` still said "short object name
(e.g. opportunity, account)" after it became an object picker; `criteria_json`
still described hand-written `FilterCondition` JSON after it became a visual
builder — and carried "(FilterCondition JSON)" in its LABEL.

All of it rewritten for the reader who actually sees it, in en / zh-CN / ja-JP /
es-ES. The engine detail was already in the object's doc comment, which is where
it stays; a comment on the first field now says so, so the next author doesn't
put it back. `active` can finally state what it does — turning it off withdraws
the access — which only became true with the reconcile fix in this branch. The
list page's subtitle and the `managed_by` / `customized` help got the same
treatment.

Browser-verified in the showcase Console: no ADR, table, column, enum or library
name survives anywhere in the create dialog, the detail page or the list header.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

Author: baozhoutao

.changeset/sharing-rule-unknown-sort-and-stale-help.md

@@ -0,0 +1,64 @@
+---
+"@objectstack/driver-sql": patch
+"@objectstack/plugin-sharing": patch
+---
+
+fix(driver-sql,sharing): an unsortable query loses its ORDER BY, not its rows (#3821)
+
+`SqlDriver.find()` already recovered from a SELECT projection naming a column
+the table lacks (retry with `select('*')`, the unknown field is simply absent
+from each row). The identical failure one clause over — an **ORDER BY** column
+the table lacks — fell through to `return []`. Because `count()` is a separate
+statement, the list endpoint answered `HTTP 200` with `records: []` and
+`total: 3`: the rows are there, none are shown, nothing is logged. Same family
+as the `$`-param footgun closed by #2926.
+
+It surfaced through the Console's sharing-rule **recipient picker**, which
+never listed a single candidate. The client mangled `'name asc'` into
+`0 n,1 a,2 m,…` (fixed separately in objectui) and the driver turned that into
+"no users exist", so no sharing rule could be authored from the UI at all.
+
+Rows now outrank their order: the retry ladder drops the projection first (the
+likelier culprit and the cheaper thing to lose), then the sort, then gives up.
+A query that cannot be sorted comes back **unordered instead of empty**. Errors
+that are not about an unknown column still propagate untouched.
+
+**A rule authored in Setup now actually applies — and switching it off actually
+withdraws access.** Writing a `sys_sharing_rule` rebound the per-record hooks,
+which only makes the rule reach records written FROM THEN ON. So an admin who
+created a rule and enabled it saw nothing happen: the recipient's list stayed
+empty until somebody happened to touch each record. The reverse was worse —
+switching a rule OFF, or deleting it, left every grant it had already issued in
+place, and boot backfill only reconciles ACTIVE rules, so those grants outlived
+restarts while the UI displayed the rule as disabled. The reconcile was reachable
+only through `POST /sharing/rules/:id/evaluate`, which the Console never calls.
+
+Each non-system write to `sys_sharing_rule` now also reconciles that rule's
+grants, chained behind the existing rebind: insert/update run the same
+diff-based `evaluateRule` the REST endpoint runs (it purges when the rule is
+inactive), and delete purges directly via the new
+`SharingRuleService.revokeRuleGrants` — `evaluateRule` can't help there because
+the row is already gone (`RULE_NOT_FOUND`), which is also why a rule deleted
+through the plain data API used to orphan its grants. Seeding and package
+bootstrap write with `isSystem` and are skipped; `kernel:bootstrapped` already
+backfills those. Reconciliation is best-effort and never fails the write.
+
+**The dialog's help text was engineering notes, shown to tenant admins.** The
+field descriptions on `sys_sharing_rule` render under each input in Setup, and
+they cited ADR numbers, table and column names (`parent_business_unit_id`,
+`sys_business_unit`), enum machine values the dropdown never shows
+(`business_unit`, `team`), a third-party library (better-auth), and engine
+vocabulary ("evaluation", "lifecycle"). Several were also stale: they still told
+admins to type an id or hand-write a `FilterCondition` after those inputs became
+a record picker and a visual builder. Rewritten for the reader who actually sees
+them — the implementation detail was already in the object's doc comment, which
+is where it stays. `criteria_json`'s LABEL loses its "(FilterCondition JSON)"
+suffix for the same reason, and `active` can finally say what it now does:
+turning it off withdraws the access.
+
+Also refreshes the `sys_sharing_rule` help text in the zh-CN / ja-JP / es-ES
+translation bundles, which still described `recipient_type` in terms of
+`department` (the enum value is `business_unit`) and told admins to enter a
+queue name for `recipient_id` (`queue` was removed in ADR-0078). The es-ES
+option labels for `position` / `unit_and_subordinates` were translated as
+"rol" — corrected to "Puesto" / "Unidad de negocio y subordinados".

packages/plugins/driver-sql/src/sql-driver-unknown-column-recovery.test.ts

@@ -0,0 +1,98 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * objectstack#3821 — `find()` must not turn an unknown column into "no rows".
+ *
+ * A SELECT projection naming a column the table lacks already had a retry
+ * (select `*` and let the missing field simply be absent). An unknown ORDER BY
+ * column did not: it fell through to `return []`, so the page came back empty
+ * while `count()` — a separate statement — still reported the true total. The
+ * UI reads that as "the records vanished". It surfaced through the sharing-rule
+ * recipient picker, whose client mangled `'name asc'` into a list of
+ * one-character column names.
+ *
+ * Rows matter more than their order, so an unsortable query returns the rows
+ * unordered. These tests pin that, and that a genuine SQL error still throws.
+ */
+
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import { SqlDriver } from '../src/index.js';
+
+describe('SqlDriver find() recovers from unknown columns (objectstack#3821)', () => {
+  let driver: SqlDriver;
+
+  beforeEach(async () => {
+    driver = new SqlDriver({
+      client: 'better-sqlite3',
+      connection: { filename: ':memory:' },
+      useNullAsDefault: true,
+    });
+
+    await driver.initObjects([
+      {
+        name: 'member',
+        fields: {
+          name: { type: 'string' },
+          rank: { type: 'integer' },
+        },
+      },
+    ]);
+
+    await driver.create('member', { id: 'm1', name: 'Carol', rank: 3 }, { bypassTenantAudit: true });
+    await driver.create('member', { id: 'm2', name: 'Alice', rank: 1 }, { bypassTenantAudit: true });
+    await driver.create('member', { id: 'm3', name: 'Bob', rank: 2 }, { bypassTenantAudit: true });
+  });
+
+  afterEach(async () => {
+    await driver.disconnect();
+  });
+
+  it('returns the rows unordered when ORDER BY names a column the table lacks', async () => {
+    const rows = await driver.find('member', {
+      orderBy: [{ field: 'nosuchfield', order: 'asc' }],
+    } as any);
+
+    expect(rows.map((r: any) => r.id).sort()).toEqual(['m1', 'm2', 'm3']);
+  });
+
+  it('still sorts when every ORDER BY column exists', async () => {
+    const rows = await driver.find('member', {
+      orderBy: [{ field: 'rank', order: 'asc' }],
+    } as any);
+
+    expect(rows.map((r: any) => r.name)).toEqual(['Alice', 'Bob', 'Carol']);
+  });
+
+  it('drops only the unknown clause when a known and an unknown sort are mixed', async () => {
+    // knex emits both columns in one ORDER BY, so the whole clause has to go;
+    // what must NOT happen is losing the rows.
+    const rows = await driver.find('member', {
+      orderBy: [{ field: 'rank', order: 'asc' }, { field: 'nosuchfield', order: 'desc' }],
+    } as any);
+
+    expect(rows).toHaveLength(3);
+  });
+
+  it('recovers from an unknown SELECT projection and an unknown sort together', async () => {
+    const rows = await driver.find('member', {
+      fields: ['name', 'nosuchfield'],
+      orderBy: [{ field: 'alsomissing', order: 'asc' }],
+    } as any);
+
+    expect(rows).toHaveLength(3);
+    expect(rows.map((r: any) => r.name).sort()).toEqual(['Alice', 'Bob', 'Carol']);
+  });
+
+  it('keeps honouring the WHERE clause while recovering', async () => {
+    const rows = await driver.find('member', {
+      where: { rank: { $gte: 2 } },
+      orderBy: [{ field: 'nosuchfield', order: 'asc' }],
+    } as any);
+
+    expect(rows.map((r: any) => r.id).sort()).toEqual(['m1', 'm3']);
+  });
+
+  it('propagates errors that are not about an unknown column', async () => {
+    await expect(driver.find('no_such_table', {} as any)).rejects.toThrow();
+  });
+});

packages/plugins/driver-sql/src/sql-driver.ts

@@ -811,7 +811,8 @@ export class SqlDriver implements IDataDriver {
   async find(object: string, query: QueryAST, options?: DriverOptions): Promise<any[]> {
     // Build everything EXCEPT the SELECT list, so the unknown-column retry
     // below can rebuild without re-deriving where/order/pagination.
-    const buildBase = () => {
+    const buildBase = (opts?: { withOrderBy?: boolean }) => {
+      const withOrderBy = opts?.withOrderBy !== false;
       const b = this.getBuilder(object, options);
       this.applyTenantScope(b, object, options);
 
@@ -821,7 +822,7 @@ export class SqlDriver implements IDataDriver {
       }
 
       // ORDER BY
-      if (query.orderBy && Array.isArray(query.orderBy)) {
+      if (withOrderBy && query.orderBy && Array.isArray(query.orderBy)) {
         for (const item of query.orderBy) {
           if (item.field) {
             b.orderBy(this.remoteColumn(object, item.field, this.mapSortField(item.field)), item.order || 'asc');
@@ -866,15 +867,31 @@ export class SqlDriver implements IDataDriver {
         // only fires when the object's schema is populated in the registry —
         // this driver backstop holds even when it isn't (notably the cloud
         // multi-tenant runtime, where the projection otherwise zeroes the list).
-        if (query.fields) {
+        //
+        // An unknown ORDER BY column is the same footgun one clause over
+        // (objectstack#3821): a client that sorts by a field the table lacks
+        // used to get an empty page with a non-zero `total` — "the rows are
+        // there but none are shown". Rows matter more than their order, so
+        // drop the sort and return them unordered rather than nothing. Ladder:
+        // projection first (it is the likelier culprit and the cheaper thing
+        // to lose), then the sort, then give up.
+        const retries: Array<() => any> = [];
+        if (query.fields) retries.push(() => buildBase().select('*'));
+        if (query.orderBy && Array.isArray(query.orderBy) && query.orderBy.length > 0) {
+          retries.push(() => buildBase({ withOrderBy: false }).select('*'));
+        }
+        results = [];
+        let recovered = false;
+        for (const retry of retries) {
           try {
-            results = await buildBase().select('*');
+            results = await retry();
+            recovered = true;
+            break;
           } catch {
-            return [];
+            // Try the next, broader fallback.
           }
-        } else {
-          return [];
         }
+        if (!recovered) return [];
       } else {
         throw error;
       }

packages/plugins/plugin-sharing/src/objects/sys-sharing-rule.object.ts

@@ -37,7 +37,7 @@ export const SysSharingRule = ObjectSchema.create({
   // We still recommend `defineSharingRule({...})` for repo-controlled
   // baselines, but admins can safely create/edit/delete from the UI.
   userActions: { create: true, edit: true, delete: true, import: false },
-  description: 'Declarative sharing rule that auto-materialises sys_record_share grants. Authored via defineSharingRule() in code or the Studio criteria builder.',
+  description: 'Grants a group of people access to the records that match a condition.',
   displayNameField: 'name',
   nameField: 'name', // [ADR-0079] canonical primary-title pointer (mirrors deprecated displayNameField)
   titleFormat: '{label}',
@@ -99,7 +99,13 @@ export const SysSharingRule = ObjectSchema.create({
       label: 'Name',
       required: true,
       maxLength: 100,
-      description: 'Unique snake_case rule name',
+      // Field `description`s on this object are ADMIN-FACING help text rendered
+      // under each input in Setup — not notes for the next engineer. They must
+      // not name tables, columns, enum values, ADRs or third-party libraries:
+      // the reader is a tenant admin who sees only the labels in the dropdown
+      // (objectstack#3821). Implementation detail belongs in the doc comments
+      // above, where it already is.
+      description: 'Identifies the rule. Lowercase letters, digits and underscores only.',
       group: 'Identity',
     }),
 
@@ -124,20 +130,20 @@ export const SysSharingRule = ObjectSchema.create({
       // instead of a free-text machine-name input. Falls back to a text input
       // when the `field:object-ref` widget is unavailable.
       widget: 'object-ref',
-      description: 'Short object name (e.g. opportunity, account)',
+      description: 'The object whose records this rule shares.',
       group: 'Target',
     }),
 
     criteria_json: Field.textarea({
-      label: 'Criteria (FilterCondition JSON)',
+      label: 'Criteria',
       required: false,
       // Rendered as a visual criteria builder scoped to the selected object's
       // fields (dependsOn: object_name), storing the same JSON FilterCondition.
       // An "Edit as JSON" fallback keeps hand-authored / advanced filters
       // editable. Falls back to a textarea when the widget is unavailable.
       widget: 'filter-condition',
       dependsOn: ['object_name'],
-      description: 'JSON FilterCondition matched against records of object_name. Empty = match all.',
+      description: 'Which records to share. Leave empty to share every record of the object.',
       group: 'Target',
     }),
 
@@ -150,7 +156,9 @@ export const SysSharingRule = ObjectSchema.create({
         label: 'Recipient Type',
         required: true,
         defaultValue: 'business_unit',
-        description: 'Kind of principal that receives access — expanded to user grants at evaluation time. `business_unit` walks the parent_business_unit_id tree; `team` is flat (better-auth); `position` expands the position\'s holders (positions are flat, ADR-0090 D3); `unit_and_subordinates` expands the named business unit PLUS every descendant unit\'s members via the sys_business_unit tree (ADR-0057 D5).',
+        // The engine detail this used to spell out (which tree is walked, which
+        // expansion is flat, the ADRs behind each) lives in the class doc above.
+        description: 'Who receives access. Picking a team, business unit or position gives access to everyone in it. "Business unit and subordinates" also covers every unit below the one you pick.',
         group: 'Recipient',
       },
     ),
@@ -166,7 +174,7 @@ export const SysSharingRule = ObjectSchema.create({
       // back to a text input when the widget is unavailable.
       widget: 'recipient-picker',
       dependsOn: ['recipient_type'],
-      description: 'business-unit id / team id / position name / user id depending on recipient_type',
+      description: 'The specific user, team, business unit or position that receives access.',
       group: 'Recipient',
     }),
 
@@ -184,7 +192,7 @@ export const SysSharingRule = ObjectSchema.create({
       label: 'Active',
       required: false,
       defaultValue: true,
-      description: 'Only active rules participate in lifecycle evaluation',
+      description: 'Turn off to withdraw the access this rule granted, without deleting the rule.',
       group: 'Lifecycle',
     }),
 
@@ -203,8 +211,7 @@ export const SysSharingRule = ObjectSchema.create({
       readonly: true,
       defaultValue: 'admin',
       description:
-        'Record provenance (unified tri-state, A4 #2920): platform = framework built-in / ' +
-        'package = app/package-declared (boot-seeded) / admin = tenant-created in Setup.',
+        'Where this rule came from: built into the platform, installed with an app, or created here in Setup.',
       options: [
         { value: 'platform', label: 'Platform' },
         { value: 'package', label: 'Package' },
@@ -219,8 +226,7 @@ export const SysSharingRule = ObjectSchema.create({
       readonly: true,
       defaultValue: false,
       description:
-        'Set when an admin edits a package-declared rule; boot seeding will no longer ' +
-        'overwrite the row (deactivations survive redeploys). Meaningless on admin rows.',
+        'Set once you edit a rule that came with an app, so your changes are kept when the app is updated.',
       group: 'System',
     }),