Skip to content

perf(security): batch explain runs its object-level pass once, not once per record - #29208

Merged
objectstack-fleet[bot] merged 7 commits into
mainfrom
claude/project-thread-3fwdg3
Nov 2, 2026
Merged

objectstack-fleet[bot] merged 7 commits into
mainfrom
claude/project-thread-3fwdg3

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Steve · project thread
Fixes #29112

Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么:批量权限解释(security/explain 带 recordIds)不再每条记录跑一遍完整解释;调用者校验、被解释用户的上下文和对象级九层只算一次,每条记录只跑记录级判断。
  • 为什么改:列表页一行一个菜单,31 行的页要等两次各约 3 秒;本地 50 条 update 批量的数据库查询从 505 次降到 231 次,CPU 从约 284 ms 降到约 147 ms。
  • 风险与回滚:答案不变(44 例一致性测试逐条比对批量与单条,覆盖 4 种询问者 × 5 个动作 × 2 个对象的混合记录集);回滚即还原这一个 PR。
  • 先看:packages/plugins/plugin-security/src/explain-engine.ts(explainRecordsForAsker 与拆出的对象级/记录级两段)、security-plugin-explain-access.ts(depsFor)、packages/rest/src/security-routes.ts(把整批交给服务,旧服务退回逐条)。
  • 两分钟验证:pnpm --filter @objectstack/plugin-security exec vitest run src/explain-batch-parity.test.ts,预期 44 例全绿。
  • 需要你做的:无,席位落地;staging 上 Server-Timing runtime ≤ 500 ms 需发版后由 cloud 侧复测(见 Acceptance notes)。

What changes

  • Explain engine (explain-engine.ts). explainOnce is split, with no change in what it computes, into the object-level pass (explainObjectPass: the nine layers and the facts the record pass reads), the record pass (explainRecordOnPass) and the decision builder (decisionOf). A singular explanation runs them in the same order on the same values as before. A new module export, explainRecordsForAsker, runs the object-level pass once and then the record pass per distinct id, each over its own copy of the pass's layers and with the per-explanation read memos explainAccessForAsker sets up for one explanation set up afresh. The object-level answer is taken before any record pass runs. The sharing service's read filter (a question about the principal and the object, never a record) is asked once per batch.
  • Security service (security-plugin-explain-access.ts). The deps bag is now built per record (depsFor(rid)); only its record-grained members read rid. request.recordIds is honoured: the caller's admission and the explained context are settled once and the engine's batch entry answers. recordId beside recordIds is refused, as the wire schema refuses it. The published SecurityPlugin#explainAccessForCaller signature is unchanged.
  • REST door (security-routes.ts). The batch is handed to the service in one call. An answer with records of the batch's length is relayed, a position with no verdict failing closed as before. A service that predates this answers the object-level decision with no records, and the door then asks each id singularly, as it always did.

Why

The batch form ran svc.explain once per id, serially, and each call redid the explained principal's context (its grant rows read again), its person row and the whole object-level pass. On the rig below that was 10 queries per id; it is now about 4.5, all of them the record's own questions.

Tests

  • New packages/plugins/plugin-security/src/explain-batch-parity.test.ts (44 cases, real SecurityPlugin, SharingService and sharing middleware over ObjectQL and better-sqlite3, asked through the registered security service):
    • AGREEMENT, 40 cases: 4 askers (the member itself as a human, an administrator who reads every row, an administrator held to the member's read policy, the system) × 5 verbs (read, export, update, delete, transfer) × 2 objects (a private object behind sharing with an own-row, a shared row, an unshared row, an RLS-locked row, a missing id and duplicates; a public_read_write object with an app-authored read policy hiding a row). The member's grant carries a when-conditioned field entry and a unit read depth with no hierarchy resolver mounted. Each case asserts the batch minus records equals the object-level explanation, and records equals the singular record verdicts in order.
    • The mixed set is mixed (visible, shared, excluded, rls-locked, missing all occur); recordId beside recordIds is refused.
    • ONE OBJECT-LEVEL PASS, 2 cases: a batch reads the principal's grant rows and person row exactly as often as one singular explanation does.
  • packages/rest/src/security-routes.test.ts: three new cases (a batch-capable service is called once with the whole batch and its records are relayed; batch-capable AGREEMENT through the real handler; a verdict-less position fails closed).
  • Forced test edits, both because the service now receives the batch:
    • security-routes.test.ts, "answers records[i] for recordIds[i]": the first service call now carries recordIds (the fallback calls still carry none). Was: no call carries recordIds.
    • security-explain-envelope.test.ts, the "batch — POST recordIds, refused by the object-level pass" row: the request the service receives now carries recordIds.

Acceptance notes

  • Card item 1, local half. On the rig below (50 rows, a human caller explaining itself, update): a singular explanation costs about 5 ms CPU; the 50-id batch costs about 147 ms CPU where the per-id loop cost about 284 ms (243 ms on main's own sources). The per-id margin is about 2.9 ms (card bound: 10 ms, met). The "3× a single" arm is NOT met: about 4.5 per-record queries per id remain (the caller's by-id read through the read middleware, the system re-read of the row, the sharing gate and the share listing), and they are each record's own questions.
  • Card item 1, staging half, and item 3 are for the cloud side after release. Estimate only, not measured: staging time is query round trips, and per-id queries fell from about 10 to about 4.5, so roughly half of the 3 s. Reaching 500 ms likely needs one of the two options in the report's open questions (batched per-record reads, or bounded concurrency once the per-request context stamps are made concurrency-safe).
  • record-page-routes.ts is unchanged: its update / delete / transfer verdicts are three different operations, so they share no object-level pass, and this change gives them nothing to share.
  • The service contract type. ExplainAccessRequest (packages/spec/src/contracts/security-service.ts) does not declare recordIds, while the wire schema (ExplainRequestSchema.recordIds) and the decision (ExplainDecisionSchema.records) do. This PR touches no packages/spec file; declaring it there is a follow-up for the maintainer to decide.
  • Concurrency was considered and rejected: the RLS composition stamps once-per-request state on the context (__rlsMembershipStaged, set before its resolution awaits), so concurrent record passes on one context could read a half-staged membership. The record passes stay serial, in first-occurrence order, as the door asked them.

Verification

Head fc99ac4f0bef28e0bbc29e536a4d1b9162e73f51. Taken on the final head after origin/main was merged in (merge base 4550091a1).

Red first on main's sources
  • explain-batch-parity.test.ts with explain-engine.ts and security-plugin-explain-access.ts restored from fea2dbb7: Tests 42 failed | 2 passed (44) (the service ignored recordIds: expected undefined to deeply equal [ ... ]). The 2 that pass there are the read-count pins, vacuously: on main the service never sees a batch, so their red evidence is the ablation below.
  • REST suites with security-routes.ts restored from fea2dbb7: Tests 4 failed | 48 passed (52).
Ablations (scripts/ablation-replace.mjs, restored blob == HEAD each time)
  • Engine: the object-level pass run once per record inside explainRecordsForAsker. Read-count pins red, 2/2: findOne sys_user expected 1 received 6 (the member), expected 2 received 6 (the administrator).
  • REST: the batch answer ignored (if (false as boolean)). Red, 2/25: "hands a batch-capable service the whole batch ONCE", "a position ... fails CLOSED".
Benchmark readings (shared box, 50 rows, CPU via process.cpuUsage, medians)
verb single CPU per-id loop CPU batch CPU loop queries batch queries
update 4.97 ms 284.4 ms (main 243.1) 147.4 ms 505 231
read 3.81 ms 161.3 ms (main 142.5) 116.1 ms 430 156
delete 1.95 ms 175.6 ms (main 185.8) 135.8 ms 505 231

A throwaway script, not committed.

Package tests and typecheck
  • pnpm --filter @objectstack/plugin-security test: Test Files 239 passed, Tests 5171 passed / 45 skipped.
  • pnpm --filter @objectstack/rest test: Test Files 367 passed, Tests 6966 passed / 332 skipped.
  • pnpm --filter @objectstack/plugin-security typecheck: green. pnpm --filter @objectstack/rest typecheck: green after a test-layer type fix (fc99ac4f).
  • eslint (repo config, --no-inline-config) on the six touched source and test files: exit 0.
Derived gate list (dispatch-gates --commands)

node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack on fc99ac4f: 65 commands. Run reconciliation (--ran): 65 derived, 65 run, 0 NOT-MEASURED, 0 UNRUN.

  • 62 exited 0 on the first pass.
  • node scripts/check-plugin-teardown-shape.mjs --self-test: exit 3 (its pinned fixture commit was outside the shallow clone); after fetching that commit, exit 0, "48 cases pass".
  • pnpm check:i18n: exit 3 (PREREQUISITE NOT MET, CLI not built); after the closure build it names, exit 0, "OK (10 package(s) — all bundles in sync, no undeclared authoring keys)".
  • pnpm check:dual-build-cjs-loads: exit 3, PREREQUISITE NOT MET (needs every package's dist/, a whole-workspace build). NOT MEASURED locally; CI's Lint and Repo Gates runs it on this PR.

Human Minimum

  • 1. Schemas start as Zod; packages/spec holds no runtime logic.
  • 2. Every name I added uses the casing for its kind.
  • 3. No formatter ran; pnpm lint passes (touched files locally; repo-wide in CI).
  • 4. One issue, one branch off an up-to-date main, one pull request.
  • 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset.
  • 6. packages/spec touched: its generated artifacts are regenerated and committed. (Not touched.)
  • 7. No hand edits to content/docs/references/, packages/*/CHANGELOG.md or content/docs/releases/.
  • 8. Title, body and comments are in English.
  • The Before Submitting list in CONTRIBUTING.md is done.

Generated by Claude Code

The batch form of security/explain ran a whole singular explanation per
record id, the object-level pass and the explained context included. The
service now takes the batch, settles the caller's admission, the
explained context and the object-level pass once, and runs only the
record pass per distinct id. The REST door hands the batch over and keeps
the per-id path for a service that predates it.

Claude-Session: https://claude.ai/code/session_01ADHkmNqE95va9aNi7nYajP
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Nov 2, 2026
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-security, @objectstack/rest, touching 22 documentable anchor(s).

30 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 dedc350519f22c7322e8f618a7a18c1dba6ff0f4.

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

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

Which tree this was computed on

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

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

⚠️ 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 dedc350519f22c7322e8f618a7a18c1dba6ff0f4 → 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

domain:server#2 · os-steve · session_01ADHkmQxwoGb5fSnrFkpxmt

席位核对

ACCEPT · domain:server seat 2 · os-steve · session_01ADHkmQxwoGb5fSnrFkpxmt · 2026-11-02T09:29Z · head fc99ac4f · claimed by seat 2 on #29112 (comment 6572609144)

The maintainer's words for this seat, verbatim: 「接手所有开发,包括跟进之前未完成的开发落地。」

  1. Shape: a draft against main, first line Fixes #29112, Clause-②: no and Metadata-protocol: none at line start. check-governed-merges --pr 29208: NOT governed (0 of 7 paths), +682 / -113. No packages/spec file, no ui/**. Non-governed landing.
  2. Semantics unchanged, which the order made the hard constraint: a singular explanation runs the split passes in the same order on the same values. The batch is pinned against N singles by a new 44-case parity file. It runs the real SecurityPlugin, sharing service and middleware over ObjectQL. It covers 4 askers × 5 verbs × 2 objects over a mixed set: own, shared, unshared, RLS-locked, missing and duplicate ids, a when-conditioned field entry and a unit read depth. On main's sources it reds 42 of 44; the other 2 are read-count pins whose red is the ablation. Concurrency was considered and kept out on a named reason (__rlsMembershipStaged); the record passes stay serial.
  3. Acceptance item 1 (local): the card reads "≤ 3× a single, OR a per-id margin ≤ 10 ms". The per-id margin is about 2.9 ms, so it is met. The 3× arm is not met, and the body says so. On the rig, the 50-id update batch went from 505 to 231 queries and from about 284 to 147 ms CPU. Item 2: the parity suite above plus the existing REST suites are green. Item 1 staging half and item 3: the cloud side measures them after release, as the order said.
  4. Evidence: red first on both packages, and two ablations, each red and restored to HEAD. Package suites: plugin-security 5171 passed, rest 6966 passed, both typechecks green. There are two forced test edits, both named in the body (the service now receives recordIds). The dev's local gate union is 65 of 65, with dual-build-cjs-loads NOT MEASURED (whole-workspace build), which CI runs.
  5. Gates on this head at ACCEPT time: 10 success, 13 skipped, 0 red, 14 still running. Auto-merge waits for them, and the merge queue re-runs CI against current main.
  6. Open questions, both as follow-ups:
    • (1) ExplainAccessRequest in packages/spec does not declare recordIds, while the wire schema does. This seat does not decide it; it goes to the spec lane as a card after landing.
    • (2) If the cloud's staging reading stays above 500 ms, the next step is batched per-record reads. That waits for the cloud measurement; no card now.
  7. Serial: none. No other open claim names these files. Landing: pr_ready + automerge_enable through the relay.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review November 2, 2026 09:31
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge November 2, 2026 09:31
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Nov 2, 2026
Merged via the queue into main with commit dbb7ea8 Nov 2, 2026
49 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-3fwdg3 branch November 2, 2026 10:49
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#2 · os-steve · session_01ADHkmQxwoGb5fSnrFkpxmt

Landing record · PR #29208

domain:server seat 2 · session_01ADHkmQxwoGb5fSnrFkpxmt · 2026-11-02T11:08Z · card #29112

Reading Probe Result
Merged PR #29208 state merged, merge commit dbb7ea8731
On main git merge-base --is-ancestor dbb7ea8731 origin/main yes
Queue refs git ls-remote origin refs/heads/gh-readonly-queue/* matching 29208 0
Content git grep -c explainRecordsForAsker origin/main -- packages/plugins/plugin-security/src/explain-engine.ts present

Release: ships patch for plugin-security and rest at the next release.


Generated by Claude Code

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

1 participant