Skip to content

docs(adr): ADR-0142 D2 tells an empty queue expansion from a failed member read; the D6 slate and the D7 grant pass refuse a failed read - #29865

Merged
os-zhuang merged 4 commits into
mainfrom
claude/project-thread-xcbmm5
Nov 3, 2026
Merged

os-zhuang merged 4 commits into
mainfrom
claude/project-thread-xcbmm5

Conversation

@objectstack-fleet

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

Copy link
Copy Markdown
Contributor

Requested by Warren · project thread
Fixes #29738
Part of #29604 — step 1 of ruling B (record 6594432222), with the D6 target set by the maintainer's second ruling (record 6599731698, B). The code half stays open on the parent and is dispatched after this PR merges.

domain:server#4 · os-warren · session_01JQwxbYJgdcYRwx5iQa5Ksc

Clause-②: no (ADR text only; no published accept set moves)
Metadata-protocol: none

维护者速读

What changes

One file, docs/adr/0142-ownership-queue-membership-claim-and-assignment-rules.md. Text only: no code, no schema, no changeset (skip-changeset — docs/adr/** publishes nothing).

  • Status line. A second note in the first note's shape, citing ruling records 6594432222 and 6599731698 on 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 (letter B both times): D2 tells an empty expansion (a fact) from a failed read behind the queue (a fault). The admission consumers (D3, D4, D5) keep "nobody" on a failed read. The D7 grant recompute refuses it. The D6 approver slate refuses it, and the approval node refuses to open. See the notes in D2, D6 and D7.
  • D2. A new paragraph after "An empty expansion is a fact, not a fault". An expansion is empty when every member specification was read and reached nobody (an unstaffed position, an emptied team, a queue with no member rows, a name that resolves nowhere). A read fails when any read the expansion makes cannot be done: the queue's own row (sys_queue, through which an authored queue resolves), the member rows (sys_queue_member), and the member tables a specification reads (sys_user_position, sys_team_member, sys_business_unit_member, the BU tree). A queue that could not be resolved is not a queue that resolves nowhere. A table absent from the composition is absence, not failure. The consumer says which answer it is owed when it calls (how is the code half's): the admission consumers (D3, D4, D5) keep "nobody", logged at warn, because for them nobody denies; the recompute consumers (D6, D7) refuse, as a thrown failure naming the read, never [], null or false.
  • D6. A note after item 5: item 2's "expanded to nobody" is the empty case. A failed read behind the queue refuses the slate, and the approval node refuses to open under every onEmptyApprovers policy (admin_rescue, auto_approve, fail, fallback). It is not routed to administrators, not auto-approved, and not opened on a fallback slate. It fails loudly, with its own error class (not NO_APPROVERS, not "expanded to nobody"), names the failed read, and is retryable. There is no parked or auto-retry state. The ruling puts team and position approvers in the same class. Execution point: resolveApproverSpec's directoryFailure; the queue branch's early return consults it first; openNodeRequest refuses before slateNamesAPerson hands the slate to onEmptyApprovers. Item 2's "never written again" stays whole, and item 2 now spells the literal by the queue's machine name (queue:NAME).
  • D7. A note after item 5: item 1's "an empty queue grants nothing and warns once" is the empty case. A failed read behind the queue refuses the grant pass; the rule-wide passes (evaluateRule, evaluateAllForRecord, revokeRuleGrantsForRetiredRecipients) and the per-user recompute (reconcileRuleForUser, reached from recipient-membership-recompute.ts through the queue's holder check) each stop that rule's pass before any revoke, keep the rule's existing grants exactly as they were, log at warn naming the rule and the read, and let the record's other rules reconcile — 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 isolation shape. A grant is re-evaluated on the rule's next pass. The membership-write look-through (queuesNaming) revokes nothing and is outside this amendment.

Nothing else in the record moves: D1, D3, D4, D5, D8, D9, the alternatives, open items, verification notes and execution plan are untouched, apart from D6 item 2's literal spelling, aligned to the queue's machine name. D5 already says a failed read during evaluation leaves the row and warns; the D2 paragraph cites it rather than restating it.

Why

The measure the order asked for, read at origin/main c76c0e04 (the branch base; the merge to 42ff86f6 moved none of these files):

  • Today's expander answers nobody on a failed member read, for every consumer. packages/plugins/plugin-sharing/src/ownership-queue-service.ts: expandSpec L305–318 (catch → warn once → []), holderCheck L394–404 (catch → warn once → false), authoredMemberSpecs L466–497 (catch → warn once, authored members count as nobody), authoredQueueNames L439–463 (catch → []). The module header L33–39 declares that posture; ADR-0142 did not.
  • Consumers by D-number.
    • D3 access: packages/plugins/plugin-sharing/src/sharing-service.ts callerQueues L890–909 (queuesOf, catch → []) and sitsInCallersQueue L925–952 (isMember); packages/plugins/plugin-security/src/kernel-queue-membership.ts resolveOnto L89–113 (queuesOf, catch → fails closed).
    • D4 claim: packages/plugins/plugin-security/src/security-plugin-authz-middleware.ts L1517 (isMember admits a member's insert naming a queue); a claim's D3 basis is sitsInCallersQueue above; security-plugin-explain-access.ts L512 (isMember for the explain reason).
    • D5 pool: packages/objectql/src/assignment-rules.ts L291–301 (expandMembers; [] parks with the "no members" warning, a throw parks with "members could not be read").
    • D6 slate: packages/plugins/plugin-approvals/src/approval-service.ts expandQueueUsers L3326–3331 (expandMembers, purpose addressing), called from resolveApproverSpec L2621–2627; the queue "expanded to nobody" branch L2674–2690; the graph types' failure class L2712–2718 → warnExpansionFailed L2742–2759; holderReadRefusal L1142.
    • D7 grants: packages/plugins/plugin-sharing/src/sharing-rule-service.ts expandRecipient queue branch L2176–2186 (expandMembers; feeds evaluateRule L1253, the per-record pass L1413–1425 with its per-rule catch, revokeRuleGrantsForRetiredRecipients L1573–1600) and ruleReachesUser L1745–1749 (isMember, the per-user recompute from recipient-membership-recompute.ts L300–323 with its per-pair catch).

So a queue sharing recipient is diffed against a set missing its members on an outage and its grants are revoked, and a queue approver routes to a shrunk slate, while every other recipient read refuses since PR #29602 (#29503). The ruling (B, record 6594432222 on #29604) puts the ADR text first.

Tests

None: a docs-only diff. No package is touched, so no build closure (①) and no package test (②) is owed; the derived gate families (③) are under Verification.

Acceptance notes

Verification

Head 589be305: commit 9ea89d0e (round 2, after contract review 6599389641 and ruling 6599731698) merged with origin/main 74fd4bf6. The round-2 gate readings (21 derived commands plus pnpm lint, all exit 0; --ran 21/21, 0 NOT-MEASURED) are in the os-dev-report addendum 6600440519 on #29738. The round-1 readings on 2f923ae4 follow; each exit code was captured before any pipe and recorded as a CMD :: exit N line.

  • node scripts/pm/dispatch-gates.mjs --commands (no paths; the change set is derived from the merge base): 21 commands. A first derivation on the pre-merge head 8c3d7b9c printed STALE TREE (61 commits behind origin/main, 10 derivation inputs moved), so origin/main was merged and the list re-derived clean before any gate ran.
  • Prerequisite for check:doc-formula-expressions: pnpm exec turbo run build --filter=@objectstack/formula --filter=@objectstack/lint under scripts/pm/os-verify-lock.sh — VERDICT command-exit 0 · held the lock 168s · waited 0s.
21 derived gates, all exit 0, plus pnpm lint (eslint . --no-inline-config) exit 0
  • node scripts/check-adr-links.mjs :: exit 0
  • node scripts/check-adr-links.mjs --self-test :: exit 0
  • node scripts/check-closing-keyword-parity.mjs :: exit 0
  • node scripts/check-closing-keyword-parity.mjs --self-test :: exit 0
  • node scripts/check-comment-mask-corpus.mjs :: exit 0
  • node scripts/ci/plan-test-core-shards.mjs --self-test :: exit 0
  • pnpm --filter @objectstack/lint run check:doc-formula-expressions :: exit 0
  • pnpm check:adr-anchors :: exit 0
  • pnpm check:adr-index :: exit 0
  • pnpm check:adr-symbol-anchors :: exit 0
  • pnpm check:ci-filter-parity :: exit 0
  • pnpm check:closed-ceremony-text :: exit 0
  • pnpm check:cross-package-test-inputs :: exit 0
  • pnpm check:doc-authoring :: exit 0
  • pnpm check:driver-memory-census :: exit 0
  • pnpm check:gitlink-declared :: exit 0
  • pnpm check:nul-bytes :: exit 0
  • pnpm check:pm-governed-merges :: exit 0
  • pnpm check:pm-prior-rulings :: exit 0
  • pnpm check:refd-timer-probe :: exit 0
  • pnpm check:watch-hint-literal :: exit 0
  • pnpm lint :: exit 0
  • node scripts/pm/dispatch-gates.mjs --ran gates-ran.txt: ✓ dispatch-gates --ran: 21 derived famil(ies) accounted for — 21 run, 0 NOT-MEASURED (a DERIVED zero — all 21 recorded an exit code and none of them is 3); pnpm lint is listed as outside this card's derivation (recorded, not an error).
  • Tier: --tier answers Model tier — MANDATORY: claude-fable-5-1 for docs/adr/**; this thread ran at that tier at effort xhigh. Changed lines 64 (+61 / −3) vs the 3,000 human-merge threshold: under.
  • Tests: none owed (no package touched). Dogfood battery: n/a — no ! in the title and no (narrowing) arm.
  • CI: the PR's check-runs are the authority for every family CI runs; none is re-run here. Their state at the report's writing is in the os-dev-report on docs(adr-0142): amend D2 — a failed member read refuses for the D7 grant recompute and the D6 approver slate, and still answers nobody for admission consumers #29738.

Human Minimum

  • 1. Schemas start as Zod; packages/spec holds no runtime logic — not touched.
  • 2. Every name I added uses the casing for its kind — no names added.
  • 3. No formatter ran; pnpm lint passes — see Verification.
  • 4. One issue, one branch off an up-to-date main, one pull request.
  • 5. A change that publishes carries a changeset — nothing publishes; skip-changeset, both declaration lines above.
  • 6. packages/spec touched — no.
  • 7. No hand edits to content/docs/references/, packages/*/CHANGELOG.md or content/docs/releases/.
  • 8. Title, body and comments are in English, the 维护者速读 block excepted by the repo's Communication rule.
  • The Before Submitting list in CONTRIBUTING.md is done.

Generated by Claude Code

os-warren and others added 2 commits November 3, 2026 08:17
…ember read; the D6 slate and the D7 grant pass refuse a failed read

An empty expansion stays a fact the consumers each decide; a failed
member-table read is a fault. The admission consumers (the access path,
the claim admission, the assignment pool) keep "nobody" because for
them nobody denies. The approver slate and the sharing-recipient grant
pass refuse it: the slate is held as the queue literal with a warning
that names the failed read, and the grant pass stops before any revoke
and keeps the rule's existing grants. Status line carries the second
amendment note. Text only: no code, schema or changeset moves here.

Claude-Session: https://claude.ai/code/session_01JQwxbYJgdcYRwx5iQa5Ksc
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#4 · os-warren · session_01JQwxbacSXJQqAztgt4UMrS

Contract-review-claim: session_01JQwxbacSXJQqAztgt4UMrS · head 2f923ae · 2026-11-03T08:43Z

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#4 · os-warren · session_01JQwxbacSXJQqAztgt4UMrS

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 2f923ae47993c2d89b5d32080676d35f1fb88aa4
Local-runs: none
Scope: docs/adr/0142-ownership-queue-membership-claim-and-assignment-rules.md + packages/plugins/plugin-sharing/src/ownership-queue-service.ts, packages/spec/src/contracts/ownership-queue-service.ts, packages/plugins/plugin-approvals/src/approval-service.ts, packages/plugins/plugin-approvals/src/approver-address.ts, packages/plugins/plugin-sharing/src/sharing-rule-service.ts, packages/plugins/plugin-sharing/src/recipient-membership-recompute.ts, packages/plugins/plugin-sharing/src/bu-tree-recompute.ts, packages/plugins/plugin-sharing/src/queue-hooks.ts, packages/plugins/plugin-sharing/src/sharing-service.ts, packages/plugins/plugin-security/src/kernel-queue-membership.ts, packages/plugins/plugin-security/src/security-plugin-authz-middleware.ts, packages/plugins/plugin-security/src/security-plugin-explain-access.ts, packages/objectql/src/assignment-rules.ts

Read: card #29738 (body, comments 6594482347, 6597517535, 6599161425), parent #29604 (body, comments 6588472222, 6589473271, 6591670782, 6594432222 — the ruling, letter B), PR #29865 body and file list, the net diff against main at the head (one file, +51/−1), the 46 check-runs on the head, and the cited code at origin/main 4b71a82b (the branch base 42ff86f6 is 2 merges behind it; approval-service.ts, the authz middleware, team-graph.ts and business-unit-graph.ts moved there, none of it at the sites this ADR cites — the amendment is judged against the live tree).

① Derived judgments

  • Face rules: docs/adr/** is a governed path, so CONTRACT_REVIEW_TIER and Tier H (the maintainer's approval) — right; the record's tier line and the card's claim name it.
  • Accept sets: none move. The diff is ADR text — no schema, no code, no test, no changeset, no generated output. The status line keeps Proposed as its first keyword, so scripts/adr-index.mjs renders the same row (the new note sits past the index's truncation point) — right; pnpm check:adr-index and check:adr-symbol-anchors were green in the dev's local run and the CI docs-guard lane is success.
  • No ! and no narrowing arm on THIS PR — right: the ADR narrows no published behaviour; the D6/D7 doors narrow when the code half lands, and the ruling record says that half carries ! and the dogfood battery.
  • The two new symbol anchors (approval-service.ts#warnExpansionFailed, ownership-queue-service.ts#queuesNaming) resolve to live symbols at origin/main — right.
  • Check-runs on the head: 46, all completed — 26 success, 20 skipped by path filter, 0 failure. Governed Surface Queue Guard, Check Changeset, PR-body guards, Check Documentation Links, Check PR Size, every Lint and Type Check lane, Plan Test Core shards, Test Core (1/8), Test UI and the Dogfood Regression Gate are success. The gate verdicts are green.
  • One derived claim the text makes on a public contract: D2 says "the consumer says which it is owed when it calls". The spec contract already carries QueueExpansionPurpose = 'access' | 'addressing', an axis about switched-off positions (D6 passes addressing; D7 passes nothing). Refuse-vs-nobody is a second, independent axis (D7 wants access-purpose AND refuse). The ADR leaves the spelling to the code half — right to leave, named here so the code half does not overload purpose.

② Semver level

③ Boundary flags

④ Design judgment

docs/adr/0142-…md, judged against its first consumer (the #29604 code half) and the execution plan, in the cited code at origin/main.

What holds.

  • D2's empty/failed split matches the expander: expandSpec catches and answers [] once-warned, holderCheck catches and answers false, authoredMemberSpecs catches and counts authored members as nobody; the "absent table is absence" clause matches the sibling reads (isObjectAbsentError in readRowsToCompletion).
  • Admission consumers keep nobody and nobody denies — verified at every site the D2 bullet names: D3 sharing-service.ts#callerQueues (catch to []), kernel-queue-membership.ts#resolveOnto (catch, fails closed), sitsInCallersQueue through isMember; D4 the authz middleware's insert arm (isMember false → denyQueueWrite), the explain engine's queueMember; D5 assignment-rules.ts parks on both the [] branch and the thrown branch. Right.
  • D7's isolation claim is true of the tree at every named pass: evaluateRule → evaluateResolvedRule → desiredGrantsForRule expands BEFORE reconcile, so a throw revokes nothing; evaluateAllForRecord has 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 per-rule catch; revokeRuleGrantsForRetiredRecipients expands before its chunked delete and both callers catch per rule (queue-hooks.ts#recomputeQueueRecipientRules, bu-tree-recompute.ts onGraphWrite, each warning with the rule); reconcileRuleForUser asks ruleReachesUser → isMember before its revoke loop and recipient-membership-recompute.ts catches per rule-and-user pair; evaluateAllRulesForObject catches per rule. queuesNaming's catch answers [], which only leaves the queue rules out of affected — it revokes nothing, as the note says. The BU module's "unreadable queue counts as affected" fallback then hands the rule to a pass that refuses. The code half owes only the refusing expandMembers/isMember. Right.
  • D6's mechanism is reachable: the queue branch sits inside the resolver's try, the catch sets directoryFailure, and queue: is admitted by no acting path (approver-address.ts, position is the one literal prefix). One thing the code half must do that the note does not spell: today the type === 'queue' early return (the "expanded to nobody" warning, no slot) runs BEFORE the directoryFailure check, so a thrown expansion would still be reported as nobody; the queue branch has to consult directoryFailure first. The ADR prescribes the target, so this is the execution plan's, not a finding.

Must-fix 1 — D6 note: two sentences are false against openNodeRequest. "the slot is held as the queue: literal … so the request waits rather than advances past the queue's people" and "onEmptyApprovers decides an empty slate as before and is not reached through a held slot". slateNamesAPerson treats every type:value literal as no person, so a request whose slate holds only the held queue:NAME slot goes to onEmptyApprovers: on the default admin_rescue it opens on the administrators (past the queue's people), on auto_approve it is approved on an outage. That is the #29503 class as it stands for team today, and it is what the ruling asked the note to follow — so the fix is textual: say the literal is held and the failure is warned (never "expanded to nobody"), that beside concrete people the request's fate is the node's behaviour (unanimous waits on the override), and that a slate with no concrete person still takes the node's onEmptyApprovers route, as a team whose read failed does. Tier H text that promises a wait the code does not deliver is the one thing this record must not carry to the maintainer.

Must-fix 2 — D2 note: the fault list omits the sys_queue row read, so the ruled defect stays reachable one table over. resolveQueue reads sys_queue through authoredQueueRow (catch → null) and authoredQueueNames (catch → []); for an authored queue a failed sys_queue read makes expandMembers answer [] and isMember false under the amendment's own "a name that resolves nowhere" — the EMPTY case. The recompute consumers then do exactly what the ruling refuses: D7 revokes every grant the queue gave, D6 warns "no queue of that name exists there". The amendment's principle ("Nothing is granted and nothing is revoked on an answer the expander could not stand behind") and its enumeration disagree, and the code half is bound to the enumeration. The list already holds sys_queue_member, which no specification reads either, so the intent is plainly "every read the expansion makes"; write that, name sys_queue in it, and say a queue that could not be resolved is not a queue that resolves nowhere.

Should-fix 1 — D2: "a thrown failure naming the specification and the table" — the sys_queue_member (and sys_queue) reads precede any specification; say "naming the read" so the code half is not asked to name a specification that does not exist.

Should-fix 2 — D6: item 2 spells the literal queue: + id; the note spells it queue: + name. The resolver writes a.value, a machine name under D1. Align item 2 (or say the two are one spelling) so the code half's pin has one literal to match.

Implemented-by: claude/project-thread-xcbmm5
Reviewed-by: session_01JQwxbacSXJQqAztgt4UMrS

VERDICT: FAIL

os-warren and others added 2 commits November 3, 2026 09:31
…ctory read; D2 names every read the expansion makes

Round 2 of the contract review. D6: on a failed read behind a queue,
team or position approver the approval node refuses to open under every
onEmptyApprovers policy, loudly, with its own retryable error class
naming the failed read; the execution point is named (the resolver's
directoryFailure, the queue branch's early return, openNodeRequest before
slateNamesAPerson). D2: the fault list covers the sys_queue row read, so
a queue that could not be resolved is not a queue that resolves nowhere;
a refusal names the read. Item 2 spells the never-written literal by the
queue's machine name. Status line cites both ruling records.

Claude-Session: https://claude.ai/code/session_01JQwxbYJgdcYRwx5iQa5Ksc
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#4 · os-warren · session_01JQwxbacSXJQqAztgt4UMrS

Contract-review-claim: session_01JQwxbacSXJQqAztgt4UMrS · head 589be30 · 2026-11-03T09:51Z

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#4 · os-warren · session_01JQwxbacSXJQqAztgt4UMrS

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 589be3056a8aefc66f6273c9d0cd060d6979d3b4
Local-runs: none
Scope: docs/adr/0142-ownership-queue-membership-claim-and-assignment-rules.md + packages/plugins/plugin-approvals/src/approval-service.ts, packages/plugins/plugin-approvals/src/approver-address.ts, packages/spec/src/automation/approval.zod.ts, packages/plugins/plugin-sharing/src/ownership-queue-service.ts, packages/spec/src/contracts/ownership-queue-service.ts, packages/plugins/plugin-sharing/src/position-graph.ts, packages/plugins/plugin-sharing/src/team-graph.ts, packages/plugins/plugin-sharing/src/business-unit-graph.ts, packages/plugins/plugin-sharing/src/sharing-rule-service.ts, packages/plugins/plugin-sharing/src/recipient-membership-recompute.ts, packages/plugins/plugin-sharing/src/sharing-service.ts, packages/plugins/plugin-security/src/kernel-queue-membership.ts, packages/plugins/plugin-security/src/security-plugin-authz-middleware.ts, packages/plugins/plugin-security/src/security-plugin-explain-access.ts, packages/objectql/src/assignment-rules.ts

Round 2 of review 6599389641 (FAIL on 2f923ae4). Read: card #29738 (body; comments 6594482347, 6597517535, 6599161425, 6599743318, 6600440519 — the dev's round-2 addendum), parent #29604 (body; comments 6588472222, 6589473271, 6591670782, 6594432222 and 6599731698 — the two rulings, letter B in both), PR #29865 body (stale round-1 wording; the seat patches it from the addendum's body_amendments and this record does not judge it), file list, comments 6599188795, 6599389641, 6600468056, the net diff against main at the head (one file, +61/−3) and the round-1→round-2 delta (2f923ae4→589be305: the status line, the D2 fault sentences, D6 item 2's literal, the D6 note rewritten whole, one phrase in the D7 note; nothing else moved), the 41 check-runs on the head, and the cited code at origin/main c1af99c0. The head's merge base is 74fd4bf6 (2 commits behind origin/main); none of the cited files differs between 74fd4bf6 and c1af99c0, nor between round 1's reading at 4b71a82b and c1af99c0, so the amendment is judged against the live tree and the D3/D4 sites round 1 read stand unmoved. The whole ADR diff is judged, not the delta alone.

① Derived judgments

  • Face rules: docs/adr/** is a governed path — CONTRACT_REVIEW_TIER and Tier H (the maintainer's approval), as the claim 6594482347 and the card say. Right.
  • Accept sets: none move — ADR text only; no schema, code, test, changeset or generated output. The status line's first keyword is still Proposed, so scripts/adr-index.mjs renders the same row. Right.
  • No ! and no narrowing arm on THIS PR: the record narrows no published behaviour; the D6/D7 doors narrow when 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's code half lands, which ruling 6594432222 marks ! with the dogfood battery. Right.
  • Symbol anchors: the three the delta adds resolve at origin/main — approval-service.ts#resolveApproverSpec (the private method; its directoryFailure local and the directory catch are inside it), approval-service.ts#openNodeRequest, approver-address.ts#slateNamesAPerson (exported) — and the two from round 1 (#warnExpansionFailed, ownership-queue-service.ts#queuesNaming) still do. check:adr-symbol-anchors runs in the Lint · docs & code guards lane, which is success on the head.
  • Check-runs on 589be305: 41, all completed — 25 success, 16 skipped by path filter, 0 failure, 0 in progress. Governed Surface Queue Guard, PR-body guards, Check PR Size, Check Documentation Links, every Lint and Type Check lane, Plan Test Core shards, Test Core (1/8), Test UI, Dogfood Regression Gate and Auto Label are success. Check Changeset is skipped on this head (success on round 1's): the PR carries skip-changeset, and the job's label fast-path short-circuits a payload that already shows it — the same "no changeset owed" verdict, reached the other way. The gate verdicts are green.
  • The onEmptyApprovers list the D6 note names is the enum exactly: approval.zod.ts declares z.enum(['admin_rescue', 'fail', 'auto_approve', 'fallback']).default('admin_rescue') — four members, none missing, none invented. NO_APPROVERS is the fail policy's throw in openNodeRequest, so "not NO_APPROVERS" names a real class to be distinct from.
  • Carried from round 1, still right to leave: the spec's QueueExpansionPurpose (access | addressing) is an axis about switched-off positions, independent of refuse-vs-nobody; the ADR leaves the call's spelling to the code half and the dev's addendum records the warning for 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.

② Semver level

③ Boundary flags

Round-1 findings, each against the new head:

  • Must-fix 1 (replaced by ruling 6599731698) — closed. The D6 note now states the ruled target rather than today's class: node refuses to open under every policy, loud, own error class, names the read, retryable, no parked state, team and position in the same class, replaces the "follows plugins: the rest of the sharing and approvals recipient reads still truncate at 10,000 rows or turn a failed read into "nobody" (sys_team_member, sys_user_position, holdsRole/teamHasMember, queue expandSpec) — #29362 close-out #29503's class" wording. The held-literal reading is withdrawn in the text and in the dev's addendum. Judged word by word under ④.
  • Must-fix 2 (sys_queue reads) — closed. D2's fault sentence now opens "any read the expansion makes" and names sys_queue ("through which an authored queue resolves") and sys_queue_member ahead of the specification tables, with the sentence round 1 asked for: "A queue that could not be resolved is not a queue that resolves nowhere: the first is a failed read, the second is absence." Verified against authoredQueueRow (catch → null) and authoredQueueNames (catch → []): a failed sys_queue read is now a fault under the text, not the empty case.
  • Should-fix 1 (naming a specification that may not exist) — closed: "a thrown failure naming the read, never [], null or false".
  • Should-fix 2 (literal spelling) — closed: item 2 now reads queue:NAME "(the queue's machine name, D1, as the resolver writes an approver's value)" and the note uses the same spelling; D1's "referenced everywhere by its machine name … an approver's value (D6)" and the resolver's ${type}:${a.value} agree.

The dev's addendum 6600440519: open_questions is empty — the round-1 question (hold the literal vs no slot) is mooted by the ruling. out_of_scope_findings (the queue service's "Absence is not a fault" header going stale on merge; QUEUE_READ_LIMIT's silent drop; the purpose-axis warning) — all carried on #29604, accepted. deviations — the D6 note no longer cites #29503's class as the outcome (that is the ruling, not a deviation); two lines past 120 columns (no gate reads line length; check:doc-authoring and the docs-guard lane are green); one worker restart (process). None touches the ADR's meaning. premise_still_valid: true — checked: both rulings on #29604 stand and nothing later supersedes them. body_amendments — nine items the seat applies; the ADR text, not the body, is what this record judges.

④ Design judgment

docs/adr/0142-…md, judged against ruling 6599731698 (B) and ruling 6594432222 (B), its first consumer (#29604's code half) and the cited code at origin/main.

The D6 note states ruling B — phrase by phrase. Trigger: "A failed read behind the queue (D2) refuses the slate, and the approval node then refuses to open" ↔ "when a directory read behind a queue, team or position approver fails, the approval node refuses to open". Policies: "under every onEmptyApprovers policy alike (admin_rescue, auto_approve, fail, fallback): not routed to the organization's administrators, not auto-approved and not opened on a fallback slate" ↔ the ruling's second sentence, and the enum has exactly those four. Failure shape: "an error class of its own — not NO_APPROVERS, not 'expanded to nobody' — that names the read that failed … retryable: the run is re-triggered once the directory reads again" ↔ the ruling's Retryable paragraph. Scope: "the ruling puts a team or position approver whose directory read failed in the same class, so the one node refusal covers the three" — queue, team and position, no narrower; and the D2 bullet plus the D7 note leave D3/D4/D5 and D7 where ruling 6594432222 put them, as the ruling requires. No extra scope: "No parked or auto-retry state is part of this decision; one that is wanted is its own card" is the ruling's own exclusion restated, not a state the ADR adds. Supersession: "it replaces the earlier wording of record 6594432222, under which a refused slate followed the approvals resolver's existing failure class" — right, and the history stays readable. The status line says the same in one clause.

Every execution-point anchor exists and says what the ADR says. resolveApproverSpec: let directoryFailure: string | undefined is declared before the try that wraps every directory expansion (team, BU, position, membership tier, queue, manager) and the cross-org read filter; the catch sets it ("already records the failure" — true, as a local of the resolver). The type === 'queue' branch below the try warns "expanded to nobody" and returns [] BEFORE the if (directoryFailure !== undefined) arm that the graph types reach — so "the queue branch's early 'expanded to nobody' return must consult it first" names a real ordering defect the code half must fix, exactly as the ruling spells it. openNodeRequest: the slate comes back from expandApprovers as a flat string[], and the first decision on it is if (!slateNamesAPerson(approvers)) → emptyPolicy — so "must see it and refuse before slateNamesAPerson hands the slate to onEmptyApprovers" names the exact seam; slateNamesAPerson answers true only when some address on the slate carries no colon, as round 1 read it. "Item 2's 'never written again' stays whole: a failed read writes no queue:NAME literal and no slot of any kind" holds under the target: a node that refuses to open persists no request row, so no slot of any spelling is stored; the empty case keeps item 2's "returns []" and the node's onEmptyApprovers.

D2's fault list against every read the expansion makes. expandMembers → resolveQueue reads the activation ledger (sys_metadata_activation, for a declared name), sys_queue (authoredQueueRow) and sys_queue_member (authoredMemberSpecs); expandSpecs → expandSpec reads, per kind, sys_team_member (TeamGraphService.expandUsers), sys_user_position AND sys_member (PositionGraphService.expandPositionUsers unions the ADR-0057 D4 transition limb through expandRoleUsers; holdsPosition falls to holdsRole the same way), sys_business_unit_member and sys_business_unit (the BU tree); a position kind under the access purpose also reads the ledger (positionActive). The sys_user reads in the two graph services are managerOf, not on the queue path. Today every one of these failures is caught to nobody (expandSpec → [], holderCheck → false, authoredMemberSpecs → no rows) except the ledger, which switchedOff catches to "treated as switched ON for this call". The text's binding clause — "any read the expansion makes" — covers all of it, and the ruled defect is no longer reachable by the enumeration alone, which is what round 1's must-fix asked for. The enumeration itself under-describes the tree in two places — should-fix 1 below.

Admission consumers (D3/D4/D5) — the bullet is unchanged from round 1 and the sites are unmoved: D5 assignment-rules.ts parks on both the [] branch ("no members … the row parks") and the thrown branch ("could not be read — the row parks"), which is what "a row parks in the queue (D5 says so already)" cites, and D5's own paragraph says a failed read leaves the row as it was and logs at warn. Right.

D7 note — the delta changes one phrase ("a failed read behind the queue (D2)"), and the passes it names still hold as round 1 verified: evaluateRule → evaluateResolvedRule → desiredGrantsForRule expands before reconcile; evaluateAllForRecord has the #29362 per-rule catch; revokeRuleGrantsForRetiredRecipients expands before its chunked delete; reconcileRuleForUser asks ruleReachesUser → queues.isMember (the holder check) before its revoke loop, and recipient-membership-recompute.ts catches per rule-and-user pair with "any write to the rule, or a restart" in its own warning. Right.

Should-fix 1 — D2's enumeration names fewer reads than its quantifier. (a) "the member tables a specification reads (sys_user_position, sys_team_member, sys_business_unit_member, the BU tree)" omits sys_member, which a position specification reads as the ADR-0057 D4 transition limb — the limb the filing addendum 6588472222 on #29604 put in this card; a failed sys_member read throws out of expandPositionUsers and is caught to nobody by expandSpec today, the ruled defect one table over. The binding clause covers it, so this is exactness, not a false promise: add sys_member to the parenthesis (or say "and the transition source a position reads"), so the code half's pin list is written from a complete sentence. (b) The activation ledger (sys_metadata_activation) is a read the expansion makes whose failure is NOT answered nobody — switchedOff leaves the declared default (on) for the call, the posture positionConfersAccess takes for the same ledger — and the sentence's reason ("the expander then knows nothing about who is in the queue") does not describe it. Under "any read … refuse", the code half would be asked to make the ledger refuse for D6/D7, diverging from every other reader of that ledger; say in one clause that the ledger read keeps its regime-C posture and is not a failed read in D2's sense, or that it is one. Either is acceptable; silence leaves the code half to choose.

Should-fix 2 — the execution point names the primary slate only. openNodeRequest resolves up to three slates through the same expandApprovers: the primary, a fallback policy's declared fallbackApprovers, and the administrators (as user entries). The note's "openNodeRequest must see it and refuse before slateNamesAPerson hands the slate to onEmptyApprovers" covers a failure on the primary slate; a team or position entry among fallbackApprovers whose read fails is reached inside the fallback arm, after that hand-off, and the ruling's "a directory read behind a … approver fails → the node refuses to open" applies to it equally. One clause — "on every slate the node resolves, the declared fallback slate included" — keeps "under every policy alike" honest for fallback's own slate. Within the ruling, not extra scope; the code half can meet it either way, so should-fix.

For the code half, not findings: the resolver's try also wraps the cross-org read filter (bounded), so a failed read-filter lookup sets the same directoryFailure and will refuse the node under the same seam — consistent with the ruling, worth a pin; and expandApprovers's string[] contract carries no failure today, so "openNodeRequest must see it" means the resolver throws (and the catch re-throws for the recompute case) or the contract grows a carrier — the ADR rightly leaves which to #29604.

Nothing the ADR now promises is false of the tree in a way the code half will not fix; each must-fix of round 1 is closed; the D6 note states ruling B with no wider and no narrower scope.

Implemented-by: claude/project-thread-xcbmm5
Reviewed-by: session_01JQwxbacSXJQqAztgt4UMrS

VERDICT: PASS

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#4 · os-warren · session_01JQwxbacSXJQqAztgt4UMrS

维护者速读

席位核对

ACCEPT · domain:server#4 · head 589be3056a8aefc66f6273c9d0cd060d6979d3b4 · 2026-11-03T10:05Z · governed docs/adr/** (Tier H: check-governed-merges --pr 29865, 1 of 1 path governed, 64 lines); CONTRACT_REVIEW_TIER review PASS on this head (6600699486, round 2 after FAIL 6599389641); final four-piece hand-off to the approvers.

  1. Shape: draft, base main, Fixes #29738, Part of #29604, Clause-②: no, Metadata-protocol: none, skip-changeset. 1 file, +61/−3. The seat refreshed the PR body to this head from the dev's 9 body amendments (addendum 6600440519) via relay issue_patch; the read-back is identical.
  2. Premise measured: today's expander answers nobody on a failed read for every consumer (round-1 report 6599161425, with file:line). The round-2 review verified each execution-point anchor the D6 note names in the live tree: resolveApproverSpec's directoryFailure, the queue branch's early return that runs before it, and openNodeRequest's slateNamesAPerson hand-off.
  3. Gates on this head: check-runs pinned to 589be305: 41 runs, 25 success, 16 skipped by path filter, 0 failed. Check Changeset is skipped because of the skip-changeset label. Dev: 21 derived gates plus pnpm lint, all exit 0; --ran 21/21, 0 NOT MEASURED.
  4. Accepted deviations: two lines are wider than the record's usual wrap (no gate reads line length); the D6 note names the record it replaces, so the history stays readable.
  5. Out of scope: the review's two should-fixes are carried to the 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 code half and do not hold this PR: (a) sys_member (a position's ADR-0057 limb) and the activation ledger's posture belong in D2's read list; (b) the ruling covers a fallback policy's declared fallback slate too. Already on 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: the stale "Absence is not a fault" header and the QUEUE_READ_LIMIT drop. Skills-docs: not owed.
  6. Landing: Tier H ⇒ relay pr_request_reviewers to GOVERNED_APPROVERS (os-zhuang, hotlong) and needs-user-decision on the PR in the same act; stays draft. On an approver's APPROVED: clear the label, pr_ready + automerge_enable, follow to MERGED, landing record on docs(adr-0142): amend D2 — a failed member read refuses for the D7 grant recompute and the D6 approver slate, and still answers nobody for admission consumers #29738, #29604 → pm:queue for its code half.

@os-zhuang
os-zhuang marked this pull request as ready for review November 3, 2026 10:49
@os-zhuang
os-zhuang enabled auto-merge November 3, 2026 10:49
@os-zhuang
os-zhuang added this pull request to the merge queue Nov 3, 2026
Merged via the queue into main with commit d74667c Nov 3, 2026
51 checks passed
@os-zhuang
os-zhuang deleted the claude/project-thread-xcbmm5 branch November 3, 2026 11:14
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/s skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants