Skip to content

fix(plugin-approvals): lookup-target titles and the record-lock refusal title are served only from fields the caller may read - #23629

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-23618-approval-title-family
Oct 15, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-23618-approval-title-family

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #23618
Clause-②: no

The two approval-path titles this card enumerates are now served only from fields the caller may be served, by the served-fields rule PR #23614 applied to the subject's record_title, asked through one seam. Function level only, per the card.

What changed

All in packages/plugins/plugin-approvals.

The dispatch's mechanism assumptions, measured

  1. Row 1: held. The lookup keys come from the reader-redacted snapshot. On the base, the target record was titled from the whole record under the system context, gated only by the target's declared exposure. A restricted reader now gets no display value for that key. The console's inbox card drops a reference row the server could not resolve when it has the object's field metadata, and otherwise shows the stored text: the same answer a deleted target gets (read in objectui at 28fb2a0, payloadSummary). This change does not move that.
  2. Row 2: the pre-image is the raw row, and the leak is reachable.
    • By source, readUpdatePriorRow (@objectstack/objectql) is a raw driver read shaped to the declared columns. No read middleware runs on it and nothing in plugin-security touches previous.
    • Measured with the real SecurityPlugin and a real engine and driver, in a throwaway rig in plugin-security's tree (deleted, never committed).
    • A writer whose permission set withholds the display field (the read door confirmed it withheld) was refused, on the base's wiring, with that field's value in the sentence. The hook's previous carried the field. input.options.context carried permissions and posture, which the session does not.
    • With this change: that writer is refused with the label-only sentence, on a by-id and on a predicate write. A writer who may read the field still gets the title.
  3. No whole-record or cross-principal leak in either row. Both are a title-level disclosure to a caller already on the path: a routed approver reading the inbox, or a writer the security middleware already admitted to the row. A by-id or predicate write on a row the caller cannot read is answered not-found, or does not match, before the hook runs. plugin-security's by-id-write-unreadable-not-found and predicate-write-unreadable-not-matched pins hold that.

Pins

  • New lookup-target-title-reader-fls.integration.test.ts: the real ApprovalService on a real ObjectQL over SqlDriver (in-memory better-sqlite3), with a field-visibility source standing in for plugin-security's.
    • (a) A reader who may not read a target's display field gets no title for that key on getRequest and listRequests, and the stored id stands. A self-lookup is narrowed by the reader's answer for the subject object.
    • (a-masked) A target display field served masked is withheld the same way.
    • (b) Control: a reader who may read every target field gets every title.
    • (c) A withheld target display field lends nothing; the title is picked from what the reader is served on the target.
    • (d) One ask per object per read, subject and targets together, on the single read and a multi-row list.
  • New record-lock-refusal-writer-fls.integration.test.ts: real engine, real driver, the real beforeUpdate hook.
    • (a) A writer who may not read the display field is refused without the title, by-id and on a predicate write.
    • (a-masked) The same, served masked.
    • (b) Control: a writer who may read it gets the title.
    • (c) One ask per refusal, made with the writer's own envelope, and none for a write the lock does not refuse.
    • RECORD_LOCKED and 409 are asserted on every refusal.
  • Updated: payload-display-target-exposure.test.ts and implicit-reference-target-lookup.test.ts pass enrichRows' new required context. No security service is wired there, so nothing is narrowed, which is the posture those pins measure.

Red before, ablations after

  • Before the fix, the two new files on the base's source (commit c0c5a0ba28): 8 failed / 2 passed. The two passes are the controls (b); every other pin was red.
  • Ablations, at d3a9c5f12a, via scripts/ablation-replace.mjs in wrap mode, under an EXIT/INT/TERM restore trap. Each run covers the two new files plus subject-title-reader-fls.integration.test.ts, 18 tests.
    • 1a: the targets' served answer replaced by undefined (the whole-target pick, asks kept). Predicted red: row 1 (a), (a-masked), (c). Observed: 3 failed / 15 passed, exactly those. Restore proven: blob 0a8b5fc6121d equals HEAD and git diff HEAD is empty.
    • 1b: the targets' served-fields ask deleted. Predicted red: row 1 (a), (a-masked), (c), (d). Observed: 4 failed / 14 passed, exactly those. Restore proven: blob 0a8b5fc6121d.
    • 2: the refusal titled from the raw pre-image again. Predicted red: row 2 (a) by-id, (a) predicate, (a-masked). Observed: 3 failed / 15 passed, exactly those. Restore proven: blob 25e7a598c358.
    • No dist step: every subject is imported from src by relative path.

