Skip to content

Commit f40c5b4

Browse files
os-helpclaude
andauthored
refactor(plugin-approvals,plugin-reports): annotate enforcement onto the full ExecutionContext (#7135) (#7206)
Services half of #7070. #6523 converged 36 contract signatures onto the complete `resolveAuthzContext` envelope (the #6206 ruling: enforcement adjudicates on the whole envelope, never a per-site subset); the implementations still named the retired six-field shape, so nothing they could read had widened. `ApprovalService`, the approval flow-node provider and `ReportService` now declare `ExecutionContext` on all 43 positions, and the casts the narrow annotation forced are gone — including `(context as any).posture` in `isOverrideActor()`, an erasure sitting on an enforcement input. `organizationId` is not a field of the envelope at all and stays cast (#5858 / `check:org-identifier`), per the #7070 boundary. A compile-time `exec-context-annotation.pin.ts` per package catches a re-narrowing, which would otherwise compile, ship and pass every test. Claude-Session: https://claude.ai/code/session_015fkdTyGmMD5s8ZtEifvuGy Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2e4274d commit f40c5b4

6 files changed

Lines changed: 344 additions & 53 deletions

File tree

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
---
2+
"@objectstack/plugin-approvals": patch
3+
"@objectstack/plugin-reports": patch
4+
---
5+
6+
refactor(plugin-approvals,plugin-reports): enforcement implementations annotate the full `ExecutionContext` (#7135)
7+
8+
The services half of #7070, mirroring what PR #7140 did for
9+
`plugin-sharing` / `plugin-audit`. #6523 converged 36 contract signatures onto
10+
the complete `resolveAuthzContext` envelope, applying the #6206 ruling —
11+
enforcement adjudicates on the whole envelope, never a per-site subset. The
12+
implementations behind those contracts still annotated their own parameters
13+
with the six-field shape the contracts used to name, so nothing they could
14+
*read* had widened.
15+
16+
`ApprovalService`, the approval flow-node provider and `ReportService` now
17+
declare `ExecutionContext` on all 43 of those positions, and the casts the
18+
narrow annotation forced are gone:
19+
20+
- `isOverrideActor()` read the derived `posture` (ADR-0095) through an
21+
unchecked `(context as any)`. That gate decides whether a platform or tenant
22+
admin may release a STUCK approval — one routed to an unstaffed position, the
23+
only in-product recovery from a permanently locked record — so an erasure sat
24+
directly on an enforcement input: a mistyped rung would have compiled and
25+
silently denied every override. It is a declared read now.
26+
- Both services' `SYSTEM_CTX` is typed as the envelope and passed as itself,
27+
retiring the `SYSTEM_CTX as unknown as …` double casts at the three sites
28+
that hand it to a contract method.
29+
- The `(context as any).userId` / `.tenantId` reads in `ApprovalService` now
30+
read declared fields.
31+
- `OwnerContextResolver` returns the envelope, which is what a scheduled report
32+
actually resolves for its owner (#2849 / #2980).
33+
34+
**No runtime behaviour changes.** The values were always complete — this
35+
family's damage was type-side — so every gate answers exactly what it answered
36+
before. Method parameters only WIDEN what they accept, so no caller is
37+
affected, and no public export changes shape.
38+
39+
Casts deliberately kept, and now documented where they sit: `organizationId`
40+
is not a field of the envelope at all — that spelling has its own history
41+
(#5858 / `check:org-identifier`) and was held out of this change by #7070. In
42+
`approval-node.ts` the single remaining assertion exists only because the
43+
literal names that key; it was reduced from `as unknown as …` to a single
44+
`as ExecutionContext`, which still requires the literal to be comparable to
45+
the envelope.
46+
47+
Because a re-narrowed annotation would compile, ship and pass every test in
48+
these packages, the convergence is pinned by a new compile-time module per
49+
package, `exec-context-annotation.pin.ts`: it hands each parameter a fresh
50+
literal naming envelope-only fields (`posture`, `accessible_org_ids`,
51+
`org_user_ids`), which TypeScript's excess-property check rejects the moment a
52+
parameter narrows back, plus negative cases so a parameter erased to `any`
53+
cannot pass either.
54+
55+
The exported `SharingExecutionContext` type itself is NOT removed here: it is
56+
defined in `packages/spec`, which is single-owner, so its retirement is a
57+
separate follow-up.

packages/plugins/plugin-approvals/src/approval-node.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,10 @@ import {
2424
APPROVAL_NODE_TYPE,
2525
type ApprovalNodeConfig,
2626
} from '@objectstack/spec/automation';
27-
import type { SharingExecutionContext } from '@objectstack/spec/contracts';
27+
// [#7135] The full `resolveAuthzContext` envelope — what
28+
// `IApprovalService.openNodeRequest` declares for its context parameter since
29+
// #6523 (the #6206 ruling: no per-site subset contracts).
30+
import type { ExecutionContext } from '@objectstack/spec/kernel';
2831
import type { ApprovalService } from './approval-service.js';
2932
import { registerApprovalReviseNode } from './approval-revise-node.js';
3033

@@ -55,7 +58,7 @@ interface MinimalLogger {
5558
warn?: (msg: any, ...rest: any[]) => void;
5659
}
5760

58-
const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as const;
61+
const SYSTEM_CTX: ExecutionContext = { isSystem: true, positions: [], permissions: [] };
5962

6063
/**
6164
* Rebuild the nested object the engine's CEL conditions see from the flow's
@@ -169,9 +172,20 @@ export function registerApprovalNode(
169172
}, {
170173
...SYSTEM_CTX,
171174
userId: context?.userId,
175+
// [#7135] ⚠️ This assertion SURVIVES the annotation widening, and it
176+
// is `as ExecutionContext` rather than the `as unknown as
177+
// SharingExecutionContext` double cast it replaces. The sole reason
178+
// a cast is still needed is `organizationId`, which is not a field
179+
// of the envelope at ALL — that spelling has its own history (#5858
180+
// / `check:org-identifier`) and was explicitly held out of this
181+
// change (#7070), so removing the key here would be a RUNTIME change
182+
// belonging to that card. Dropping the second hop matters: `as
183+
// unknown as` erases the value entirely, while a single assertion
184+
// still requires the literal to be comparable to the envelope, so a
185+
// `userId: 42` here is once again a compile error.
172186
organizationId: context?.organizationId,
173187
tenantId: context?.tenantId,
174-
} as unknown as SharingExecutionContext);
188+
} as ExecutionContext);
175189

176190
// #3447 P2: empty slate + onEmptyApprovers: 'auto_approve' — nobody to
177191
// ask, no request row. Complete (don't suspend) straight down the

0 commit comments

Comments
 (0)