Skip to content

fix(plugin-approvals,plugin-sharing): the approver a pending step waits on reads the record its request is about - #26397

Merged
objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/project-thread-crp41o
Oct 27, 2026
Merged

objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/project-thread-crp41o

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Leehom · project thread

Fixes #26180

Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么:审批请求处于待审时,当前步骤审批人名单上的人(按用户 id 或账号邮箱)和覆盖权持有人,可以只读打开该请求对应的那一条记录;步骤一结束就收回。
  • 为什么改:审批人此前常常打不开要审的记录,只能看快照就批合同;这次按裁决 A 从名单推导读权,不新增任何元数据键或应用声明。
  • 风险与回滚:新增的是一条只读的行级读权,写入口径不读它;查不到或审批服务缺席时一律不授(fail-closed),也不跨组织。回滚即 revert 本 PR,无数据迁移。
  • 先看:plugin-approvals/src/pending-approver-read-grant.ts(谁有读权、组织围栏、按请求缓存)、plugin-sharing/src/sharing-service.ts 的 buildReadFilter 并入处与 fail-closed 分支、plugin-security/src/explain-engine.ts 的 approval request 归因。
  • 两分钟验证:pnpm --filter @objectstack/dogfood exec vitest run test/approval-pending-approver-record-read.dogfood.test.ts,预期 1 passed:审批人 200、陌生人 404、PATCH 403、批准/驳回后 404。
  • 需要你做的:无,席位落地。

What changes

While an approval request is pending, each person on its current step's slate and the override holder may read the one record the request is about. The grant is derived from state plugin-approvals already keeps (the pending-approver index sys_approval_approver), so nothing is declared: no metadata key, no app setting (ruling 6429330267, letter A).

  • @objectstack/plugin-approvals: new PendingApproverReadGrantSource (pending-approver-read-grant.ts), exposed as ApprovalService.pendingApproverReadGrants(object, context), which answers record id → pending request ids.
    • A person is a slot naming the caller's user id or the email their own account carries, taken from actingAddresses with no positions (the two addresses slateNamesAPerson calls a person). A position:/team: literal names no person and grants nothing.
    • The override holder is isOverrideActor asked with the caller's active organization: every pending request of that organization.
    • Both probes are fenced to the caller's active organization (an org-less caller reaches org-less requests only). The platform admin's override crosses the wall to decide, not to read.
    • The request row is re-read with status: 'pending', and the index only holds rows while pending, so a step that is approved, rejected or advanced stops granting at the next read.
    • Cost: memoized per execution context and per engine write epoch (the permissionSetMemo precedent). A person's first read is one sys_user read and one indexed sys_approval_approver read; a caller on no slate stops there for every object. A caller on a slate, or the override holder, pays one sys_approval_request read per object. Never a read per row. Only settled answers are memoized; a failed read rejects and is asked again next time.
  • @objectstack/plugin-sharing: SharingService.buildReadFilter ORs those record ids into the id $in arm beside the caller's record shares, so every read door composing the sharing filter serves them. The probe is late-bound to the approvals service and fails closed: no approvals service, no member, a lookup or read that throws, or an on-behalf-of principal (ADR-0090 D10) all grant nothing, with one warn line when a read fails. Write gates read sys_record_share access levels only, so the member cannot write. New listApprovalReadGrants(object, recordId, context) for explain.
  • @objectstack/plugin-security: explain attributes the grant on the sharing layer as a record_share rule with via: "approval request N": admits on read, neutral on writes. A failing lookup attributes nothing.
  • Attachments: no new child reach. ADR-0104's parent-read inheritance follows the record read as for any reader. 【能力需求】plugin-approvals 审批请求可见性:对业务记录有读权者应可只读查看审批动态(或提供可见性 hook) #8652 is untouched.

Tests

  • packages/qa/dogfood/test/approval-pending-approver-record-read.dogfood.test.ts (new, real bootStack, red first on main): a private object, a two-step flow with approvers by email. The first approver gets GET 200 and the record in the list; a stranger gets 404; the approver's PATCH is 403 with nothing landed; POST /security/explain names "approval request N"; the override holder (organization_admin_no_bypass) reads; approving advances the step (first approver 404, second approver 200); the final approval revokes (second approver and overrider 404); a rejection revokes.
  • packages/plugins/plugin-approvals/src/pending-approver-read-grant.integration.test.ts (new, ObjectQL + SqlDriver on better-sqlite3): person by id and by email, position literal grants nobody, stranger/submitter get nothing; override holder; decided requests (approve, reject) grant nothing, also on the same context; the org wall holds, for the platform admin and for an org-less caller too; the cost pin (five reads of one object plus one of another = sys_user, sys_approval_approver, two sys_approval_request reads; a stranger costs two reads; a write retires the memo); a rejected read is rethrown and not memoized.
  • packages/plugins/plugin-sharing/src/sharing-service.test.ts: seven cases on the OR-in, the join with record shares, no write gate, not asked for null/deny, delegated principal gets none, the fail-closed variants, and listApprovalReadGrants.
  • packages/plugins/plugin-security/src/explain-engine.test.ts: attribution on read (admits) and update (neutral), and a failing lookup attributes nothing.
  • approver-address-readers.test.ts: the new slot comparison site is classified as a reader.

Acceptance notes

  • The approval grant is the row-level reach of a read share: the approver still needs read on the object itself, as for any record share.
  • A unanimous or quorum voter leaves the slate once they vote (the index drops their row), so they lose the read while the step stays pending for the others. That follows "each resolved person on the current step's slate".
  • Explain reuses kind record_share with via: "approval request N", because a dedicated kind is a spec enum edit plus a ui label, outside this card.
  • The org-wall test lives at the service level: the dogfood boot is single-organization.
  • Overlap: feat!: ownership queues and assignment rules (ADR-0142 phase 2) #26337 (ownership queues) edits the same insertion points in sharing-service.ts (options, field, constructor, the buildReadFilter tail), sharing-plugin.ts and explain-engine.ts. git merge-tree reports content conflicts in those three files, all adjacent inserts. Whichever lands second keeps both: the queue disjunct and the approval ids in the id $in arm. No other open PR touches these files.
  • Dogfood Regression Gate does not run on PRs; the dogfood file above was run locally on the final head.

Verification

All on head e8fb0d3 unless noted.

Red on main

The five source files reverted to base 6390fe5 (trap restoring from HEAD), the three plugins rebuilt, ablation-dist-preflight.mjs --absent confirmed the dist: the dogfood test failed at line 183, expected 404 to be 200 (the first approver's read). Restore proved by an empty git diff HEAD, then the dist was rebuilt and the preflight presence check passed.

Suites
  • plugin-approvals: Test Files 100 passed, Tests 1286 passed (lock VERDICT command-exit 0).
  • plugin-sharing: Tests 1240 passed; plugin-security: 4962 passed, 45 skipped (both packages unchanged since 1639ac9, where they were run).
  • dogfood file: 1 passed. Typecheck of the three plugins: exit 0.
  • repo-level census projects (core 83, objectql 5, rest 280 + 18 skipped, runtime 947, spec 1279, types 11): all exit 0.
  • eslint --no-inline-config on the three plugin packages and the dogfood file: exit 0, no findings.
Derived gates

Derived with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack (74 commands), each run with timeout 900, exit captured before any pipe.

  • 73 exit 0.
  • pnpm check:dual-build-cjs-loads: exit 3, PREREQUISITE NOT MET (no dist/ for packages this diff does not touch: studio, client-react, embedder-openai, knowledge-memory and others). NOT MEASURED locally; CI builds them.
  • --ran reconciliation: "74 derived famil(ies) accounted for, 73 run, 1 NOT-MEASURED (1 DERIVED from a recorded exit 3)", exit 0.

Generated by Claude Code

…is about (red first)

Claude-Session: https://claude.ai/code/session_016NZtJpJqVWVJA1XDU9dbnz
Co-authored-by: Claude <noreply@anthropic.com>
…ts on reads the record its request is about

While an approval request is pending, each person on its current step's
slate and the override holder may read the one record it is about: read
only, ending when the step resolves, never across organizations. The
approvals service derives the record ids from the slate it already
resolves; the sharing read filter ORs them in beside the record shares and
fails closed; explain names the grant via the approval request.

Claude-Session: https://claude.ai/code/session_016NZtJpJqVWVJA1XDU9dbnz
Co-authored-by: Claude <noreply@anthropic.com>
…he memo-retirement pin

Claude-Session: https://claude.ai/code/session_016NZtJpJqVWVJA1XDU9dbnz
Co-authored-by: Claude <noreply@anthropic.com>
…rom approver-address.ts, and classify the site

Claude-Session: https://claude.ai/code/session_016NZtJpJqVWVJA1XDU9dbnz
Co-authored-by: Claude <noreply@anthropic.com>
…ending approver's memo

A rejected read leaves nothing behind, with no cleanup step; the system
context needs no branch of its own (the sharing filter bypasses it).

Claude-Session: https://claude.ai/code/session_016NZtJpJqVWVJA1XDU9dbnz
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/plugin-approvals, @objectstack/plugin-security, @objectstack/plugin-sharing, touching 35 documentable anchor(s).

18 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json abb9046057b6713b78712dfe53a329deefa534ba.

⛔ 6 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 10 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 55 of 207 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 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; 97 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 — 21 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 abb9046057b6713b78712dfe53a329deefa534ba → packageMentionDocs.

Which tree this was computed on

This run read content/docs from ede3cf2accc1d81540ccc0b685257742d599b206 — the merge of head e8fb0d3bb12fbb202af1ba9789227e8585cb055c into base abb9046057b6713b78712dfe53a329deefa534ba, 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 ede3cf2accc1d81540ccc0b685257742d599b206 && git checkout ede3cf2accc1d81540ccc0b685257742d599b206
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin abb9046057b6713b78712dfe53a329deefa534ba e8fb0d3bb12fbb202af1ba9789227e8585cb055c && git checkout -B drift-repro abb9046057b6713b78712dfe53a329deefa534ba && git merge --no-ff e8fb0d3bb12fbb202af1ba9789227e8585cb055c

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

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

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs abb9046057b6713b78712dfe53a329deefa534ba → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 · ACCEPT · domain:server seat 1 · os-project-manager · session_01R1q2xdtYgU6FKrZ7LgjPCC · 2026-10-27T02:16Z

Head e8fb0d3bb1, report 6431829080 (on #26180), claim 6430888682. This is a security boundary, so the seat's order owed a review at CONTRACT_REVIEW_TIER: PASS on this head (6431927981, the record just above).

item reading
Shape Draft, base main, Fixes #26180, patch changesets for @objectstack/plugin-approvals, @objectstack/plugin-sharing and @objectstack/plugin-security; Clause-②: no, Metadata-protocol: none. Not governed (0 of 12 paths), 1,051 changed lines. Security card: no advisory, no new issue.
Fix Ruling 6429330267 letter A. A resolved person on a pending step's slate (user id or their own e-mail; position:* literals grant nobody), and the override holder inside their own organization, may read the one record the request is about. The grant enters only through SharingService.buildReadFilter as id $in the pending requests' record ids on that object. No write door reads it (PATCH 403 pinned). It ends when the step resolves (syncApproverIndex plus a status: 'pending' re-read, with the memo keyed by engine.writeEpoch). It is fenced to the request's organization, and explain names "via approval request N". It fails closed with one warn, and costs one bounded probe per context and epoch, never one per row.
Evidence 36 check runs succeeded and 11 were expected skips on this head, 0 failed; mergeable clean. Integration and dogfood tests cover the ruling's seven cases: slate member reads, stranger does not, revoked on approve, reject or advance, no write, explain, override holder, and the org wall at the service. Red first on main. Derived gates: 73 of 74 exit 0. dual-build-cjs-loads was not measured locally (dist missing for untouched packages); CI's build answers it.
Carried Reviewer notes, no action owed: explain reuses kind record_share (a dedicated kind needs a spec enum and a ui label, outside this card). A unanimous or quorum voter who has voted loses the read while the step stays pending for others, which is literal to the ruling. Overlap: #26337 (not this lane's) has adjacent inserts in sharing-service.ts, sharing-plugin.ts and explain-engine.ts; whichever lands second keeps both arms. Merge-tree with #26389 (#26181, same plugin) is clean. The record was taken while CI ran; the seat re-read the conclusions on this head before arming.
Landing 「按照 pm-dispatch 规则复核过的都授权」 (Leehom, 2026-10-25T11:35Z): ready and auto-merge through the relay, once every check on this head is green or an expected skip.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 27, 2026 02:17
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 27, 2026 02:17
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 27, 2026
Merged via the queue into main with commit b7b04e5 Oct 27, 2026
48 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-crp41o branch October 27, 2026 02:45
zhuangjianguo pushed a commit that referenced this pull request Oct 27, 2026
Main's pending-approver read grant (#26397) and this branch's ownership
queue both extend the sharing read filter, the sharing plugin's service
options and the explain sharing layer. Both are kept: the read filter
ORs the owner match, the queue-sitting match and the granted ids (record
shares plus pending-approval reads); explain reports the approval read
grants, then the queue membership and claim rows.

Claude-Session: https://claude.ai/code/session_01GTntg542RUmmRRbMY6wMZD
Co-authored-by: Claude <noreply@anthropic.com>
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/xl tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

plugin-approvals: the approver a step waits on gets no read of the record it is about — "无法打开此记录" on the request they must decide

2 participants