Skip to content

[finding] ApprovalRequestRow declares no organization_id, but openNodeRequest stamps one on the row it writes and returns #10331

Description

@os-warren

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:

constctxOrg=(contextasany)?.organizationId??context?.tenantId??input.organizationId??null;// ...organization_id: ctxOrg,

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-measureplugin-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:

  1. the omission is an oversight, so the contract should declare organization_id?: string | null;
  2. 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.

Metadata

Metadata

Assignees

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions