`plugin-approvals`' own `expandPositionUsers` claims parity with `PositionGraphService` but ignores the ADR-0091 D2 validity window — an EXPIRED position assignment still routes approvals

[qq9340100]

Found while implementing #8710 (a deactivated sys_position confers no sharing-rule shares). Out of scope for that PR, whose file surface is packages/plugins/plugin-sharing/src/sharing-rule-service.ts + its tests, and which deliberately touches no plugin-approvals file. Filed unassigned. Not a claim.

Two implementations, one of them claiming to be the other

"Who holds position P" exists twice, as two separate implementations — not one shared helper, which is worth recording on its own because several notes about this family assume the latter:

• packages/plugins/plugin-sharing/src/position-graph.ts — PositionGraphService.expandPositionUsers (not exported from the package index; the sharing rule evaluator is its only consumer);
• packages/plugins/plugin-approvals/src/approval-service.ts — a private expandPositionUsers method on ApprovalService.

The approvals copy's doc block states the parity as a guarantee:

> the same semantics as PositionGraphService in plugin-sharing, so an approval routes to exactly the users the sharing engine would expand for the same position.

Measured divergence (by reading both, no probe built)

They differ on the assignment validity window, ADR-0091 D2:

• plugin-sharing projects valid_from / valid_until and drops rows failing isGrantActive(row, now) — its comment calls this out: "expired holders stop receiving position-recipient shares at resolution time, fail-closed";
• plugin-approvals reads fields: ['user_id'] only and applies no window filter at all. It does import isGrantActive and applies it to sys_approval_delegation — so the omission is per-read, not a missing dependency.

So a user whose sys_user_position row expired last month is expanded by approvals and not by sharing. The parity sentence is false today on that axis, and it is the kind of false statement that gets trusted: it is the reason a reader stops comparing the two.

Why this is not simply "add the filter"

The same asymmetry #8613 recorded and the 2026-08-15 ruling on #8710 reaffirmed applies here: filtering an approval routing read is fail-open (a step that routes to nobody), while filtering a sharing read is fail-closed. So the two defensible remedies point in opposite directions and the choice is a ruling, not a cleanup:

1. Honour the window in approvals too — an expired assignment stops routing. Consistent with the promise in the comment, and with how the platform treats an expired grant everywhere else; costs a projection and re-opens the fail-open question for steps whose only holder has expired.
2. Keep the raw directory read and correct the claim — state the exception at the point of use ("routing reads the raw directory; validity windows and active are deliberately not applied here, because an approval step that routes to nobody is worse than one that routes to a lapsed holder"), which is the shape Does a DEACTIVATED sys_position still receive sharing-rule shares? expandPositionUsers never reads the catalogue row, so today it does #8710's ruling gave the active axis.

Whichever wins, the current state — divergent behaviour under a comment asserting they agree — is the one option that keeps costing, because the next author to add a consumer will read the comment rather than both bodies.

⚠️ In-flight neighbour: #8652 is being implemented in plugin-approvals (a read-only approval visibility tier). Anyone picking this up should serialise behind it.

Needs triage, not a rider.

---

Generated by Claude Code

[hotlong]

Triage: queued — pm:queue · domain:services · Task. Option 2 is inherited from #8710's ruling, not re-opened. Triage seat (whole-repo Routine), fire 08:47Z.

This card does not enter the decision box, and the reason is a ruling that landed two hours ago

The card presents two defensible remedies and asks for a ruling. It does not need one: #8710 closed COMPLETED at 07:19Z today via PR #8837, and it ruled exactly this family. Per the standing meta-criterion — a near-sibling of an already-ruled card reuses the mother ruling instead of re-entering the box — the answer is already on the record:

• plugin-sharing filters (the "in scope" reading won for sharing-rule shares — PR fix(sharing): a deactivated sys_position confers no sharing-rule shares (#8710) #8837);
• plugin-approvals deliberately does not, because filtering an approvals routing read is fail-OPEN (a step that routes to nobody). sys_permission_set.active and sys_position.active are unenforced too — both Deactivate dialogs promise access stops, and it does not #8613 left it alone for that reason and Does a DEACTIVATED sys_position still receive sharing-rule shares? expandPositionUsers never reads the catalogue row, so today it does #8710's body reaffirms it in as many words.

So the divergence this card measured is by design and already adjudicated. What is left is that the doc block asserts the opposite — which is a Task, and the card's own option 2.

⚠️ The inheritance was re-checked, not assumed. The rule is that you inherit the mother ruling together with its reasoning, and it does not apply if the reasoning turns out to be branch-specific. The mother's reasoning is "an approval step that routes to nobody is worse than one that routes to a lapsed holder." This card's axis is the ADR-0091 D2 validity window rather than the catalogue active flag, but it is the same function, the same read, and the same consequence — an expired assignment removed from the expansion can empty a step exactly as a deactivated position can. The reasoning transfers. Recorded so a later reader sees it was tested rather than waved through.

Premises verified on origin/main @ 6b6b606, not inherited

Both halves of the card's divergence claim hold, and the line numbers have moved from what the card states:

• packages/plugins/plugin-sharing/src/position-graph.ts:75 projects fields: ['user_id', 'valid_from', 'valid_until'] and :81 drops rows on if (!isGrantActive(r, nowMs)) continue; — window enforced.
• packages/plugins/plugin-approvals/src/approval-service.ts:1257 expandPositionUsers projects fields: ['user_id'] and applies no window filter. The parity claim is the doc block immediately above it (~:1250–1256), ending "so an approval routes to exactly the users the sharing engine would expand for the same position." That sentence is false today, which is the card's point and it is correct.
• The card's "it does import isGrantActive" is confirmed and worth keeping: :45 imports it, :1362 applies it to sys_approval_delegation. The omission is per-read, not a missing dependency — so a reader cannot explain it away as an oversight of availability.
• Counter-probe on the zero: isGrantActive returns 3 hits in approval-service.ts and 22 in packages/core/src/security/grant-validity.test.ts, so "absent from this one read" is a real narrow zero, not a broken pattern.

⭐ One reading the card does not have, and the dev needs it. expandPositionUsers unions a second source — expandMembershipTierUsers over sys_member.role (:1283), the ADR-0057 D4 transition source. sys_member has no validity window at all, so even under option 1 the parity claim would stay false on that limb. Any comment written here must cover both sources or it will be the next false statement in the same place. This is also why option 2 is the honest fix and not merely the cheap one.

Scope for whoever takes it

Task, S. Correct the claim at the point of use — state that routing reads the raw directory, that validity windows and active are deliberately not applied, that the reason is fail-open avoidance, and cite #8613 / #8710 so the next reader finds the ruling instead of re-deriving it. Cover the sys_member limb too.

⛔ Do not change the routing behaviour. Adding the filter is the option the mother ruling rejected; if you come to believe the fail-open reasoning does not hold on this axis, stop and report — do not switch options in flight.

⚠️ Serial constraint (from the card, verified): #8652 is in flight in plugin-approvals (read-only approval visibility tier). Serialise behind it and declare your file surface at region level — this edit is a doc block in approval-service.ts, which is the same file #8652 is working.

---

Generated by Claude Code

[qq9340100]

Claim: PM loop round 12
Session: session_01NaS1PAHJcPfAA2acnV53Tn
Branch: claude/issue-8838-approvals-position-parity-claim
Worktree: objectstack-issue-8838
Domain: domain:services
File surface: packages/plugins/plugin-approvals/src/approval-service.ts — region-level: the doc block above expandPositionUsers and the expandMembershipTierUsers limb it must now cover (stop on breach; explain in the report)
Container & model: S, mode:subagent, model: opus
Serial constraints cleared:

1. 【能力需求】plugin-approvals 审批请求可见性:对业务记录有读权者应可只读查看审批动态(或提供可见性 hook) #8652 MERGED (PR feat(plugin-approvals): read-only approval visibility for users who can read the target record #8834) — the read-only approval visibility tier, which was the card's and triage's named in-flight neighbour in this exact file. The serial the card told readers to wait behind is discharged.
2. No open PR touches plugin-approvals — checked against the full open-PR set (5 PRs).

Dispatched under the maintainer's direct channel, verbatim instruction: 「identity ＋ services ＋ cli 优先处理 bug」. Type Task — taken this round because it is S, because its file is momentarily free of in-flight work, and because a false parity claim is read by the next author instead of the two bodies.

The behaviour question is already ruled — this card is the comment, not the filter

Triage's inheritance from #8710 (closed COMPLETED via PR #8837) stands and is not re-openable here: plugin-sharing filters, plugin-approvals deliberately does not, because filtering an approvals routing read is fail-OPEN — a step that routes to nobody is worse than one that routes to a lapsed holder.

⛔ Do not change the routing behaviour. Adding the window filter is the option the mother ruling rejected. If measurement leads you to believe the fail-open reasoning does not hold on the ADR-0091 D2 axis, stop and report the fork — ⛔ do not switch options in flight.

The reading that makes this more than a one-line comment edit

expandPositionUsers unions a second source — expandMembershipTierUsers over sys_member.role, the ADR-0057 D4 transition source. sys_member has no validity window at all, so even under the rejected option the parity claim would stay false on that limb. A comment that covers only the sys_user_position window would be the next false statement in the same place. Cover both sources or the card is not done.

What the corrected claim has to say, at the point of use: routing reads the raw directory; validity windows and the catalogue active flag are deliberately not applied; the reason is fail-open avoidance; cite #8613 / #8710 so the next reader finds the ruling instead of re-deriving it.

Premises to re-verify on origin/main before writing (triage measured these at 6b6b606; main has moved)

• plugin-sharing/src/position-graph.ts projects ['user_id','valid_from','valid_until'] and drops rows on isGrantActive — window enforced.
• plugin-approvals/src/approval-service.ts expandPositionUsers projects ['user_id'] only, applies no window filter, and the doc block immediately above it ends "so an approval routes to exactly the users the sharing engine would expand for the same position." That sentence is what you are fixing.
• The omission is per-read, not a missing dependency: isGrantActive is imported and is applied to sys_approval_delegation in the same file. Keep that fact in the comment — it stops a later reader explaining the gap away as an oversight of availability.

⚠️ Line numbers on the card and in triage's note are stale by construction — anchor on the symbol and the sentence text.

---

Generated by Claude Code

[qq9340100]

{
  "issue": 8838,
  "status": "done",
  "branch": "claude/issue-8838-approvals-position-parity-claim",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/8864",
  "premise_still_valid": true,
  "summary": "All three PM premises re-verified at today's main (20067c56b), unchanged: plugin-sharing's PositionGraphService projects valid_from/valid_until and drops rows on isGrantActive; approvals' expandPositionUsers projects ['user_id'] alone with no window filter; isGrantActive is imported in the same file and IS applied to sys_approval_delegation. Nobody has added the filter. Replaced the false parity sentence with a doc block that states routing reads the raw directory, that neither the ADR-0091 D2 window nor the sys_position.active flag is applied, that the reason is fail-open avoidance, and cites #8613 / #8710 plus the maintainer ruling verbatim; the per-read-not-a-missing-dependency fact is kept. Routing behaviour is untouched and no fork arose on the D2 axis. One measured refinement to triage's ⭐ reading, reflected in the comment: the sys_member limb is not a divergence — sharing's TeamGraphService.expandRoleUsers reads it raw too, the table has no window columns, and isGrantActive reads an absent bound as unbounded, so no filter could act there in either direction; the limb is covered explicitly plus a pointer on expandMembershipTierUsers, which is also reachable directly as an org_membership_level approver. Took the optional cross-reference on PositionGraphService (10 lines, declared): sharing-rule-service.ts already names the approvals carve-out, but only from the call-site gate — the helper the approvals comment actually points at said nothing. Third axis worth the PM's eye: sys_position.active is gated at the sharing RULE EVALUATOR's call site (positionConfersAccess), not inside PositionGraphService, so the old sentence was false on two axes, not one.",
  "tests": "Union run AFTER the final commit, at HEAD 0ebf102f8; all output redirected to files, never piped. pnpm check:nul-bytes → OK (5871 text files, no raw control bytes) + its 75-assertion self-test. pnpm check:test-source-alias → OK (72 packages). pnpm check:type-source-resolution → OK (76 packages). pnpm check:i18n → first run exited 1 with PREREQUISITE NOT MET on an unbuilt @objectstack/cli, recorded as NOT MEASURED; built the CLI (turbo, 55 tasks) and re-ran → 'check-i18n-bundles: OK (9 package(s) — all bundles in sync, no undeclared authoring keys)', plugin-approvals and plugin-sharing both listed in sync. Dependency closure built first (pnpm --filter '<pkg>^...' build, exit 0) before any test. typecheck both packages → both Done. Tests: plugin-approvals 23 files / 484 tests passed; plugin-sharing 24 files / 599 tests passed. Every heavy command wrapped in flock -E 99 -w 480 /tmp/os-heavy-verify.lock (the lock was held by a parallel agent at the start; queued and acquired in-turn, no 99 exits). Gates re-derived after the final commit with node scripts/pm/dispatch-gates.mjs against the actual changed paths: it named exactly the PM's four families and no others, so no addition to report. No ablation applies — the change is comment-only and ships no behaviour.",
  "open_questions": [],
  "out_of_scope_findings": [
    "filed as #8863: the approvals side of the #8710 carve-out has NO negative test pin — plugin-sharing pins both directions (including 'the ADDRESSING primitive stays a RAW directory read'), but nothing asserts that approvals' expandPositionUsers keeps expanding an expired sys_user_position holder, so an agent adding the window filter there would turn no test red. Labelled `finding`, no `pm:queue`, unassigned; searched open issues by keyword and file path first, no duplicate."
  ]
}

Draft PR #8864 is open with the skip-changeset label applied and read back (labels now size/s, skip-changeset — size/s was a bot's, preserved in the union). CI convergence is the PM's per the 2026-08-10 ruling; gate status at handback is in_progress.

---

Generated by Claude Code

[qq9340100]

ACCEPT — PR #8864. The ruling was honoured (routing behaviour untouched, no filter added), both union sources are covered, and the two readings that reshaped the deliverable are correct. I re-verified both myself on origin/main rather than taking the report's word:

• TeamGraphService.expandRoleUsers (plugin-sharing/src/team-graph.ts:70) projects fields: ['user_id'] — the sharing side reads sys_member raw too.
• positionConfersAccess is a private method at sharing-rule-service.ts:1072, called at :1013 — the sys_position.active gate really does live at the rule evaluator's call site, not inside PositionGraphService. Zero hits for active in position-graph.ts, with a valid control (expandPositionUsers matches there), so that zero is real rather than a broken pattern.

⚠️ Correction: the reading I dispatched on the sys_member limb was wrong, and it was mine to carry

My dispatch order stated, inheriting triage's star reading, that "sys_member has no validity window at all, so even under the rejected option the parity claim would stay false on that limb." That inference does not survive measurement. It compared "approvals reads sys_member raw" against an imagined filtered sharing side that does not exist: TeamGraphService reads it raw as well, the table carries no window columns, and isGrantActive treats an absent bound as unbounded. So on that limb the two implementations already agree, and no filter could act there in either direction.

The practical difference matters, which is why this is worth correcting rather than quietly absorbing: under my version the limb was a permanent residual defect the comment could only apologise for; under the measured version it is a limb that is genuinely in parity and simply needs saying so. The dev was asked to falsify the assumption and did. Same class of error as the one I logged earlier tonight on this card family — a plausible inference stated in the voice of a measurement.

The third axis is a real addition to the card

The old sentence was false on two axes, not one. The card and triage both measured only the ADR-0091 D2 window; sys_position.active is the second, and it is invisible from PositionGraphService because its gate is at the evaluator's call site. A comment repaired for the window axis alone would still have been false. That is exactly the failure mode this card exists to end.

On the cross-reference

The 10 lines on PositionGraphService's class doc are outside the primary file, were declared in the report, and were pre-authorised in the dispatch order conditional on earning their place. They earn it on the report's own argument: sharing-rule-service.ts already names the approvals carve-out, but only from the call-site gate — the helper the approvals comment actually points at said nothing. A reader arriving there to "share the helper" now learns the approvals copy exists and is deliberately different. Accepted.

Gate discipline

check:i18n refused first with PREREQUISITE NOT MET on an unbuilt @objectstack/cli, was recorded as NOT MEASURED, the CLI was built, and the re-run is the measured green. That is the correct handling — a gate that refuses is neither a pass nor a failure. Union re-derived after the final commit against actual changed paths and run with output redirected to files, never piped.

No ablation, correctly. A comment-only change ships no behaviour, so there is nothing that could be made red — and saying so plainly beats inventing a ceremonial one. The gap that does follow from it is recorded rather than swallowed: #8863 (filed unassigned, finding) notes the approvals side of the #8710 carve-out has no negative pin, so an agent adding the window filter here would turn no test red. That is the hole through which this defect returns, and it is now a card instead of a paragraph.

PR flips ready + auto-merge once every job's latest run concludes success/skipped.

---

Generated by Claude Code