You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[finding] ApprovalRequestRow declares no organization_id, but openNodeRequest stamps one on the row it writes and returns #10331
Found while implementing #10153. Not fixed there — that card is fenced to the manager approver branch, and this is a published-contract question.
What was measured
packages/spec/src/contracts/approval-service.ts lines 84-262 declare the whole of ApprovalRequestRow. grep -n "organization_id" over that file returns nothing (counter-check: grep -n "pending_approvers" in the same file hits line 96, so the search works).
Meanwhile packages/plugins/plugin-approvals/src/approval-service.ts writes the field on the row it inserts and returns:
So the value is on the object at runtime and absent from the type every consumer reads it through.
How it surfaced
A test asserting on the request's organization wrote req.organization_id and got
error TS2339: Property 'organization_id' does not exist on type 'ApprovalRequestRow'.
only under pnpm check:type-check-debt --re-measure — plugin-approvals excludes **/*.test.ts from its tsconfig.json, so pnpm --filter @objectstack/plugin-approvals typecheck exits 0 over the same file. #10153's pins now read the stamp off the persisted sys_approval_request row instead, which is a workaround, not a resolution.
Why it is worth recording
ApprovalRequestRow is the shape IApprovalService publishes to every consumer of an approval read. A field that exists on the wire and not in the type is one every caller has to cast past. The tenancy stamp is a poor field to make invisible: it is exactly what a caller filtering or asserting across organizations needs.
Not asserting the fix shape
Two readings, and they are not equivalent:
the omission is an oversight, so the contract should declare organization_id?: string | null;
the omission is deliberate — the row's organization is an internal placement fact and consumers are meant to reach it another way — in which case the write path should say so, and the field should not ride the returned object either.
Nobody has stated which, so this is filed for triage rather than argued. Whoever decides should check the sibling rows (ApprovalActionRow and the inbox index rows) for the same asymmetry in the same pass.
Activity
feat(spec): declare organization_id on ApprovalRequestRow and Approva…
test(spec): contracts and conversions test titles state each cited de…