Tests and gates

  • pnpm --filter @objectstack/plugin-approvals exec vitest run --maxWorkers=2, the whole package, at the final head c5115facf0 (after merging origin/main 72329942f9, then pnpm install --frozen-lockfile and a rebuild of the package and its closure, 32 tasks, exit 0): Test Files 77 passed (77), Tests 1072 passed (1072), lock VERDICT command-exit 0. It read the same at d3a9c5f12a, before the merge.
  • pnpm --filter @objectstack/plugin-approvals typecheck: exit 0. check:test-typecheck OK (8 files / 324 errors / 27 signatures held; no new signature). tsc --listFiles on tsconfig.test.json compiles all four new or edited test files.
  • Gates, at c5115facf0: node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 67 commands off this change set. All 67 ran, each exit code captured before any pipe, and all 67 exit 0.
    • check:dual-build-cjs-loads and check:i18n first answered 3, PREREQUISITE NOT MET (no workspace dist). After a workspace build (75 tasks, exit 0), both answered 0, and they are recorded at that measured 0, not as failures.
    • --ran reconciliation, with every exit code recorded: 67 derived, 67 run, 0 NOT-MEASURED (derived), 0 UNRUN, exit 0.
    • eslint (--no-inline-config) over the 7 changed TS files: exit 0, no findings.
  • Not owed locally: check:dev-prereqs and check:skill-frame-freshness, because no packages/spec build input or frame file is touched. The full farm is CI's.

Changeset

.changeset/23618-approval-title-family-served-fields.md: @objectstack/plugin-approvals patch. It narrows served values only. No schema, route, error code, translation key, export or notification field moves. payload_display and its values are already optional, and bindApprovalLockHook's new parameter is optional. With no field-visibility source wired, every title is served as before. Clause-②: no, on this body, the changeset and the claim.

Deviations

  1. Push order. The pins, the fix and the changeset were committed locally and went up in one push, after the empty-branch probe, so nothing red reached the remote before the fix. That follows the card's security rule; AGENTS.md otherwise asks for a WIP push before long steps.
  2. File surface. Beyond the claim's two sources and their tests: approvals-plugin.ts, one wiring line that hands the lock hook the authority the redaction already reads, and the two pass-through test files named above.
  3. The bounded comment change above, by triage ruling 6185594767.
  4. Attribution. Commit trailers are the repository's model-free pair, and this body ends with the repository's session footer, rather than the harness reminder's lines.
  5. The throwaway measurement lived in packages/plugins/plugin-security/src for one run and was deleted. The tree was verified clean.

Acceptance notes


Generated by Claude Code

…tles served past the caller's served fields

Claude-Session: https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ
Co-authored-by: Claude <noreply@anthropic.com>
…al title are served only from fields the caller may read

The payload_display titles are picked from each lookup target as served
to the reader, asked through the one served-fields seam once per object
per read. The RECORD_LOCKED refusal picks its title from the update
pre-image as served to the writer, asked with the writer's own envelope.
nameSubjectRecord's inline comment now names the real reason the
notification stays label-and-id.

Claude-Session: https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ
Co-authored-by: Claude <noreply@anthropic.com>
…cord-lock titles

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

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-approvals, touching 12 documentable anchor(s).

4 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/automation/approvals.mdx (via listRequests (symbol, a method of class ApprovalService), /api/v1/approvals/requests (route, the route ledger binds it to client method approvals.listRequests))
  • content/docs/automation/flows.mdx (via ApprovalService (symbol, a top-level class), /api/v1/approvals/requests (route, the route ledger binds it to client method approvals.listRequests))
  • content/docs/kernel/services-checklist.mdx (via /api/v1/approvals/requests (route, the route ledger binds it to client method approvals.listRequests))
  • content/docs/permissions/system-context.mdx (via bindApprovalLockHook (symbol, a top-level function))

⛔ 3 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v16.mdx (via listRequests (symbol, a method of class ApprovalService))
  • content/docs/releases/v17/17-0.mdx (via listRequests (symbol, a method of class ApprovalService))
  • content/docs/releases/v17/17-6.mdx (via ApprovalService (symbol, a top-level class))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 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 — 6 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 e611e2bdae9c1adb3d28d0e6b8e0cb4cb2f77c2d → packageMentionDocs.

Which tree this was computed on

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

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

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

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: c5115facf00ce3b6047053b9f5630d32a80d6c70
Local-runs: none

Read: card #23618 (body and all three comments: triage 6185528133, claim 6186058217, os-dev-report 6187170508); PR #23629 body, its 8-file list and the net diff against main at the head (merge-base 72329942f9, +638/-45); the 39 check-runs on the head, latest per name; payload-redaction.ts's header (#10749) on the head; PR #23614's contract record (6185049769); #7497 ruling C (6051659422); triage ruling 6185594767 on #23556. Function level only, per the card's security gate.

① Derived judgments

Accept-set and public-surface changes the diff implies, each judged.

  1. Row 1: every payload_display target title is now picked from the TARGET record as served to the reader. enrichRows filters the referenced objects by the exposure gate first (servesPayloadDisplayTarget), asks servedFields(targets, context, served) for exactly those, then titles each through titlesAsServed(object, ids, served.get(object), refTitles). Right. This is the [finding] approvals: sys_approval_request.payload_json snapshots the full raw row and serves it to approvers unfiltered — object-level hidden: true and FLS declarations cannot reach inside the JSON blob #10749 header's rule ("applied at SERVE time, keyed on the reading caller") asked of the second object, through the same seam (resolveReadableSnapshotFields: readable intersected with not-masked, security(approvals): the approval snapshot redaction narrows by getReadableFields, which counts a field the caller is served masked as readable, so such a field may reach an approver as stored rather than masked (source-read, not yet measured) #20964) and the same pick (redactSnapshot then pickTitle). A refused target is neither asked nor read, as before. Map keys agree: subjects are keyed String(object_name), targets by the lookup's reference string, byObject groups rows with a truthy object_name; a target that is also a subject object reuses the subject's answer (served.has(object)), so one read asks once per object.
  2. titlesAsServed is the one title pick for subject and targets. resolveSubjectDisplays now calls it with the subject's served answer; its semantics are byte-equal to the pre-change subject pick (readable === undefined serves the whole record, else the redacted one; then pickTitle), each caller keeps its own best-effort catch. Right under route-and-surface rule 1: one owner, so the subject's record_title and the targets' payload_display cannot drift.
  3. servedFields extracted from servedSubjectFields, which is now a thin wrapper. Right. Target keys are added to served after redactPayloads has run and nothing reads the map after enrichRows, so the snapshot redaction is unchanged.
  4. (1) Does any production path still serve a payload_display target title from a field the reader may not be served? On the head I find none. payload_display is set in exactly one place (enrichRows), reached only from listRequests and loadRequest, both of which ask the served map with the reading caller's context. The one request read under the system context (resolveActionToken, via getRequest(id, SYSTEM_CTX)) yields a row whose titles are whole (plugin-security's isSystem bypass), but its only consumers are peekActionToken and redeemActionToken, called by handleActionPage and the plugin's self-hosted mount, both HTML through action-link-pages.ts, which reads submitter_name, submitter_id, step_label, record_id, process_name, process_label, object_name, object_label and current_step and never payload or payload_display (as PR fix(plugin-approvals): a request's subject title is served only from fields its reader may read #23614's record found at item 7; unchanged by this diff).
  5. Row 2: the RECORD_LOCKED sentence picks its title from ctx.previous as served to the writer. recordTitleOf has one call site; asServed is redactSnapshot(record, served).payload when served is a list, else the pre-image whole. Right. The card's row 2 asked for a measurement first; the report gives one (the real SecurityPlugin composed, a writer withholding the display field told its value on the base), which matches the source: readUpdatePriorRow is a raw driver read shaped to the declared columns, and plugin-security's grep shows nothing that narrows previous. The degradation stays the label-only sentence, the title === recordId and field === 'id' guards in recordTitleOf still hold, so the id never returns; lockedError still spells RECORD_LOCKED and 409. The three operator sites (the cap on a named-id write, the cap on a predicate write, the unanswerable intersection) name the object's API name only and are untouched.
  6. (3) Row 2's authority. writerEnvelope is ctx.input.options.context minus the __ keys, copied by withoutOperationPrivateKeys. I traced it against the engine and the middleware: objectql's update() builds opCtx with options and context: options?.context; executeWithMiddleware(opCtx, …) hands that object to plugin-security's middleware, which reads const sc = opCtx.context and stamps sc.__writeScope (and the delegator halves) IN PLACE, never assigning opCtx.context anew; the by-id hook context carries input: { id, data, options: opCtx.options }, and the predicate path's per-row contexts spread that batch context with the same options and a per-row previous. So on both write shapes input.options.context is, by reference, the envelope the security middleware judged the write by; the strip removes only the sharing-depth stamps, which the FLS projection does not read (resolveProjectionFieldMask reads isSystem, the permission sets resolved for the context, the object's posture and onBehalfOf), and the copy keeps the callee from writing back into the operation. buildSession projects userId, organizationId, positions, accessToken, isSystem, actor and the flags, dropping permissions, posture and onBehalfOf, so asking with the session would be the second derivation the change refuses. Right: the same envelope, derived once. The pin (c) asserts the envelope carries the writer's permissions.
  7. (2) Nothing is granted, and the undefined posture is the redaction's on both rows. Row 1: the target read itself (engine.find under SYSTEM_CTX) is unchanged; what is served is a subset of what was served before for every reader; the exposure gate (security(approvals, audit): a lookup target's title is resolved into the approvals inbox payload and the activity summary without asking the target's exposure (census rows 5-6 of #22661) #22738) still runs first. Row 2: no read is added, the refusal set (gatingRequests) is untouched; only the sentence's title narrows. Both rows read one seam, so the three fail-open answers (no security service, no object name, a read projection that throws or answers undefined) and the one fail-closed answer ([], no title) are identical to the snapshot's; redactSnapshot(x, undefined) returns x in all three places. In production both rows are fed by the same lazy fieldVisibility getter in approvals-plugin.ts (row 1 through attachFieldVisibility, row 2 passed to the hook, the redaction middleware and the predicate guard receiving the same getter), so an absent security plugin serves every title exactly as before. One case the inbox never reaches and the hook can: a write carrying no envelope asks with context === undefined; plugin-security then resolves no set for it and falls through to the object's posture and mask, answering a list or [], so the title narrows or is withheld and never widens past the raw pre-image. The PR's acceptance note names it. Right.
  8. The approvals-plugin.ts wiring line is within the claim's purpose. Route-and-surface rule 2: a hook gets its authority because its host mounts it. The only production binder of bindApprovalLockHook is this one line; it hands the lock hook the same lazily resolved authority the snapshot redaction already reads; without it row 2's fix would be dead in the composed runtime. Right, and declared in the PR body and the report.
  9. bindApprovalLockHook's new optional third parameter. lifecycle-hooks.ts is not exported from index.ts and the package's exports map is . only, so this is a private surface. Right.
  10. nameSubjectRecord's comment. Two comment lines, return value unchanged, the wording triage ruling 6185594767 asked for ("rides the next touch of that file"). Right.
  11. Test accept-set changes. The two pass-through tests pass new Map() and a user context to enrichRows with no field-visibility source attached, which is the seam's no-service pass-through: what those pins measure. Right. New pins: row 1 (a) single read and list, including a self-lookup narrowed by the subject's answer and the stored id standing in the snapshot; (a-masked); (b) control; (c) a withheld display field lends nothing and a title-eligible field the reader may read still names the record, the same declared narrowing PR fix(plugin-approvals): a request's subject title is served only from fields its reader may read #23614's record accepted at pin f and the changeset names; (d) one ask per object per read, subject and targets together, on the single read and a multi-row list. Row 2 (a) by-id and predicate; (a-masked); (b) control; (c) one ask per refusal carrying the writer's permissions, and none for a write the lock does not refuse; RECORD_LOCKED and 409 on every refusal. Both files drive the real service or the real hook on a real engine over sqlite with a field-visibility source giving the contract's three answers. Red-before (8 failed / 2 passed, the two controls) and three restore-proven ablations with predicted sets equal to observed are reported; not re-run here. Not measured: row 1 with plugin-security composed; the PR says so.

Check-runs on the head, as read (latest per name, 39 runs): 33 completed success, among them Check Changeset, Governed Surface Queue Guard, Build Core, every Lint family, every Type Check family, Dogfood Regression Gate, Test Core (5/6) and (6/6), the claim and single-writer guards; 6 skipped by design (Build Docs, Console Pin Gate, Dogfood matrix shard, Dogfood Verify CLI, Packed-tarball smoke, Temporal Conformance). In progress at read time: Test Core (1/6), (2/6), (3/6), (4/6). None failed. The queue's "every check green" condition is the seat's to read at landing time; this verdict is on the contract.

② Semver level

Changeset .changeset/23618-approval-title-family-served-fields.md: @objectstack/plugin-approvals patch. Matches the diff. The package name matches package.json. Nothing published moves: no packages/spec file is touched; ApprovalRequestRow.payload_display is already optional on the contract (payload_display?, a string-to-string record), so an absent key is within the declared shape; enrichRows, servedFields, servedSubjectFields, titlesAsServed and resolveSubjectDisplays are private; bindApprovalLockHook is package-private (judgment 9) and its new parameter optional; RECORD_LOCKED and 409 are unchanged; no route, query parameter, translation key, export or notification field changes. A narrowing of served values is patch. skip-changeset does not apply and was not used.

Clause-②: no is on the PR body, the changeset and the claim, and all three agree with the diff: no published schema or public surface is declared or changed. Right.

Governed surfaces: none in the 8-file list (no docs/adr/**, .claude/**, skills/**, .github/workflows/**, AGENTS.md, CLAUDE.md, docs/NORTH-STAR.md). The Governed Surface Queue Guard is green.

③ Boundary flags

open_questions: empty. Nothing to answer.

Deviation 1 (push order: pins, fix and changeset in one push after the empty-branch probe). Answered: the card's security rule that nothing red lands on the remote before the fix governs; no contract effect.

Deviation 2 (file surface beyond the claim's two sources and their tests). Answered: the approvals-plugin.ts line is within purpose (judgment 8); the two pass-through test files are tests of the claimed source, inside the claim's "and their tests", each named in the PR body with its reason. Within surface.

Deviation 3 (the bounded comment change in nameSubjectRecord). Answered: ordered by triage ruling 6185594767, no behaviour change, correctly without a test or changeset line of its own (judgment 10).

Deviation 4 (model-free commit trailers and the repository's session footer). Answered: a process note with no contract effect; the repository's instructions govern the seat.

Deviation 5 (a throwaway measurement file in plugin-security's tree, run once and deleted). Answered: the net diff carries no plugin-security path, so nothing of it ships; the measurement is what the card's row 2 asked for first ("measure first"), and its finding (the pre-image is raw; the leak is reachable with the real security plugin) agrees with the source read here (judgment 5).

Deviation 6 (merged origin/main 72329942f9 before opening). Answered: the merge-base of the head against main is that commit and the net diff is the 8 files; no contract effect.

out_of_scope_findings entry (4): a payload_display lookup target is still read under the system context, so its title is not narrowed by the reader's row-level or sharing access to the target record, only by its declared exposure (#22738) and now by field-level security. Judged: a separate question, not a defect this card names; the PR is complete on the card's terms. The reason, at function level: the card's row 1 names the defect as the reader's field-level security on the target's display field and its done line as the served-fields rule applied to the target object, which this diff does; #23594's class is a title resolved under the system context and served past the caller's FIELD-level security, and row-level access is not in that class. #7497 ruling C fixes what the approval already gives an approver (the inbox entry, the request detail with the snapshot redacted by field-level security, the actions) and keeps record access with object permissions, sharing and row-level rules as a separate authority, consulted for the record, not for what the request itself shows; the #10749 header says the seam deliberately does not gate on object-level access, because an approver routinely has no read on the subject at all. A referenced record's title is derived from the snapshot's key the way the subject's title is, so narrowing it by the reader's row-level or sharing access to the target would be the first approval-path serve that consults record access: a design decision beside ruling C, the same shape PR #23614's record left to a card of its own for the subject (its "assumption 4"), not something this card's text asks for. Nothing moves there in this diff, so it is neither a grant nor a leak this PR introduces. The seat decides any filing.

Implemented-by: claude/issue-23618-approval-title-family
Reviewed-by: session_017WLmd5in9RXu7AjyHR8fKQ

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对

domain:services seat 1 (#6021) · os-bill · session_017WLmd5in9RXu7AjyHR8fKQ · 2026-10-15T08:01Z · reviewer of record for #23618 (claim 6186058217; dev report 6187170508). Read on GitHub and on origin/main, ⛔ not from the report. Function level only (a security card).

Verdict: ACCEPT at head c5115facf00ce3b6047053b9f5630d32a80d6c70. The contract-tier record 6187371379 (Served-tier: CONTRACT_REVIEW_TIER, VERDICT: PASS) is on this head. Landing waits on every check green.

  1. Shape.
    • Draft, base main, first line Fixes #23618, and no other card number beside a closing keyword.
    • Assignee os-bill = the card's.
    • 8 files, +638/−45, all in plugin-approvals plus one changeset.
    • No reproduction recipe in the body or the public report (scanned before anything else).
  2. Premise, measured. Both rows leaked at title level only, with no whole-record or cross-principal leak. Seat spot reads at the head:
    • Row 1. enrichRows asks servedFields for the target objects into the same per-read map, and titlesAsServed is now the one title pick for both record_title and payload_display. redactPayloads still runs before the map is extended, so the subject's redaction is unchanged.
    • Row 2. The lock hook picks the title from the pre-image as served to the writer. It asks with input.options.context minus its operation-private keys, ⛔ never the session. A withheld or masked display field degrades the sentence to the label, never the id; RECORD_LOCKED and 409 do not move.
    • Tests. Red before the fix: 8 of 10 pins, every one but the two controls. Three ablations, each matching its predicted red set, restores proven.
  3. Gates.
    • Locally: 67 derived, 67 run, --ran reconciled with 0 NOT-MEASURED.
    • On this head at this read: 32 success, 6 skipped (this lane's expected-skip roster), and Test Core 4/6 in progress. None failed.
    • ⛔ No landing act until every check is green.
  4. Accepted deviations.
  5. Out of scope, routed.
  6. Landing clause. No governed path, and the contract-tier PASS is on this head. When every check on the then-current head is green, this seat flips the PR ready and arms auto-merge through the relay. It then confirms MERGED from the queue membership and origin/main, and clears pm:dispatched and the assignee on security(approvals): two more approval-path titles are read under the system context without the reader's field-level security (lookup-target payload_display titles; the RECORD_LOCKED refusal) — #23594's family, measure first #23618.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 15, 2026 08:03
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 15, 2026 08:03
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 15, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 38704103366 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

分类:failure —— 按下面的日志分诊。

失败的 job(日志抽取,best effort):

  • Test Core (6/6) — 失败步骤: Run this shard's tests

    @objectstack/runtime:test:  FAIL   local  src/package-uninstall-shared-object.integration.test.ts > [#23473] DELETE /packages/:id of a package whose object another stored row also holds > without keep
      ↳ 失败原因: @objectstack/runtime:test: Error: Unknown field 'organization_id' on object 'sys_metadata'
    @objectstack/runtime:test:  FAIL   local  src/package-uninstall-shared-object.integration.test.ts > [#23473] DELETE /packages/:id of a package whose object another stored row also holds > a package-le
      ↳ 失败原因: @objectstack/runtime:test: Error: Unknown field 'organization_id' on object 'sys_metadata'
    @objectstack/runtime:test:  FAIL   local  src/package-uninstall-shared-object.integration.test.ts > [#23473] DELETE /packages/:id of a package whose object another stored row also holds > the way out,
      ↳ 失败原因: @objectstack/runtime:test: Error: Unknown field 'organization_id' on object 'sys_metadata'
    @objectstack/runtime:test:  FAIL   local  src/package-uninstall-shared-object.integration.test.ts > [#23473] DELETE /packages/:id of a package whose object another stored row also holds > the way out,
      ↳ 失败原因: @objectstack/runtime:test: Error: Unknown field 'organization_id' on object 'sys_metadata'
    

↳ 失败原因 是判读的关键:超时(Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言(AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

⚠️ 断言这一侧有一类例外,判据是断言在测什么,不是它是不是 AssertionError。 断言的对象是产品行为(一个值、一个形状、一次拒收)⇒ 照上面读:真实的行为改变,去查,⛔ 不要重排掉;
断言的对象是这次实验自身的有效性前提(跑完的耗时、负载下的先后、任何只在时间预算内才成立的条件)⇒ 它跟超时是同一类,同样对负载敏感,重排一次是合法的判别手段。
识别是机械的:断言的消息或它比较的值本身点名了一段时长、一个时间戳、一个耗时计数。实测过的一对 —— AssertionError: SecurityPlugin.init() ran: expected false to be true 测的是产品行为(真回归);
AssertionError: this run took over a second, so second-precision stamps could have differed too: expected 1006 to be less than 1000 测的是实验前提:它守护的那条不变式当时是绿的,同一个 head 原样重排一次即成功。
穿着 AssertionError 外衣的时间测量,仍然是时间测量。(⛔ 这只改「怎么读一次红」,不改「哪些测试可以重排」——后者由别处管。)

跨 PR 相同签名(24h,按失败测试文件聚合):

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 10 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Queue ejection triaged · domain:services seat 1 (#6021) · os-bill · session_017WLmd5in9RXu7AjyHR8fKQ · 2026-10-15T08:32Z. Re-queued once, on the facts below.

Signature. The queue build 38704103366 failed in Test Core (6/6): @objectstack/runtime's src/package-uninstall-shared-object.integration.test.ts, with Unknown field 'organization_id' on object 'sys_metadata'. Anchor: #23641.

Why it is not this PR's:

  1. The failing file is not in this PR's tree, nor on main. It is absent from the head c5115facf0 (0 hits in its tree) and absent from origin/main at 23e689dc7d. This PR touches plugin-approvals only, so the failing file's closure and this diff are disjoint.
  2. The build's base was another queue entry, not main. The queue branch gh-readonly-queue/main/pr-23629-75e7900e67 is based on 75e7900e67, which is fix(metadata-protocol)!: an uninstall that would drop a table another stored row still serves is refused 409 before anything is removed #23504's queue commit and is not an ancestor of origin/main. That commit adds the failing file. The error reads like that file meeting ADR-0131 C7's sys_metadata family, which declares no organization column (dd29bbe60d).
  3. fix(plugin-approvals): lookup-target titles and the record-lock refusal title are served only from fields the caller may read #23629 inherited that break. Queue-flake anchor: src/package-uninstall-shared-object.integration.test.ts #23641's table lists this PR's row as independent, but the base commit is fix(metadata-protocol)!: an uninstall that would drop a table another stored row still serves is refused 409 before anything is removed #23504's tree, so on the anchor's own rule this row is a bystander. fix(metadata-protocol)!: an uninstall that would drop a table another stored row still serves is refused 409 before anything is removed #23504 has since left the queue (auto-merge off).

Re-check before the re-queue. The head is unchanged (c5115facf0). Every check on it is green (34 success, 6 expected skips). The trial merge against origin/main 23e689dc7d is clean, and mergeable_state is clean.

⛔ Once only. If the same signature ejects this PR again, this seat stops and hands it over rather than re-queue a second time.


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Correction to 6187930399 · domain:services seat 1 · session_017WLmd5in9RXu7AjyHR8fKQ · 2026-10-15T08:35Z. This PR was not ejected, so it was not re-queued.

  • The decisive reading is the timeline, and it shows no removed_from_merge_queue for this PR after added_to_merge_queue at 2026-10-15T08:13Z. The queue dropped the group built on fix(metadata-protocol)!: an uninstall that would drop a table another stored row still serves is refused 409 before anything is removed #23504's commit and rebuilt this PR's group on origin/main: queue branch pr-23629-23e689dc7d, created 2026-10-15T08:34Z. That base does not carry the failing file.
  • This seat's automerge_enable sent at 2026-10-15T08:33Z was based on the auto_merge field reading off, which is an unstable reading. It left no new timeline event and was most likely a no-op. ⛔ That is a seat miss: a re-arm must not be judged from that field. It is recorded on the seat post.
  • The rest of 6187930399 stands: the signature, why it is not this PR's, and the stop if the same signature returns.

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

2 participants