Skip to content

Commit b3fddd1

Browse files
fix(plugin-approvals): an out-of-office delegation applies only in its own organization (#25439)
<!-- claude-projects-attribution: {"github_login":"os-sales"} --> _Requested by **Sales** · [project thread](https://claude.ai/code/project/chan_01D5bWFY3rFzgJcytD5gD4Uu?thread=cmsg_01D5bWFY3rFzgJcytD5gD4UuGEYT5aVm7ttZRBJJNvF8Bj)_ Fixes #25280 Clause-②: no (narrowing) Metadata-protocol: none ## What changed `ApprovalService.lookupActiveDelegation` now reads a `sys_approval_delegation` row the way `grantAppliesInTenant` (`@objectstack/core`, `resolve-authz-context.ts`) reads a stored position or permission-set grant: the row reroutes an approver only when its `organization_id` is the request's organization, and both must name one. | Delegation row | Request | Before | After | |---|---|---|---| | org A | org A | reroutes | reroutes | | org A | org B | no | no | | no organization (`NULL`, absent, blank) | any | **reroutes in every organization** | no, and a `warn` names the row | | any | no organization | **takes every active row** | no | - `organizationNamed()` normalizes `NULL`, absent and `''` to "none", as core's module-private `grantOrganizationOf` does. It is applied to both sides of the comparison. - An active organization-less row that the lookup skips is logged at `warn`. The structured fields are `{ delegationId, delegatorId, delegateId, requestOrganizationId }`, and the message gives the remedy. Its author still sees the row in Setup, and nothing else says it has stopped applying. - The rule is the same in every posture (ADR-0131 D11). There is no posture branch. - `sys_approval_delegation.organization_id` help, which said "null = applies across tenants for this delegator", now says what the code does: - `en` is regenerated with `node scripts/check-i18n-bundles.mjs --write`. - `zh-CN`, `es-ES` and `ja-JP` are translated by hand. The `es-ES` and `ja-JP` leaves were byte copies of the old English, so their source-hash entries drop on extract. - The object docblock and `content/docs/automation/approvals.mdx` (Out-of-office delegation) state the organization scope. ## Who is affected **Under a wall (`group`/`isolated`), only stored state is affected.** No current write produces an organization-less delegation: - A session insert is stamped with the active organization (`injectTenantOnInsert` in `sql-driver.ts`). - An elevated insert with no organization is refused (`resolveSystemInsertOrganization`, D9). - An update that empties the column is refused (`denyOrganizationRemoval`, `security-plugin-write-tenant-wall.ts`). What remains are rows stored before 18.0. The D10 ceremony has no anchor for this table under a wall (inventory fate 3), so it reports such a row and leaves it `NULL`. Before this change, that row rerouted every organization's approvals for its delegator, onto a delegate who may hold no membership there. **Under `single`, a delegation written on 18.x applies exactly as before.** D9 stamps the Default Organization on both the delegation and the record, and the request takes its record's organization. Only a pre-ceremony organization-less row stops applying, and `os migrate organization-ownership --apply` attributes it. The changeset declares the break (`**BREAKING**`, ADR-0087 `not-required (no-migration-prescription)` marker) and gives the operator remedy for each posture. Precedent: `.changeset/24583-7a2-organization-less-grant-readers-retired.md`. ## Tests New file `ooo-delegation-organization.test.ts` (18 cases). Each walled case runs under both `isolated` and `group`: - A's delegation does not reroute an approval in B, and the request lands in B. - The same-organization delegation still applies, with the audit comment unchanged. - The request's organization is its record's: a member signed in to A, opening on B's record, gets no reroute from A's delegation. - An organization-less row reroutes nothing in either organization and is reported once per lookup, with the named fields. - A blank or absent organization counts as organization-less. - Beside an organization-less row, the same-organization row is the one that applies. - A chain stops at an organization-less hop. - A request with no organization takes no delegation. - Under `single`, a Default-Organization delegation reroutes, and an organization-less row does not and is reported. `approval-service.test.ts`: the pin "a null-org rule applies across tenants" is replaced by its inverse. Fixtures that wrote an organization-less delegation and expected a reroute now write it in the request's organization: - `admin-exemption-retired.test.ts`: `seedDelegation` writes `'t1'`. - `admin-rescue-named-slate.integration.test.ts`: `dlg_admin` writes `ORG`, and `sys_approval_delegation` leaves the `relaxTenantColumns` list. - `approval-notification-body.integration.test.ts`: `dlg_1` and `dlg_2` write `orgOfDeal(...)`, and `sys_approval_delegation` leaves the `relaxTenantColumns` list. The three `installDefaultOrganization` suites that insert a delegation (`actor-lookups-hold-ids`, `approval-notification-keyed-text`, `approval-requested-notification`) pass unchanged. D9 stamps the Default Organization on both sides there. `scripts/engine-double-contract.pinned.json` records the new test file's double (`check-engine-double-contract --write`). Its `update`/`delete` open with `assertEngineUpdateDispatch`/`assertEngineDeleteDispatch`, and its `find` holds the caller's `limit`. ## Verification - `pnpm --filter @objectstack/plugin-approvals exec vitest run`: 95 files, 1232 tests, all passed. `pnpm --filter @objectstack/plugin-approvals typecheck` passed, including `check:test-typecheck`. - Ablation (`scripts/ablation-replace.mjs`): the old filter was restored in `approval-service.ts`, then the new file, `approval-service.test.ts` and the three re-stamped fixtures were run. - 10 tests went red: the organization-less, blank or absent, chain, no-organization-request and `single` organization-less cases, plus the replaced pin. - The cross-organization and same-organization cases stay green under the old filter, as expected: the old filter already kept a scoped row in its own organization. - After the restore, the file matched HEAD and `git diff HEAD` was empty. - `node scripts/check-i18n-bundles.mjs`: OK, all bundles in sync. - `node scripts/check-adr-0087-registration.mjs --base origin/main`: exit 0. - Local gates (`dispatch-gates --commands`): on the first pass 95 of 99 exited 0. The other four were handled as follows: - `check:engine-double-contract` and `check:objectql-double-limit`: fixed (see above). - `check:dual-build-cjs-loads` and `check:skill-examples`: exit 3 because their prerequisite dists were missing. Those packages are now built. - The full re-run of the gates on the final head and the full dogfood battery (the `(narrowing)` arm) are running now. Their counts go in the `os-dev-report` on #25280. ## Acceptance notes - The branch is the harness-assigned `claude/project-thread-4a6wrl`, not `claude/issue-25280-…`. - `content/docs/automation/approvals.mdx` gains one paragraph on the organization scope, so the guide states the behaviour this PR changes. - #25342 (delegator prefill, in flight) edits the same object file and bundles, but only the `delegator_id` default and help. Whichever lands second merges main and regenerates the bundles. - The remedy for a walled organization-less row is in the warning and the changeset: recreate the delegation while signed in to the right organization, then delete the old row. No automated rewrite is offered, because under a wall the row names nothing to attribute it by. --- _Generated by [Claude Code](https://claude.ai/code/session_012KEX9o2giH3Wzt4jgeNTAK)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_012KEX9o2giH3Wzt4jgeNTAK --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 17ef032 commit b3fddd1

16 files changed

Lines changed: 389 additions & 20 deletions
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
'@objectstack/plugin-approvals': patch
3+
---
4+
5+
fix(plugin-approvals): an out-of-office delegation applies only in its own organization, and an organization-less delegation reroutes no approval
6+
7+
Clause-②: no (narrowing)
8+
Metadata-protocol: none
9+
10+
<!-- adr-0087: not-required (no-migration-prescription) No key of any metadata schema is removed, renamed or re-shaped, so there is nothing for `objectstack migrate meta` to rewrite and no tombstone. What changes is how the approval service reads a stored `sys_approval_delegation` row: the reading ADR-0131 D1 retired for every tenant column, and that `grantAppliesInTenant` already applies to grant rows. -->
11+
12+
**BREAKING** for a stored `sys_approval_delegation` row that carries no `organization_id` (`NULL`, absent or blank). No write produces such a row any more under a walled posture, so only a database upgraded from before 18.0 can hold one.
13+
14+
- **Which delegation reroutes an approver.** FROM: an organization-less delegation applied in every organization ("null = applies across tenants for this delegator"), and a request with no organization took every active delegation of its approver, whatever organization it belonged to. TO: a delegation reroutes an approver only when its organization is the request's organization, the rule a stored position or permission-set grant follows. An organization-less delegation reroutes nothing anywhere, and a request with no organization takes no delegation.
15+
- **Why.** Under `group` or `isolated`, an organization-less delegation rerouted every organization's approvals for its delegator, onto a delegate who may hold no membership in that organization. ADR-0131 D1: nothing reads a NULL tenant value as an owner.
16+
- **Under `single`** a delegation written on 18.x applies as before: the engine stamps the Default Organization on a delegation its author writes and on the record a request is about, and the request takes its record's organization. Only an organization-less row left from before the ownership ceremony stops applying.
17+
- **Where such a row comes from.** Under a wall a member's insert is stamped with the active organization, an elevated insert with none is refused, and an update that empties the column is refused. A row stored before that, on a database upgraded from 17.x or an 18.0 pre-release, can still be organization-less: `os migrate organization-ownership` attributes it to the Default Organization under `single`, and under a wall, where the row names nothing to attribute it by, reports it and leaves it NULL.
18+
- **It is not silent.** Each time a lookup skips an active organization-less delegation, the approval service logs a `warn` naming the delegation, its delegator and delegate, the request's organization, and the remedy.
19+
- **Setup.** The delegation form's Organization help says what the code does, in every shipped locale.
20+
21+
**What an operator does.** On a `single` deployment, run `os migrate organization-ownership --apply` if it has not run yet; it attributes these rows to the Default Organization. On a walled deployment, ask the delegator to create the delegation again while signed in to the organization whose approvals it should cover, then delete the organization-less row the warning names.

‎content/docs/automation/approvals.mdx‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -900,6 +900,13 @@ background job — so a lapsed delegation simply stops applying. Chains
900900
identity**; the reroute is recorded as an `ooo_substitute` audit action and both
901901
the delegate and the skipped approver are notified.
902902

903+
A delegation belongs to the organization it is created in, and reroutes only the
904+
approvals opened in that organization; a request's organization is its record's.
905+
To cover approvals in two organizations, create a delegation in each. A
906+
delegation that names no organization, which only a database upgraded from
907+
before 18.0 can hold, reroutes nothing, and the approval service logs a warning
908+
naming it (ADR-0131).
909+
903910
<Callout type="info">
904911
**Group-routed approvers are not rerouted.** `type: position` / `team` /
905912
`department` / `org_membership_level` keep their whole membership, so there is

‎packages/plugins/plugin-approvals/src/admin-exemption-retired.test.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,11 +202,14 @@ describe('#4839 evidence — a delegation cannot rescue an IN-FLIGHT approval',
202202
svc = new ApprovalService({ engine: engine as any, clock: { now: () => new Date(baseTime + (n++) * 1000) } });
203203
});
204204

205-
/** Declare an out-of-office delegation directly (bypassing the write guard). */
205+
/**
206+
* Declare an out-of-office delegation directly (bypassing the write guard),
207+
* in the submitter's organization: a delegation applies only there (ADR-0131).
208+
*/
206209
const seedDelegation = (delegatorId: string, delegateId: string) => {
207210
engine._tables['sys_approval_delegation'] = [{
208211
id: 'del1', delegator_id: delegatorId, delegate_id: delegateId,
209-
valid_from: null, valid_until: null, reason: 'sick leave', organization_id: null,
212+
valid_from: null, valid_until: null, reason: 'sick leave', organization_id: 't1',
210213
}];
211214
};
212215

‎packages/plugins/plugin-approvals/src/admin-rescue-named-slate.integration.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ describe('an empty admin_rescue slate opens on the organization\'s administrator
188188
await engine.syncSchemas();
189189
// The organization-less rows below exist only on a store built before 18.x
190190
// (ADR-0131 D1 makes a fresh tenant column NOT NULL): this is that store.
191-
await relaxTenantColumns(engine, ['sys_user', 'sys_user_permission_set', 'crm_deal', 'sys_approval_delegation']);
191+
await relaxTenantColumns(engine, ['sys_user', 'sys_user_permission_set', 'crm_deal']);
192192
bindSecurityCatalogReader(engine, catalogServing([
193193
'organization_admin', 'organization_admin_no_bypass', 'admin_full_access',
194194
]));
@@ -292,7 +292,7 @@ describe('an empty admin_rescue slate opens on the organization\'s administrator
292292
// it writes stays, and no case after it reads that slate.
293293
it('an administrator out of office is covered by their delegate, as any named approver is', async () => {
294294
await engine.insert('sys_approval_delegation', {
295-
id: 'dlg_admin', delegator_id: 'u_admin', delegate_id: 'u_cover', reason: 'leave',
295+
id: 'dlg_admin', delegator_id: 'u_admin', delegate_id: 'u_cover', reason: 'leave', organization_id: ORG,
296296
valid_from: '2026-09-01T00:00:00.000Z', valid_until: '2026-12-31T00:00:00.000Z',
297297
created_at: '2026-09-01T00:00:00.000Z', updated_at: '2026-09-01T00:00:00.000Z',
298298
}, { context: SYSTEM } as any);

‎packages/plugins/plugin-approvals/src/approval-notification-body.integration.test.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ describe('every approval notification carries its text in body', () => {
216216
await engine.syncSchemas();
217217
// The organization-less rows below exist only on a store built before 18.x
218218
// (ADR-0131 D1 makes a fresh tenant column NOT NULL): this is that store.
219-
await relaxTenantColumns(engine, ['sys_user', 'sys_approval_delegation']);
219+
await relaxTenantColumns(engine, ['sys_user']);
220220
for (const u of USERS) await engine.insert('sys_user', u, { context: SYSTEM } as any);
221221

222222
svc = new ApprovalService({
@@ -263,8 +263,9 @@ describe('every approval notification carries its text in body', () => {
263263
await svc.remind(ids.bare, { actorId: 'u_sub' }, asUser('u_sub'));
264264

265265
// ── Out of office: the routing substitutes a delegate ──
266+
// A delegation applies only in its own organization (ADR-0131): the deal's.
266267
await engine.insert('sys_approval_delegation', {
267-
id: 'dlg_1', delegator_id: 'u_ooo', delegate_id: 'u_bob', reason: 'leave',
268+
id: 'dlg_1', delegator_id: 'u_ooo', delegate_id: 'u_bob', reason: 'leave', organization_id: orgOfDeal('ooo'),
268269
valid_from: '2026-09-01T00:00:00.000Z', valid_until: '2026-12-31T00:00:00.000Z',
269270
created_at: '2026-09-01T00:00:00.000Z', updated_at: '2026-09-01T00:00:00.000Z',
270271
}, { context: SYSTEM } as any);
@@ -273,7 +274,7 @@ describe('every approval notification carries its text in body', () => {
273274
// [#24065] The control: the absent approver names no account, so no name
274275
// can be read for them; the delegate the same hop routes to does.
275276
await engine.insert('sys_approval_delegation', {
276-
id: 'dlg_2', delegator_id: GONE, delegate_id: 'u_alice', reason: 'leave',
277+
id: 'dlg_2', delegator_id: GONE, delegate_id: 'u_alice', reason: 'leave', organization_id: orgOfDeal('ooo_gone'),
277278
valid_from: '2026-09-01T00:00:00.000Z', valid_until: '2026-12-31T00:00:00.000Z',
278279
created_at: '2026-09-01T00:00:00.000Z', updated_at: '2026-09-01T00:00:00.000Z',
279280
}, { context: SYSTEM } as any);

‎packages/plugins/plugin-approvals/src/approval-service.test.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3134,10 +3134,13 @@ describe('ApprovalService — out-of-office delegation (#1322)', () => {
31343134
expect(req.pending_approvers).toEqual(['alice']);
31353135
});
31363136

3137-
it('applies a cross-tenant (null org) rule regardless of request tenant', async () => {
3137+
// ADR-0131 D1: an organization-less row is no "every organization" rule.
3138+
// The full matrix, per posture, is `ooo-delegation-organization.test.ts`.
3139+
it('an organization-less rule reroutes nothing, in the request tenant or any other', async () => {
31383140
seedDelegation([{ delegator_id: 'alice', delegate_id: 'bob', organization_id: null }]);
31393141
const req = await svc.openNodeRequest(openInput(['alice']), CTX);
3140-
expect(req.pending_approvers).toEqual(['bob']);
3142+
expect(req.pending_approvers).toEqual(['alice']);
3143+
expect(engine._tables['sys_approval_action'].some((a: any) => a.action === 'ooo_substitute')).toBe(false);
31413144
});
31423145
});
31433146

‎packages/plugins/plugin-approvals/src/approval-service.ts‎

Lines changed: 57 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1222,6 +1222,19 @@ function csvSplit(raw: unknown): string[] {
12221222
return String(raw).split(',').map(s => s.trim()).filter(Boolean);
12231223
}
12241224

1225+
/**
1226+
* [ADR-0131] The organization a value names, or `null` for none — `NULL`,
1227+
* absent or blank. The normalization `grantOrganizationOf`
1228+
* (`@objectstack/core`, `resolve-authz-context.ts`) gives a stored grant row's
1229+
* organization; `null` names no organization and matches none, its own
1230+
* included. Read by {@link ApprovalService.lookupActiveDelegation} on both
1231+
* sides of its comparison: the delegation row's and the request's.
1232+
*/
1233+
function organizationNamed(value: unknown): string | null {
1234+
if (value === null || value === undefined || value === '') return null;
1235+
return String(value);
1236+
}
1237+
12251238
/**
12261239
* Is a submitted decision output blank — i.e. does it fail a `required`
12271240
* declaration (objectui#2955)?
@@ -3227,6 +3240,32 @@ export class ApprovalService implements IApprovalService {
32273240
* shared `isGrantActive` half-open window (ADR-0091 D2), enforced here at
32283241
* resolution time — never by a background job. When several rows are active,
32293242
* the one expiring soonest wins (the most specific coverage window).
3243+
*
3244+
* **A delegation applies only in its own organization (ADR-0131).** A row
3245+
* reroutes a request's approver only when the row's `organization_id` is the
3246+
* request's organization: the rule `grantAppliesInTenant`
3247+
* (`@objectstack/core`, `resolve-authz-context.ts`) applies to a stored
3248+
* position or permission-set grant, read the same way here.
3249+
*
3250+
* - A delegation of another organization reroutes nothing in this one.
3251+
* - An organization-less delegation reroutes nothing anywhere. "NULL means
3252+
* every organization" is the reading ADR-0131 retired (D1: nothing reads a
3253+
* NULL tenant value as an owner). Under a wall no write produces such a row
3254+
* any more: a session insert is stamped with its active organization, an
3255+
* elevated insert with none is refused (D9), and the write wall refuses an
3256+
* update that empties the column. What remains is stored state from before.
3257+
* The D10 ceremony attributes it to the Default Organization under
3258+
* `single`; under a wall it has no anchor, so the ceremony reports it and
3259+
* leaves it NULL. Such a row is reported here at `warn` with its remedy,
3260+
* because its author still sees it in Setup and nothing else says it has
3261+
* stopped applying.
3262+
* - A request with no organization takes no delegation: a missing
3263+
* organization has no "every organization" reading either.
3264+
*
3265+
* The rule is the same in every posture (ADR-0131 D11). Under `single`, D9
3266+
* stamps the Default Organization on the delegation its author writes and on
3267+
* the record a request is about, and the request takes its record's
3268+
* organization, so a delegation written there applies as it did before.
32303269
*/
32313270
private async lookupActiveDelegation(
32323271
delegatorId: string,
@@ -3243,10 +3282,24 @@ export class ApprovalService implements IApprovalService {
32433282
context: SYSTEM_CTX,
32443283
} as any);
32453284
} catch { return null; } // table absent on minimal stacks — no OOO, resolve as-is
3246-
const active = (rows ?? []).filter((r: any) =>
3247-
isGrantActive(r, now)
3248-
// A null-org rule applies across tenants; a scoped rule only within its tenant.
3249-
&& (organizationId == null || r.organization_id == null || String(r.organization_id) === String(organizationId)));
3285+
const requestOrg = organizationNamed(organizationId);
3286+
const active: any[] = [];
3287+
for (const r of rows ?? []) {
3288+
if (!isGrantActive(r, now)) continue;
3289+
const rowOrg = organizationNamed(r?.organization_id);
3290+
if (rowOrg === null) {
3291+
this.logger?.warn?.(
3292+
`[approvals] out-of-office delegation '${r?.id}' (${delegatorId} → ${r?.delegate_id}) names no `
3293+
+ `organization, so it reroutes no approval: a delegation applies only to approvals in the `
3294+
+ `organization it belongs to. Create it again while signed in to the organization whose `
3295+
+ `approvals it should cover, and delete this one; on a single-organization deployment, `
3296+
+ `'os migrate organization-ownership --apply' attributes it to the Default Organization instead.`,
3297+
{ delegationId: r?.id, delegatorId, delegateId: r?.delegate_id, requestOrganizationId: requestOrg },
3298+
);
3299+
continue;
3300+
}
3301+
if (rowOrg === requestOrg) active.push(r);
3302+
}
32503303
if (!active.length) return null;
32513304
active.sort((a: any, b: any) => {
32523305
const au = a.valid_until ? Date.parse(String(a.valid_until)) : Number.POSITIVE_INFINITY;

0 commit comments

Comments
 (0)