Skip to content

fix(plugin-approvals)!: an approval node refuses to open when a team or position directory read fails or a queue expansion rejects, and the ownership-queue reads page to completion - #30288

Merged
objectstack-fleet[bot] merged 9 commits into
mainfrom
claude/project-thread-1wujme
Nov 5, 2026
Merged

objectstack-fleet[bot] merged 9 commits into
mainfrom
claude/project-thread-1wujme

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Requested by Bill · project thread

domain:server#4 · os-bill · session_01Utp6CZMkHvuDeVYgTD73qF

维护者速读

  • 改了什么: 审批节点里 team、position 审批人的名录读取(含跨组织读过滤)失败,或 queue 审批人展开时报错,节点不再开单:四种 onEmptyApprovers 策略(含 fallback 名单)一律拒开,报可重试的 APPROVER_DIRECTORY_UNREADABLE,点名是哪次读取失败。队列成员与队列列表改为按 id 分页读完,不再在第 10,000 行截断。
  • 为什么改: 以前名录表一出故障,节点就当这些人不存在:只开给读得到的人、转给 fallback 或管理员,甚至自动通过(裁定 6599731698)。队列超过 1 万成员时,第 10,001 人静默不在队列里。
  • 风险与回滚: 只在名录读取失败时多拒开一次,流程可在名录恢复后重跑;department、BU、成员等级、manager 不变,决定时的旧路径保留字面值(有 pin)。队列服务自身读成员表失败时仍答「无人」,queue 审批人照旧不占位,留待第 2 部分(等 spec: IOwnershipQueueService lets the caller ask for a refusal on a failed member read (onFailedRead), which ADR-0142 D2 as amended needs for the D6 and D7 consumers #30251)。回滚即撤 PR。
  • 先看: approval-service.ts 的 ApproverDirectoryUnreadableError 与 resolveApproverSpec catch 里的一处 throw;ownership-queue-service.ts 三处 readMembershipToCompletion。
  • 两分钟验证: pnpm --filter @objectstack/plugin-approvals exec vitest run src/approval-service.test.ts -t "refuses the node",预期 18 条全过;pnpm --filter @objectstack/plugin-sharing exec vitest run src/ownership-queue.test.ts -t "page to completion",预期 5 条全过。
  • 需要你做的: 无,席位落地;第 2 部分等 spec 车道的 spec: IOwnershipQueueService lets the caller ask for a refusal on a failed member read (onFailedRead), which ADR-0142 D2 as amended needs for the D6 and D7 consumers #30251。

Refs #29604 (part 1)

Clause-②: yes (narrowing) — an approval node no longer opens when a team or position approver (static, or an expression approver with resolveAs team / position) cannot be expanded because a directory read or the cross-organization read filter failed, or when a queue approver's expansion rejects. Under every onEmptyApprovers policy, the fallbackApprovers slate included, it now fails the node with a retryable APPROVER_DIRECTORY_UNREADABLE: error naming the read, where it used to open on the rest of the slate, on the fallback slate or on the administrators, or auto-approve. The registered ownership-queue service does not reject yet: a failed member read inside it still answers nobody until #29604 part 2 (#30251), so a queue approver refuses here only when expandMembers itself rejects. Every other input answers as before. Nothing is widened: the new error class is not on the package entry.
Metadata-protocol: none
Skills-docs: none. No page in skills/** or content/docs/** states what an approval node does when a directory read fails, so no text moves.

What changes

This is part 1 of #29604, as the seat ruled (route A, comment 6648946388): the parts that do not wait on the spec contract. The queue's own refusal on D6 and D7 is part 2, behind the spec-lane card #30251, and will carry Fixes #29604.

1. The approval node refuses to open when a team or position directory read fails, or a queue approver's expansion rejects (ruling 6599731698, letter B; ADR-0142 D6 as amended).

  • ApproverDirectoryUnreadableError (plugin-approvals, not on the package entry) carries approverType, approverValue, failedRead and retryable: true, and the original error as cause. Its message starts APPROVER_DIRECTORY_UNREADABLE: (the message-prefix family this service already uses for NO_APPROVERS:), names the approver, the organization and the read, and says that the node is not opened on the rest of the slate, the fallback slate or the administrators, and is not auto-approved. It never says NO_APPROVERS.
  • In resolveApproverSpec, the catch that records directoryFailure now throws that error first when the approver type is team, position or queue. Because the throw sits in the catch, it runs before the queue branch's "expanded to nobody" return, which used to answer an unreadable queue as an empty one. That is the reorder the ruling asks for. On this head it fires for a queue only when expandMembers rejects: the registered OwnershipQueueService still answers its own failed member reads with nobody until part 2, so a queue backed by it adds no slot on such a failure, as before.
  • resolveExpressionApprovers throws the same error for resolveAs: 'team' and 'position', since those values are read by the same expanders. Its ADR-0105 D9 filter call now sits in its own try and refuses the same way, naming the filter's read; any other failure there fails the node as it always did.
  • The fallback slate is expanded by the same resolver, so a fallbackApprovers slate whose own team or position read fails refuses too, rather than degrading to admin_rescue.
  • The position expander's sys_member limb (the transition read) is a failed read like any other, as the carried item says (pinned on a real sys_member).
  • The approval node executor already turns a thrown error into { success: false } with the runtime failure class, which is retryable. approval-node.ts is not touched.
  • expandApprovers takes onDirectoryFailure: 'refuse' | 'literal' (default 'refuse'). Only decideNode's pre-snapshot re-resolution passes 'literal': it runs for requests opened before the approver snapshot existed, after the decision's action row is written, so a refusal there would fail a decision that is already recorded. It keeps the literal slot it always kept.
  • The static resolver's cross-organization read filter (bounded) sits inside its try, so its failure is now rethrown as the cross-organization read filter (…) could not be read: …, and the warning and the refusal name that read rather than the directory's (carried item). One helper words that read for the static and the expression resolver. filterApproversWhoCanRead itself still answers its own failed sys_member read by keeping the slate (ADR-0105 D9), so in practice this naming is reached only when the filter throws past its own catch.

2. The ownership-queue reads page to completion (the 10,000-row reading, 6591670782).

queuesNaming, authoredQueueNames and authoredMemberSpecs read sys_queue / sys_queue_member through readMembershipToCompletion (team-graph.ts, the keyset walk the team and position reads already use since #29503), where each read one page of QUEUE_READ_LIMIT = 10000 rows and answered on it. QUEUE_READ_LIMIT is gone. What a failed read answers is unchanged until part 2: the catch keeps its warn-once and contributes nobody. The warning carries the read's reason only: the driver's error, or for a walk that could not vouch for completion its sentence without the walk's own "so the expansion refuses instead", which is not this service's answer yet. A walk that fails on a later page now answers like a failed read, never with the pages already read. authoredQueueRow is a one-row lookup by name and stays as it is.

Tests

  • approval-service.test.ts, new top-level describe "a team, position or queue approver whose directory read fails refuses the node (ADR-0142 D6)", 14 cases on the shared fake engine with one table's reads made to throw:
    • team (sys_team_member) and position (sys_user_position) × admin_rescue, auto_approve, fail, fallback: rejects with ApproverDirectoryUnreadableError (approverType, approverValue, retryable: true, message prefix, failedRead names the driver error, never NO_APPROVERS), and no sys_approval_request, sys_approval_approver or sys_approval_action row is written. 8 cases.
    • Beside a staffed user approver, the node still refuses.
    • A fallback slate naming a team whose read fails refuses, with the primary slate readable and empty.
    • A queue whose expandMembers rejects refuses, and the "expanded to nobody" warning is not logged.
    • Control: the readable slate opens as before.
    • Control: a failed department read still opens the node (not named by the ruling).
    • decideNode on a pre-snapshot request still decides while the team read fails.
  • Existing integration pins on driver-sql moved from "keeps the literal" to the refusal, each followed by the 'literal' reader keeping the old slot and warning: team-position-holders-complete (team, position), member-tier-holders-complete (position whose sys_member limb fails; org_membership_level keeps its literal), business-unit-holders-complete (expression with resolveAs: 'team').
  • approval-service.test.ts, a second new top-level describe "a failed cross-organization read filter refuses the node, named as that read (ADR-0142 D6)", 4 cases on the shared fake engine and the cross-org scenario of approver-cross-org.integration.test.ts. filterApproversWhoCanRead is wrapped by a pass-through spy declared at the top of the harness (vi.mock of ./approver-org-scope.js, every other case runs the real filter through it): a static position approver and an expression approver resolving as position, each targeted at the group with organization: '$root'. Each reaches the group CFO when the filter answers (control), and refuses with ApproverDirectoryUnreadableError whose failedRead is exactly the filter's sentence when the filter rejects, writing no request.
  • ownership-queue.test.ts, new top-level describe "the queue reads page to completion (plugins: a queue sharing recipient still revokes grants on a member-table outage, because the queue expander answers "nobody" for every consumer; refuse for the sharing-grant pass and the approver slate (ADR-0142 amendment first) #29604)", 5 cases. Four at 10,001 rows: a queue's last member is expanded, is a member, and lists the queue in queuesOf; an organization's 10,001st queue is listed; queuesNaming finds the queue naming the recipient on row 10,001; a read failing on its second page answers [] / false with one warning carrying the page's error. The fifth: a full page whose last row has no id answers [], and its one warning carries the reason without the walk's "refuses instead" clause. The test double learned $gt and orderBy, which the keyset walk sends. The existing fixture rows gained ids, which a keyset walk needs.

Ablations, one per new refusal (scripts/ablation-replace.mjs, wrap mode, under os-verify-lock.sh)

Every ablated test imports its subject by relative source path, so no build sits between the mutation and the run. Each restore was proven (blob == HEAD, git diff HEAD empty).

# mutation red which
A1 static refusal limited to queue 13 of 485 the 8 team/position × policy cases, beside-a-person, fallback slate, and the three driver-sql refusal pins
A2 static refusal skips queue 1 of 485 the queue case
A3 expression refusal off 1 of 485 resolveAs: 'team' on driver-sql
A4 decide's 'literal' removed 1 of 485 the pre-snapshot decide case
A5 the three queue reads cut at 10,000 rows 3 of 46 member, queue and naming past one page
A6 a failed page answers with the pages read 3 of 46 the second-page case, and two existing failed-read pins
B1 the expression filter's refusal off 1 of 482 the expression filter case
B2 the static filter's failure rethrown raw 1 of 482 the static filter case
B3 the queue warning keeps the walk's closing clause 1 of 47 the incomplete-walk case

A1 to A6 ran on 98d62fd6c; B1 to B3 on c18905922, the commit that added them. B1 and B2 ran again on 1a3854d28, after the pins moved into the shared harness: each still reds exactly its one case (1 of 459).

Gates, local, on head c4c64fdee (origin/main merged once, at 98d62fd6c)

node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 62 families on the changed paths. The whole workspace was built first (turbo run build --filter='!@objectstack/docs', 119 of 119 tasks), and each exit code was written to a file before any pipe, then reconciled:

Run reconciliation — 62 derived, 62 run, 0 NOT-MEASURED, 0 UNRUN.
✓ dispatch-gates --ran: 62 derived famil(ies) accounted for — 62 run, 0 NOT-MEASURED (a DERIVED zero — all 62 recorded an exit code and none of them is 3).

All 62 exited 0. Also exit 0: the four artifact-roster gates the derivation marks as keeping their roster under a changed directory (check:authz-resolver, check:changeset-fixed, check:error-code-casing, check:filter-alias-parity), pnpm lint, pnpm --filter @objectstack/plugin-approvals --filter @objectstack/plugin-sharing typecheck (the test-layer ledger included), and both packages' full suites (plugin-sharing 1392 passed). The same 62 also exited 0 on 98d62fd6c.

On 1a3854d28 (the cross-org pins moved into the shared harness, a test-only change): plugin-approvals 1465 passed, its typecheck (test-layer ledger included) exit 0, node scripts/check-engine-double-contract.mjs exit 0, pnpm check:objectql-double-limit exit 0, pnpm lint exit 0.

Dogfood battery, once, on head 98d62fd6c (the title carries ! and the declaration a narrowing arm)

Not re-run after the review rounds: since 98d62fd6c the source changes are the expression resolver's filter catch and the queue warning's text; the rest is tests and the changeset.

The whole workspace built, then pnpm --filter @objectstack/dogfood test under os-verify-lock.sh:

 Test Files  1 failed | 418 passed | 3 skipped (422)
      Tests  1 failed | 3491 passed | 36 skipped (3528)
os-verify-lock: VERDICT command-exit 1 · held the lock 1445s (24m05s)

The one failure is not this change's. test/permission-set-clone-door.dogfood.test.ts › "a platform set (the lock-and-clone rule's commonest source) copies into its shipping package's pair" expects 200 from cloning member_default and receives 422 INVALID_METADATA: the clone grants on sys_comment and sys_activity, which the permission validator says no package registers. It fails the same way alone (1 failed | 10 passed), and fails the same way with both of this PR's source files (approval-service.ts, ownership-queue-service.ts) restored to origin/main 771b2db2 and both packages rebuilt (the built dist then carries 0 occurrences of ApproverDirectoryUnreadableError and 4 of QUEUE_READ_LIMIT). Neither package touches permission sets or object registration. Reported to the seat.

Acceptance notes

  • Part 2 owes: the queue service's own failed reads (it still answers [] / false and logs once), the D7 grant recompute keeping grants, and the D6 slate refusing on a queue whose member table fails inside the service. All three need the call-time signal spec: IOwnershipQueueService lets the caller ask for a refusal on a failed member read (onFailedRead), which ADR-0142 D2 as amended needs for the D6 and D7 consumers #30251 adds to the contract. Part 1's queue refusal fires only when expandMembers itself rejects.
  • Types the ruling does not name keep their answer: department / business_unit / bu, org_membership_level and manager (and resolveAs: 'department') still fall back as before on a failed read, with the plugins: sharing expandRole and approvals role-tier recipients read sys_member at a 10,000-row bound and turn a failed read into an empty list, so a large or unreadable membership silently shrinks the set #29362 warning naming the failure. Pinned for department and org_membership_level.
  • Activation-ledger read, decided: switchedOff keeps answering "switched on" when sys_metadata_activation cannot be read. On the D6 slate and the D7 grant pass that errs toward keeping people, never toward dropping them, so it cannot shrink a slate or revoke a grant. On the admission paths it can let a switched-off position confer access during that outage; that posture predates this card and is not changed here.
  • No error code field. The refusal travels as a message prefix and a class, as NO_APPROVERS: does. A code would enter check:dispatcher-error-vocabulary, whose table lives in packages/runtime, outside the claim.
  • Contract review 6650659075 (PASS, three suggested changes), all taken: the queue wording now names a queue approver whose expansion rejects and says the service's own failed reads answer nobody until part 2; the expression resolver's cross-organization filter refuses as the static one does, pinned on both; the queue warning no longer quotes the walk's "refuses instead". Commits c18905922, 34e14c1ef, 50deed3e8, and c4c64fdee for the engine double ESLint flagged (it declared update / delete the node never sends and ignored limit).
  • Incremental review 6651366716 (PASS, two suggested changes), both taken: this body's Clause-②: line and summary use the changeset's wording, and the cross-org pins live in the shared approval-service.test.ts harness (1a3854d28).
  • Assignee and reviewers: none set. The dispatch forbids GitHub MCP writes, so the session's usual reviewer step is left to the seat.

Verification

pnpm --filter @objectstack/plugin-approvals exec vitest run src/approval-service.test.ts -t "refuses the node"
pnpm --filter @objectstack/plugin-sharing exec vitest run src/ownership-queue.test.ts -t "page to completion"

Expect 18 and 5 passed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh

… position or queue directory read fails; queue reads page to completion

Part 1 of #29604 (seat ruling 6648946388, route A).

- plugin-approvals: a failed directory read behind a team, position or
  queue approver (and an expression's resolveAs team/position value)
  throws ApproverDirectoryUnreadableError, so openNodeRequest opens on no
  slate under any onEmptyApprovers policy, the fallback slate included.
  decide's pre-snapshot re-resolution keeps the literal slot.
- plugin-sharing: the queue service's authored sys_queue / sys_queue_member
  reads go through the shared keyset walk instead of a 10,000-row cut; a
  failed read keeps today's answer until part 2.

Claude-Session: https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh
Co-authored-by: Claude <noreply@anthropic.com>
…ery policy and the queue reads past one page

The approval node refuses to open, and writes nothing, when a team, position
or queue approver's directory read fails, under admin_rescue, auto_approve,
fail and fallback alike, beside a staffed person, and on a fallback slate. A
department failure and decide's legacy re-resolution keep their answers.

The queue reads find the 10,001st member, queue and naming row, and a read
that fails on its second page answers as a failed read does today.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh
…prover directory, plugin-sharing pages its queue reads

Also states the request row the legacy-decide pin reads as present, which the
test-layer type-check ledger counts.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh
@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

16 anchor(s) derived from 2 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 103 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 13 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json bfca0efb79ac5df2049f392cdecd3bbeeea9ec61 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 5a47c49ef795781562a3ff3b012f5b573aa56354 — the merge of head 1a3854d288f1e5e2bac081863ee5281783bfe012 into base bfca0efb79ac5df2049f392cdecd3bbeeea9ec61, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 5a47c49ef795781562a3ff3b012f5b573aa56354 && git checkout 5a47c49ef795781562a3ff3b012f5b573aa56354
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin bfca0efb79ac5df2049f392cdecd3bbeeea9ec61 1a3854d288f1e5e2bac081863ee5281783bfe012 && git checkout -B drift-repro bfca0efb79ac5df2049f392cdecd3bbeeea9ec61 && git merge --no-ff 1a3854d288f1e5e2bac081863ee5281783bfe012

node scripts/docs-audit/affected-docs.mjs --json bfca0efb79ac5df2049f392cdecd3bbeeea9ec61

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#4 · os-bill · session_01Utp6CZMkHvuDeVYgTD73qF

Contract-review-claim: session_01Utp6CZMkHvuDeVYgTD73qF · head 98d62fd · 2026-11-05T05:14Z

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#4 · os-bill · session_01Utp6CZMkHvuDeVYgTD73qF

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 98d62fd6cacef2d6489674e725d65c5a72d458ed
Local-runs: none
Scope: packages/plugins/plugin-approvals/src/approval-service.ts, packages/plugins/plugin-approvals/src/approval-service.test.ts, packages/plugins/plugin-approvals/src/business-unit-holders-complete.integration.test.ts, packages/plugins/plugin-approvals/src/member-tier-holders-complete.integration.test.ts, packages/plugins/plugin-approvals/src/team-position-holders-complete.integration.test.ts, packages/plugins/plugin-sharing/src/ownership-queue-service.ts, packages/plugins/plugin-sharing/src/ownership-queue.test.ts + packages/plugins/plugin-approvals/src/approval-node.ts, packages/plugins/plugin-approvals/src/index.ts, packages/plugins/plugin-sharing/src/team-graph.ts, packages/plugins/plugin-sharing/src/objects/sys-queue-member.object.ts, packages/types/src/keyset-walk.ts, packages/spec/src/contracts/ownership-queue-service.ts, packages/spec/src/automation/approval.zod.ts, docs/adr/0142-ownership-queue-membership-claim-and-assignment-rules.md (D2, D6, D7 as amended)

Rendered 2026-11-05T05:37Z. Inputs: card #29604 (body and all eleven comments, the rulings 6594432222, 6599731698, 6600766777 and the seat's route-A ruling 6648946388 included), PR #30288 (body, eight-path file list, net diff against main at the head's merge base 771b2db2), and the check-runs on the head. The narrowing face: no schema and no governed path in the diff; the floor is every path outside the prose faces, listed first above, then the files the hunks cite, read at the head (refs/pm-review/30288-head, fetched into a ref of my own; nothing checked out, built or run). CI on the head at posting time: every gate-carrying context (Build Core, Test Core 1–8, Lint · *, Type Check · *, Governed Surface Queue Guard, Check Changeset, Dogfood Regression Gate) concluded success; the UI and opt-in contexts skipped; no failure.

① Derived judgments

  1. Accept-set narrowing at openNodeRequest — a team or position approver (static, or an expression approver with resolveAs team / position) whose directory read throws now rejects with ApproverDirectoryUnreadableError from resolveApproverSpec's directory catch (head :2757) and from resolveExpressionApprovers' catch (:3035), ahead of slateNamesAPerson and every onEmptyApprovers arm. The fallback slate (:4368) and the administrators slate (:4448) go through the same expandApprovers with the default 'refuse'. Nothing is written before the primary expansion (:4309): the reads before it are the existing-request lookup, the live record and the submitter's acting addresses. Right — ADR-0142 D6 as amended; ruling 6599731698, letter B.
  2. The queue arm — the throw sits in the catch, so it runs before the "expanded to nobody" return (:2803), which now also distinguishes a failed read from an empty queue on the literal reader. Right as the ADR's execution point. On this head it fires only when expandMembers itself rejects, and OwnershipQueueService on the same head never rejects: expandSpec (:322) catches to [] and authoredMemberSpecs (:516) to an empty map. So in the shipped composition a sys_queue / sys_queue_member / member-table outage behind a queue approver still opens the node on its policy. That is part 2's gap (route A, spec: IOwnershipQueueService lets the caller ask for a refusal on a failed member read (onFailedRead), which ADR-0142 D2 as amended needs for the D6 and D7 consumers #30251), not made worse here — see ② and ④.
  3. decideNode's pre-snapshot re-resolution passes 'literal' (:5113). Right: D6 puts the refusal at the node's open; a decision whose action row is already written must not be failed by a read made for its tally, and main's answer there is kept unchanged.
  4. The cross-organization read filter (bounded, :2715–2731) now rethrows a filter failure named as that read, inside the static resolver's try, so it refuses under the class naming the right read. Right (carried item 6600766777). In the expression resolver the same filter runs outside the try (:3047) — ④ should-fix 2.
  5. Types the ruling does not name — department / business_unit / bu, org_membership_level, manager, resolveAs: 'department' — keep the literal slot and the plugins: sharing expandRole and approvals role-tier recipients read sys_member at a 10,000-row bound and turn a failed read into an empty list, so a large or unreadable membership silently shrinks the set #29362 warning. Right: 6599731698 names queue, team, position only, and canonicalApproverType folds only the deprecated role spelling, so the refusing set is closed.
  6. Public surface — ApproverDirectoryUnreadableError is exported from the module and absent from src/index.ts (a named export list, no star export); package.json exposes only .. expandApprovers / resolveApproverSpec are private, their onDirectoryFailure parameter internal; QUEUE_READ_LIMIT was module-private. packages/spec is untouched and IOwnershipQueueService unchanged. Right: nothing published widens.
  7. plugin-sharing paging — queuesNaming, authoredQueueNames and authoredMemberSpecs read through readMembershipToCompletion (keyset on id, pages of 10,000, $and-composed cursor, a refusal on a failed page or a walk it cannot vouch for). A widening at the consumer: a queue's 10,001st member is now a member. Right — the contract on main declares "the queue's members" with no bound, so the cap was an undeclared narrowing and this delivers the declared capability (reading 6591670782). A failed or unverifiable walk answers [] / false with one warn carrying the driver's reason — today's posture, which the route-A ruling says part 1 keeps. authoredQueueRow (one row by name) and switchedOff (bounded by names.length) are rightly left alone. hasAnyQueue now reads an organization's authored queue rows to the end to answer a boolean where it read at most 10,000; same work class, no contract change.
  8. The error carries no code; it travels as class plus the APPROVER_DIRECTORY_UNREADABLE: prefix, the NO_APPROVERS: precedent in the same service — consistent. retryable: true is informational: approval-node.ts (:245) turns any throw into { success: false, error } and nothing in the tree reads the property; the ruling's "retryable" is met by nothing having been written, so re-running the flow is the retry.
  9. Tests — the new pins are top-level describes in the components' shared harness files (ruling 6583487846); the three driver-sql pins move from "keeps the literal" to the refusal and keep the literal reader's old answer beside it; the queue double gained $gt and orderBy, and the page-2 failure case is faithful to keysetWalk (the first page carries the bare where, the second the $and cursor).

② Semver level

.changeset/29604-approver-directory-refusal-queue-paging.md: patch for @objectstack/plugin-approvals and @objectstack/plugin-sharing; ! in the title; a BREAKING banner; Clause-②: yes (narrowing) and Metadata-protocol: none line-initial in both the PR body and the changeset; one adr-0087: not-required (no-migration-prescription) marker, a category the gate names. No spec key, export or config field moves, so the Post-Task Checklist's rule applies: a non-protocol breaking change ships patch with its banner and disposition. Matches what the diff publishes. The (narrowing) arm reads the narrowing face whichever word precedes it (reviewFace), which is the face this record serves. The changeset's queue sentence is read against the tree in ④ should-fix 1.

Clause-②: yes (narrowing) — right for the diff: the approval node's accept set narrows at open, and nothing on a published entry widens.

③ Boundary flags

  • Dev report 6648911579, open question 1 (the route): answered by the seat in 6648946388, route A; this PR is part 1 under it. Closed.
  • Dev report 6650309375: open_questions: []. The decisions it states in the PR's acceptance notes, each read: (a) part 2 owes the service's own refusal, D7 and the D6 queue arm — consistent with the ruling; (b) unnamed types keep their answer — consistent with 6599731698; (c) the activation-ledger read keeps "switched on" when it fails — carried item 6600766777 asked that this be decided and said; it is said in the PR and the service header, and on D6/D7 it errs toward keeping people, never dropping them: accepted; (d) no code field — accepted on the NO_APPROVERS: precedent; (e) assignee and reviewers unset — the seat's hand-over step.
  • Out-of-scope finding (dogfood permission-set clone door, 422 cloning member_default): carrier seat, noted; the head's Dogfood Regression Gate check-run is success, so CI did not reproduce it. Not this PR's.
  • Skills-docs: none — no page under skills/** or content/docs/** states the node's failed-read behaviour; the Docs Drift Check listed nothing. Accepted.
  • Nothing outstanding; nothing escalated.

④ Design judgment

Read against ADR-0142 D2 / D6 / D7 as amended on main and the cited code. The node refusal makes the spec's own refusal real: D6 as amended says the node refuses to open under every policy, with a class of its own naming the read, before slateNamesAPerson hands the slate to onEmptyApprovers, and the tree does exactly that, the queue early return consulting directoryFailure first. No declared capability is taken from a consumer: onEmptyApprovers's declared input is the EMPTY slate (D2's "an empty expansion is a fact"); a failed read was never a declared input to it. The paging change delivers a capability the contract declared and the implementation had cut. Tried to break it: every expandApprovers caller takes 'refuse' except the recorded-decision tally; no write precedes the primary expansion; the fallback and administrator slates are expanded by the same resolver; resolveAs admits only user / department / position / team, so the refusing set is closed; the literal reader answers as main did. The meantime after part 1 alone: a queue whose member tables fail still expands to nobody inside plugin-sharing, so D7 still revokes on it and the D6 queue arm still opens on the policy (the measured table in 6648911579). Both are ruled to part 2; nothing in this diff moves a failed-read answer, so part 1 leaves no new inconsistency and nothing less safe than main.

Findings:

  • should-fix 1 — the changeset's queue sentence (Prime Directive 10: a claim no wider than the enforcement). The changeset — the text that ships as CHANGELOG.md — says a failed directory read behind a queue approver refuses the node, and its Clause-② line says the same. On this head that holds only when the ownership-queue service's expandMembers rejects, which the shipped service never does until part 2; a sys_queue_member or member-table outage behind a queue approver still opens on the policy. Narrow the queue wording in the changeset body and the Clause-② line to what ships ("a queue approver whose expansion rejects"), with one line saying the service's own member-table failures keep answering nobody until part 2. The PR body's acceptance note says this already; the changeset must too.
  • should-fix 2 — the expression resolver's filter sits outside the refusing try. resolveExpressionApprovers calls filterApproversWhoCanRead after its catch (:3047), so a cross-organization filter failure on a resolveAs: 'team' | 'position' slate propagates as a raw error: the node still fails (approval-node.ts :245) but not under ApproverDirectoryUnreadableError and not naming the read, unlike the static path's bounded. Move the filter inside the try, or wrap it as bounded does, so both paths refuse under the one class. Narrow window: the filter keeps the slate on its own sys_member read failure.
  • should-fix 3 — a log line that states the wrong consequence. readFailureReason logs a truncated walk's own sentence, which ends "so the expansion refuses instead" (team-graph.ts holderReadRefusal, expansion kind), while the queue service then answers nobody. Log the walk's reason without the consequence clause, or word the queue warning to say the answer it gave.

No must-fix.

Implemented-by: claude/project-thread-1wujme
Reviewed-by: session_01Utp6CZMkHvuDeVYgTD73qF

VERDICT: PASS

…tion read filter refuses the node, named as that read

The ADR-0105 D9 read filter on an expression approver's team and position
values ran outside the refusing try, so its failure failed the node with the
raw error. It now refuses with ApproverDirectoryUnreadableError naming the
filter's read, as the static resolver's filter does; one helper words that
read for both. Pinned on both paths with the filter made to reject.

The ownership-queue warning for a walk that could not vouch for completion
logs the reason only, without the walk's "so the expansion refuses instead",
since the queue still answers nobody.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh
…jects until part 2

The ownership-queue service does not reject a failed member read yet, so the
narrowing names a queue approver whose expansion rejects, and says the
service's own failed reads still answer nobody until the contract can carry
the refusal.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh
…the node config takes it

The test-layer type-check program admits no new file with errors.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh
…ours limit and carries no write verbs it is never sent

The node under these pins never updates or deletes through the engine, so
the double declares neither, and its find cuts at the caller's limit as the
engine does.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#4 · os-bill · session_01Utp6CZMkHvuDeVYgTD73qF

Contract-review-claim: session_01Utp6CZMkHvuDeVYgTD73qF · head c4c64fd · 2026-11-05T06:04Z

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#4 · os-bill · session_01Utp6CZMkHvuDeVYgTD73qF

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: c4c64fdeec4c995324b0fff29eef614c5f984024
Local-runs: none
Scope: packages/plugins/plugin-approvals/src/approval-service.ts, packages/plugins/plugin-approvals/src/approver-directory-refusal-cross-org.test.ts, packages/plugins/plugin-sharing/src/ownership-queue-service.ts, packages/plugins/plugin-sharing/src/ownership-queue.test.ts, packages/plugins/plugin-approvals/src/approval-service.test.ts, packages/plugins/plugin-approvals/src/business-unit-holders-complete.integration.test.ts, packages/plugins/plugin-approvals/src/member-tier-holders-complete.integration.test.ts, packages/plugins/plugin-approvals/src/team-position-holders-complete.integration.test.ts + packages/plugins/plugin-approvals/src/approver-org-scope.ts, packages/plugins/plugin-approvals/src/approver-cross-org.integration.test.ts, packages/plugins/plugin-sharing/src/team-graph.ts, docs/adr/0142-ownership-queue-membership-claim-and-assignment-rules.md (D2, D6, D7 as amended)

Rendered 2026-11-05T06:26Z — a delta review on the four commits above the head my record 6650659075 passed (98d62fd6, PASS, three should-fixes): c1890592 (the expression resolver's cross-organization filter refusal), 34e14c1e (changeset wording), 50deed3e and c4c64fde (the new pin file's double). No force push; the earlier head's four commits are unchanged beneath them. Inputs: the diff 98d62fd6..c4c64fde (five paths, +189/−18) read at refs/pm-review/30288-head2 (fetched into a ref of my own; nothing checked out, built or run), the current PR body, the PR and card comments since my record (the seat's claim 6651048324 for this head), and the check-runs on c4c64fde. Everything my first record judged on the untouched paths stands; this record judges the delta and the three should-fixes. CI on this head at posting time: every gate-carrying context (Build Core, Test Core 1–8, every Lint · * including Lint · ESLint and Lint · CI, release & engine gates, every Type Check · *, Governed Surface Queue Guard, Check Changeset, Dogfood Regression Gate) concluded success; the UI and opt-in contexts skipped; no failure.

① Derived judgments

  1. should-fix 2 — resolved. resolveExpressionApprovers now wraps filterApproversWhoCanRead in its own try (head2 :3056–3072): on resolveAs team / position under 'refuse' it throws ApproverDirectoryUnreadableError with failedRead from the new shared crossOrgFilterReadFailure (:1188), the same wording the static resolver's bounded uses (:2706), expression: source set so the message names the expression form, and cause carried. Every other case (department, or the 'literal' reader) rethrows the raw error exactly as this head's predecessor and main did, so no admission or decision path moves. The accept set at the node is the one my first record judged, with the expression path's filter now inside it. Right.
  2. should-fix 3 — resolved. readFailureReason logs the driver's cause when the walk carries one and, for a walk that could not vouch for completion, the walk's sentence with its closing "…, so the expansion refuses instead." clause cut (regex anchored on refuses instead. at the end; the clause holds no period before its own, so the cut stops at the reason). Pinned by the new case in ownership-queue.test.ts (a full page whose last row has no id: expandMembers answers [], one warning, error ending at the walk's reason). The coupling to team-graph.ts's wording is string-level, but the pin will say so the day the wording moves. Right; the answer itself (nobody, until part 2) is unchanged.
  3. should-fix 1 — resolved in the published text. The changeset title, Clause-② line, BREAKING banner and a new "Not yet" bullet now say a queue approver refuses only when expandMembers itself rejects, and that the registered ownership-queue service still answers nobody on a failed member read until plugins: a queue sharing recipient still revokes grants on a member-table outage, because the queue expander answers "nobody" for every consumer; refuse for the sharing-grant pass and the approver slate (ADR-0142 amendment first) #29604 part 2 (spec: IOwnershipQueueService lets the caller ask for a refusal on a failed member read (onFailedRead), which ADR-0142 D2 as amended needs for the D6 and D7 consumers #30251). That is what the tree does. Right. The PR body's own line-initial Clause-②: line and its reader summary still carry the earlier wording ("team, position or queue approver … because a directory read failed") — ④ should-fix 1 of this record.
  4. New pin file approver-directory-refusal-cross-org.test.ts: a module-level vi.mock of ./approver-org-scope.js with a pass-through vi.fn(actual.filterApproversWhoCanRead) and mockRejectedValueOnce per failing case; controls prove the group CFO is reached through the filter once, the refusal cases prove the class, approverType / approverValue, retryable, the filter-named failedRead, and no request row written — for the static position approver and the expression one alike. mockClear in beforeEach. The double answers find honouring limit, insert, count, and the two hook verbs, nothing it is never sent (the lint reds the coordinator named came from the earlier shape; the head's Lint · ESLint and Lint · CI, release & engine gates are success). Right as a pin; its placement is ④ should-fix 2.
  5. Public surface: unchanged by the delta. crossOrgFilterReadFailure is a module-private function; index.ts and package.json are untouched; packages/spec untouched.

② Semver level

Unchanged: patch × 2, ! title, BREAKING banner, Clause-②: yes (narrowing) and Metadata-protocol: none line-initial in the changeset, one adr-0087: not-required (no-migration-prescription) marker. The changeset's narrowing sentence now matches the tree (① item 3). Check Changeset is success on this head. Matches what the diff publishes.

Clause-②: yes (narrowing) — right for the diff; the delta narrows nothing further on a published surface and widens nothing (the expression filter case was already a node failure, now under the class).

③ Boundary flags

  • No new dev report and no open_questions on the delta; the three commits answer the three should-fixes of record 6650659075 and nothing else was raised. The seat's claim 6651048324 names this head.
  • Nothing outstanding; nothing escalated.

④ Design judgment

The delta tightens the design my first record passed without moving any ruled answer: the one class now covers the cross-organization filter on both resolvers, the published text now claims exactly what ships for queue, and the queue warning no longer asserts a refusal the service does not make. Tried to break it: the expression try rethrows raw under 'literal' and for department, which is main's behaviour on both; crossOrgFilterReadFailure takes directoryOrg and organizationId in the same order at both call sites; the regex in readFailureReason cannot strip a driver's own message (it is only reached when the walk carried no cause, and only cuts a tail ending in refuses instead.); the new pin's vi.mock is a pass-through, so its controls exercise the real filter. The interim after part 1 (D7 still revokes, the D6 queue arm still opens on a member-table outage inside plugin-sharing) is as my first record stated, ruled to part 2, and now said in the shipped CHANGELOG text as well.

Findings:

  • should-fix 1 — the PR body's declaration still carries the pre-delta queue claim. The line-initial Clause-②: line in the PR body (and the maintainer summary above it) still reads "a team, position or queue approver … cannot be expanded because a directory read failed no longer opens", which the changeset no longer says and the tree does not do for the shipped queue service. Align the body's Clause-②: line and the summary with the changeset's wording (a body edit, no code; read the body back after the write per the footer rule).
  • should-fix 2 — the new pin lives in a new file. approver-directory-refusal-cross-org.test.ts pins ApprovalService, whose shared harness is approval-service.test.ts, and its header names approver-cross-org.integration.test.ts as the same scenario; the rule (ruling record 6583487846, AGENTS.md Build & Test) puts a new pin for a component with a shared harness file in that file as a top-level describe. The module mock is a pass-through and would not disturb the harness's other cases, so fold the two describe bodies into approval-service.test.ts (or the scenario's integration file), or state in the header the measured reason a separate file is needed.

No must-fix.

Implemented-by: claude/project-thread-1wujme
Reviewed-by: session_01Utp6CZMkHvuDeVYgTD73qF

VERDICT: PASS

…n the shared approval-service harness

Moved as one top-level describe into approval-service.test.ts, on the shared
fake engine, with the pass-through spy on the read filter declared there; the
separate file is gone.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh
@objectstack-fleet objectstack-fleet Bot changed the title fix(plugin-approvals)!: an approval node refuses to open when a team, position or queue directory read fails, and the ownership-queue reads page to completion fix(plugin-approvals)!: an approval node refuses to open when a team or position directory read fails or a queue expansion rejects, and the ownership-queue reads page to completion Nov 5, 2026
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#4 · os-bill · session_01Utp6CZMkHvuDeVYgTD73qF

席位意见:ACCEPT(#29604 第 1 部分)。审批节点上 team、position 审批人的名录读取(含跨组织读过滤)失败,或 queue 审批人展开时报错,节点不再开单,改报可重试的 APPROVER_DIRECTORY_UNREADABLE;队列成员读取按 id 分页读完,不再在 10,000 行截断。已发布的 ownership-queue 服务读成员表失败时仍答「无人」,D6 queue 臂与 D7 留给第 2 部分(等 spec 卡 #30251),卡保持派发。

席位核对

Head-sha: 1a3854d288f1e5e2bac081863ee5281783bfe012 · read 2026-11-05T07:02Z

Item Reading
Shape Draft → main, author objectstack-fleet[bot] (relay pr_create); attribution block, then signature line, Refs #29604 (part 1) (no closing keyword: part 2 waits for #30251), Clause-② / Metadata-protocol at line start. 8 files, +595/−67. Title narrowed by the seat (2026-11-05T06:40Z) to "…when a team or position directory read fails or a queue expansion rejects…", matching the changeset.
Ruling Record 6599731698 on #29604, letter B (ADR-0142 D6 as amended by PR #29865). Seat route A (6648946388): part 1 here, part 2 behind spec card #30251.
Gates (check-runs on this head) 38 success, 17 skipped, 0 failure, 0 pending; the one cancelled row is a Check Changeset run superseded by the title edit, whose rerun is success. Dev's local runs on this head: plugin-approvals 1465 pass, typecheck 0, pnpm lint 0, check-engine-double-contract 0, objectql-double-limit 0; the B1/B2 ablations re-run at the pins' new place, each one red, restored.
Contract review CONTRACT_REVIEW_TIER record 6650659075 on 98d62fd6: PASS, three should-fixes. Delta record 6651366716 on c4c64fde: PASS, the three resolved, two new should-fixes, no must-fix.
Delta since the record (c4c64fde..1a3854d2, read by the seat) Test-only: the four cross-org pins move as one top-level describe into the shared harness approval-service.test.ts (on makeFakeEngine), with the pass-through vi.mock('./approver-org-scope.js') at the harness top; approver-directory-refusal-cross-org.test.ts is deleted. No source file moves. The PR body's Clause-②: line and summary now carry the changeset's wording. Both should-fixes of 6651366716 are closed.
Semver patch × 2 with !, BREAKING banner, Clause-②: yes (narrowing), Metadata-protocol: none, one adr-0087: not-required (no-migration-prescription) marker; judged right by both records.
Landing Not governed (check-governed-merges --pr 30288: 0 of 8 paths), 662 lines ≤ 3000. Seat lands: pr_ready + automerge_enable.

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review November 5, 2026 07:03
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge November 5, 2026 07:03
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Nov 5, 2026
Merged via the queue into main with commit 13c0c5b Nov 5, 2026
56 of 57 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-1wujme branch November 5, 2026 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant