feat(approvals): cross-organization approver targeting (ADR-0105 D9) - #3824
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 3 package(s): 104 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
One organization id decided three different things at once in
`openNodeRequest`: where the request row lives, where its inbox index rows
live, and where its approvers are looked up. The first two are the request's
own organization by definition. The third is not — a group CFO holds her `cfo`
position in the GROUP organization while the purchase order she signs off
lives in the PLANT organization. `expandPositionUsers('cfo', <plant>)` matched
nobody, the slot fell back to the dead `position:cfo` literal, and a group
escalation could not be expressed at all.
An approver may now declare which organization's directory resolves it:
- { type: position, value: plant_manager, group: plant }
- { type: position, value: cfo, organization: $root, group: finance }
Four design points the ADR left open, settled from what the code and the
authoring model require (ADR text amended in this commit):
- PER APPROVER, not per node. The node-level form cannot express the commonest
group shape — one node requiring a plant manager AND a group CFO in parallel.
Splitting into serial nodes changes the semantics rather than the syntax. A
node-level default remains addable later as sugar; the reverse is not true.
- SYMBOLS FIRST (`$root` / `$parent`). Flow metadata is portable across
environments; an organization id is data minted per deployment, so a literal
id in a flow is unportable by construction and an AI author cannot know one.
The symbols walk D6's `parent_organization_id` tree with zero deployment
knowledge. A slug covers what they cannot — notably a SIBLING organization
(a shared-services centre approving payables for every plant).
- LEGALITY IS "SHARES A ROOT", not "is an ancestor" — the sibling case above is
first-class. The rule reads only the organization tree, never the submitter,
so one flow routes identically for everyone and a routing bug is reproducible.
- NON-`group` POSTURES REFUSE at runtime. Posture is environment configuration,
so the same portable metadata deploys into any posture and no static check
can see which; ignoring the declaration would let a group → isolated
migration reroute approvals with no signal.
Two failure modes are made loud rather than silent: an approver type with no
org-scoped directory (`user`/`field`/`manager`/`team`) refuses the declaration
instead of ignoring it, with a new `approval-approver-cross-org-unsupported`
lint catching it at author time; and a targeted approver holding no membership
in the request's organization is dropped with a warning naming them — D2's
union wall would otherwise hide the request from someone already routed to,
leaving a task she cannot open. Dropping hands it to the node's existing
`onEmptyApprovers` policy.
An approver without `organization` is unchanged: same resolution, same
queries, no extra reads.
Tests: 17 resolver units (symbols, sibling/foreign slugs, posture, cycles,
unreadable-membership), 7 service integration (including the pre-D9 gap, the
mixed plant+group node, and that the request still belongs to the request org),
4 lint. plugin-approvals 305, lint 500, spec 6736, cli 682 all green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015FebXPaaGrLhGKw1LHPbpL
The drift check flagged `automation/approvals.mdx` — correctly. D9 adds an authoring capability, and the hand-written guide is where an author (human or AI) actually looks; a feature reachable only from the generated schema reference is a feature most authors never find. Adds a section covering what the reference table cannot: which value to reach for ($root over a slug, because flow metadata is portable and organization ids are not), and the three things the field deliberately does NOT do — it does not move the request, it is not a route to an arbitrary tenant, and it does not apply to approver kinds that consult no organization directory. Plus the read-visibility precondition, which is the one way a correctly-authored cross-org approval still ends up empty. Also points the approver-kinds paragraph at the new section, so the fact that only some kinds accept `organization` is discoverable from where the kinds are listed rather than only from the lint that rejects it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015FebXPaaGrLhGKw1LHPbpL
os-zhuang
force-pushed
the
claude/adr-0105-group-tenancy-posture-5womi2
branch
from
July 28, 2026 08:33
3545221 to
ce10d23
Compare
os-zhuang
marked this pull request as ready for review
July 28, 2026 08:46
os-zhuang
added a commit
that referenced
this pull request
Jul 28, 2026
… types (#3873) `user`, `field` and `manager` return EARLY in `resolveApproverSpec` — they name a person outright instead of expanding a directory. D9's org resolution (#3824) sat AFTER those returns, so an `organization` declared on one of them never reached the check and was silently INERT. That is precisely the behaviour ADR-0105 D9 rules out and the authoring docs promise against ("`organization` on those is refused at runtime"). The `os lint` rule caught it at author time, but the runtime claim was false — and a flow stored before the lint existed, or assembled programmatically, got no signal. Resolution moves to the top of `resolveApproverSpec`, above every early return, so the refusal reaches all three types. The ordinary path is unchanged and still free: with no `organization` declared the resolver returns the request's organization without reading anything. Why the existing tests missed it: `approver-org-scope.test.ts` calls the resolver DIRECTLY, so it never traverses the early return, and the service-level integration test only covered `position`. Both gaps are closed — the new case opens a real request for each of the three directory-less types and asserts the refusal, plus one that a directory-less approver with NO declaration is untouched, so the guard cannot start costing the ordinary case. Found by cloud's group-posture dogfood on a real `group` boot. The two links that dogfood was written to check both hold: a cross-organization approver does reach her queue through the D2 union wall, and `decide()` does accept her. Claude-Session: https://claude.ai/code/session_015FebXPaaGrLhGKw1LHPbpL Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ADR-0105 Phase 2 D9. Tracking: #3541. Follows D8 (#3663, #3674, #3695, objectui#2868, cloud#886).
The gap
A plant document that needs a group-side sign-off could not be expressed. One organization id decided three different things at once in
openNodeRequest:A group CFO holds her
cfoposition in the GROUP organization; the purchase order she signs off lives in the PLANT organization. SoexpandPositionUsers('cfo', <plant>)matched nobody, the slot fell back to the deadposition:cfoliteral, and the request routed intoonEmptyApprovers. Silently.The first test in the integration suite pins exactly that pre-D9 behaviour, so the gap this closes is a fact in the repo rather than a claim in a PR body.
The change
Only the third job moves. The request row, its
sys_approval_approverindex rows, and its audit actions all still belong to the request's organization — asserted, because relocating them would cost the plant its own audit trail.Four design points the ADR left open
The ADR said "an approval chain node may name a target organization". Implementing it surfaced four questions it didn't answer; the ADR text is amended in this PR with the reasoning.
1. Per approver, not per node. The node-level form cannot express the commonest group shape — one node requiring a plant manager and a group CFO in parallel (
per_group). Expressing that as two serial nodes changes the semantics (parallel co-sign becomes sequential approval), so node-level distorts the model rather than merely inconveniencing the author. A node-level default is a strict special case of the per-approver form and stays addable later as sugar; the reverse is not true. It also puts the knob whereresolveAsandgroupalready live.2. Symbols first —
$root/$parent. Flow metadata is portable across environments; an organization id is data minted per deployment. A literal id in a flow is unportable by construction, and an AI author cannot know one at authoring time — so the value space must have a deployment-independent spelling for the common intents.$root(escalate to group) and$parent(division sign-off in a three-tier group) walk D6'sparent_organization_idtree. A slug covers what they cannot, notably a sibling organization: a shared-services centre approving payables for every plant.3. Legality is "shares a
parent_organization_idroot", not "is an ancestor" — because of that sibling case, and downward targeting too. Critically the rule reads only the organization tree, never the submitter: one flow routes identically for everyone, which is what makes a routing bug reproducible. (Bounding by the submitter'saccessible_org_idswas the alternative; it would have made flow behaviour vary per submitter and blocked the main use case, since the plant submitter is usually not a member of the group org.) A deployment with no grouping metadata gets a refusal naming D6, not a silent pass.4. Non-
grouppostures refuse at runtime. Posture is environment configuration, so the same portable metadata may be deployed into any posture and no static check can see which — this cannot be a lint. Silently ignoring the declaration would let agroup→isolatedmigration reroute approvals with no signal, which is an audit event, not a config detail. Refusal follows theexpression-approver philosophy already in this file: a config that cannot run is a routing bug, never "condition not met".Two silent failures made loud
A type with no org-scoped directory (
user/field/manager/team— the last becausesys_team_membercarries no organization column) refuses the declaration rather than ignoring it: an author who wrote it believed it did something. Newapproval-approver-cross-org-unsupportedlint (error) catches it at author time. The applicability table issatisfies-exhaustive, so a newApproverTypethat doesn't declare whether targeting applies fails the build.A targeted approver with no membership in the request's organization is dropped, with a warning naming them. The ADR says cross-org reads "are covered by D2 (membership union)" — true, but D2's union is
organization_id IN accessible_org_idsand the request is stamped with the request's org, so a group-side approver reaches it only if she also holds a membership there. That is the intended group shape (the ADR-0105 acceptance dogfood already models it) but nothing enforced it. Unchecked, the failure is silent and expensive: routing succeeds, the inbox row is written, and she opens a task the wall hides. Dropping converts it into the node's existingonEmptyApproverspolicy. An unreadable membership table does not drop anyone — emptying a live slate on an infrastructure hiccup is worse than routing optimistically.Verification
The resolver tests cover the boundaries rather than the happy path:
$rootclimbing more than one level (a one-level implementation would pass a shallower test), a sibling accepted, a foreign-group org refused, an id-instead-of-slug refused with a message saying why, a cycle in the grouping metadata terminating, and a non-group posture refusing.Compatibility
An approver without
organizationtakes the same path as before — same resolution, same queries, no extra reads (asserted). No schema migration; the field is optional and additive.🤖 Generated with Claude Code
https://claude.ai/code/session_015FebXPaaGrLhGKw1LHPbpL
Generated by Claude Code