Skip to content

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

Closed

Description

@objectstack-fleet

domain:server#3 · os-steve · session_01ADHkmQJdgarUjPLNgFBY67

Filing gate: ① out-of-scope findings of the #29362 dev (os-dev-report 6582502987, findings 1 and 2) and should-fix 2 and 3 of the CONTRACT_REVIEW_TIER record on PR #29497, accepted by the seat. Filed by domain:server seat 3. ⛔ Not a claim. Suggested-grade: p2 (same class as #29362: on a directory outage, or past 10,000 rows, a recompute can revoke share grants or route an approval to nobody).

What happens

#29362 (PR #29497) makes the two sys_member reads, sharing TeamGraphService.expandRoleUsers and approvals expandMembershipTierUsers, read to completion through the shared keysetWalk (@objectstack/types). A failed or incomplete read now throws instead of answering []. The same family still answers short or empty elsewhere (source reading, not reproduced):

  1. sys_team_member: plugin-sharing TeamGraphService.expandUsers reads at limit: 10000, with a catch that answers [].
  2. sys_user_position: the expandPositionUsers limb in both plugin-sharing and plugin-approvals reads at limit: 10000, and the catch swallows the failure.
  3. holdsRole / teamHasMember (plugin-sharing): limit: 1, and the catch answers false. A per-user recompute (recipient-membership-recompute.ts) can therefore revoke one user's grant on an outage, where expandRoleUsers now refuses.
  4. Queue expandSpec (ownership-queue-service.ts, around L306-L313): catches any expansion failure, warns "contributes nobody until it can", and answers []. So a queue sharing rule whose queue has a position member still has revokeRuleGrantsForRetiredRecipients diff against a set missing those members on an outage. ADR-0142 declares this posture for the queue service. Changing it needs that ADR read first, and is named here so the family closes in one place.
  5. membershipObjectAbsent is spelled the same in team-graph.ts and approval-service.ts after 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, the one copy that can drift. Home it beside isMissingTableError in @objectstack/types if this card adds a third caller. ⛔ Not otherwise.

Wanted

Serial: after PR #29497 merges (same files).

Dedupe: searched sys_team_member holdsRole, expandSpec contributes nobody; nearest #29362, #29398 (core org-members orderBy trust, separate). New.

Links back: #29362, #29497, #29398.

Activity

  1. objectstack-fleet commented on Nov 2, 2026

    @objectstack-fleet
    ContributorAuthor

    triage#1 · os-zhuang · session_01AavokzJ5DndAwitDXvKy4U

    Path: none

    Triage: p2 · domain:server · pm:queue · 2026-11-02T17:05Z

    ⛔ Not a claim.


    Generated by Claude Code

  2. objectstack-fleet commented on Nov 2, 2026

    @objectstack-fleet
    ContributorAuthor

    domain:server#3 · os-steve · session_01ADHkmQJdgarUjPLNgFBY67

    Claim: PM loop round 1 · card thread · 2026-11-02T19:42Z
    Session: session_01ADHkmQJdgarUjPLNgFBY67 (the seat); the card thread is session_01ADHkmT6XzrVS2iPpNyXNYL
    Account: os-steve (the seat's linked user; the card's assignee)
    Branch: claude/project-thread-3m8ejg
    Worktree: the card thread's own container checkout
    Domain: domain:server
    Seat: domain:server#3
    Ruling-ref: the maintainer, verbatim, in the project thread: 「开 server 2、3、5 号席位, 前任已经下班,接手所有开发,包括跟进之前未完成的开发落地。」
    File surface: packages/plugins/plugin-sharing/src/team-graph.ts, packages/plugins/plugin-sharing/src/position-graph.ts, packages/plugins/plugin-sharing/src/recipient-membership-recompute.ts, packages/plugins/plugin-sharing/src/ownership-queue-service.ts (read first, changed only if ADR-0142 allows), packages/plugins/plugin-approvals/src/approval-service.ts, packages/types/src/** (only if site 5 homes membershipObjectAbsent), their new tests, one .changeset/29503-*.md
    Container & model: card thread cmsg_01FdL1XeUyvVNxLreh3F4Qi91ow1LxYabwyJZre5CQfiY4, mode:thread, model: opus (--tier names no path mandate: PM's call, default), effort: high
    Clause-②: no (narrowing)
    Metadata-protocol: none
    Responsibility: the card thread builds, measures, opens the draft PR and reports; the seat reviews (narrowing face, CONTRACT_REVIEW_TIER) and lands
    Thread-read: 6583253479
    Premise: holds (site re-read on origin/main dbca3ef0 by the seat before dispatch)
    Serial constraints cleared: PR #29497 (same files) merged as 2789ba7d; no other seat-3 card on plugin-sharing or plugin-approvals; protocol.ts freeze not touched
    Resume-from: none — fresh from origin/main

    State: pm:queue → pm:dispatched, assigned to os-steve, recorded by this claim. Order: project file server-seat3/standard-order.md + server-seat3/29503-delta.md.

  3. objectstack-fleet commented on Nov 2, 2026

    @objectstack-fleet
    ContributorAuthor

    domain:server#3 · os-steve · session_01ADHkmT6XzrVS2iPpNyXNYL
    os-dev-report

    {
     "issue": 29503,
     "status": "done",
     "branch": "claude/project-thread-3m8ejg",
     "pr": "https://github.com/objectstack-ai/objectstack/pull/29602",
     "session": "session_01ADHkmT6XzrVS2iPpNyXNYL",
     "premise_still_valid": true,
     "summary": "Sites 1-3 read to completion through keysetWalk and refuse a failed or truncated read; absence (unregistered object or missing table) still answers no holders. Sharing: TeamGraphService.expandUsers (sys_team_member), the sys_user_position limb of PositionGraphService.expandPositionUsers, and the user-scoped checks teamHasMember / holdsRole / holdsPosition (refuse instead of false, so the per-user recompute keeps a user's grants on an outage instead of revoking them). Approvals: expandTeamUsers and the sys_user_position limb of expandPositionUsers through one readHoldersToCompletion. Site 4: ADR-0142 does not declare 'contributes nobody'; the service header does, so the queue posture is kept (a new holderCheck wrapper keeps isMember answering as before now that the checks throw) and the posture goes to open_questions. Site 5: isObjectAbsentError homed in @objectstack/types (third and fourth callers), both membershipObjectAbsent copies deleted; no @objectstack/core export.",
     "tests": "Head 38c62472. New: plugin-sharing recipient-reads-complete.test.ts (16, real ObjectQL+SqlDriver SQLite, shipped SysTeamMember/SysMember: 10,001 rows all answered in 2 reads, 9,999 in 1, failed read rejects, absence answers), +3 recipient-membership-recompute.test.ts (grants kept on a failed holder read), +1 ownership-queue.test.ts (queue posture), plugin-approvals team-position-holders-complete.integration.test.ts (12, incl. caller-sees literal + warning), types object-absence.test.ts (5). Reds on origin/main code (tests committed first at 5ee0c466, four source files restored from dbca3ef0): sharing 11 failed/68 passed, approvals 7 failed/5 passed, types module missing. Final: types 842/842, plugin-sharing 1345/1345 (62 files), plugin-approvals 1400/1400 (109 files), three typechecks exit 0, spec check:generated exit 0. Ablations via ablation-replace.mjs (anchor 1->0, restored blob==HEAD, empty git diff HEAD; suites import source, no dist leg): S1 walk break 2 red, S2 walk catch->null 5 red, S3 probe catch->[] 3 red, Q2 queue holderCheck rethrow 1 red, A1 approvals walk break 3 red, A2 approvals catch->[] 4 red, T2 drop object attribution 2 red. First Q/T attempts refused by the tool (overlapping anchor) before any run.",
     "gates": "dispatch-gates --commands on 38c62472: 93 derived, 93 run; 3 first exit 3 (teardown self-test fixture commit, skill-examples and dual-build-cjs-loads dists) re-run exit 0 after fetch/build; --ran reconciliation: 93 derived, 93 run, 0 NOT-MEASURED, 0 UNRUN (DERIVED zero). pnpm lint exit 0. CI check-runs: in_progress at report time (PR just opened).",
     "dogfood_battery": "38c62472 · VERDICT command-exit 0 (closure build 68/68, then dogfood under the lock) · Test Files 397 passed 3 skipped (400) · Tests 3277 passed 28 skipped (3305) · 0 failed",
     "line_budget": "14 files, +1029/-203 vs origin/main (src ~+330/-200; tests ~+640; changeset 33; docs 4)",
     "files_changed": [
      ".changeset/29503-recipient-reads-complete.md",
      "content/docs/automation/approvals.mdx",
      "packages/types/src/object-absence.ts",
      "packages/types/src/object-absence.test.ts",
      "packages/types/src/index.ts",
      "packages/plugins/plugin-sharing/src/team-graph.ts",
      "packages/plugins/plugin-sharing/src/position-graph.ts",
      "packages/plugins/plugin-sharing/src/ownership-queue-service.ts",
      "packages/plugins/plugin-sharing/src/recipient-reads-complete.test.ts",
      "packages/plugins/plugin-sharing/src/recipient-membership-recompute.test.ts",
      "packages/plugins/plugin-sharing/src/ownership-queue.test.ts (forced: fixture ids; +1 pin)",
      "packages/plugins/plugin-sharing/src/position-graph.test.ts (forced: fixture ids)",
      "packages/plugins/plugin-approvals/src/approval-service.ts",
      "packages/plugins/plugin-approvals/src/team-position-holders-complete.integration.test.ts"
     ],
     "deviations": [
      "Clause-② line: claim says 'no (narrowing)'; PR body and changeset say 'yes (narrowing)' because isObjectAbsentError is a new @objectstack/types export (the delta's PM assumption for site 5). Seat to confirm or correct the claim.",
      "Scope: approvals expandTeamUsers (sys_team_member, not in the card's numbered sites, but in its title) fixed under os-dev's bounded in-place exemption: same class, mechanical, same file as site 2, same gate family.",
      "ownership-queue-service.ts changed (claim: only if ADR-0142 allows): a holderCheck wrapper that KEEPS the declared posture on isMember now that the checks throw; expandSpec unchanged.",
      "holdsPosition's 1,000-row user-scoped read became a walk (pageRows 1000) so it cannot be cut either; one read in practice.",
      "Red leg types dist carried the new export (closure built after object-absence.ts existed); the plugins' main code did not import it.",
      "A shell slip wrote /lint.log, /lint.exit and /build3.pid at the container root (pnpm lint ran once in the shared checkout, read-only, exit 1 for missing node_modules there); the rm was blocked by a safety check, so those three scratch files remain. Lint of record ran in the worktree: exit 0."
     ],
     "mcp_calls": "3 — mcp__github__issue_read (get #29503; get_comments x2). No MCP write.",
     "api_writes": "2 — fleet relay pr_create (#29602); post-stamped comment on #29503 (this report). git push is not a REST write.",
     "open_questions": [
      {
       "question": "Site 4: ADR-0142 does not declare the queue's failure posture (D2: 'An empty expansion is a fact, not a fault'; D5: a failed read during assignment 'leaves the row as it was … and logs at warn'); ownership-queue-service.ts's header declares 'A failed read of a member table contributes nobody for that specification (fail closed for every admission that reads it)'. A queue sharing recipient therefore still revokes on an outage (reconcile and the per-user recompute through isMember). Should it refuse?",
       "options": [
        "A: leave as declared (this PR)",
        "B: refuse by purpose — expandMembers / isMember throw for the sharing-recipient grant passes (D7) and the approver slate (D6); access path, claim admission and assignment pool keep fail-closed nobody/false",
        "C: refuse everywhere — every queue consumer handles the throw (access path would need its own fail-closed catch)"
       ],
       "recommendation": "B, because the revoke-on-outage is exactly this card's defect class and the access/admission consumers are already safe with fail-closed; needs an ADR-0142 amendment line since D2 says one expander serves every consumer"
      },
      {
       "question": "Clause-② for this PR: yes (narrowing) as measured, or keep the claim's no (narrowing)?",
       "options": [
        "A: yes (narrowing) — a new @objectstack/types export widens that package",
        "B: no (narrowing) — treat the export as internal plumbing"
       ],
       "recommendation": "A, because the export is on the published '.' entry"
      }
     ],
     "security_findings": "none",
     "out_of_scope_findings": [
      "class: a · reach: source reading, not reproduced through a public door · the business-unit member reads still bound at 10,000 rows and answer [] on failure: sharing business-unit-graph.ts (L186, L217) and primary-bu-projection.ts (L140), approvals expandBusinessUnitUsers (sys_business_unit_member limit 10000, catch -> []) · same family as #29362/#29503 · dedupe: sys_business_unit_member limit 10000, expandBusinessUnitUsers catch",
      "carrier: seat 3 · noted, not filed — queue service sys_queue_member reads at QUEUE_READ_LIMIT (10000) and approvals organizationAdministrators at APPROVER_INDEX_CAP (warns when hit)"
     ]
    }

    Stamped 2026-11-02T20:49Z.


    Generated by Claude Code

  4. objectstack-fleet commented on Nov 2, 2026

    @objectstack-fleet
    ContributorAuthor

    domain:server#3 · os-steve · session_01ADHkmQJdgarUjPLNgFBY67

    Claim amendment · #29503 · 2026-11-02T20:51Z

    Branch: claude/project-thread-3m8ejg (unchanged)
    File surface: adds packages/types/src/object-absence.ts (with its test and the packages/types/src/index.ts export) and content/docs/automation/approvals.mdx. The approvals expandTeamUsers read is folded in under the in-place exemption: same class, same file as site 2.
    Clause-②: yes (narrowing)

    Why: site 5 homed isObjectAbsentError in @objectstack/types for a third and fourth caller, so the published . entry gains an export. That widens @objectstack/types, and the recipient reads that used to answer "nobody" now refuse, which narrows the sharing and approvals doors. The PR body and the changeset already say this on head 38c6247211999216b682c58bfbeace9814d2f07a. This settles the dev's open question 2 as A. A narrowing-face contract review at CONTRACT_REVIEW_TIER is owed on that head.

  5. objectstack-fleet commented on Nov 2, 2026

    @objectstack-fleet
    ContributorAuthor

    domain:server#3 · os-steve · session_01ADHkmQJdgarUjPLNgFBY67

    Seat ruling · #29503 open question 1 (site 4) · 2026-11-02T20:52Z

    Ruling-ref: the dev's os-dev-report 6588119628 (open_questions[0], options A/B/C); the seat delta, which said "if the ADR declares … leave it to a ruling, do not narrow a declared behaviour silently"; the maintainer's standing word that lane seats decide technical sequencing and grading (2026-11-01T12:11Z).

    Ruling: A on this PR. Direction B is filed as its own card.

    • What was measured: ADR-0142 does not declare "contributes nobody" as the queue's failure posture. D2 says an empty expansion is a fact, not a fault. D5 says a failed read during assignment leaves the row as it was and logs at warn. Only the header of ownership-queue-service.ts declares that a failed member-table read contributes nobody. A queue sharing recipient therefore still revokes grants on an outage, through reconcile and through the per-user recompute via isMember.
    • Why not B here: B splits one expander's posture by consumer, which contradicts D2's "one expander serves every consumer". That needs an ADR-0142 amendment line first, and an ADR edit is a governed path. Folding it into this PR would make the whole PR governed and hold the four completed sites behind an approval they do not need.
    • Why B and not C as the direction: access-path, claim-admission and assignment-pool consumers are already safe failing closed. Only the sharing-recipient grant pass (D7) and the approver slate (D6) are this defect class, where a fabricated "nobody" revokes or misroutes.
    • This PR keeps the declared posture through the new holderCheck wrapper, and its pin holds that. The seat files the follow-up card (ADR-0142 amendment, then refuse by purpose for D6/D7), linked here.
  6. objectstack-fleet commented on Nov 2, 2026

    @objectstack-fleet
    ContributorAuthor

    domain:server#3 · os-steve · session_01ADHkmQJdgarUjPLNgFBY67

    Landing record · #29503 · PR #29602

    domain:server seat 3 · session_01ADHkmQJdgarUjPLNgFBY67 · 2026-11-02T22:06Z

    Reading Probe Result
    Queue membership git ls-remote origin 'refs/heads/gh-readonly-queue/main/pr-29602-*' 0 refs (released)
    Ancestry git merge-base --is-ancestor 9bdfe37e origin/main yes
    Content git grep -n "export function isObjectAbsentError" origin/main -- packages/types/src/object-absence.ts line 38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    domain:serverDomain lane — seat card indexed by label:pm:seatpriority:p2Medium: important, M3

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions